fix(gateway): ignore inverters reported without a serial

[mgazza]

Problem

A gateway hub briefly reported a status listing one GivEnergy inverter with an empty serial and no battery block. The gateway component treated it as a real unit:

• _needs_reconfigure() counted '' as a newly discovered serial and re-ran auto-config (new inverter(s) discovered ['']).
• automatic_config() bound inverter 0 to that entry, so every entity arg used an empty suffix (number.predbat_gateway__reserve_soc, ...) and every control command went out with "serial": "". The writes failed their read-back (Inverter 0 write to reserve failed), SoC read 0, and soc_max fell back to its default with a soc_max element None validation error.
• _inject_entities() published predbat_gateway__* entities for the phantom unit.

Control only recovered when the real serial reappeared, counted as "new" again and triggered another re-config.

Fix

A blank serial only means "missing" for GivEnergy units (inverter, EMS, Gateway). GivEnergy discovery always reads the serial, whereas the other hub drivers never report one, so their inverters always arrive with serial "". For every other type nothing changes: a blank serial binds exactly as before.

For a GivEnergy unit reported without its serial (_serial_missing()):

• Re-config: _needs_reconfigure() never treats it as a new inverter.
• Binding: automatic_config() never binds it, but it still counts when deciding the control point. Dropping it first could choose wrong: a second AIO without its serial would stop the Gateway being chosen.
• Unidentified EMS or Gateway: while one is listed, the control point itself is uncertain, so auto-config skips that status (including a Gateway with a single AIO). It also skips when nothing addressable was chosen. In both cases the existing binding and _auto_configured are left alone, so a first-run config retries on the next status. When the serial is reported it is a new serial and re-runs auto-config.
• Status with only such units: it isn't counted as inverter telemetry. The last good status and the telemetry freshness are kept, so a long spell shows up through the existing staleness check. The gateway-online, EV charger and EMS aggregate entities are still published, and new EV chargers are registered straight away.
• Entities: no per-inverter or inverter_time entities, so no predbat_gateway__* entities appear. EMS aggregates carry no serial in their names and are still published.
• EV chargers when auto-config skips: a new charger is registered as well, so it doesn't re-trigger auto-config (and its log lines) on every status.
• Warnings: the missing-serial and auto-config-skipped warnings are logged at most once every ten minutes each.

Tests

All in TestGatewayUnitControlBinding, most driven through _process_telemetry():

• Next to a real inverter on first telemetry: a blank serial (empty and whitespace, with and without a battery block) is not bound, remembered or published.
• After a good config: a blank-only status keeps the last good status and telemetry time. A mixed status triggers no re-config. The binding and configured serials are unchanged, and a forced automatic_config() keeps the binding and warns.
• Blank only on first telemetry: the gateway is not configured, the API is not started and the status is not counted as telemetry. The next real status configures normally.
• Unidentified EMS, or Gateway with two AIOs or with one: nothing is bound, and EMS aggregates are still published. Once the serial arrives the right control point is bound. After a good config an unidentified Gateway doesn't move the binding.
• Second AIO without a serial behind a Gateway: it still makes the Gateway the control point.
• Blank-only status with an EV charger: the charger entities are published and the car registered, the binding and telemetry time stay put, and the warning is logged once until the interval passes.
• Skipped auto-config with a new EV charger: the charger is registered, with one "skipped" and one "new charger" log line over repeated statuses. This holds both on first run and on a configured site.
• Non-GivEnergy types (Solis, Deye, unknown) with serial "": these still configure as before.

On main all but the non-GivEnergy test fail (that one checks main's behaviour is kept). I also reverted each guard individually (type scoping, whitespace strip, coordinator rule, binding filter, EMS lookup, entity filter, telemetry early-out, re-config filter, configured-serials filter, EV registration in both paths, rate limit), and each revert makes at least one test fail. ./run_all --test gateway passes (284) and ./run_all --quick passes.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

Address the critical topology-selection issue, stale telemetry handling, and warning rate limiting.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Serial-less AIO omitted from Gateway selection · New

What changed in this PR

Fixes GivEnergy gateway handling when inverter serials are temporarily missing.

Changes:

• Filters unidentified units from binding and entity publication.
• Preserves topology detection, telemetry, and EV registration.
• Adds regression tests and warning throttling.

File	Summary
apps/​predbat/​tests/​test_gateway.py	Adds coverage for missing serials, topology, EV handling, and compatibility.
apps/​predbat/​gateway.py	Implements serial filtering and telemetry handling.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[springfall2008]

Automated reply from the triage bot.

Already addressed in 16c8b90, which was pushed after this review was posted.

gateway.py:1190-1198 now counts unidentified GivEnergy AIOs towards the topology decision, by id() rather than membership (protobuf messages compare by value, so two blank entries would otherwise collapse into one), while the binding filter at gateway.py:1223 still drops them from addressable.

For the shape named here — Gateway + one real AIO + a serial-less AIO with no battery block — aios is [real AIO] and unidentified_aios is [blank AIO], so topology_aio_count is 2 and the Gateway is selected; it is then bound because it carries its own serial.

Covered by test_blank_serial_aio_with_no_battery_still_counts_towards_gateway_topology, which constructs the blank AIO with no battery block set at all and asserts the Gateway suffix is what gets bound.

No further change was made for this thread. The branch has since been merged with current main (5210f42); pre-commit and the gateway suite (290 tests) pass on the merge result.

[springfall2008]

Automated comment from the triage bot (PR cleanup).

Merged current main into this branch (c046604) to clear a conflict in apps/predbat/gateway.py. The conflict came from the module constants: this branch adds _WARN_REPEAT_INTERVAL, and main (#5220 plus the command-ack work) adds _COMMAND_ACK_WINDOW, _COMMAND_ACK_IDS_KEPT and _monotonic(). The resolution keeps all of them. test_gateway.py merged automatically.

Checked where the two sides interact:

• _inject_entities still publishes main's gateway_read_only device-level entity when a status lists only serial-less units. That is correct, because it has no per-inverter suffix.
• _check_inverter_resets and the command-ack tracking only use serials from _suffix_to_serial, and this branch never adds a blank serial there.

Results on the merge commit: ./run_pre_commit passes, including the quick suite. ./run_all --test gateway passes: 333 tests, covering all the blank-serial tests and main's new ack tests.

No review feedback was outstanding. The one Copilot thread already has a reply (fixed in 16c8b90), and CI was green before the merge.