Repository navigation
fix(inverter): settle before verifying a GivTCP discharge target write - #4492
Merged
Merged
Conversation
GivTCP's runAll status is a separately-cached snapshot refreshed on its own polling cycle, not synchronously with the setDischargeTarget POST. Reading it back immediately after the write can catch data from before GivTCP has applied and exposed the change, reporting a spurious "REST failed to setExportTarget got 0" even though the write actually succeeded - confirmed against multiple reporters' logs on #4421, where GivTCP's own log shows the write landing at the exact moment Predbat reports the failure. Adds a short settle delay between the POST and each readback in rest_setDischargeTarget, so a slow-to-update cache isn't misread as a failed write.
…rge target write Traced against GivTCP's own source (write.py's setDischargeTarget()): Control.Discharge_Target_SOC_1 is updated synchronously the moment GivTCP accepts a write, via updateControlCache(). raw.invertor.discharge_target_soc_1, which rest_setDischargeTarget() was checking exclusively, is only refreshed by GivTCP's separate background self_run poll cycle - tens of seconds away on some installs, not remotely covered by a short settle delay. Check Control.Discharge_Target_SOC_1 first, falling back to the raw register only if Control doesn't have the key. The settle delay from the previous commit is still useful for the much smaller residual window (the physical inverter itself taking a moment to apply the write).
This was referenced Aug 14, 2026
Closed
springfall2008
added a commit
that referenced
this pull request
Aug 15, 2026
…ng whether to write (#4518) * fix(inverter): also prefer Control.Discharge_Target_SOC_1 when deciding whether to write adjust_force_export() still decided whether a discharge target write was needed by reading raw.invertor.discharge_target_soc_1 alone - the same slow, self_run-poll-refreshed field #4492 moved away from as the primary signal inside rest_setDischargeTarget() itself. On hardware where that field never catches up, the write looked permanently "needed" and fired every cycle even when nothing had changed (#4517). rest_readDischargeTarget() centralises the same Control-first/raw-fallback read used for write verification, so the caller now correctly skips the write once it's actually landed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Trefor Southwell <48591903+springfall2008@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
4 tasks done
springfall2008
pushed a commit
that referenced
this pull request
Aug 16, 2026
…nverters (#4517) (#4543) AC Coupled inverters don't have a working Discharge_Target_SOC_1 register - GivTCP still reports each write as successful, but it never persists between cycles, so adjust_force_export() saw a permanent mismatch and rewrote it every single cycle indefinitely. Confirmed against GivEnergy's own firmware archive (github.com/DJBenson/giv-firmware): "AC Coupled" is a single product line with exactly two firmware releases ever published, not an early/late generational split the way the Hybrid line has Gen1/2/3 - so there's no newer AC-coupled variant this would need to keep working for. Skip the write outright when raw.invertor.model reports "Ac", rather than attempting one already known to be doomed. Supersedes the earlier enable-before-write approach on this same issue - that theory relied on a correlation with a second reporter's data that turned out to be from before #4492/#4518 existed and was confounded by the exact race condition those fixes address, so it was never solid evidence that AC Coupled's register works anywhere. Dropped rather than kept alongside this fix, since it has no remaining supporting evidence. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
2 participants
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.
Summary
Fixes #4421 - spurious "REST failed to setExportTarget got 0" warnings even though GivTCP's own log confirms the write succeeded.
rest_setDischargeTarget()POSTs the write, then immediately GETs GivTCP's/runAllstatus to verify it landed. That status is a separately-cached snapshot, refreshed on GivTCP's own polling cycle rather than synchronously with the POST - reading it back with no delay can catch data from before GivTCP has applied and exposed the change. Traced against multiple reporters' logs on the issue: writes do succeed at inconsistent retry counts (rules out a broken field path, which would fail every time), and GivTCP's own log consistently shows the write landing at the exact moment Predbat reports the failure.Fix: a short settle delay between the POST and each readback (every attempt, not just between retries).
Test plan
test_discharge_target_settle_delayintest_inverter.py) simulating the exact race: a stale cached readback followed by the real post-write value, asserting the write is still recognised as successful and that a settle sleep happens before each readback../run_all --quick- all passing./run_pre_commit- clean🤖 Generated with Claude Code