DisconnectedError handling#802
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves disconnect robustness in the UI by preventing DisconnectedError exceptions from bubbling out of long-running telemetry streams (battery, pose, motor logging) and updates type annotations to satisfy pre-commit linting.
Changes:
- Add
DisconnectedErrorhandling to streaming loops for battery, pose, and motor telemetry to avoid terminal errors on disconnect. - Add
from __future__ import annotationsand expand/adjust function and method type annotations in affected UI modules. - Introduce/standardize relevant type imports (e.g.,
Crazyflie,InputReaderInterface,Callable/Coroutine/Future).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/cfclient/ui/tabs/FlightTab.py |
Adds DisconnectedError handling for motor telemetry streaming and adds extensive method annotations. |
src/cfclient/ui/pose_logger.py |
Adds DisconnectedError handling for pose streaming and adds/adjusts type annotations. |
src/cfclient/ui/main.py |
Adds DisconnectedError handling for battery streaming and improves type annotations for commander pipeline and input-device UI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PySide6's QtAsyncio prints tracebacks for unhandled task exceptions before done callbacks run, so the existing _task_done_callback catch was too late. Wrap coroutines in create_task to catch DisconnectedError before it reaches PySide6, and remove the now-redundant per-coroutine except blocks.
gemenerik
left a comment
gemenerik
left a comment
There was a problem hiding this comment.
Good find! The DisconnectedError on disconnect tracebacks were ugly!
We already had code in _task_done_callback that was supposed to catch DisconnectedError for all create_task coroutines, and it did catch them (preventing os._exit), but QtAsyncio still prints the traceback before the done callback runs, so they still showed up in the terminal. Rather than having to add except DisconnectedError: pass in every streaming coroutine, I've made a PR into your branch that wraps coroutines in create_task so DisconnectedError is caught before PySide6 can see it. The rest of your PR (annotations, setup-inside-try, disconnect log message) is unaffected. See #806
Makes it possible to identify which task was interrupted when multiple background tasks are torn down by a single disconnect.
DisconnectedError is intentionally swallowed by the wrapper as a normal shutdown case; the original docstring suggested all exceptions propagate.
Catch DisconnectedError in create_task wrapper instead of per-coroutine
This PR adds
DisconnectedErrorhandling for 3 features in the Flight tab (battery, pose, and motors). In all 3, thewhile Trueloop has no except, so whenstream.next()raisesDisconnectedErroron disconnect, we get an error in the terminal.It also fixes the annotations in main.py, pose_logger.py, and FlightTab.py, to pass pre-commit checks.