Skip to content

fix: surface HA request error bodies + dedupe Growatt charge-rate writes (#741) - #743

Open
pra wants to merge 6 commits into
johanzander:mainfrom
pra:fix/issue-741-charge-rate-dedupe
Open

fix: surface HA request error bodies + dedupe Growatt charge-rate writes (#741)#743
pra wants to merge 6 commits into
johanzander:mainfrom
pra:fix/issue-741-charge-rate-dedupe

Conversation

@pra

@pra pra commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Together these address #741 (surface the body + reduce write churn).

What we observed (and what we don't claim)

On a Growatt Open API V1 (cloud / growatt_server) install, control writes intermittently fail with growattServer.exceptions.GrowattV1ApiError: Error during writing parameter (ac_charge / charge_power / time segment). The API returns a generic error — no explicit rate-limit or quota message — so the exact cause is unconfirmed. The failure pattern (rejections cluster later in active periods, then recover) is consistent with a write-rate/quota limit, but we're not asserting that as fact.

What we can say empirically: after #402 (grid_charge/discharge_rate dedup) and #568 (TOU write deferral), the charge-rate register was still re-sent every scheduler tick from adjust_charging_power's direct-write path (power monitor disabled), because it bypasses _write_period_to_hardware's dedup. Cutting those surplus writes coincided with fewer rejections in a multi-day before/after (below). This is a likely contributor to the failures — the same framing #402 used ("a likely contributor to GrowattV1ApiError").

The failure was also hard to diagnose: _api_request logged only str(e) ("500 Server Error … for url …"), discarding response.text.

Fix

  • ha_api_controller._describe_request_error()str(e) plus response.text when present; used in the retry and final-failure logs. No behaviour change beyond logging; timeouts/connection errors (no response body) are unaffected. In field use it surfaced, e.g., a startup-timing {"result":"error","message":"System is not ready with state: setup"} that the old format hid; for the Growatt/Nordpool cloud errors it (correctly) shows the body is a generic 500, pointing to integration-level debug.
  • InverterController.write_charge_rate_if_changed() — mirrors the existing _last_written_* / dedupe_register_writes pattern; skips a charge-rate write when the value is unchanged, leaves _last_written unset on a failed write so the next tick retries. adjust_charging_power's direct-write path routes through it. SolaxModbusGrowatt TOU (dedupe_register_writes = False) still writes unconditionally, unchanged.

Control/apply layer only — no optimizer, intent, or DP change.

Test plan

  • write_charge_rate_if_changed: RED→GREEN unit tests — unchanged rate skips the write, changed rate writes, failed write retries next call.

  • _describe_request_error: body appended when present, plain str(e) when absent/timeout.

  • core/bess fast suite 1672 passed, slow suite 564 passed, 0 failures. (backend/tests bot-pipeline failures on the dev machine were a missing-jq env gap, unrelated.)

  • Live before/after on a real Growatt-cloud install (small sample, absolute counts — not a rigorous failure-rate %):

    before (no charge-rate dedup) after (with dedup)
    charge_power write rejections ~6 in one afternoon Sep 7: 4, Sep 8: 0, Sep 9: 4, Sep 10: 0
    dropped writes (retries exhausted) 0 0 over 3 days

    Reading: a reduction, not elimination — rejections still occur in clusters on active evenings (likely legitimate per-period rate changes the dedup can't remove, plus other writes). No charge/discharge window was dropped. Not proof of a quota, and the sample is small.

Addresses #741.

pra and others added 3 commits September 6, 2026 17:16
The charge-power-rate register is written from
BatterySystemManager.adjust_charging_power (power monitor disabled), which
bypasses _write_period_to_hardware's johanzander#402 dedup. It therefore re-sent an
unchanged rate to the Growatt cloud on every scheduler tick — the last
per-tick write-churn source after johanzander#402 (grid_charge/discharge_rate dedup)
and johanzander#568 (TOU write deferral). Field logs on a Growatt Open API V1 (cloud)
install showed these surplus writes exhaust the account's daily parameter-
write quota, after which control writes fail with GrowattV1ApiError
("Error during writing parameter") and planned charge/discharge windows are
silently dropped.

Add InverterController.write_charge_rate_if_changed(), mirroring the existing
_last_written_* / dedupe_register_writes pattern, and route the direct-write
path through it. Unchanged rates are skipped; a failed write leaves
_last_written unset so the next tick retries. Control/apply layer only — no
optimizer, intent, or DP change. SolaxModbusGrowatt TOU (dedupe_register_writes
= False) still writes unconditionally, unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nder#741)

The Growatt/switch/number write path (and every other _api_request caller)
logged only str(e) on failure — "500 Server Error ... for url: ...", with no
body. The body carries the real reason (a Growatt cloud rate-limit message,
InfluxDB's "no database", etc.), so a one-line misconfiguration or a
rate-limit surfaced as an opaque 500 with no path to the cause.

Add _describe_request_error(e) — str(e) plus response.text when present — and
use it in both the retry-attempt and final-attempt log lines. No behaviour
change beyond logging; timeouts/connection errors (no response) are unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@bess-agent

Copy link
Copy Markdown
Collaborator

Reviewed the diff: well-scoped, follows the existing _last_written_* / dedupe_register_writes dedup pattern (#402), RED→GREEN unit tests added, CHANGELOG updated, no optimizer/intent/DP changes. No correctness issues found — set_charging_power_rate has a single production writer on this path (power_monitor.py:268 is mutually exclusive by power_monitoring_enabled), so the new cache stays coherent, and a failed write leaves _last_written_charge_rate unset for retry.

Minor (non-blocking): the _last_written_* block comment in inverter_controller.py still says these are written by _write_period_to_hardware, which no longer holds for _last_written_charge_rate.

This will be approved once the live Growatt-cloud observation box in the description is checked and CI is green.

pra and others added 2 commits September 10, 2026 20:32
…ed mechanism)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…hanism)

The daily-quota mechanism is inferred from the failure pattern, not confirmed
(the Growatt API returns a generic error). Reword the write_charge_rate_if_changed
docstring, the adjust_charging_power comment, the test docstring, and the
_describe_request_error example to say 'appears to contribute to intermittent
write rejections / exact cause unconfirmed' rather than asserting a quota.
No code behaviour change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@pra
pra marked this pull request as ready for review September 10, 2026 18:39
CI "Code quality" (mypy) flagged 5 new errors the local run had missed
because mypy was not installed in the venv:

- battery_system_manager: union-attr on the new write_charge_rate_if_changed
  call — narrowed the InverterController | None attribute with an assert
  (is_configured already guarantees it, but the property can't narrow it).
- inverter_controller: typed write_charge_rate_if_changed's parameters and
  added a TYPE_CHECKING import for HomeAssistantAPIController.
- test_controller_coverage: annotated the 3 new TestChargeRateWriteOnChange
  methods and imported MockHomeAssistantController for the type hint.

No behaviour change — annotations and an assert only. The 3 johanzander#741 tests
still pass; black/ruff clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants