[shelly] Tolerate third-party BLE script event data in NotifyEvent frames - #21385
Open
ML19821 wants to merge 3 commits into
Open
[shelly] Tolerate third-party BLE script event data in NotifyEvent frames#21385ML19821 wants to merge 3 commits into
ML19821 wants to merge 3 commits into
Conversation
…ames Gen2/Gen3 devices running a third-party BLE-proxy script (e.g. Home Assistant's) emit NotifyEvent frames with event "ble.scan_result" whose "data" member is an array. Shelly2NotifyEvent hard-typed that member to Shelly2NotifyBluEventData, so Gson failed the whole frame with "Expected BEGIN_OBJECT but was BEGIN_ARRAY", discarding every event in it (including unrelated ones such as button pushes) and logging the full payload several times per second on affected devices. The "data" member is now kept as a raw JsonElement and resolved lazily via getBluData(), which returns the BLU payload only when the element actually is a JSON object; the binding's own oh-blu.* scanner format parses exactly as before. ble.scan_result events are skipped explicitly so they are neither re-parsed per event nor logged as unhandled. Signed-off-by: Martin Littkovsky <2018turtle@proton.me> AI-assisted-by: Claude Code
ML19821
force-pushed
the
fix/shelly-notifyevent-data-shape
branch
from
August 14, 2026 05:11
488043d to
11d6a06
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Shelly Gen2/Gen3 event handling to tolerate third-party BLE scanner payloads without discarding entire frames.
Changes:
- Stores polymorphic event data as
JsonElementand lazily parses BLU payloads. - Silently skips
ble.scan_resultevents. - Adds tests for object, array, and absent data shapes.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
Shelly2ApiJsonDTO.java |
Adds raw event data handling and BLE scan constant. |
Shelly2RpcSocket.java |
Routes BLU data lazily and skips third-party scans. |
Shelly2ApiRpc.java |
Adds fallback handling for skipped scans. |
ShellyBluApi.java |
Uses lazy BLU payload conversion. |
Shelly2NotifyEventDataShapeTest.java |
Tests supported event data shapes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…data Deserializing the BLU payload directly through Gson let a JsonSyntaxException escape for an object-shaped but malformed payload. Before this PR the whole frame went through ShellyUtils.fromJson, which wraps that into the checked ShellyApiException - and that is what both call sites catch, so the unchecked exception would have escaped Shelly2RpcSocket.onMessage and ShellyBluApi.onNotifyEvent instead of being logged with the payload. getBluData() now routes the conversion through the same helper and declares the checked exception. A malformed object therefore fails exactly as it did before, rather than silently returning no BLU data: the device did send a BLU event, and dropping it without a trace would hide a real problem. The array-shaped case that this PR is about is unaffected - it returns before any conversion happens. Signed-off-by: Martin Littkovsky <2018turtle@proton.me> AI-assisted-by: Claude Code
wborn
reviewed
Aug 15, 2026
wborn
left a comment
wborn
left a comment
Member
There was a problem hiding this comment.
AI review before manual maintainer review.
The ble.scan_result parsing fix looks good, and the follow-up commit correctly preserves the existing ShellyApiException boundary. One issue remains around mixed NotifyEvent frames, plus one smaller robustness suggestion.
A mixed-frame routing test covering ble.scan_result together with multiple regular events would also help guard the behavior this PR introduces.
…once onNotifyEvent() walks the whole frame itself, but the frame was handed to it once per non-BLU event in that frame. A frame carrying two regular events was therefore processed twice, so both events counted twice - a button press counted as two. This is older than the polymorphic data handling, but that change is what makes mixed frames parseable in the first place: a third-party ble.scan_result next to regular events used to fail the whole frame, so this path was never reached for them. The frame is now forwarded once, after the loop has determined that it contains at least one regular event. ShellyBluApi additionally deserialized the data of every event as a BLU payload before looking at the event name. Only oh-blu.* events carry one; unrelated payloads were misread, and since the complete hub frame reaches this handler, a malformed unrelated payload could abort the whole frame. Adds routing tests for a frame with several regular events, a mixed frame, a scan-result-only frame and a single-event frame. Signed-off-by: Martin Littkovsky <2018turtle@proton.me> AI-assisted-by: Claude Code
wborn
reviewed
Aug 15, 2026
wborn
left a comment
wborn
left a comment
Member
There was a problem hiding this comment.
The two previous AI review findings are addressed in 009219c7: NotifyEvent frames are now forwarded to the hub handler only once, and BLU payload conversion is restricted to oh-blu.* events. The added mixed-frame routing tests cover the duplication regression and the new ble.scan_result handling.
AI found no further issues in the current changes. A human maintainer should still review the PR before merge.
lsiepel
approved these changes
Aug 15, 2026
lsiepel
left a comment
lsiepel
left a comment
Contributor
There was a problem hiding this comment.
Thanks, LGTM
Awaiting confirmation from @markus7017
lsiepel
added
the
enhancement
Contributor
|
I come back here when #21287 is merged, from a first quick check it looks ok |
4 tasks
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.
Description
Fixes #21384.
Gen2/Gen3 devices running a third-party BLE-proxy script (e.g. the one Home Assistant's Shelly integration installs to use the device as a Bluetooth proxy) emit
NotifyEventframes with eventble.scan_resultwhosedatamember is an array.Shelly2NotifyEventhard-typeddataasShelly2NotifyBluEventData(the object shape of the binding's ownoh-blu.*scanner script), so Gson failed the whole frame withExpected BEGIN_OBJECT but was BEGIN_ARRAY, discarding every event in it — including unrelated ones such as button pushes sharing the frame — and dumping the full payload into the DEBUG log several times per second per affected device.Changes:
Shelly2NotifyEvent.datais now a rawJsonElementwith a lazygetBluData(Gson)accessor that returns the BLU payload only when the element actually is a JSON object. The two call sites that read the old field were converted; the binding's ownoh-blu.*object format parses exactly as before (same plainGsoninstance, no custom adapters in the bundle).ble.scan_resultevents are skipped explicitly inShelly2RpcSocket.onMessage; since the second review round the frame is forwarded toonNotifyEventonce, after the loop, and only when it carries a regular event (a scan-only frame is not forwarded at all). A belt-and-braces no-op case inShelly2ApiRpc.onNotifyEvent's switch covers the mixedoh-blu.*path. At most a trace log — the DEBUG log stays quiet.Side effect worth noting: a frame carrying both a
ble.scan_resultand a real event (e.g. a button push) was previously discarded wholesale; the real event is now processed.Testing
Shelly2NotifyEventDataShapeTestcovers the threedatashapes (object → populated BLU data; Home-Assistant-style array → parses without exception, no BLU data; absent → no BLU data). Full bundle suite: 715 tests, 0 failures (710 at submission, +1 in review round 1, +4 mixed-frame tests in round 2); spotless/checkstyle/PMD/SpotBugs pass. The pre-existing BLU tests double as the regression proof for theoh-blu.*object format.ble.scan_resultevents skipped in the first two minutes (visible at TRACE).Closing:
Signed-off-by (in commit): Martin Littkovsky 2018turtle@proton.me