fix(leds): add initial color parameter and implement retry logic for … - #102
Merged
Conversation
…device color updates
There was a problem hiding this comment.
🟡 Changes recommended
It introduces a couple of correctness edge cases (config-driven color normalization and retry delay indexing safety) that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR improves the BLE LED controller startup behavior by allowing the initial LED color to come from vehicle configuration and by adding bounded retry logic for the initial (startup) dispatch, with tests covering the new behavior.
Changes:
- Added
initial_colorparameter toBleLedControllerand wired it fromvehicle_config["theme"]["main"]. - Implemented bounded retry attempts for the startup color dispatch only, refactoring send logic into retry wrapper + single-attempt method.
- Expanded unit tests to cover startup retry behavior and ensure later updates do not retry; bumped project version.
File summaries
| File | Description |
|---|---|
src/services/led_service.py | Adds initial_color, startup-only retry logic, and refactors device send into retry + single-attempt methods. |
main.py | Passes the configured theme color into BleLedController as initial_color. |
tests/test_led_service.py | Adds/updates tests for initial color queueing and startup retry vs. subsequent update behavior. |
VERSION | Bumps version from 2.0.1-rc.15 to 2.0.1-rc.16. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
33
to
36
| def __init__(self, storage=None, catalog: Optional[DeviceCatalog] = None, | ||
| registry: Optional[ProtocolRegistry] = None): | ||
| registry: Optional[ProtocolRegistry] = None, | ||
| initial_color: str = "#48B8FF"): | ||
| super().__init__("Leds", storage) |
Comment on lines
+286
to
+287
| delay_index = min(attempt_index, len(self.STARTUP_RETRY_DELAYS) - 1) | ||
| await asyncio.sleep(self.STARTUP_RETRY_DELAYS[delay_index]) |
| with mock.patch.object(service, "_send_to_device", side_effect=capture): | ||
| await service._ble_worker() | ||
| self.assertEqual(attempts, [3, 1]) |
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
This pull request introduces improvements to the LED controller service, focusing on more robust initial color dispatch and retry logic for BLE device communication. The changes allow the initial LED color to be set from the configuration, implement bounded retry attempts for startup color dispatch, and add comprehensive tests for these behaviors.
LED Controller Improvements:
BleLedControllernow accepts aninitial_colorparameter, allowing the initial LED color to be set from the vehicle configuration instead of being hardcoded. (main.py,src/services/led_service.py) [1][2][3]STARTUP_RETRY_DELAYS), improving reliability in case of transient connection issues. Subsequent color updates do not use retries. (src/services/led_service.py) [1][2][3]_send_to_devicenow supports bounded retries and returns a boolean indicating success, while the actual send logic is moved to_send_to_device_once. (src/services/led_service.py) [1][2][3][4]Testing Enhancements:
tests/test_led_service.py) [1][2]Other:
2.0.1-rc.15to2.0.1-rc.16. (VERSION)