fix(inverter): retry a service call the HA layer rejected instead of deduplicating it away

[chalfontchubby]

Written by Claude, posted on behalf of @chalfontchubby.

Fixes #4876. Originally raised in discussion #4866.

call_service_template() recorded its deduplication hash before making any of the calls, and discarded each call's return value:

else:
    # Record the last service called
    self.base.last_service_hash[hash_index] = this_service_hash   # recorded up front
    service_repeat = False
...
    self.base.call_service_wrapper(service_name, **service_data)  # result discarded

A rejected call was therefore remembered as done, and every later cycle carrying the same target logged Skipped service ... as it was previously called. Predbat believed it had set a control it had not, with nothing to correct it until the payload itself changed — which for a held charge target can be hours.

Reported twice, on unrelated hardware

• battery left on HoldChrg @ 7% during IOG off-peak period #2903 — a Libbi left un-charged for a 3.5 hour cheap-rate window after a failed switch.myenergi_libbi_charge_from_grid turn-on. Plan was correct throughout (Charging target 7%-100%); the switch call simply never landed and was never retried.
• charge_limit/program_point writes via SK driver show same "verification discarded" pattern as #4845 #4876 — a Deye SUN-12K via Solar Assistant left in its idle program, confirmed on the physical display, while predbat.status read Charging. The reporter re-sent the identical select_option by hand from Developer Tools and the battery moved from ~-30W to +850W within seconds.

What this changes

Record the hash only once every call has been accepted, and drop any earlier record when one is rejected, so the next cycle reissues it naturally.

That is exactly what repeat: True on a service template already achieves manually (documented in docs/inverter-setup.md). Neither reporter had it set, and needing it is the thing being removed.

What this deliberately does not do

It is not read-back verification, which is what #4866 originally asked for. As noted in that discussion, the integrations involved cache writes asynchronously, so re-reading an entity cannot settle whether a value actually reached the inverter — the same report's charge_limit path shows a readback trailing the write by a full cycle, which is a separate false-failure problem rather than this one.

This checks only that the service call itself was accepted. That is a fact the HA layer already returns and this code was throwing away.

The return value is left at True. Callers use it to choose a fallback service:

if not self.call_service_template("charge_freeze_service", ...):
    self.call_service_template("charge_stop_service", ...)

Reporting a transient failure there would silently downgrade a freeze into a stop instead of retrying it, so the returned value keeps its existing "was a service configured and dispatched" meaning.

Caveat worth a reviewer's eye

call_service_wrapper returns None on an explicit websocket failure or a two-minute timeout, and the standalone loopback branch returns False when the service is not in EVENT_LISTEN_LIST. trigger_callback's docstring states that shared True/False contract is intentional, and Predbat.com does not drive inverters through HA services — but a vendor-specific service called in loopback mode would now look permanently rejected and be reissued each cycle rather than skipped.

Relationship to #4845

Adjacent, not overlapping. #4845 covers the same act-on-an-unverified-write class at the ten adjust_* notify/MQTT sites, whose failure mode is announcing changes that did not happen. This path has no write_and_poll_value() at all and fails the opposite way — silently, with no retry. #4876's log shows both halves within one cycle of each other.

Testing

New test_call_service_template_retry covering: a rejected call is retried next cycle; an accepted call is still deduplicated as before; and a later rejection drops the earlier record rather than leaving it stale. Confirmed it fails without the fix:

ERROR: a rejected call was recorded as done - the next cycle will skip it
ERROR: the call was not retried after being rejected - this is the #4876 hang
ERROR: the hash from the previous accepted call survived a rejection

./run_all --quick and ./run_all --test debug_cases both pass.

[Copilot]

🟡 Changes recommended

The test cleanup mutates shared runner state and must restore its original values.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes inverter service deduplication so Home Assistant-rejected calls are retried instead of skipped.

Changes:

• Records service hashes only after successful dispatch.
• Clears stale hashes after rejected calls.
• Adds retry and deduplication regression tests.

File summaries

File	Review
apps/predbat/tests/test_inverter.py	Adds regression coverage, but must restore all shared state—including retry_test—during cleanup.
apps/predbat/inverter.py	Updates deduplication state based on service-call acceptance.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced

---

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