Feat: add bms_cooldown_on_error option and clear raw BLE stream buffer during JK-BMS AT-command flood - #392
aka-raveren wants to merge 5 commits into
Conversation
…r during JK-BMS AT-command flood
…dalone mode in _create_client
|
Thanks for digging into this, and for the detailed writeup. The problem you're describing is real — I agree that one BMS failing should not take the others down, and #391 shows the same thing. But I can't merge this as it stands. Going through it in the order the diff appears:
Worth knowing the history too: clearing the buffer is what the code used to do, and it was removed on purpose in 2.04 — "resync framing on the header instead of clearing the buffer (dropped a frame when a packet held two), fixing Also,
So the soft tier logs "Soft programmatic flush completed" while having done nothing but sleep 1 s. That is worse than not having it, because the log claims a recovery happened. The hard tier is actively dangerous. It fires on any exception from any BMS — a single transient On the actual symptoms, two fixes have landed since you opened this, both from #391 — worth retesting on 2.14 before adding recovery machinery:
What I would take. The isolation idea, done narrowly: in serial mode all BMS share one |
|
Thanks for digging into this — the symptom is real and #370 needed attention. I've reworked it rather than merged it, because most of the recovery machinery doesn't actually execute:
The The jikong iteration went the right way — stripping only leading What's landing instead:
I dropped For the record, my first cut of the escalation had a worse bug than anything in this PR: I keyed the counter on the forced-disconnect branch, which requires If you can still reproduce the BlueZ |
… junk (#392) A proposed fix for the JK-PB 'AT\r\n' flood flushed self._buffer whenever feed_frames reported dropped bytes. But the junk is already deleted at the moment it is counted, so what is left buffered is the header-aligned head of the frame *behind* the junk - and dropped>0 together with a partial frame is exactly what an interleaved flood produces. Flushing there turns a recoverable split frame (#377) into a lost one, and the following tail then counts as junk too, so it repeats. Check the real invariant instead: feed_frames consumes or trims on every iteration, so it cannot return with a whole frame still buffered. If it ever does, log it and resync rather than letting the buffer grow silently.
…ts (#392) Recovery for a host stack that wedges and answers every connect with "Operation already in progress". Lives next to the existing per-BMS backoff rather than in main.py's serial fetch loop, which saw only "some BMS raised" and so cycled the radio for a single hiccup on any one of them. It escalates only after repeated error-forced reconnects of the same BMS have failed to recover it, resets on that BMS's next good sample, skips wired and virtual BMS, and is rate-limited process-wide because all samplers share the controller. Goes through bt_power() rather than hciconfig so the ble_stack guards apply: with bumble the adapter is owned by bumble, and with esphome there is no local adapter to cycle. Off by default - it drops every BLE connection, including Home Assistant's own.
… BLE-only (#392) Three defects found reviewing 120a513: The escalation counter only advanced inside the `bms.is_connected` forced- disconnect branch, so it never advanced in the case the option exists for: a host answering every connect with "Operation already in progress" never gets the link up, so is_connected stays False and the count sat at zero forever. 50 consecutive failures produced no cycle. Count the saturation round whether or not there was a link to drop. The old tests missed this because they forced is_connected=True on a fake; the real property is read-only. Being cancelled between power-off and power-on left the radio down for good, and main.py cancels the pending fetch loops whenever one returns. Hand the power-on to a task that outlives the cancellation. Escalation also fired for any exception the generic handler caught, including failures raised after fetch() had already returned a sample - a downstream sink bug would cycle a perfectly healthy radio. Require a BLE-ish error. Also: bt_power() swallows bluetoothctl failures internally, so the old "power cycle done" claimed more than was verified; and a rate-limited skip consumed the per-BMS history, making the BMS earn its way back from zero while still wedged.
The Problem:
Some JK BMS (Jikong) models running specific inverter-interfacing firmware versions actively broadcast non-protocol telemetry text messages starting with
AT\r\n(AT-command flood) onto the shared BLE/UART bus (as referenced in issue #370).When heavy load, solar inverter EMI, or prolonged connection sessions cause data packet fragmentation, these junk bytes accumulate exponentially inside
BleakClientincoming streams. Under the original code logic,bmslib/models/jikong.pycounts these bytes (discarded junk bytes) but leaves them insideself._buffer. Within a few poll cycles, this unmanaged binary noise chokes theasyncioloop queue, leading to continuousTimeoutErrorspikes. Worse, the Linux host Bluetooth daemon (BlueZ) eventually drops into a critical deadlock state (org.bluez.Error.InProgress / Operation already in progress), which completely freezes data fetching for all other healthy, connected BMS units (e.g., JBD).The Solution:
This PR introduces a two-tier escalation recovery strategy controlled via
options.json / config.yaml:Low-Level Junk Buffer Suppression (
bmslib/models/jikong.py):Directly inside the
if dropped:routine, we execute.clear()onself._bufferon the fly. This instantaneously vaporizes theAT\r\ntext flood out of the active Python RAM before it can bottleneck the async parser. Crucially, the valid static device configuration mappings inside_resp_table(such asnum_cellsindex mapping) are completely preserved, preventingKeyErrororIndexErrorcode crashes.First Tier: Software Isolation (
bms_cooldown_on_error):If a sampling timeout still slips through, the main loop catches the error, forces a programmatic
.close()sequence on the troubled client, and wipes the static Bleak GATT descriptors memory cache usingBleakClient._gatt_cache.clear(). This isolates the faulty battery, giving its internal MCU buffer exactly one polling period of silent cooldown time to auto-reset, while allowing other adjacent healthy BMS nodes to continue reporting data to Home Assistant without a single dropped packet.Second Tier: Hardware Remediation (
bt_power_cycle_on_error):If the software cache flush fails to bypass a severe OS-level kernel freeze, this option executes a hot power cycle of the
hci0radio controller interface utilizinghciconfig hci0 down/uphooks with proper hardware delay sleep constraints (3 seconds), safely reviving the host adapter state without restarting the daemon container.Testing:
Tested in a real-world multi-BMS ecosystem (JK BMS + JBD BMS) running continuously for hours under volatile grid load. Programmatic cache and stream clearing successfully bypasses 3800+ junk byte/sec streams on the fly, reducing host CPU overhead and achieving unbroken long-term data collection stability in Home Assistant Core.