fix(givtcp): fill per-inverter arg slots by REST endpoint index, not discovery order (#5209)

[springfall2008]

This is an automated draft PR generated from issue #5209 — a maintainer should review it before merging.

Fixes #5209

Summary

GivTCPComponent.automatic_config() built each per-inverter arg list by position, from the endpoints that answered discovery. If endpoint 0 was down at startup, endpoints 1 and 2 moved into slots 0 and 1. Hand-set per-inverter args such as inverter_limit_charge: [2600, 2600, 3000] stayed where they were. So Predbat inverter 1 wrote 2600 W to the AC3 on endpoint 2, and inverter 2 wrote 3000 W back to it every cycle. When endpoint 0 came back, rediscover() appended it, which turned the shift into a permanent rotation: [givtcp_1, givtcp_2, givtcp_0].

_keep_configured_tail() is replaced by _per_endpoint_values(), which always puts REST endpoint n in slot n:

• a discovered endpoint gets its own entity
• any other slot keeps what is already configured for it (apps.yaml, or live args)
• a gap slot with nothing configured, below the highest discovered endpoint, gets the entity that endpoint will have once it answers

num_inverters now covers at least max(discovered) + 1. The order of self.discovered no longer carries meaning, so appending on re-probe is harmless. The #5029 tail behaviour is unchanged: past the last discovered endpoint, the configured entries are kept, a scalar is broadcast, and a short list stays short.

This also fixes the reporter's battery_calibration ... expected 3 warnings. An unconfigured claimed key now gets one entry per inverter instead of coming out short.

Review round 1

• A capability-gated key (pause, discharge target, battery voltage, discovery sensors) claimed while a gap endpoint was down is now handed back to what it held before the claim if a re-run's gate fails, for example a late endpoint that turns out to be GivTCP v2. Previously that stale claim stayed until restart.
• run() now publishes again straight after rediscover() adopts an endpoint. Without this, the re-run's discovery-key gates saw nothing published for the adopted inverter and failed for every key. On main that left the old, shorter claim in place; with the handback it would have dropped the keys for the whole fleet.
• Gap slots are logged separately from tail slots, with a Warn line naming the URL and what it costs.
• The automatic_config() header comment and the debug-journal GH#5029 row and PR fix(inverter): auto-create charge_rate entity for script-driven "power" inverters (#3311) #4645 bullet are updated to the per-endpoint invariant.

Testing

• tools/triage_test.sh givtcp_component: fails without the fix (104/108, 4 failed) and passes with it. The red run shows the reporter's exact lists: charge_rate shifted to [givtcp_1, givtcp_2, inv2], battery_calibration two entries short of three, and the [givtcp_1, givtcp_2, givtcp_0] rotation after recovery. After review round 1: 118/118.
• New test_leading_endpoint_down_does_not_shift_the_fleet reproduces the Predbat GivTCP REST repeatedly setting inverter charge/discharge rate to wrong value, then correct value #5209 setup: 3 endpoints, endpoint 0 down, num_inverters: 3, then endpoint 0 recovers.
• New test_gated_claim_is_handed_back_when_a_late_endpoint_lacks_the_capability, test_gap_and_tail_slots_are_logged_separately, and a battery_calibration assertion in test_rediscovery_picks_up_an_inverter_that_was_down_at_startup cover the round 1 changes.
• ./run_pre_commit passes: all hooks plus the quick suite (4 slow tests skipped).

Notes

• Behaviour change to review: two existing tests asserted the old compression, and I have rewritten both.
  • test_automatic_config_maps_predbat_inverter_to_the_live_endpoint is now ..._keeps_the_live_endpoint_in_its_own_slot. It had givtcp_rest: [dead, live] becoming one inverter driven by endpoint 1. It now becomes two inverters, with slot 0 pointing at endpoint 0's own (not yet published) entities.
  • test_rediscovery_appends_so_running_inverters_keep_their_identity is now test_rediscovery_keeps_inverter_identity_by_endpoint.
  • At startup, a dead endpoint cannot be told apart from a leading placeholder URL. I chose identity-by-index because compressing silently misroutes real hardware, which is what this issue shows. It also matches how inverters were numbered before refactor(givtcp): move GivTCP REST handling into its own component #4864, when inverter n read givtcp_rest[n]. The shipped templates only ever put placeholders at the tail, and a tail placeholder is unaffected.
  • I considered using configured num_inverters to choose between the two behaviours, and rejected it: the GivTCP template tells REST users they can delete num_inverters, so the users most exposed to this bug often won't have it set.
• Cost of the gap slot (the phantom inverter): when a non-last endpoint has not answered and nothing is configured for its slot, Predbat builds an inverter for it whose entities are not published yet. Its soc_max read falls through to Inverter's 8 kWh default and its control writes have nowhere to land. That lasts until the hourly re-probe adopts the endpoint, or indefinitely if the URL belongs to a decommissioned inverter. Main planned correctly in that case for users with no per-inverter lists, because it compressed the fleet down to what answered. I did not cap the gap fill: a cap has to fall back to compressing, and compressing is the Predbat GivTCP REST repeatedly setting inverter charge/discharge rate to wrong value, then correct value #5209 misroute for anyone who does have per-inverter settings. It would also have to be decided fleet-wide rather than per key, because deciding it per key reintroduces the index mismatch between keys. The startup log now names the URL and says to remove it from givtcp_rest if that inverter no longer exists.
• The reporter's follow-up asks for battery_calibration to be added to templates/givenergy_givtcp.yaml. I left that out of scope: with givtcp_rest set, that key is auto-configured, and after this fix it comes out full length.
• Blast radius: GitNexus impact rated automatic_config and _keep_configured_tail LOW, and their only caller is run(). detect_changes shows only the GivTCP run → rediscover → automatic_config flows. I found nothing else that reads the order of self.discovered.
• Debug-journal context: the GH#5029 row (PR fix(givtcp): only ever raise num_inverters in automatic config #5037, _keep_configured_tail) and the PR fix(inverter): auto-create charge_rate entity for script-driven "power" inverters (#3311) #4645 bullet, which used the per-inverter list shape as a per-index source proxy, are updated in this PR. The proxy still holds past the last discovered endpoint, but gap slots below it are now filled.

🤖 Generated with Claude Code