fix(inverter): stop spurious "REST failed to setExportTarget" warnings - #4413
Merged
Conversation
rest_setDischargeTarget verified the write with a raw string against an int - GivTCP reports the raw invertor registers as strings, so '4' == 4 was always False and the write never counted as successful. Every export slot burned all five retries (five redundant register writes) and then recorded an error status, even though the register had actually taken the value. rest_setReserve already coerces with int(float()); do the same here and report the value read back on failure. adjust_force_export also collapsed an unreadable target to 0 in both branches. That sentinel was harmless while the guard was "current > reserve_percent", but since it became "current != target_soc" an inverter that does not expose the register looks like a target parked below the reserve and gets written on every cycle. Keep it as None and skip the write instead, so the bidirectional tracking still applies to real values. Also raise INVERTER_REST_TIMEOUT from 5 to 10 seconds to give a busy GivTCP more room to respond. Fixes #4404 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes spurious export-target warning noise for GivEnergy GivTCP REST users by making export-target read-back verification type-safe and by avoiding writes when the export-target value is unreadable/missing (e.g., inverter models without that register).
Changes:
- Treat REST read-back values as strings and coerce before comparing so successful export-target writes can be recognised.
- Avoid writing export-target when the current value cannot be read (REST v3 and entity paths), preventing repeated warnings/errors on inverters lacking the register/entity.
- Increase REST request timeout to better tolerate slower/busier local GivTCP instances, and add regression tests for the above behaviours.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/predbat/inverter.py | Fixes REST export-target read-back verification; skips export-target writes when current target is unreadable; improves failure messaging. |
| apps/predbat/const.py | Increases REST timeout from 5s to 10s to reduce timeouts on busy REST endpoints. |
| apps/predbat/tests/test_inverter.py | Adds regression test covering string read-back success and “missing/unreadable target” skip behaviour for REST + entity paths. |
Comment on lines
+3395
to
+3399
| # GivTCP reports the raw registers as strings, so coerce before comparing or a | ||
| # successful write reads back as '4' and never matches the int target | ||
| result = self.rest_data.get("raw", {}).get("invertor", {}).get("discharge_target_soc_1", None) | ||
| try: | ||
| result = int(float(result)) |
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 #4404
Problem
Since v8.47.1, GivEnergy REST (GivTCP) users get
Warn: Inverter 0 REST failed to setExportTargeton every export slot, and the run endsExporting with Errors reported. @davemilsom2 bisected it to 8.47.1 and pointed at #4384, which is right — that PR is the onlyinverter.pychange in v8.47.0..v8.47.1.There are two defects, one old and one new. They only combine from 8.47.1.
1.
rest_setDischargeTargetcould never report success (pre-existing)The write was verified with:
GivTCP reports the raw invertor registers as strings. From the attached debug file:
So
'4' == 4is always False. Every call burned all five retries — five redundant register writes and fiverunAllpolls — and then recorded an error, even when the register had taken the value.The reporter's log proves the write worked:
Set export target slot 1 ... via REST failed, 0 successesCurrent discharge target is already set to 4.0— that path inadjust_force_exportcoerces withfloat(), so it sees the value the verify loop refused to match4.0rest_setReserveimmediately above already does this correctly withint(float(...)).2.
0was doing double duty as "could not read it" (the regression)Both branches of the export target block fall back to
current = 0when the value is None or unparseable. While the guard wascurrent > self.reserve_percentthat sentinel was harmless — it never triggered a write. #4384 changed it tocurrent != target_soc, so an unreadable register now looks like a target parked below the reserve and gets written every cycle.That is @russdan's second symptom, on the entity path:
Predbat writing to an entity that isn't there.
Fixes
rest_setDischargeTarget— coerce the read back value withint(float(...))before comparing, guarded for missing/None, and name the value read in the failure message so the next report is diagnosable.adjust_force_export— an unreadable target staysNoneinstead of collapsing to0, and is skipped rather than written, on both the REST v3 and entity paths. #4384's bidirectional tracking is untouched for real values. The skip is a plain log line, not aWarn:— an inverter that simply has no such register would otherwise print a warning every cycle.INVERTER_REST_TIMEOUT5 → 10 seconds, to give a busy GivTCP more room to respond.Tests
test_discharge_target_read_backinapps/predbat/tests/test_inverter.py:Written first and confirmed failing on the pre-fix code (all six assertions), passing after. The test wraps
record_statusso it asserts only on export target errors, rather than on unrelated fixture noise from the H M format time entities.The existing
test_discharge_target_tracks_reservefrom #4384 used ints inraw, which is why it missed this — real GivTCP sends strings../run_all --quickand./run_pre_commitboth exit 0.Not addressed
@russdan also reported
REST failed to setDischargeSlot1. That is a different comparison (Timeslots, strings on both sides, no type mismatch) and the debug file failed to upload, so there is no evidence to work from yet. It may be collateral: fix 1 removes four spurious POSTs plus fiverunAllpolls per export cycle, which was hammering GivTCP immediately before the slot write. Worth a retest on this branch before chasing it separately.🤖 Generated with Claude Code