Adding a basic ELRS/CRSF bridge. by gbin · Pull Request #498 · copper-project/copper-rs

gbin · 2025-11-12T20:50:01Z

This is again useful in the drone space to send telemetry or remote control over the waves at long distance (up to the curvature of the earth in line of sight)

Copilot

Pull Request Overview

This PR introduces a new CRSF (Crossfire) bridge component for the Copper-rs framework, enabling long-range radio communication for drone telemetry and remote control using ELRS (ExpressLRS) protocol.

Key Changes:

New bridge component cu-crsf with support for RC channels and link quality statistics
Platform-agnostic design supporting both std and no_std environments
Serial port abstraction with registry system for embedded targets

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`Cargo.toml`	Adds new `cu_crsf` bridge component to workspace members
`components/bridges/cu_crsf/Cargo.toml`	Defines dependencies and features for CRSF bridge (std/no_std support)
`components/bridges/cu_crsf/build.rs`	Build script for setting up log index directory environment variable
`components/bridges/cu_crsf/src/messages.rs`	Wrapper types for CRSF message payloads with serialization support
`components/bridges/cu_crsf/src/lib.rs`	Main bridge implementation with serial communication and channel management

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-12T22:10:25Z

+        tx_channels: &[BridgeChannelConfig<<Self::Tx as BridgeChannelSet>::Id>],
+        rx_channels: &[BridgeChannelConfig<<Self::Rx as BridgeChannelSet>::Id>],
+    ) -> CuResult<Self>
+    where
+        Self: Sized,
+    {
+        let _ = tx_channels;
+        let _ = rx_channels;
+


The variables tx_channels and rx_channels parameters are explicitly ignored with let _ = .... If these parameters are not used in the current implementation, consider documenting why they're not needed or if there's planned future functionality. Alternatively, prefix the parameter names with underscore (_tx_channels, _rx_channels) which is the idiomatic way to indicate unused parameters in Rust.

Suggested change

tx_channels: &[BridgeChannelConfig<<Self::Tx as BridgeChannelSet>::Id>],

rx_channels: &[BridgeChannelConfig<<Self::Rx as BridgeChannelSet>::Id>],

) -> CuResult<Self>

where

Self: Sized,

{

let _ = tx_channels;

let _ = rx_channels;

_tx_channels: &[BridgeChannelConfig<<Self::Tx as BridgeChannelSet>::Id>],

_rx_channels: &[BridgeChannelConfig<<Self::Rx as BridgeChannelSet>::Id>],

) -> CuResult<Self>

where

Self: Sized,

{

Copilot · 2025-11-12T22:10:25Z

+impl LinkStatisticsPayload {
+    pub fn inner(&self) -> &LinkStatistics {
+        &self.0
+    }


The inner_mut() method is provided but LinkStatisticsPayload doesn't implement it, while RcChannelsPayload does. This creates an inconsistent API between the two similar wrapper types. Consider adding an inner_mut() method to LinkStatisticsPayload for consistency.

Suggested change

}

}

/// Returns mutable access to the inner CRSF structure.

pub fn inner_mut(&mut self) -> &mut LinkStatistics {

&mut self.0

}

Copilot · 2025-11-12T22:10:26Z

