Augmenting CRSF telemetry parser for BATT frames with trailing sensor-id

[wimalopaan]

The CRSF protocol definition allows to augment existing packet definitions in a backward compatible manner. It allows to extend the payload if the overall frame structure remains valid.

The problem:
The crsf BATT sensor type does not support multiple instances. This leads to problems, if e.g. multiple ESCs sends this kind of telemetry sensor frames.
Also, the BATT sensor is the only actually available type to measure current.
There is a proposal to the CRSF protocol definition to add also a multi-instance current sensor frame type like VOLTS, RPM or TEMP: tbs-fpv/tbs-crsf-spec#38

But this is not expected to be setup in the near future.

The solution:
The CRSF protocol definition allows to extend CRSF frames in a backward compatible manner by extending the payload with addition al fields.
This could be used by CRSF devices to add an instance id to the BATT payload (at the end).
This new frametype can be detected by the crsf parser easily and then handle the old type of BATT sensor frames as well as the new frames with instance id included.

This PR modifies the existing CRSF parser to handle old and new BATT sensor types correctly.

As an example, the follwing branch adds the new BATT frame structure to the well known ESCape32 ESC firmware: https://github.com/wimalopaan/ESCape32/tree/wm_crsf_batt_id

Summary by CodeRabbit

• Bug Fixes
  • Improved Crossfire telemetry handling for battery data from systems with multiple sensor instances.
  • Battery voltage, current, capacity, and remaining-capacity readings now preserve the correct sensor-instance identifier when such details are present in the incoming telemetry frame, reducing ambiguity between readings.

[coderabbitai]

📝 Walkthrough

Walkthrough

Crossfire telemetry processing now supports per-sensor identifiers. Battery frames conditionally extract sensorID values and pass them through voltage, current, capacity, and remaining-capacity telemetry updates.

Changes

Crossfire telemetry

Layer / File(s)	Summary
Propagate battery sensor identifiers radio/src/telemetry/crossfire.cpp	processCrossfireTelemetryValue combines the optional sensor ID with the base telemetry ID, while BATTERY_ID extracts and forwards sensor IDs for battery metrics.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title accurately summarizes the main change: adding trailing sensor-id support to CRSF BATT telemetry parsing.
Description check	✅ Passed	The description explains the problem, solution, and context; it only omits the template's exact section headings.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

radio/src/telemetry/crossfire.cpp (1)

366-381: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Align length checks with sibling parsers and fix indentation.

While rxBufferCount > 12 correctly identifies the extended battery frame, other handlers in this file (such as BARO_ALT_ID) check the parsed crsfPayloadLen variable to determine the presence of optional trailing data. Additionally, the new logic uses deeper indentation (up to 10 spaces) instead of standard 2-space increments.

Consider swapping to crsfPayloadLen >= 11 for consistency and adjusting the indentation to conform to the project's formatting style.

♻️ Proposed refactor

     case BATTERY_ID:
     {
       uint8_t sensorID = 0;
-      if (rxBufferCount > 12) {
-          if (getCrossfireTelemetryValue<1>(11, value, rxBuffer)) {
-              sensorID = value;
-          }
-      }
-      if (getCrossfireTelemetryValue<2>(3, value, rxBuffer))
-          processCrossfireTelemetryValue(BATT_VOLTAGE_INDEX, value, sensorID);
-      if (getCrossfireTelemetryValue<2>(5, value, rxBuffer))
-          processCrossfireTelemetryValue(BATT_CURRENT_INDEX, value, sensorID);
-      if (getCrossfireTelemetryValue<3>(7, value, rxBuffer))
-          processCrossfireTelemetryValue(BATT_CAPACITY_INDEX, value, sensorID);
-      if (getCrossfireTelemetryValue<1>(10, value, rxBuffer))
-          processCrossfireTelemetryValue(BATT_REMAINING_INDEX, value, sensorID);
+      if (crsfPayloadLen >= 11) {
+        if (getCrossfireTelemetryValue<1>(11, value, rxBuffer)) {
+          sensorID = value;
+        }
+      }
+      if (getCrossfireTelemetryValue<2>(3, value, rxBuffer))
+        processCrossfireTelemetryValue(BATT_VOLTAGE_INDEX, value, sensorID);
+      if (getCrossfireTelemetryValue<2>(5, value, rxBuffer))
+        processCrossfireTelemetryValue(BATT_CURRENT_INDEX, value, sensorID);
+      if (getCrossfireTelemetryValue<3>(7, value, rxBuffer))
+        processCrossfireTelemetryValue(BATT_CAPACITY_INDEX, value, sensorID);
+      if (getCrossfireTelemetryValue<1>(10, value, rxBuffer))
+        processCrossfireTelemetryValue(BATT_REMAINING_INDEX, value, sensorID);
     }
       break;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@radio/src/telemetry/crossfire.cpp` around lines 366 - 381, Update the
extended battery parsing block to use crsfPayloadLen >= 11 instead of
rxBufferCount > 12 when deciding whether to read sensorID, matching sibling
parsers such as BARO_ALT_ID. Reformat the block with the project’s standard
two-space indentation while preserving the existing telemetry extraction and
processing behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@radio/src/telemetry/crossfire.cpp`:
- Around line 366-381: Update the extended battery parsing block to use
crsfPayloadLen >= 11 instead of rxBufferCount > 12 when deciding whether to read
sensorID, matching sibling parsers such as BARO_ALT_ID. Reformat the block with
the project’s standard two-space indentation while preserving the existing
telemetry extraction and processing behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e394637c-577f-4361-a589-7c03f801e7bc

📥 Commits

Reviewing files that changed from the base of the PR and between e184cf3 and dafdcaf.

📒 Files selected for processing (1)

• radio/src/telemetry/crossfire.cpp