Conversation
There was a problem hiding this comment.
Pull request overview
This PR enables the Color LED tab in the UI and migrates its Crazyflie interactions to the newer async cflib2-style APIs (async params + log streaming), aligning it with the newer connection model used by other tabs like ParamTab.
Changes:
- Enabled
ColorLEDTabin the tab loader (availablelist + import). - Reworked Color LED deck detection, param writes, and thermal throttling monitoring to use async cflib2 calls and
create_task()scheduling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/cfclient/ui/tabs/__init__.py |
Uncomments/imports ColorLEDTab and adds it to the available tab list so it can be loaded. |
src/cfclient/ui/tabs/ColorLEDTab.py |
Migrates to async cflib2 param/log APIs, adds async connection setup, and implements thermal log stream reading via background tasks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Custom colors are not grayed out when disconnected. (They may not have been in the old client, but anyways) |
|
Nice catch! This issue exists in the old client as well. It is fixed for the new client in 0ae1059. |
ArisMorgens
force-pushed
the
Aris/cflib2/ColorLEDTab
branch
from
0ae1059 to
caecf09
Compare
ArisMorgens
force-pushed
the
Aris/cflib2/ColorLEDTab
branch
from
7940a4e to
5b502d1
Compare
gemenerik
left a comment
gemenerik
left a comment
There was a problem hiding this comment.
Several places replace specific exception types with bare except Exception. The risk here is that it hides; programming mistakes, unexpected runtime issues, new exception types from cflib2. They would all be silently logged as a debug message while the code moves on. It's better to catch only the exceptions you actually expect.
| self._cf, present_decks | ||
| ) | ||
| for position, stream in streams.items(): | ||
| create_task(self._run_thermal_stream(position, stream)) |
There was a problem hiding this comment.
The thermal stream tasks created here aren't stored, so they can't be canceled on disconnect. Currently they only terminate because the Disconnected exception is caught by the general except Exception handler, but catching exceptions is meant to catch things going wrong, not to handle a normal disconnect. The disconnect path should use explicit cancellation of the task. Store the task references and cancel them in disconnected(), like with how _connected_task is handled.
| style = button.styleSheet() # type: ignore | ||
| if "background-color:" not in style: | ||
| return | ||
| hex_color = style.split("background-color:")[-1].split(";")[0].strip() |
There was a problem hiding this comment.
not this pr but this feels fragile
No description provided.