+                            Packet::RcChannels(channels) => {
+                                self.last_rc = Some(channels);
+                                for (i, value) in self.last_rc.iter().enumerate() {
+                                    debug!("RC Channel {}: {}", i, value.as_ref());


The debug message references value.as_ref() but value is of type &u16, which doesn't have an as_ref() method that makes sense in this context. This should likely be just value or *value to dereference and print the u16 value.

Suggested change

debug!("RC Channel {}: {}", i, value.as_ref());

debug!("RC Channel {}: {}", i, *value);

Copilot · 2025-11-12T22:10:26Z

+            _ => {
+                error!("CRSF: Serial port read error");


The error message silently swallows the actual error from the serial port read operation. Consider including the error details in the log message to aid debugging: error!("CRSF: Serial port read error: {:?}", err);

Suggested change

_ => {

error!("CRSF: Serial port read error");

Err(err) => {

error!("CRSF: Serial port read error: {:?}", err);

Copilot · 2025-11-12T22:10:27Z

+impl<S, E> CuCrsfBridge<S, E>
+where
+    S: SerialFactory<E>,
+{


The register_serial public method lacks documentation. Users in no_std environments will need to understand how and when to call this method to register their serial ports.

Suggested change

{

{

/// Registers a serial port for use with the bridge in `no_std` environments.

///

/// # Arguments

///

/// * `slot` - The slot index to associate with the serial port. Each slot corresponds to a bridge instance.

/// * `serial_port` - The serial port implementation to register. Must implement `SerialFactory<E>`.

///

/// # Usage

///

/// Call this method before creating or using a bridge instance to ensure the serial port is available.

/// This is required in `no_std` environments where dynamic allocation and registration are not handled automatically.

///

/// # Errors

///

/// Returns an error if the registration fails (e.g., if the slot is already occupied).

Copilot · 2025-11-12T22:10:27Z

+                                for (i, value) in self.last_rc.iter().enumerate() {
+                                    debug!("RC Channel {}: {}", i, value.as_ref());


The iteration over self.last_rc on line 231 is incorrect. self.last_rc is an Option<RcChannels>, not an iterable collection. This code will not compile. You likely meant to iterate over self.last_rc.unwrap().0 or handle the Option properly with if let Some(channels) = &self.last_rc.

Suggested change

for (i, value) in self.last_rc.iter().enumerate() {

debug!("RC Channel {}: {}", i, value.as_ref());

if let Some(channels) = &self.last_rc {

for (i, value) in channels.0.iter().enumerate() {

debug!("RC Channel {}: {}", i, value.as_ref());

}

Copilot · 2025-11-12T22:10:27Z

+pub trait SerialFactory<E>: Write<Error = E> + Read<Error = E> + Send + 'static {
+    fn try_new(config: Option<&ComponentConfig>) -> CuResult<Self>
+    where
+        Self: Sized;
+}


The SerialFactory trait lacks documentation. Public traits should have doc comments explaining their purpose and when/how they should be implemented.

Copilot · 2025-11-12T22:10:28Z

+pub struct CrsfRx;
+
+impl CrsfRx {
+    pub const LQ: BridgeChannel<RxId, LinkStatisticsPayload> =
+        BridgeChannel::new(RxId::LinkQuality, "lqrx");
+    pub const RC_CHANNELS: BridgeChannel<RxId, RcChannelsPayload> =
+        BridgeChannel::new(RxId::RCChannels, "rcrx");
+}
+impl BridgeChannelSet for CrsfRx {
+    type Id = RxId;
+
+    const STATIC_CHANNELS: &'static [&'static dyn BridgeChannelInfo<Self::Id>] =
+        &[&Self::LQ, &Self::RC_CHANNELS];
+}
+
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum TxId {
+    LinkQuality,
+    RCChannels, // only support those for now, there are a bunch of others
+}
+
+pub struct CrsfTx;


The CrsfRx and CrsfTx structs lack documentation. These are public API types that users will reference when configuring channels, so they should have doc comments explaining what they represent.

Copilot · 2025-11-12T22:10:28Z

+                TxId::LinkQuality => {
+                    let lsi = Self::payload_as_lq(payload).inner().clone();
+                    debug!(
+                        "CRSF: Sent LinkStatistics: Downlink LQ:{}",
+                        lsi.downlink_link_quality
+                    );
+                    let ls = Packet::LinkStatistics(lsi);
+                    let raw_packet = ls.into_raw(PacketAddress::Transmitter);
+                    self.serial_port
+                        .write_all(raw_packet.data())
+                        .map_err(|_| CuError::from("CRSF: Serial port write error"))?;
+                }
+                TxId::RCChannels => {
+                    let rc = Self::payload_as_rc(payload).inner().clone();
+                    for (i, value) in rc.iter().enumerate() {
+                        debug!("Sending RC Channel {}: {}", i, value);
+                    }
+                    let rc = Packet::RcChannels(rc);
+                    let raw_packet = rc.into_raw(PacketAddress::Transmitter);
+                    self.serial_port
+                        .write_all(raw_packet.data())
+                        .map_err(|_| CuError::from("CRSF: Serial port write error"))?;
+                }
+            }


[nitpick] There's significant code duplication between the LinkQuality and RCChannels cases in both the send and receive methods. The pattern of extracting payload, converting to CRSF type, creating packet, and writing to serial is identical except for the types. Consider extracting this logic into a helper method that can be parameterized by type to reduce duplication and improve maintainability.

This is again useful in the drone space to send telemetry or remote control over the waves at long distance (up to the curvature of the earth in line of sight)

lots of bridges do have a natural ID -> Route map and suddenly this mandatory route parameted make them stutturing for example an AIO ESC have 4 motors, you cannot route anything but the API was forcing you to add a random string to it. This will be followed by some helper to generate more complex bridges like middleware ones where the user needs to generate it at the last minute with all the types they want to expose to each ID.

* Moved route on cubridges to optional lots of bridges do have a natural ID -> Route map and suddenly this mandatory route parameted make them stutturing for example an AIO ESC have 4 motors, you cannot route anything but the API was forcing you to add a random string to it. This will be followed by some helper to generate more complex bridges like middleware ones where the user needs to generate it at the last minute with all the types they want to expose to each ID. * removing some AI stupidity * Mopping up dumb errors from CI.

lots of bridges do have a natural ID -> Route map and suddenly this mandatory route parameted make them stutturing for example an AIO ESC have 4 motors, you cannot route anything but the API was forcing you to add a random string to it. This will be followed by some helper to generate more complex bridges like middleware ones where the user needs to generate it at the last minute with all the types they want to expose to each ID.

gbin requested a review from Copilot November 12, 2025 22:06

Copilot started reviewing on behalf of gbin November 12, 2025 22:06 View session

Copilot finished reviewing on behalf of gbin November 12, 2025 22:08

Copilot AI reviewed Nov 12, 2025

View reviewed changes

gbin added 2 commits November 12, 2025 16:26

Adding a basic ELRS/CRSF bridge.

375ec1a

This is again useful in the drone space to send telemetry or remote control over the waves at long distance (up to the curvature of the earth in line of sight)

snafu in pure copper logging.

7621dc9

gbin force-pushed the gbin/crsf branch from 4bd547b to 7621dc9 Compare November 12, 2025 22:26

gbin added 13 commits November 13, 2025 12:01

Bunch of quality of life improvements for CuBridges

6116f13

merge snafu

f41230d

missing a spot on baremetal

25487df

Fix for id case handling mismatched between the 2 macros

ed85790

Bunch of quality of life improvements for CuBridges

9295197

merge snafu

3f56f3a

missing a spot on baremetal

178a17b

Fix for id case handling mismatched between the 2 macros

c7a3f57

WIP

e1f8596

fixes with new API

b6648e0

gbin changed the base branch from master to gbin/bridge_helper November 13, 2025 20:45

Merge branch 'gbin/bridge_helper' into gbin/crsf

e80d9c1

Base automatically changed from gbin/bridge_helper to master November 13, 2025 22:47

gbin added 7 commits November 13, 2025 16:47

Merge branch 'master' into gbin/crsf

e07291d

pass integrating to the bdshot demo

7b1c81b

Should be a good base for an end to end demo with elrs

4fdf1a3

renaming of crates

5b9ec68

fix dep versions

506c53b

missed a CI spot for embedded

6ee56be

dep cleanup

ade1222

gbin merged commit 86a4f93 into master Nov 18, 2025
7 checks passed

gbin deleted the gbin/crsf branch November 18, 2025 09:36

makeecat added the no-std No-std support effort label Jan 5, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding a basic ELRS/CRSF bridge.#498

Adding a basic ELRS/CRSF bridge.#498
gbin merged 23 commits into
masterfrom
gbin/crsf

gbin commented Nov 12, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-    }
+    }
+    /// Returns mutable access to the inner CRSF structure.
+    pub fn inner_mut(&mut self) -> &mut LinkStatistics {
+        &mut self.0
+    }

	debug!("RC Channel {}: {}", i, value.as_ref());
	debug!("RC Channel {}: {}", i, *value);

-            _ => {
-                error!("CRSF: Serial port read error");
+            Err(err) => {
+                error!("CRSF: Serial port read error: {:?}", err);

-{
+{
+    /// Registers a serial port for use with the bridge in `no_std` environments.
+    ///
+    /// # Arguments
+    ///
+    /// * `slot` - The slot index to associate with the serial port. Each slot corresponds to a bridge instance.
+    /// * `serial_port` - The serial port implementation to register. Must implement `SerialFactory<E>`.
+    ///
+    /// # Usage
+    ///
+    /// Call this method before creating or using a bridge instance to ensure the serial port is available.
+    /// This is required in `no_std` environments where dynamic allocation and registration are not handled automatically.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the registration fails (e.g., if the slot is already occupied).

		for (i, value) in self.last_rc.iter().enumerate() {
		debug!("RC Channel {}: {}", i, value.as_ref());

Conversation

gbin commented Nov 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants