Skip to content

Commit e5bfe79

Browse files
sensei-hackerclaude
andcommitted
Fix CRSF telemetry corruption from PR iNavFlight#11025
This commit fixes the critical bugs that caused PR iNavFlight#11025 to be reverted (PR iNavFlight#11139). The original implementation caused telemetry stream corruption by emitting malformed frames when sensors were unavailable. Root Cause: The original PR scheduled telemetry frames unconditionally (if feature compiled in) but frame functions only wrote data when sensors were available. This resulted in frames containing only [SYNC][CRC] instead of the proper [SYNC][LENGTH][TYPE][PAYLOAD][CRC] structure, corrupting the CRSF protocol stream and breaking ALL telemetry (not just new frames). Fixes Implemented: 1. RPM Frame (Bug #1 - CRITICAL): - Added conditional scheduling: only schedule if ESC_SENSOR_ENABLED and motorCount > 0 - Added protocol limit enforcement: max 19 RPM values per CRSF spec - Location: src/main/telemetry/crsf.c:705-707, 337-343 2. Temperature Frame (Bug #2 - CRITICAL): - Added conditional scheduling: only schedule if temperature sources are actually available (ESC sensors OR temperature sensors) - Added bounds checking: prevent buffer overflow beyond 20 temperatures - Location: src/main/telemetry/crsf.c:709-732, 361-381 3. Buffer Overflow Protection (Bug #4): - Added MAX_CRSF_TEMPS constant and bounds checks in loops - Prevents array overflow if >20 temperature sources configured - Location: src/main/telemetry/crsf.c:361, 368, 376 4. Protocol Limit Enforcement (Bug #5): - Added MAX_CRSF_RPM_VALUES constant (19 per CRSF spec) - Clamp motorCount to protocol limit before sending - Location: src/main/telemetry/crsf.c:337, 342-344 Implementation Pattern: Follows the GPS frame pattern: ✓ Conditional scheduling - only schedule if sensor available ✓ Unconditional writing - always write complete frame data when called Testing: - Code compiles successfully (verified with SITL build) - No syntax errors or warnings - All fixes follow existing code patterns in crsf.c Related Issues: - Original PR: iNavFlight#11025 (merged Nov 28, 2025, reverted same day) - Revert PR: iNavFlight#11139 - Investigation: claude/developer/sent/pr11025-root-cause-analysis.md Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent f8dc0bc commit e5bfe79

1 file changed

Lines changed: 34 additions & 5 deletions

File tree

src/main/telemetry/crsf.c

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -334,9 +334,15 @@ int24_t rpm_value[]; // 1 - 19 RPM values with negative ones representing
334334
*/
335335
static void crsfRpm(sbuf_t *dst)
336336
{
337+
const uint8_t MAX_CRSF_RPM_VALUES = 19; // CRSF protocol limit: 1-19 RPM values
337338
uint8_t motorCount = getMotorCount();
338339

339340
if (STATE(ESC_SENSOR_ENABLED) && motorCount > 0) {
341+
// Enforce protocol limit
342+
if (motorCount > MAX_CRSF_RPM_VALUES) {
343+
motorCount = MAX_CRSF_RPM_VALUES;
344+
}
345+
340346
sbufWriteU8(dst, 1 + (motorCount * 3) + CRSF_FRAME_LENGTH_TYPE_CRC);
341347
crsfSerialize8(dst, CRSF_FRAMETYPE_RPM);
342348
// 0 = FC including all ESCs
@@ -358,22 +364,22 @@ int16_t temperature[]; // up to 20 temperature values in deci-degree (tenths of
358364
*/
359365
static void crsfTemperature(sbuf_t *dst)
360366
{
361-
367+
const uint8_t MAX_CRSF_TEMPS = 20; // Maximum temperatures per CRSF frame
362368
uint8_t tempCount = 0;
363369
int16_t temperatures[20];
364370

365371
#ifdef USE_ESC_SENSOR
366372
uint8_t motorCount = getMotorCount();
367373
if (STATE(ESC_SENSOR_ENABLED) && motorCount > 0) {
368-
for (uint8_t i = 0; i < motorCount; i++) {
374+
for (uint8_t i = 0; i < motorCount && tempCount < MAX_CRSF_TEMPS; i++) {
369375
const escSensorData_t *escState = getEscTelemetry(i);
370376
temperatures[tempCount++] = (escState) ? escState->temperature * 10 : TEMPERATURE_INVALID_VALUE;
371377
}
372378
}
373379
#endif
374380

375381
#ifdef USE_TEMPERATURE_SENSOR
376-
for (uint8_t i = 0; i < MAX_TEMP_SENSORS; i++) {
382+
for (uint8_t i = 0; i < MAX_TEMP_SENSORS && tempCount < MAX_CRSF_TEMPS; i++) {
377383
int16_t value;
378384
if (getSensorTemperature(i, &value))
379385
temperatures[tempCount++] = value;
@@ -702,10 +708,33 @@ void initCrsfTelemetry(void)
702708
}
703709
#endif
704710
#ifdef USE_ESC_SENSOR
705-
crsfSchedule[index++] = BV(CRSF_FRAME_RPM_INDEX);
711+
if (STATE(ESC_SENSOR_ENABLED) && getMotorCount() > 0) {
712+
crsfSchedule[index++] = BV(CRSF_FRAME_RPM_INDEX);
713+
}
706714
#endif
707715
#if defined(USE_ESC_SENSOR) || defined(USE_TEMPERATURE_SENSOR)
708-
crsfSchedule[index++] = BV(CRSF_FRAME_TEMP_INDEX);
716+
// Only schedule temperature frame if we have temperature sources available
717+
bool hasTemperatureSources = false;
718+
#ifdef USE_ESC_SENSOR
719+
if (STATE(ESC_SENSOR_ENABLED) && getMotorCount() > 0) {
720+
hasTemperatureSources = true;
721+
}
722+
#endif
723+
#ifdef USE_TEMPERATURE_SENSOR
724+
if (!hasTemperatureSources) {
725+
// Check if any temperature sensors are configured
726+
for (uint8_t i = 0; i < MAX_TEMP_SENSORS; i++) {
727+
int16_t value;
728+
if (getSensorTemperature(i, &value)) {
729+
hasTemperatureSources = true;
730+
break;
731+
}
732+
}
733+
}
734+
#endif
735+
if (hasTemperatureSources) {
736+
crsfSchedule[index++] = BV(CRSF_FRAME_TEMP_INDEX);
737+
}
709738
#endif
710739
#ifdef USE_PITOT
711740
if (sensors(SENSOR_PITOT)) {

0 commit comments

Comments
 (0)