Repository navigation
fix(hanchu): event-loop write fix, #5672 review fixes, model ratings, no Freeze Export, hold backstop, docs - #5684
Conversation
…n loop Predbat presses the Hanchu write switch from its plan thread through ha.py's run_async(), which runs the event on a throwaway event loop. The component wrote from there with its persistent aiohttp session and asyncio write_lock, both of which belong to the loop run() ticks on, so every press failed with "Timeout context manager should be used inside a task". The retries also blocked the plan thread for about 12 s. The change then reached the device only on the component's next tick, about a minute later. run() now records its loop, and _handle_control_event hands the write over to it with run_coroutine_threadsafe - the same hand-over gateway.py's _publish_raw() makes. The wait is shielded and capped at HANCHU_LOOP_HANDOVER_TIMEOUT, so a stuck loop cannot hold the plan thread and giving up the wait never cancels the write. On the component's own loop, or with that loop not running, the write runs in place as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Power ceiling: raise the menu's 0-5000 W range to the highest value the device has ever been seen holding (persisted with the config cache), not the value held now - Predbat's own write overwrote it, so the 6 kW raise undid itself after the first write (finding 1). - Drift: compare each control read-back with the applied payload and forget it on a mismatch, so a change made in the Hanchu app, a device reset or a lost write is re-sent (finding 2). - HTTP 401: re-authenticate on the status before the body's shape is checked, so an empty or HTML 401 body no longer skips re-auth (finding 3). - Units: the below-10-means-kW heuristic applies only to fields with no unit sibling (loadPwr), not to grid and battery power (finding 7). - Hold: decide the zero charge rate after the hold slot is placed, so a hold with no room leaves the plan's charge rate alone (finding 8). - Midnight: clamp a 24:00:00 end to 23:59 so TCT_END is never 86400 (finding 9). - Logging: the API trace is now hanchu_api_debug (default off), and "settings unchanged" is only logged at debug level with the trace on (finding 10). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e backstop - battery_rate_max and inverter_limit come from a devModel -> rating table (HANCHU_MODELS) built from the console's pcsEnMap and the manuals, with the menu/held-value derivation kept for unknown codes and the user overrides still winning. The table's battery limit also raises the power clamp ceiling, so a 10 kW unit can be asked for 10 kW. The model name is logged once and reported in the discovery record. The default-power warning waits for the first status poll, and is not given for a model the table covers (#5664). - support_discharge_freeze is False for HanchuCloud: CHG_PWR_LMT only limits charging from the grid, so a zero charge rate cannot stop solar filling the battery (#5665). - During a hold the grid-charge limit (DTU_AC_CHG_SOC_LMT) is capped at the current SoC, so a 0 W charge slot whose write did not land cannot grid charge to a later window's target. A limit already written at or below the SoC is kept, so solar raising the SoC does not send a write every percent (#5667). - templates/hanchu.yaml: say that Predbat clears the Hanchu app's time slots at its first write and why, that Freeze Export is unavailable, and how the ratings are found; drop the stale "every request is traced" line (#5668). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs/inverter-setup.md had no section for the native component, and its Hanchu iESS section still said there was no native integration. Add a "Hanchu ESS Cloud" section and table row covering setup, control behaviour (app slots cleared, no Freeze Export, hold grid-charge cap, SoC limits, write pacing), the devModel power ratings and the diagnostics CLI, and point the Service API section at it. Also correct _apply_model's docstring: device_detail reaches the static cache only at the next static refresh, not straight away. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
springfall2008
left a comment
There was a problem hiding this comment.
Automated comment from the triage bot.
Review of head 0ac65fd2 (the combined branch superseding #5682, #5674 and #5680): 1 finding inline (low; it is the body's own open point 3, confirmed still present in this head), and nothing new beyond it. The prior PR-by-PR findings carry over cleanly into the combined history.
Checked against the head itself (read of the checkout, not main):
HANCHU_MODELS(apps/predbat/hanchu_const.py) matches #5664's table line for line — all 32 codes and the battery/AC figures, including the battery entries marked "not stated" for 28 (HESS-HY-S-6K-S) and 29 (HESS-HY-S-12K-S)._note_external_change()'s slot-drift coverage is real in production, not just in the test's mock: the iotGet read-backkeysargument isHANCHU_CONTROL_KEYS(hanchu_const.py:347), which includes all twelveTCT_/TDT_slot keys as well as the six scalars, so an app-edited slot time actually is compared. (I went in suspecting the mock was generous here — it is not.)- The 401 fix covers both body shapes: a genuinely empty 401 body makes
response.json()raiseValueError, which the new innerexceptcatches intopayload=None— the same path the test'sunparseable_response(401, ValueError(...))models for an HTML body — so both re-authenticate. inverter_record()(apps/predbat/coordinator.py:1148) takesinfo=natively and drops an empty dict, so"info" in recordbeing False for an unknown model matches the builder's own convention.- The 38/38 registration claim on
test_hanchu_control.pyholds: 38def test_hanchu_*definitions, 38 registered tuples, no duplicate names and no duplicate registrations. split_window_seconds()'s clamp now agrees withhhmmss_to_seconds()for a24:00:00end;HANCHU_TTL_CONFIG = 30matches the new doc's "(every 30 minutes)"; the CLI keeps the trace on by default (hanchu.py:2491,api_debug = not args.no_trace).ha.pyrun_async()(apps/predbat/ha.py:50) confirmed as the throwaway-loop press path the handover fixes, withgateway.py:2687as the precedent.
On the two decision points the body asks about:
- The table raising the write clamp above the menu (open point 1) — I would keep the current shape. The refusal path is loud where it matters: a partial accept is not cached as applied (
hanchu.py:1846), so it is rebuilt and resent each paced cycle, a per-key refusal from the ack map is named in the log, and the user is notified after three consecutive failures. The "safer" shape has the opposite cost: a fresh 10 kW unit whose settings still hold 5000 W would be planned at 10000 and then have every window's write clamped back to 5000 behind a single one-time clamp warning — quiet, systematic derating that nobody is told about. If a larger unit does refuse, the failure is diagnosable from the log and reversible in one line; the derating of the alternative is neither. - The in-window hold keeping the written grid limit (open point 2) — agree it is harmless, and it is actually correct as written: at a target-met hold, SoC >= target, so the cap is the target itself and any kept lower written value is below the SoC — grid charging cannot reach the limit even when the zero-power write did not land, so the fallback charges no more than the plan asked for. A written value above the cap (SoC having fallen below it) is re-derived, as the test's third case shows. I see nothing that needs restricting.
The body's still-open items (#5672 findings 5 and 6, cross-component restart schedule write) are as agreed — nothing to add from this review.
| merged[key] = (low, int(held)) | ||
| if (sn, "held", key) not in self._clamp_logged: | ||
| self._clamp_logged.add((sn, "held", key)) | ||
| self.log("Info: Hanchu {} holds {} {:.0f} W, above the menu's {} W maximum (a range the menu shares across models), so {:.0f} W is used as its limit".format(sn, key, held, high, held)) |
There was a problem hiding this comment.
Automated comment from the triage bot.
Confirmed in this head, and it is open point 3 in the body — this line can now quote a figure that came from HANCHU_MODELS, not the menu. After the table raise above (battery_limit > high), high here is the table's battery limit wherever the menu said nothing or said 5000: the T-06K-and-above models are not in the menu's power item at all (#5664's menu dump), so device_ranges holds no bound, merged is the built-in (0, 5000), the table raises high to e.g. 6000 for a HESS-HY-T-06K, and any held value above 6000 logs "above the menu's 6000 W maximum" — where the menu said nothing and 6000 came from the table. Cosmetic and once per (device, key) via _clamp_logged, but the provenance it claims is now wrong. Simplest fix is to reword this one line to what the figure actually is, e.g. "above the {} W power ceiling (menu or model)".
There was a problem hiding this comment.
Fixed in 66edcb61 and d6256b0a. The model table no longer raises the ceiling at all: ranges_for() computes min(max(menu, highest held), battery limit). The "held above the menu" line now fires only when the result is actually above the menu's bound, and it quotes the menu's own figure and the capped limit used. Covered by test_hanchu_model_battery_limit_caps_a_higher_held_setting.
… to accept The devModel table raised the CHG/DSCHG write clamp to the model's battery limit, above the settings menu's 0-5000 W, with nothing showing the cloud accepts that. A 7-10 kW unit that refused e.g. CHG_PWR_LMT=10000 would turn every write into a partial accept, resent each cycle with failure notifications and no applied_payload. The table now sets only battery_rate_max and inverter_limit. What is written stays within the menu's maximum or a higher limit the device has been seen holding. Where the rating is above that, the gap is logged once per key after a settings read. This also removes the case where the "holds ... above the menu's maximum" log quoted the table's figure as the menu's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…limit A HESS-HY-S-6.0K holds DSCHG_PWR_LMT 6000 against the menu's 0-5000 W but still exports at its 5000 W battery limit, so a held setting above the battery limit moves no more power. ranges_for() now caps the ceiling at the model's battery limit where HANCHU_MODELS knows it, and the 6.0K's export power control no longer advertises 6000 W. A larger unit seen holding above the menu keeps that, up to its own battery limit, and an unknown model is unchanged. Also corrects the rationale repeated in ranges_for(), inverter_limit() and a test docstring, that taking the menu's word "would cap every discharge below the inverter's rating" on the 6.0K. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
springfall2008
left a comment
There was a problem hiding this comment.
Automated comment from the triage bot.
Final review of head d6256b0: 2 findings inline, both low, 0 in the body. No correctness issue blocks merging. No code was run, only read; CI's pre-commit job (which runs the quick suite) passed at this head.
What I re-checked across the combined change, beyond each part's own earlier review:
- Event-loop handover vs #5672 drift detection. A handed-over write now runs on the same loop as
refresh_config()._note_external_change()skips any read that began before a write'slast_write_time, or withinHANCHU_READBACK_SETTLEof it, so an event write cannot be judged as drift by a read already in flight. - Model table vs
power_ceiling.ranges_for()is nowmin(max(menu, highest held), battery limit)for a known model, and unchanged for an unknown one. The 6.0K (held DSCHG 6000) gives 5000/5000 with no raise or gap log. A 10 kW unit gets min(held, 10000), and the "held above the menu" log quotes the capped figure. The published power controls'maxand the write clamp both read this, so they agree. The daemon's earlier point (the log quoting a table figure as the menu's) is gone, because the table no longer raises a ceiling. - Hold backstop vs drift and partial accepts.
_hold_grid_target()'s keep rule readsapplied_payload. That is popped on drift and never set on a partial accept, so a stale value is never kept. Open point 2 in the PR body (the keep rule also applies to the in-window target-met hold) is still present and still harmless; I have not re-posted it. support_discharge_freeze: False.INVERTER_DEF,HANCHU_CAPABILITIESandtest_hanchu_inverter_def_completeagree. execute.py forces freeze export off fleet-wide, and the docs quote the log line it writes.- Tests and docs. Every new test is registered (the control and API runners, with no duplicates after the cherry-pick merge). Each change's test asserts the behaviour it changes. The docs in
docs/inverter-setup.mdandtemplates/hanchu.yamlmatch the code at this head, including the battery-limit cap and the 30-minute drift re-read.
From the final review of #5684. _on_component_loop() only caught TimeoutError, so two shutdown/restart paths misbehaved: - asyncio.run() ending on the component's loop cancels the write in flight, and the CancelledError escaped the hand-over, killing run_async()'s thread with a traceback. It is now logged and returns False, while a cancellation of the caller itself is still re-raised. - The loop could stop between the is_running() check and the queued write starting, which then never ran, and the caller waited the full 120 s. The wait is now taken in HANCHU_LOOP_HANDOVER_POLL steps and abandoned as soon as the loop is no longer running. Also reword HANCHU_LOOP_HANDOVER_TIMEOUT's comment: 120 s is not above a write's worst case, and it does not need to be, because giving up only stops the waiting. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r debugging hanchu_api_debug now defaults to True. The Hanchu API is undocumented, so a customer's log needs the request/response trace when something goes wrong, and asking them to turn it on and wait for the problem to recur loses the evidence. This matches the Sunsynk and AlphaESS components, which also trace by default. hanchu_api_debug: False quietens the log once things work. The template and docs say so. MockHanchu passes api_debug=False explicitly, so the other tests' log assertions don't wade through traces. test_hanchu_api_trace_is_on_unless_turned_off pins the real default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One PR for the Hanchu ESS cloud component's open fixes. It supersedes #5682, #5674 and #5680, which are closed in favour of it. Each original is one or two commits here, in merge order, so the history still reads PR by PR:
0cf78975,4692095c268827b2f39a5a06,0ac65fd266edcb61d6256b0a47806948c5b4dad9Closes #5664, closes #5665, closes #5667, closes #5668. Addresses #5672 findings 1, 2, 3, 7, 8, 9 and 10 (4 is answered by hardware evidence; 5 and 6 are still open, see below).
1. Plan-thread writes failed and arrived a minute late (#5682) - a bug in v9.4.3
When Predbat's plan thread pressed the schedule-write switch, every write failed with
Hanchu /gateway/app/ha/iotSet transport failure: Timeout context manager should be used inside a task. The component's own tick wrote the payload about a minute later, so every plan change reached the inverter roughly a minute late. The doomed retries also blocked the plan thread for about 12 s per press.Cause: the press arrives through
ha.py's loopbackcall_service→run_async(trigger_callback(...)), which runs it on a throwaway event loop._handle_control_eventthen wrote from there, using the persistentaiohttpsession and theasynciowrite lock, both of which belong to the looprun()ticks on.Fix:
run()records its loop.apply_schedulethrough_on_component_loop(), which hands it over withrun_coroutine_threadsafe, the same hand-overgateway.py's_publish_raw()makes.HANCHU_LOOP_HANDOVER_TIMEOUT(120 s). A stuck loop cannot hold the plan thread, and giving up the wait never cancels the write.47806948), a write its loop cancels is logged rather than escaping as aCancelledError, and a write the stopped loop will never run is given up withinHANCHU_LOOP_HANDOVER_POLL(1 s) rather than after the full timeout.End-to-end check (scratch, not committed): a real
ClientSessionon an owner loop, a local aiohttp server standing in for the cloud, and the press driven through the realrun_async.The trap is recorded in
tools/debug-journal.md.2. Code review findings from #5672 (#5674)
ranges_for()raises the menu's ceiling to the highest value ever seen per device and key (power_ceiling), saved with the config cache (seeded from cacheddevice_settingsfor an older cache).refresh_config()compares each iotGet read-back withapplied_payloadand, on a mismatch, logs the keys and forgets the applied payload, so the next reconcile resends it. Keys missing from the read-back are ignored. A read withinHANCHU_READBACK_SETTLE(60 s) of a write, or one that started before it, is not judged.loadPwr) use it.CHG_PWR_LMT=0was written when no hold slot could be placed24:00:00end gaveTCT_END_1=86400hhmmss_to_seconds()does.api_debug = Truewas hard-codedhanchu_api_debugoption. Sincec5b4dad9it defaults on, at the maintainer's request, so a customer's log carries the trace for debugging, matching Sunsynk/AlphaESS/DEYE.hanchu_api_debug: Falsequietens it. The CLI harness traces unless--no-trace.3. Follow-ups from upton68's tickets (#5680)
HANCHU_MODELStable keyed ongetDeviceStatusdevModel(names from the console'spcsEnMap, ratings from the manuals, both as posted on #5664). Battery limit →battery_rate_max, AC rating →inverter_limit, which is what Predbat plans with. The table never raises what is written: CHG/DSCHG power limits stay within the menu's maximum or a higher limit the device has been seen holding (66edcb61). Where a rating is above that, the gap is logged once per key. The writable limit is also capped at a known model's battery limit (d6256b0a): a HESS-HY-S-6.0K holdingDSCHG_PWR_LMT=6000still exports at its 5000 W battery limit, so a held setting above the battery limit moves no more power, and the 6.0K's export power control no longer advertises 6000 W. Unknown codes, and 28/29 (no stated battery limit), fall back to the menu/held derivation, and user overrides still win. The model is logged once and reported asinfo.modelin the discovery record. The default-power warning waits for the first status poll and is not given for a model the table covers.support_discharge_freeze: False, becauseCHG_PWR_LMTonly limits grid charging, so a zero charge rate cannot stop solar charging the battery. The ≥50% "Maximum Charge SOC" alternative is not done: it needs a guaranteed restore.DTU_AC_CHG_SOC_LMTis capped atmax(int(SoC), 20)in the sameiotSet, so a 0 W write that did not land cannot grid-charge to a later window's target. A limit already written at or below the SoC is kept, so solar raising the SoC does not send a write every percent.templates/hanchu.yamlsays Predbat clears the app's six slots at its first write, and why, and that Freeze Export is unavailable.docs/inverter-setup.mdgains a Hanchu ESS Cloud section and table row (setup, control behaviour, thedevModelratings, the diagnostics CLI), and its Hanchu iESS section no longer says there is no native integration.Review points from #5680
66edcb61. The table's battery limit used to raise the write clamp above the menu's 0-5000 W, and a 7-10 kW unit that refused e.g.CHG_PWR_LMT=10000would have made every write a partial accept. Now the table only sets the ratings, writes stay within proven-accepted limits, andtest_hanchu_model_rating_never_raises_what_is_writtencovers the payload. The trade-off: on such a unit, forced charge and export windows run at the writable limit while the plan models the rating, until the device is seen holding more.Decisions for review
--live-window freeze-exportrun on a DC-coupled unit decides whether it can be relaxed per unit, via thegrid_charge_onlyattribute.Still open, not in this PR
#5672 finding 5 (the export floor is applied only once the window is active, which needs a behaviour decision) and finding 6 (a restart can write an all-disabled schedule). upton68 confirmed finding 6 on a standalone Docker instance, but the same pattern exists in alphaess, fox, deye and sunsynk, so it belongs in a cross-component fix.
Tests
test_hanchu_control.py's test functions andrun_hanchu_control_testsregistration list. Both sides were kept: 38 tests defined, 38 registered, no duplicates../run_all --quickpass on the combined head (4 slow tests skipped), and pre-commit is clean on every changed file.hanchu.py. Grep shows the changed symbols are used only byhanchu.py,hanchu_const.pyand their tests, plus the HanchuCloudINVERTER_DEFrow and thehanchu_api_debugregistry/schema entries.🤖 Generated with Claude Code