Color LED tab by ArisMorgens · Pull Request #793 · bitcraze/crazyflie-clients-python

ArisMorgens · 2026-03-12T15:55:34Z

No description provided.

Copilot

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 ColorLEDTab in the tab loader (available list + 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.

Copilot

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.

gemenerik · 2026-03-23T15:32:06Z

Custom colors are not grayed out when disconnected. (They may not have been in the old client, but anyways)

ArisMorgens · 2026-03-25T12:19:48Z

Nice catch! This issue exists in the old client as well. It is fixed for the new client in 0ae1059.

gemenerik

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.

gemenerik · 2026-03-31T11:47:57Z

+                self._cf, present_decks
+            )
+            for position, stream in streams.items():
+                create_task(self._run_thermal_stream(position, stream))


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.

Added proper handling of the thermal_stream_tasks upon disconnection in 506a7f9.

gemenerik · 2026-03-31T12:28:55Z

not this pr but this feels fragile

Fixed in 3530d38.
Instead of retrieving a button's color by reading its stylesheet, we now store the color directly on the button using Qt's setProperty(). This happens both on preset buttons once during connection, and on custom buttons when they're created.

ArisMorgens · 2026-05-05T11:43:02Z

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.

I replaced the broad except Exceptions with specific ones from cflib2.error in e3833c2. I tried to match the exception with the relevant errors from each call (e.g. detect_decks() calls cf.param().get(), so I used ParamError). @gemenerik Is this the appropriate approach of catching the expected exceptions?

gemenerik · 2026-05-06T09:15:53Z

+                self._process_thermal_data(log_data.data)
+        except asyncio.CancelledError:
+            raise
+        except Exception as e:


One last trailing except Exception

Whoops. Fixed in 7443322.

gemenerik · 2026-05-06T09:27:17Z

        ]
        for btn in color_buttons:
+            style = btn.styleSheet()
+            hex_color = style.split("background-color:")[-1].split(";")[0].strip()


Still has this potentially fragile parsing. Again not this PR but maybe nice to fix.

In 6994368 I replaced the chained split() with a regex that explicitly targets the QPushButton{} block, so the parsing should be more robust now. What do you think @gemenerik?

ArisMorgens requested review from Copilot and gemenerik March 12, 2026 15:55

Copilot started reviewing on behalf of ArisMorgens March 12, 2026 15:56 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

Comment thread src/cfclient/ui/tabs/ColorLEDTab.py Outdated

Comment thread src/cfclient/ui/tabs/ColorLEDTab.py Outdated

Comment thread src/cfclient/ui/tabs/ColorLEDTab.py Outdated

Comment thread src/cfclient/ui/tabs/ColorLEDTab.py Outdated

Comment thread src/cfclient/ui/tabs/ColorLEDTab.py

ArisMorgens changed the title ~~Added a cflib2-based Color LED tab~~ Color LED tab Mar 12, 2026

gemenerik requested a review from Copilot March 23, 2026 13:52

Copilot started reviewing on behalf of gemenerik March 23, 2026 13:52 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

Comment thread src/cfclient/ui/tabs/ColorLEDTab.py Outdated

Comment thread src/cfclient/ui/tabs/ColorLEDTab.py Outdated

Comment thread src/cfclient/ui/tabs/ColorLEDTab.py

ArisMorgens force-pushed the Aris/cflib2/ColorLEDTab branch 2 times, most recently from 7940a4e to 5b502d1 Compare March 25, 2026 12:53

gemenerik requested changes Mar 31, 2026

View reviewed changes

ArisMorgens added 11 commits May 5, 2026 14:23

Added a cflib2-based Color LED tab

cfc6cc9

Added storing and cancelling the _on_connected task

2b1fce7

Updated start_monitoring to return the streams

22a0da0

Updated _run_thermal_stream to handle exceptions

18e7d95

Made custom colors grey-out when the Crazyflie disconnects

8505f4e

Re-formated the modified files

d675d74

Removed suppression comment

5cc9881

Fixed a custom color bug where they all set the LED to white

dc6dd74

Replaced except Exception with specific exception types

f986b64

Added better handling of the thermal_stream_tasks upon disconnection

e76b464

Store the color on each button using setProperty()

1ef9f60

ArisMorgens force-pushed the Aris/cflib2/ColorLEDTab branch from 3530d38 to 1ef9f60 Compare May 5, 2026 12:24

gemenerik requested changes May 6, 2026

View reviewed changes

ArisMorgens added 2 commits May 7, 2026 17:02

Replaced a remaining except Exception with LogError

7443322

Replaced split() with a regex to fix fragile parsing

6994368

Conversation

ArisMorgens commented Mar 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemenerik commented Mar 23, 2026

Uh oh!

ArisMorgens commented Mar 25, 2026

Uh oh!

gemenerik left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArisMorgens commented May 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants