feat: add support for AK001-ZJ21413 (0xB6) Surplife outdoor lighting#498
feat: add support for AK001-ZJ21413 (0xB6) Surplife outdoor lighting#498asxzy wants to merge 18 commits intolightinglibs:masterfrom
Conversation
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
47b30e7 to
995c338
Compare
|
Hi codeowners, I have a outdoor light by Surplife installed few days ago. The controller is not supported by the project -- "Cannot determine protocol" . The protocol is supported, but the device is not mapped in. I did most of the code via AI, and the feature is verified manually. Let me know if anything is missing before merging it in. Once it is in, I can request change to integrate this with home assistant. Yi |
flux_led/aiodevice.py
Outdated
| # Handle protocol determination for extended state | ||
| if ( | ||
| self._determine_protocol_future | ||
| and not self._determine_protocol_future.done() | ||
| ): | ||
| assert self._protocol is not None | ||
| self._set_protocol_from_msg(msg, self._protocol.name) | ||
| self._determine_protocol_future.set_result(True) |
There was a problem hiding this comment.
Looks like duplicated code.. I think we have this elsewhere
There was a problem hiding this comment.
Thank you for the feedback. I will update the code later today.
flux_led/base_device.py
Outdated
| if len(full_msg) >= 20 and full_msg[0] == 0xEA and full_msg[1] == 0x81: | ||
| # Extended state format - use extended state byte positions | ||
| self._model_num = full_msg[LEDENET_EXTENDED_STATE_MODEL_POS] | ||
| self._model_data = get_model(self._model_num, fallback_protocol) | ||
| version_num = ( | ||
| full_msg[LEDENET_EXTENDED_STATE_VERSION_POS] | ||
| if len(full_msg) > LEDENET_EXTENDED_STATE_VERSION_POS | ||
| else 1 | ||
| ) | ||
| else: | ||
| # Standard state format - use standard state byte positions | ||
| self._model_num = full_msg[LEDENET_STATE_MODEL_POS] | ||
| self._model_data = get_model(self._model_num, fallback_protocol) | ||
| version_num = ( | ||
| full_msg[LEDENET_STATE_VERSION_POS] | ||
| if len(full_msg) > LEDENET_STATE_VERSION_POS | ||
| else 1 | ||
| ) |
There was a problem hiding this comment.
This could be extracted into a helper
There was a problem hiding this comment.
Pull request overview
This PR adds support for the Surplife outdoor permanent LED lighting system (model 0xB6, AK001-ZJ21413). The device exclusively uses the extended state format (0xEA 0x81) introduced in PR #428, requiring modifications to message handling priority and protocol determination logic to support devices that only respond with this format.
Changes:
- Modified message processing order to check extended state format before regular state format to support extended-state-only devices
- Enhanced protocol determination to extract model and version information from extended state messages
- Added byte position constants for parsing state messages to improve code maintainability
- Added complete model and hardware definitions for the 0xB6 device with RGB+CCT color modes
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| flux_led/aiodevice.py | Reordered message processing to prioritize extended state format and added protocol determination support for extended state responses |
| flux_led/base_device.py | Enhanced _set_protocol_from_msg() to handle both standard and extended state formats using byte position constants |
| flux_led/protocol.py | Added byte position constants and extended state validation methods to base protocol classes |
| flux_led/models_db.py | Added model definition for 0xB6 Surplife device and hardware configuration entry |
| tests/test_sync.py | Added sync API test for 0xB6 device identification and protocol validation |
| tests/test_aio.py | Added async API tests covering extended state setup and protocol validation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def is_valid_extended_state_response(self, raw_state: bytes) -> bool: | ||
| """Check if a state response is valid.""" | ||
| return raw_state[0] == 0xEA and raw_state[1] == 0x81 and len(raw_state) >= 20 |
There was a problem hiding this comment.
The method lacks bounds checking before accessing array indices. If raw_state has length less than 2, accessing raw_state[0] and raw_state[1] could raise an IndexError. The length check (len(raw_state) >= 20) happens after the index access. The checks should be reordered to: return len(raw_state) >= 20 and raw_state[0] == 0xEA and raw_state[1] == 0x81
| return raw_state[0] == 0xEA and raw_state[1] == 0x81 and len(raw_state) >= 20 | |
| return len(raw_state) >= 20 and raw_state[0] == 0xEA and raw_state[1] == 0x81 |
tests/test_sync.py
Outdated
| if calls == 2: | ||
| # Standard state response (sync mode expects this during setup) | ||
| # Real device uses extended state, but mocking standard for compatibility | ||
| self.assertEqual(expected, 12) | ||
| return bytearray(b"\x23\x61\x24\x64\x00\x00\x00\x00\x01\x00\xf0\x34") |
There was a problem hiding this comment.
The test comment states this device "ONLY responds with extended state format" but the test actually uses a standard state response (0x81) instead of extended state (0xEA 0x81). This doesn't accurately test the device behavior described in the PR. Consider adding a test that properly validates the device with extended state format to ensure the sync API can handle extended-state-only devices.
|
Hi @bdraco , I originally thought I can add the device in with existing protocol and call it a day. The existing protocol can perform on/off solid color change well, but the app is providing more features that is not use by other devices. When I doing more package capturing and analysis, I found this device is taking more command that change the pattern and segments. I think it will be cleaner to start a new protocol instead. Will update this PR later. |
|
@bdraco codecov failed and I can't see the detailed message. Let me know if anything need to be adjusted. |
|
flux_led/fluxled.py does not have unit test and it is out of scope of this PR. I will skip it for now. |
|
@bdraco let me know if anything is missing. |
Summary
New Features
Device Support:
Extended Custom Effects (0xE1 0x21):
Segment Colors (0xE1 0x22):
New API Methods:
Technical Changes
Test plan