[EXAMPLE] Using signals instead of callbacks#1154
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces an example migration from per-callsite callbacks to a centralized Blinker-based signal system for broadcasting drone error and connection-status events to the Socket.IO frontend.
Changes:
- Added
radio/app/signals.pydefiningDroneErrorandDroneConnectStatussignals with connected Socket.IO emit handlers. - Removed the legacy
droneConnectStatusCbcallback plumbing fromutils.py,Drone, and thecomPorts/autopilotendpoints. - Updated
DroneandArmControllerto demonstrate emitting errors/status via signals.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| radio/app/utils.py | Removes legacy droneConnectStatusCb TypedDict + callback from utils. |
| radio/app/signals.py | Adds Blinker signals and Socket.IO-connected handlers for error + connect status. |
| radio/app/endpoints/comPorts.py | Stops passing droneConnectStatusCb into Drone. |
| radio/app/endpoints/autopilot.py | Stops carrying/passing droneConnectStatusCb during reboot reconnect. |
| radio/app/drone.py | Switches connection status and error emission to signals; removes callback field/arg. |
| radio/app/controllers/armController.py | Demonstrates using Drone.error() / DroneError signal on exceptions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from typing import TypedDict, NotRequired | ||
| from app.customTypes import Number |
There was a problem hiding this comment.
NotRequired is imported from typing, which will raise ImportError on Python 3.10 (e.g., Ubuntu 22.04 default). Use typing_extensions.NotRequired (and optionally typing_extensions.TypedDict for consistency with the rest of the radio code) or add a version-conditional import fallback.
| except Exception as e: | ||
| self.drone.logger.error(e, exc_info=True) | ||
| if self.drone.droneErrorCb: | ||
| self.drone.droneErrorCb(str(e)) | ||
| self.drone.error(str(e)) | ||
|
|
||
| ## Or you can have just have: (TODO: remove) | ||
| DroneError.send(str(e)) | ||
|
|
There was a problem hiding this comment.
This exception path will emit the same error twice: self.drone.error(...) already sends DroneError, then DroneError.send(...) is called again. Please remove the duplicate send (and the TODO comment) so the frontend doesn't receive duplicate drone_error events.
| DroneConnectStatus.send(payload) | ||
| except Exception: | ||
| self.logger.exception("Connection status callback failed") | ||
| return |
There was a problem hiding this comment.
The log message still refers to a “callback” even though the implementation now uses a signal (DroneConnectStatus). Updating this message will make operational logs easier to understand when diagnosing connection issues.
|
Ok, looks like the sender thing isnt just recommended but required (even though it worked without??) Will need to look back tomorrow... |
This is the basics of using signals, in its simplest form its easiest to just move what was the callbacks in utils to being handlers of their own route in
signals.pyDroneConnectStatus is a full replacement of the old callback and so is the simplest example
DroneError shows two options, you can either directly send in each place you would have called the callback, or you can add a function to drone that essentially represents the old callback and you route all the calls through that.