fix(gateway): write entities for the bound control target, not just primary units - #4360
Merged
Merged
Conversation
…rimary units On a multi-AIO site automatic_config picks the Gateway/EMS as the control target and binds every PredBat arg to its serial suffix. _inject_entities only ever wrote entities for units flagged primary — and firmware never flags a Gateway or EMS primary — so none of the bound entities were written. They held whatever value they had and silently froze. Observed in production: a site froze for 69 minutes with byte-identical SoC, battery power, grid power, load power and PV power, while PredBat continued to plan and issue control against a stale 76% SoC. The only escaping symptom was the clock-skew warning, because inverter_time is the one bound value whose staleness is self-evident against wall clock — which sends users to fix an inverter clock that is not broken. Inject a unit when it is primary OR when automatic_config bound args to it (tracked in _suffix_to_serial), and write inverter_time under the bound suffix rather than the primary's. Non-primary units that are not the control target stay skipped, preserving the existing guard against double-counted power readings. Adds TestBoundEntitiesAreWritten, which asserts the invariant directly: every entity automatic_config binds must actually be written by _inject_entities. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| continue | ||
| suffix = inv.serial[-6:].lower() if len(inv.serial) > 6 else inv.serial.lower() | ||
| self._inject_inverter_entities(inv, suffix) | ||
| self._inject_inverter_entities(inv, _serial_suffix(inv.serial)) |
Owner
There was a problem hiding this comment.
Why not just publish them all?
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
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.
Fixes #4356.
The bug
On a multi-AIO site
automatic_config()picks the Gateway/EMS as the control target (GivTCP rules) and binds every PredBat arg to its serial suffix:But
_inject_entities()only ever wrote entities for units flaggedprimary— and firmware never flags a Gateway or EMSprimary(it is set on battery inverters). So on those sites none of the bound entities were ever written. They kept whatever value they last held and silently froze.Impact
Seen in production: a site froze for 69 minutes with byte-identical readings every cycle —
— while the battery was really force-exporting at ~5.5 kW. PredBat kept planning and issuing control against a stale 76% SoC.
The only symptom that escaped was the clock-skew warning, because
inverter_timeis the one bound value whose staleness is self-evident against wall clock. It grew linearly (−37 → −42 → … → −98 min) and told the user to "fix your inverter time" — misleading, since the inverter clock was fine. Everything else failed silently, because nothing about a stale SoC looks wrong on its own.Verified against a live device: the gateway was publishing healthy frames throughout (current
timestamp, all inverters present) and PredBat's broker session was receiving them. Nothing was wrong with the device or the transport — only the entity binding.There is a second divergence mode with the same cause:
_needs_reconfigure()only re-runs on a new serial or new EV charger, so if theprimaryflag moves between already-known units the binding goes stale the same way. This PR fixes the write side, which covers both.The fix
primaryor whenautomatic_configbound args to it._suffix_to_serialis already populated byautomatic_configwith exactly the control-target suffixes, so it is the authoritative record of what PredBat reads.inverter_timeunder the bound suffix rather than the primary's.Tests
New
TestBoundEntitiesAreWrittenasserts the invariant directly — every entityautomatic_configbinds must actually be written by_inject_entities— using a Gateway + 2 AIO topology matching the field case.Confirmed RED first. Before the fix it reported:
After: all 251 gateway tests pass, and
unit_test.py --quickis green.flake8output is unchanged from themainbaseline (the remainingC901/E402are pre-existing);interrogatestays at 100%.