Send a power setpoint, and stop pre-judging overlap - #274
Draft
Poshy163 wants to merge 2 commits into
Draft
Conversation
Two things from testing on CharlesGillanders#269. dragon2611 saw the discharge entry appear in the AlphaESS portal and do nothing, because it had no rate to run at. setTimeChargeBySn accepts a period with no chargePower and then ignores it. We only sent one when a previous periodic read had given us a value to carry over, which for a system with no existing periodic schedule is never. Fall back to the inverter's own rated power from getEssList. benbrown249 hit "charge period 13:30-02:45 overlaps discharge" on a system whose app schedule was empty. That window comes from the legacy store, which on a migrated system is a separate store the app doesn't show, and because it wraps midnight it spans thirteen hours and collides with anything. So every write was skipped. The overlap guard was written from the documentation, before the library could report return codes. It can now, so drop the guess and send the schedule: if the API objects with 6008 we say so, and name the periods we sent. That also removes a guard that could never have been right, since we don't know whether the API applies the overlap rule when a cycle is disabled.
Swept the rest of the endpoints for the two shapes behind the CharlesGillanders#269 reports: an optional field whose absence silently does nothing, and us deciding a call will fail instead of making it. chargePower was the only optional field with that behaviour, and it is now sent, so that one is closed. The EV control pre-check is the same mistake as the overlap guard, and worse. It refused to send start/stop unless the cached charger status matched our own idea of which states allow which command. That status is only as fresh as the last poll, which is configurable up to an hour, and if the status endpoint had failed there was no status at all, so every command was blocked indefinitely. Absorbing API errors on reads made that more likely. The command now always goes out and we report what the charger says. can_control_ev stays for the Can Start/Stop Charging sensors, where a hint is all it ever was. The two services also passed times through untouched. The API wants zero padded HH:mm on the quarter hour, so "9:00" comes back 6001 and an off-grid minute is accepted and then unusable. They are normalised now, the same way the time entities already did it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two things from testing on #269.
@dragon2611 saw the discharge entry turn up in the AlphaESS portal and then just sit there doing
nothing. It had no rate to run at:
setTimeChargeBySnhappily accepts a period with nochargePowerand then ignores it. We were only setting one if an earlier read had handed us avalue to reuse, and if you've never had a periodic schedule there's nothing to reuse. It now falls
back to the inverter's own rated power.
@benbrown249-wq got "charge period 13:30-02:45 overlaps discharge" on a system whose schedule in
the app was completely empty. That window isn't a stale cache. It comes from the legacy store,
which on a migrated system is a different store the app doesn't show you, and because it wraps past
midnight it covers thirteen hours. So it clashed with everything and we skipped every single write.
I'd written that overlap check from the docs, back when the library couldn't tell us what the API
actually said. It can now, so I've dropped the guessing: send the schedule, and if the API comes
back with
6008, log it and name the periods we sent. The old check was shaky anyway, since itassumed the overlap rule still applies when a cycle is disabled and nobody has confirmed that.