Skip to content

feat: dedicated ProtocolLEDENETExtendedCustom + LED count for 0xB6#539

Merged
bdraco merged 5 commits into
lightinglibs:masterfrom
asxzy:feat/add-0xb6-protocol
Jul 1, 2026
Merged

feat: dedicated ProtocolLEDENETExtendedCustom + LED count for 0xB6#539
bdraco merged 5 commits into
lightinglibs:masterfrom
asxzy:feat/add-0xb6-protocol

Conversation

@asxzy

@asxzy asxzy commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Merge order: #538 → this → effects-api → timers → cli. Draft until #538 lands.

The 0xB6 replies only with the extended state frame (0xEA 0x81), never the standard 0x81. #538 made basic on/off work by reusing ProtocolLEDENET25Byte (which already parses that frame). This PR introduces the device's own protocol class so the effect/segment/scribble/timer methods (later PRs) have a home, and exposes the configured pixel count.

Changes

File What / why
protocol.py (+110) New ProtocolLEDENETExtendedCustom (subclass of ProtocolLEDENET25Byte). Adds the 21-byte extended-state probe fork, named_raw_state extended conversion, and extended_state_led_count() (+ LEDENET_EXTENDED_STATE_LED_COUNT_POS).
base_device.py (+17) Store the parsed LED count in process_extended_state_response; add the led_count property and supports_extended_custom_effects.
device.py (+13) Sync-path support for the extended protocol (probe/state handling).
models_db.py (±10) Repoint 0xB6 to PROTOCOL_LEDENET_EXTENDED_CUSTOM.
tests/test_aio.py (+75), tests/test_sync.py (+38) Protocol/state/led_count tests, incl. real captured frames.

Protocol: extended state response (EA 81, 27 bytes, after unwrap)

ea 81 01 00 b6 09 [PWR] 66 01 64 f0 00 00 00 00 64 05 00 [COUNT] 00 00 00 20 02 01 00 03
 0  1  2  3  4  5    6    7  8  9 10                15 16 17   18
  • byte 4 = model (b6), byte 5 = fw version (9), byte 6 = power (0x23 on / 0x24 off), byte 7 = preset (0x66 in scribble/static), byte 18 = configured LED count (0x64=100, 0x50=80, verified against two real frames).

Testing

  • test_extended_state_led_count: asserts 100 and 80 from the two captured frames and None for an invalid/short frame. Existing protocol/state tests extended for 0xB6.
  • pytest green, ruff clean, no new mypy errors.

On-device verification (device 192.168.1.78):

