Add CRSF sensor input on dedicated UART#11379
Add CRSF sensor input on dedicated UART#11379sensei-hacker wants to merge 1 commit intoiNavFlight:maintenance-9.xfrom
Conversation
Add support for receiving sensor data from CRSF-speaking devices connected to a dedicated flight controller UART, similar to MSP sensor input. Incoming CRSF frames are parsed and dispatched to INAV's real sensor subsystems so the data is automatically available to OSD, blackbox, navigation, and outgoing CRSF telemetry. Supported sensor types: - GPS (0x02): New GPS_CRSF provider feeding gpsSolDRV - Barometer (0x09): New BARO_CRSF driver with altitude-to-pressure conversion via ISA formula - Battery (0x08): New CRSF voltage/current source following the SmartPort integration pattern - Vario (0x07): Stored and used as preferred source for outgoing CRSF vario telemetry frames Configuration: serial <n> 16777216 420000 0 0 0 set gps_provider = CRSF set baro_hardware = CRSF set vbat_meter_type = CRSF set current_meter_type = CRSF Feature is gated on MCU_FLASH_SIZE > 512 to exclude F722 and other flash-constrained targets.
sensei-hacker
added
hardware needed
|
Just tested RM ERS CU 01. At a first quick glance it works ;-) Will test the other sensors too if there is more time, and report back. |
|
Just an idea: would it also be possible to support the ERS CV01 cells voltages sensors? |
|
One additional observation: according to my knowledge the Radiomaster Sensor Configuration LUA sends MSP-over-CRSF messages as we discussed earlier. I also traced the tx-line of the FC uart where the sensors are connected. The tx-line is permanently low and no messages show up here. I think I missed the configuration of INav to forward the MSP-over-CRSF to the port the sensors are connected. How should I enable that? This is needed e.g. to query some parameters of the sensors or e.g. to set the capacity, set the Led or start the calibration of the baro sensor. |
Review Summary by QodoAdd CRSF sensor input on dedicated UART with multi-sensor support WalkthroughsDescription• Add CRSF sensor input on dedicated UART with frame parsing • Support GPS, barometer, battery, and vario sensor data integration • Feed parsed sensor data into INAV's real subsystems automatically • Gate feature on MCU_FLASH_SIZE > 512 to exclude flash-constrained targets Diagramflowchart LR
CRSF["CRSF Device"]
UART["Dedicated UART"]
ISR["ISR Frame Assembly"]
TASK["Scheduler Task"]
GPS["GPS Provider"]
BARO["Barometer Driver"]
BATT["Battery Sensor"]
VARIO["Vario Storage"]
CRSF -->|"420kbaud"| UART
UART -->|"crsfSensorDataReceive"| ISR
ISR -->|"crsfSensorFrameDone"| TASK
TASK -->|"crsfSensorDispatchFrame"| GPS
TASK -->|"crsfSensorDispatchFrame"| BARO
TASK -->|"crsfSensorDispatchFrame"| BATT
TASK -->|"crsfSensorDispatchFrame"| VARIO
File Changes1. src/main/io/crsf_sensor.c
|
Code Review by Qodo
1. crsfSensorFrame[1] not validated
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| const int fullFrameLength = crsfSensorFramePosition < 3 ? 5 : crsfSensorFrame[1] + CRSF_FRAME_LENGTH_ADDRESS + CRSF_FRAME_LENGTH_FRAMELENGTH; | ||
|
|
||
| if (fullFrameLength > CRSF_FRAME_SIZE_MAX) { | ||
| crsfSensorFramePosition = 0; | ||
| return; | ||
| } | ||
|
|
||
| if (crsfSensorFramePosition < fullFrameLength) { | ||
| crsfSensorFrame[crsfSensorFramePosition++] = (uint8_t)c; | ||
| if (crsfSensorFramePosition >= fullFrameLength) { | ||
| crsfSensorFrameDone = true; | ||
| crsfSensorFramePosition = 0; |
There was a problem hiding this comment.
1. crsfsensorframe[1] not validated 📘 Rule violation ⛯ Reliability
The ISR frame assembler uses the received frameLength byte to compute fullFrameLength without validating a minimum/valid range, which can cause frame assembly to stall/drop indefinitely or read an invalid CRC location. This violates the requirement to validate external message payload shape/length before use.
Agent Prompt
## Issue description
The CRSF sensor UART ISR computes `fullFrameLength` from `crsfSensorFrame[1]` (external input) without validating a minimum valid range (e.g., type+CRC and known frame length constraints). Invalid/too-small lengths can prevent frame completion and lead to incorrect indexing/CRC handling.
## Issue Context
Compliance requires validating message payload length/shape before reading/indexing. The ISR currently only checks `fullFrameLength > CRSF_FRAME_SIZE_MAX`.
## Fix Focus Areas
- src/main/io/crsf_sensor.c[265-276]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| BARO_CRSF = 12, | ||
| BARO_FAKE = 13, | ||
| BARO_MAX = BARO_FAKE |
There was a problem hiding this comment.
3. Baro enum breaks upgrades 🐞 Bug ✓ Correctness
BARO_CRSF is inserted before BARO_FAKE, renumbering BARO_FAKE from 12->13, but the barometer parameter group version is unchanged, so existing EEPROM configs can be misinterpreted after upgrade (e.g., previously-stored BARO_FAKE value now selects BARO_CRSF).
Agent Prompt
### Issue description
`baroSensor_e` was renumbered (inserting `BARO_CRSF` before `BARO_FAKE`). Because barometer config is persisted as raw bytes and its PG version was not bumped, existing EEPROM configurations can be silently reinterpreted (e.g., old `BARO_FAKE` value becomes `BARO_CRSF`).
### Issue Context
INAV parameter groups are loaded via memcpy when PG version matches. Changing enum numeric values without a PG version bump (and/or migration) is backward-incompatible.
### Fix Focus Areas
- src/main/sensors/barometer.h[25-40]
- src/main/sensors/barometer.c[60-68]
- src/main/config/parameter_group.c[86-94]
### Suggested fix approaches
- Prefer: keep numeric compatibility by assigning `BARO_CRSF` a new unused numeric value (e.g., after `BARO_FAKE`) so existing values remain unchanged.
- Or: bump `PG_BAROMETER_CONFIG` version and add migration/reset logic (accepting that older configs will reset to defaults for this PG).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| static void crsfSensorDataReceive(uint16_t c, void *rxCallbackData) | ||
| { | ||
| UNUSED(rxCallbackData); | ||
|
|
||
| const timeUs_t currentTimeUs = microsISR(); | ||
|
|
||
| if (cmpTimeUs(currentTimeUs, crsfSensorFrameStartAtUs) > CRSF_SENSOR_TIME_NEEDED_PER_FRAME_US) { | ||
| crsfSensorFramePosition = 0; | ||
| } | ||
|
|
||
| if (crsfSensorFramePosition == 0) { | ||
| crsfSensorFrameStartAtUs = currentTimeUs; | ||
| } | ||
|
|
||
| const int fullFrameLength = crsfSensorFramePosition < 3 ? 5 : crsfSensorFrame[1] + CRSF_FRAME_LENGTH_ADDRESS + CRSF_FRAME_LENGTH_FRAMELENGTH; | ||
|
|
||
| if (fullFrameLength > CRSF_FRAME_SIZE_MAX) { | ||
| crsfSensorFramePosition = 0; | ||
| return; | ||
| } | ||
|
|
||
| if (crsfSensorFramePosition < fullFrameLength) { | ||
| crsfSensorFrame[crsfSensorFramePosition++] = (uint8_t)c; | ||
| if (crsfSensorFramePosition >= fullFrameLength) { | ||
| crsfSensorFrameDone = true; | ||
| crsfSensorFramePosition = 0; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| void crsfSensorInputInit(void) | ||
| { | ||
| const serialPortConfig_t *portConfig = findSerialPortConfig(FUNCTION_CRSF_SENSOR); | ||
| if (!portConfig) { | ||
| return; | ||
| } | ||
|
|
||
| crsfSensorPort = openSerialPort(portConfig->identifier, | ||
| FUNCTION_CRSF_SENSOR, | ||
| crsfSensorDataReceive, | ||
| NULL, | ||
| CRSF_SENSOR_BAUDRATE, | ||
| MODE_RX, | ||
| CRSF_SENSOR_PORT_OPTIONS | ||
| ); | ||
|
|
||
| crsfSensorVario = 0; | ||
| crsfSensorVarioLastUpdateMs = 0; | ||
| } | ||
|
|
||
| bool crsfSensorVarioIsValid(void) | ||
| { | ||
| return (crsfSensorVarioLastUpdateMs > 0) && | ||
| ((millis() - crsfSensorVarioLastUpdateMs) < CRSF_SENSOR_VARIO_TIMEOUT_MS); | ||
| } | ||
|
|
||
| int16_t crsfSensorGetVario(void) | ||
| { | ||
| return crsfSensorVario; | ||
| } | ||
|
|
||
| // Called from scheduler to process completed frames outside ISR context | ||
| void crsfSensorProcess(void) | ||
| { | ||
| if (crsfSensorFrameDone) { | ||
| crsfSensorFrameDone = false; | ||
| crsfSensorDispatchFrame(); | ||
| } |
There was a problem hiding this comment.
4. Crsf frame buffer race 🐞 Bug ⛯ Reliability
CRSF sensor input uses a single global frame buffer written by the UART ISR and later parsed by a 100Hz scheduler task without copying/locking, so frames can be overwritten mid-dispatch and multiple frames can collapse into one—causing CRC failures and dropped sensor updates.
Agent Prompt
### Issue description
`crsf_sensor` assembles frames in an ISR into a single global buffer and parses them later in a 100Hz task. Because the ISR can start writing the next frame immediately after setting `frameDone`, the task can read a partially-overwritten buffer (length/CRC/payload), producing CRC failures and sensor dropouts. Also, multiple frames arriving between task ticks are collapsed (only one boolean flag).
### Issue Context
CRSF can send frames up to ~150Hz. The task is 100Hz and runs at IDLE priority.
### Fix Focus Areas
- src/main/io/crsf_sensor.c[66-71]
- src/main/io/crsf_sensor.c[251-278]
- src/main/io/crsf_sensor.c[312-318]
- src/main/fc/fc_tasks.c[637-643]
### Suggested fix approaches
- Implement double buffering: ISR fills buffer A; when complete, atomically swaps a pointer/index to buffer B for task consumption.
- Or implement a small ring buffer of fixed-size frames with head/tail indices updated atomically.
- Ensure `crsfSensorProcess()` drains all queued frames per call (while queue not empty) to reduce backlog.
- Consider raising task rate/priority if needed after correctness is ensured.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| uint16_t groundSpeed = crsfSensorReadU16(payload + 8); // km/h * 10 | ||
| uint16_t heading = crsfSensorReadU16(payload + 10); // degrees * 100 | ||
| uint16_t altitude = crsfSensorReadU16(payload + 12); // meters + 1000 offset | ||
| uint8_t numSat = payload[14]; | ||
|
|
||
| gpsSolDRV.llh.lat = lat; // degree / 10^7, same as INAV | ||
| gpsSolDRV.llh.lon = lon; | ||
| gpsSolDRV.llh.alt = (int32_t)(altitude - 1000) * 100; // meters to cm | ||
| gpsSolDRV.groundSpeed = (groundSpeed * 100 + 18) / 36; // km/h*10 to cm/s |
There was a problem hiding this comment.
5. Gps altitude wraparound 🐞 Bug ✓ Correctness
CRSF GPS altitude is encoded as uint16 meters with a +1000m offset, but decoding subtracts 1000 while still in unsigned arithmetic, so altitudes below 0m (altitude<1000) wrap to a huge positive value and corrupt gpsSolDRV altitude.
Agent Prompt
### Issue description
CRSF GPS altitude decoding underflows for altitudes below 0m because the subtraction is performed in `uint16_t` domain before casting.
### Issue Context
CRSF encodes altitude as `uint16` with a -1000m offset. Values <1000 are valid and should decode to negative meters.
### Fix Focus Areas
- src/main/io/crsf_sensor.c[109-117]
- src/main/telemetry/crsf.c[211-220]
### Suggested fix
- Change to: `gpsSolDRV.llh.alt = ((int32_t)altitude - 1000) * 100;`
- (Optional) Add bounds checking if you want to guard against nonsense values from the external device.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Can we verify that, as we discussed earlier? You mentioned a logic analyzer - that would certainly do the trick. I don't want to spam the sensor with garbage it can't use 100 times per second by passing it all CRSF from the radio as originally requested. That would be needlessly taking time away from our PID loop and possibly making the sensor less reliable by sending it "random junk" that it doesn't understand. I'd prefer to send it only what it is set up to handle. |
Sure. I'll be back with LA traces for these packages. For the beginning, I'll use the BATT sensor CU01 |
The sensors make sense on the RM NEXUS-X / -XR. Unfortunately, the firmware does not allow this to be tested on the target. I have an ERS-GPS available here. |
|
The following screenshots are for the three basic operations:
The LUA defines the following operations: CRSF_FRAMETYPE_MSP_REQ = 0x7A but I could only see 0x7c and 0x7b. The LUA also uses the following fixed extended addresses (for all devices): local CRSF_ADDRESS_BETAFLIGHT = 0xC8 Clearly, the first sensor in the daisy-chain of sensors receives also all other packages like rc-channels and link-stats. But the sensors do not forward these packages, so from he second sensor on, no rc-channels and link-stat packages are seen. If you need more, please let me know! Ping: 
Ping response: 
Device select: 
Device select response: 
Parameter write: 
Parameter write response: 
|
Yes unfortunately the F722 flash is full, and has been been for two years. |
Yes, it's a shame, but I understand the problem. @wimalopaan was kind enough to provide a target that I could test with. With your change, the RM ERS-GPS is functional in any case. Personally, I would prefer to use a normal GPS, but I could probably use a CU sensor. I can't think of anything right now that could be dispensed with. Maybe you could name two or three features that take up a lot of memory. |
3 participants
Summary
Add support for receiving sensor data from CRSF-speaking devices connected to a dedicated flight controller UART. Incoming CRSF frames are parsed and fed into INAV's real sensor subsystems, making the data automatically available to OSD, blackbox, navigation, and outgoing CRSF telemetry.
Supported Sensors
GPS_CRSFprovider — feedsgpsSolDRVlike MSP GPSBARO_CRSFdriver — converts altitude to pressure via ISA formulaConfiguration
Changes
FUNCTION_CRSF_SENSORserial function (repurposesFUNCTION_UNUSED_2, bit 24)crsf_sensor.c/h,gps_crsf.c,barometer_crsf.c/h,battery_sensor_crsf.c/hMCU_FLASH_SIZE > 512to exclude F722 and other flash-constrained targetsKnown Limitations