Check Command Result
Discovery flux_led -s ✅ found 7C3E821AE33F / 192.168.1.78
Info / state / model flux_led <ip> -i ✅ recognized as 0xB6 "Surplife Outdoor Permanent Lighting"; power + state parsed
Power --on / --off ✅ strip on / dark
LED count toggled device 100→80 led_count (state byte 18) tracked the change; bitmap sizing follows

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
flux_led/base_device.py 96.38% <100.00%> (+0.73%) ⬆️
flux_led/device.py 76.04% <100.00%> (+3.55%) ⬆️
flux_led/models_db.py 98.90% <ø> (ø)
flux_led/protocol.py 99.17% <100.00%> (+0.15%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

asxzy added 2 commits June 30, 2026 10:39
Introduces a dedicated protocol class for the Surplife 0xB6 device, switches the
model to it, and adds synchronous-path support.

- protocol.py: PROTOCOL_LEDENET_EXTENDED_CUSTOM, LEDENET_EXTENDED_STATE_RESPONSE_LEN,
  and ProtocolLEDENETExtendedCustom (name, 21-byte state length, extended-only
  is_valid_state_response, white-off-aware extended_state_to_state, and
  named_raw_state that converts extended -> standard for the sync path)
- device.py: read the full 21-byte extended state and accept it while probing
- base_device.py: register the protocol and add supports_extended_custom_effects
- models_db.py: point the 0xB6 model at PROTOCOL_LEDENET_EXTENDED_CUSTOM

Stacked on the reuse PR.
@asxzy asxzy force-pushed the feat/add-0xb6-protocol branch from 12c8d8b to 6608f52 Compare June 30, 2026 16:46
@asxzy asxzy marked this pull request as ready for review June 30, 2026 16:53
@asxzy

asxzy commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Hello @bdraco , this PR is ready for review.

@bluetoothbot

bluetoothbot commented Jun 30, 2026

Copy link
Copy Markdown

Previous review — superseded by a newer review below.

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking issues found.

  • led_count only populated on the async path — sync WifiLedBulb.led_count is always None

@asxzy asxzy requested a review from bluetoothbot June 30, 2026 17:15
@bluetoothbot

bluetoothbot commented Jun 30, 2026

Copy link
Copy Markdown

Previous review — superseded by a newer review below.

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking issues found.

  • Sync read length (21) is shorter than the real 27-byte device frame

@asxzy asxzy force-pushed the feat/add-0xb6-protocol branch from 5a60df2 to e0381b2 Compare June 30, 2026 17:53
@bluetoothbot

bluetoothbot commented Jun 30, 2026

Copy link
Copy Markdown

Previous review — superseded by a newer review below.

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking issues found.

  • Sync read length (21) truncates the real 27-byte extended frame

LEDENET_EXTENDED_STATE_RESPONSE_LEN was 21 but the real 0xB6 device frame
is 27 bytes, so the sync read (device.py::_determine_protocol and
ProtocolLEDENETExtendedCustom.state_response_length -> query_state) left 6
trailing bytes in the reused socket buffer, desyncing later same-socket
reads. Set it to 27 (0xB6-only: keyed purely off the 0xEA 0x81 header, the
sole extended-only device), and replace the synthetic test frames in
test_setup_0xB6_surplife / test_0xB6_surplife_sync with the real 27-byte
capture so extended_state_to_state is exercised against real wire data.
@asxzy asxzy requested a review from bluetoothbot June 30, 2026 19:04
@bluetoothbot

Copy link
Copy Markdown

PR Review — feat: dedicated ProtocolLEDENETExtendedCustom + LED count for 0xB6

Solid follow-up to #538 — gives 0xB6 its own protocol class and exposes led_count. The three prior bot blockers are already resolved in this HEAD; remaining items are minor cleanups.

What's done well:

  • Read length is correctly 27 bytes in both the constant and the sync _determine_protocol branch, and both the async (test_setup_0xB6_surplife) and sync (test_0xB6_surplife_sync) tests now feed the real 27-byte captured frame.
  • Sync/async parity for led_count: the async path stores it in process_extended_state_response, the sync path captures it in process_state_response before named_raw_state flattens the extended frame — verified reachable via query_stateupdate_state.
  • The new white-brightness guard genuinely prevents a ValueError from scaled_color_temp_to_white_levels (confirmed it raises on temp/brightness >100); the parent class lacks this hardening.
  • Good edge-case test coverage (out-of-range white, truncated frame, invalid-frame → None).

What needs attention (non-blocking):

  • extended_state_to_state duplicates ~50 lines of the parent's conversion; drift risk on future fixes.
  • A few docstrings still say "21-byte" though the frame is 27.

Note: PR is marked Draft pending #538 — merge order is a process gate, not a code issue.


🟢 Suggestions

1. extended_state_to_state largely duplicates the parent implementation
flux_led/protocol.py:1643-1697

ProtocolLEDENETExtendedCustom.extended_state_to_state copies nearly the entire body of ProtocolLEDENET25Byte.extended_state_to_state (the byte unpacking, the HSV→RGB conversion, and the output bytes(...) assembly). The only real deltas are:

  • the new white guard (white_temp > 100 or white_brightness == 0 or white_brightness > 100 → both whites off), and
  • mode = raw_state[8] if preset_pattern == 0x25 else 0 instead of the parent's hardcoded mode = 0.

Why it matters: two near-identical copies of the HSV/white conversion will drift. A future fix to the conversion (e.g. the colorsys.hsv_to_rgb path mishandles h > 1.0 when hue byte exceeds 180) would need to be applied in both places, and it's easy to patch only one.

Suggested fix: factor the shared conversion into a helper (or move the white guard up into the parent, which would also harden the parent against the same crash) and have the subclass override only the mode selection. Not blocking — the override is functionally correct and the white guard is a genuine improvement over the parent.

if white_temp > 100 or white_brightness == 0 or white_brightness > 100:
    cool_white = 0
    warm_white = 0
else:
    levels = scaled_color_temp_to_white_levels(white_temp, white_brightness)

Checklist

  • Protocol detection handles extended-only (0xEA 0x81) device
  • Sync read consumes the full 27-byte device frame
  • Sync/async parity for led_count
  • State conversion guards against out-of-range white (no crash)
  • Conversion tested against real device frames
  • No unnecessary code duplication — suggestion #1
  • Docstrings accurate

Automated review by Kōan (Claude) HEAD=aa7572e 4 min 55s

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

Comment thread tests/test_aio.py Outdated
@bdraco

bdraco commented Jun 30, 2026

Copy link
Copy Markdown
Member

  1. extended_state_to_state largely duplicates the parent implementation
    flux_led/protocol.py:1643-1697
    ProtocolLEDENETExtendedCustom.extended_state_to_state copies nearly the entire body of ProtocolLEDENET25Byte.extended_state_to_state (the byte unpacking, the HSV→RGB conversion, and the output bytes(...) assembly). The only real deltas are:
  • the new white guard (white_temp > 100 or white_brightness == 0 or white_brightness > 100 → both whites off), and
  • mode = raw_state[8] if preset_pattern == 0x25 else 0 instead of the parent's hardcoded mode = 0.

Why it matters: two near-identical copies of the HSV/white conversion will drift. A future fix to the conversion (e.g. the colorsys.hsv_to_rgb path mishandles h > 1.0 when hue byte exceeds 180) would need to be applied in both places, and it's easy to patch only one.

Suggested fix: factor the shared conversion into a helper (or move the white guard up into the parent, which would also harden the parent against the same crash) and have the subclass override only the mode selection. Not blocking — the override is functionally correct and the white guard is a genuine improvement over the parent.

if white_temp > 100 or white_brightness == 0 or white_brightness > 100:
    cool_white = 0
    warm_white = 0
else:
    levels = scaled_color_temp_to_white_levels(white_temp, white_brightness)

That's likely going to bite us in the future. Should be refactored into a helper

@bluetoothbot

Copy link
Copy Markdown

@bdraco: > ###

Already refactored in this diff. Shared _extended_state_to_state helper holds the byte unpacking, HSV->RGB, and assembly. Parent and subclass both delegate. Only the white guard and mode selection differ now. No drift risk remaining.

@bluetoothbot

Copy link
Copy Markdown

PR Review — feat: dedicated ProtocolLEDENETExtendedCustom + LED count for 0xB6

Solid, well-scoped protocol addition — the two earlier blockers are resolved and the duplication concern is already refactored away. Merge-ready pending the noted process gate (#538).

Strengths:

  • Sync read path now consumes the full 27-byte extended frame (LEDENET_EXTENDED_STATE_RESPONSE_LEN), fixing the earlier truncation.
  • led_count is captured on both paths — process_extended_state_response (async first frame) and process_state_response (sync + async subsequent frames), verified consistent against the actual _async_process_message routing.
  • The prior duplication review comment is genuinely addressed: the parent's HSV/assembly body is extracted into a shared _extended_state_to_state, with parent and subclass delegating and only white-guard/mode differing.
  • The out-of-range white guard (white_brightness > 100) hardens against a scaled_color_temp_to_white_levels crash and is tested; real captured 27-byte frames back the led_count assertions.

Needs attention (both non-blocking):

  • Comment annotation Model number (0x25 = pattern ID, otherwise 0) is attached to the wrong byte — the 0x25 logic keys off the preset byte (raw_state[7]).
  • The mode = raw_state[8] (preset 0x25) branch has no test and no consumer until the follow-up effects PR.

🟢 Suggestions

1. mode = raw_state[8] branch (preset 0x25) is untested and unused by current callers
flux_led/protocol.py:1672

The mode = raw_state[8] if raw_state[7] == 0x25 else 0 selection only diverges from the parent when the preset byte is 0x25. None of the added tests exercise that path — every captured/synthetic frame has byte 7 = 0x66, so mode is always 0 in tests, matching the parent's hardcoded behavior.

Why it matters: the effect/pattern support that actually consumes a non-zero mode lands in the follow-up effects-api PR. Until then this branch is forward-looking code with no caller and no coverage, so a regression in the pattern-ID extraction would go unnoticed.

Fix (optional): add one extended_state_to_state test with byte 7 = 0x25 asserting mode/color-mode resolution, or defer this line to the effects PR that gives it a consumer. Not blocking — the branch is a no-op for the frames this PR handles.

mode = raw_state[8] if raw_state[7] == 0x25 else 0

Checklist

  • Protocol detection handles extended-only (0xEA 0x81) device
  • Sync read consumes the full 27-byte device frame
  • Sync/async parity for led_count
  • State conversion guards against out-of-range white (no crash)
  • Conversion tested against real device frames
  • No unnecessary code duplication
  • Docstrings and comments accurate — suggestion #1
  • New branches covered by tests

Automated review by Kōan (Claude) HEAD=fc5f07e 4 min 8s

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

@bdraco bdraco merged commit 5bcd932 into lightinglibs:master Jul 1, 2026
8 checks passed
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.

3 participants