diff --git a/02_telemetry_bridge/lib/terseCRSF/src/terseCRSF.cpp b/02_telemetry_bridge/lib/terseCRSF/src/terseCRSF.cpp index fa44076..b400451 100644 --- a/02_telemetry_bridge/lib/terseCRSF/src/terseCRSF.cpp +++ b/02_telemetry_bridge/lib/terseCRSF/src/terseCRSF.cpp @@ -352,7 +352,18 @@ void CRSF::sendSBUS() //======================================================== uint8_t CRSF::decodeTelemetry(uint8_t *_buf, uint8_t len) { - uint8_t crsf_frm_lth = _buf[1]; + // PATCH LOCALE: validazione lunghezza + CRC8 prima di decodificare. + // L'upstream non valida nulla qui (la CRC era controllata solo in + // readCrsfFrame(), non usato perche' i frame arrivano da ESP-NOW): un + // frame corto o corrotto veniva decodificato leggendo byte oltre il + // frame e producendo telemetria spazzatura. 'len' = byte disponibili. + // _buf: [0]=sync [1]=lth(type+payload+crc) [2]=type ... [lth+1]=crc + uint8_t crsf_frm_lth = _buf[1]; + if (len < 4) return 0; + if (crsf_frm_lth < 2) return 0; // almeno type+crc + if ((uint16_t)crsf_frm_lth + 2 > len) return 0; // frame non completo + uint8_t calc_crc = crc8_dvb_s2_sbuf_accum(&_buf[2], crsf_frm_lth - 1); + if (calc_crc != _buf[crsf_frm_lth + 1]) return 0; // CRC errata -> scarta uint8_t crsf_id = _buf[2]; if (crsf_id == 0) { diff --git a/02_telemetry_bridge/lib/terseCRSF/src/terseCRSF.h b/02_telemetry_bridge/lib/terseCRSF/src/terseCRSF.h index 34cd81e..a45cd81 100644 --- a/02_telemetry_bridge/lib/terseCRSF/src/terseCRSF.h +++ b/02_telemetry_bridge/lib/terseCRSF/src/terseCRSF.h @@ -12,6 +12,10 @@ // - terseCRSF.cpp, BATTERY_ID: bat_fuel_drawn leggeva 4 byte // (bytes2int32) includendo il byte "percent" come LSB e gonfiando i // mAh consumati (es. 350 -> 89678). Corretto a lettura uint24 a 3 byte. +// - terseCRSF.cpp, decodeTelemetry(): aggiunta validazione lunghezza + +// CRC8 in testa. L'upstream non validava nulla (frame corti/corrotti da +// ESP-NOW venivano decodificati leggendo oltre il frame). Ora un frame +// incompleto o con CRC errata viene scartato (return 0). // // Per aggiornare la lib upstream: ri-applicare queste patch a mano. // ============================================================ diff --git a/02_telemetry_bridge/test/test_embedded_crsf_decode/test_decode.cpp b/02_telemetry_bridge/test/test_embedded_crsf_decode/test_decode.cpp index 420bd72..d1ee70b 100644 --- a/02_telemetry_bridge/test/test_embedded_crsf_decode/test_decode.cpp +++ b/02_telemetry_bridge/test/test_embedded_crsf_decode/test_decode.cpp @@ -1,14 +1,17 @@ // ============================================================ // Test on-device (Unity via USB, nessuna catena RF) per il path // di decodifica realmente usato in produzione: terseCRSF's -// CRSF::decodeTelemetry(), chiamato da mode_human.cpp/mode_ltm.cpp -// esattamente come qui, su frame CRSF sintetici byte-esatti -// (nessun ESP-NOW/radio necessario). +// CRSF::decodeTelemetry(), chiamato dai modi esattamente come qui, +// su frame CRSF sintetici byte-esatti (nessun ESP-NOW/radio). // // I vettori attesi (valori decodificati) sono stati calcolati -// indipendentemente in Python replicando le formule di -// terseCRSF.cpp (big-endian, *0.1, RADS2DEGS, ecc.), non copiati -// dall'implementazione. +// indipendentemente replicando le formule di terseCRSF.cpp +// (big-endian, *0.1, RADS2DEGS, ecc.), non copiati dall'impl. +// +// NB: dalla patch locale, decodeTelemetry() valida lunghezza + CRC8 +// (poly 0xD5, sui byte type+payload). Ogni frame qui include quindi +// la CRC corretta nell'ultimo byte (buf[lth+1]); le CRC sono state +// calcolate offline. Un test dedicato verifica il rigetto su CRC errata. // ============================================================ #include #include @@ -24,6 +27,7 @@ void test_decode_gps(void) { buf[0] = 0xC8; buf[1] = 17; buf[2] = GPS_ID; const uint8_t payload[] = {27,25,73,180, 5,122,72,96, 0,125, 35,40, 4,128, 11}; memcpy(&buf[3], payload, sizeof(payload)); + buf[18] = 0xB4; // CRC8 valida (type+payload) uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf)); TEST_ASSERT_EQUAL_UINT8(GPS_ID, id); @@ -40,6 +44,7 @@ void test_decode_attitude(void) { buf[0] = 0xC8; buf[1] = 8; buf[2] = ATTITUDE_ID; const uint8_t payload[] = {6,209, 252,151, 235,140}; memcpy(&buf[3], payload, sizeof(payload)); + buf[9] = 0xF6; // CRC8 valida uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf)); TEST_ASSERT_EQUAL_UINT8(ATTITUDE_ID, id); @@ -49,28 +54,22 @@ void test_decode_attitude(void) { TEST_ASSERT_FLOAT_WITHIN(0.01f, 330.0f, crsf.attiF_yaw); } -// NB: documenta un comportamento della libreria terseCRSF vendorizzata -// (non codice nostro): il commento dice "uint24_t mAh drawn" ma -// bat_fuel_drawn legge bytes2int32(&_buf[7]), cioe' 4 byte (buf[7..10]), -// mentre lo standard CRSF prevede capacity_used a 3 byte + 1 byte percent -// (buf[7..9] capacita', buf[10] percent). Il risultato e' che bat_fuel_drawn -// include anche il byte percent come LSB e ignora correttamente solo il -// confine a 3 byte: il valore "mAh" mostrato da HUMAN/LTM non corrisponde -// alla capacita' realmente trasmessa. Se la cosa diventa rilevante in -// pratica, va corretto a monte (fork/patch locale di terseCRSF), non qui. -void test_decode_battery_documents_upstream_fuel_drawn_quirk(void) { +// Battery: lth corretto a 10 (type + 8 payload + crc). bat_fuel_drawn ora +// e' 350 (uint24 su buf[7..9]) dopo la patch locale, NON piu' 89678. +void test_decode_battery(void) { uint8_t buf[12] = {0}; - buf[0] = 0xC8; buf[1] = 9; buf[2] = BATTERY_ID; + buf[0] = 0xC8; buf[1] = 10; buf[2] = BATTERY_ID; // voltage=164(*0.1=16.4V) current=12(*0.1=1.2A) capacity=350mAh(3 byte) percent=78 const uint8_t payload[] = {0,164, 0,12, 0,1,94, 78}; memcpy(&buf[3], payload, sizeof(payload)); + buf[11] = 0xC3; // CRC8 valida uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf)); TEST_ASSERT_EQUAL_UINT8(BATTERY_ID, id); TEST_ASSERT_FLOAT_WITHIN(0.01f, 16.4f, crsf.batF_voltage); TEST_ASSERT_FLOAT_WITHIN(0.01f, 1.2f, crsf.batF_current); - TEST_ASSERT_EQUAL_UINT8(78, crsf.bat_remaining); // corretto - TEST_ASSERT_EQUAL_UINT32(89678, crsf.bat_fuel_drawn); // NON e' 350: bug upstream + TEST_ASSERT_EQUAL_UINT8(78, crsf.bat_remaining); + TEST_ASSERT_EQUAL_UINT32(350, crsf.bat_fuel_drawn); // patch uint24 } void test_decode_link_statistics(void) { @@ -78,6 +77,7 @@ void test_decode_link_statistics(void) { buf[0] = 0xC8; buf[1] = 12; buf[2] = LINK_ID; const uint8_t payload[] = {52, 0, 100, 247, 0, 2, 3, 60, 98, 245}; memcpy(&buf[3], payload, sizeof(payload)); + buf[13] = 0x3D; // CRC8 valida uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf)); TEST_ASSERT_EQUAL_UINT8(LINK_ID, id); @@ -92,31 +92,37 @@ void test_decode_link_statistics(void) { void test_decode_flight_mode(void) { uint8_t buf[10] = {0}; buf[0] = 0xC8; - buf[1] = 7; // flight_mode_lth = buf[1]-3 = 4 ("ACRO") + buf[1] = 7; // type + "ACRO\0"(5) + crc ; lth-3 = 4 buf[2] = FLIGHT_MODE_ID; - memcpy(&buf[3], "ACRO", 4); + memcpy(&buf[3], "ACRO", 4); // buf[7]=0 (terminatore) gia' azzerato + buf[8] = 0x80; // CRC8 valida (type + "ACRO\0") uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf)); TEST_ASSERT_EQUAL_UINT8(FLIGHT_MODE_ID, id); TEST_ASSERT_EQUAL_STRING("ACRO", crsf.flightMode.c_str()); } -// Caratterizzazione: decodeTelemetry() NON valida sync byte ne' CRC. -// La validazione CRC esiste solo in readCrsfFrame() (letture da UART), -// che qui non usiamo perche' i frame arrivano da ESP-NOW. Un frame con -// sync sbagliato e CRC spazzatura viene comunque decodificato. -void test_decode_does_not_validate_sync_or_crc(void) { +// Dalla patch locale decodeTelemetry() VALIDA la CRC: un frame con CRC +// errata viene scartato (return 0). (Il sync byte resta non validato by +// design, ma una CRC sbagliata basta a rigettare il frame.) +void test_decode_rejects_bad_crc(void) { uint8_t buf[20] = {0}; - buf[0] = 0x00; // sync sbagliato (dovrebbe essere 0xC8/0xEA) - buf[1] = 17; - buf[2] = GPS_ID; + buf[0] = 0xC8; buf[1] = 17; buf[2] = GPS_ID; const uint8_t payload[] = {27,25,73,180, 5,122,72,96, 0,125, 35,40, 4,128, 11}; memcpy(&buf[3], payload, sizeof(payload)); - buf[18] = 0xFF; // crc "spazzatura" + buf[18] = 0x00; // CRC errata (corretta sarebbe 0xB4) uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf)); - TEST_ASSERT_EQUAL_UINT8(GPS_ID, id); - TEST_ASSERT_FLOAT_WITHIN(0.0001f, 45.46421f, crsf.gpsF_lat); + TEST_ASSERT_EQUAL_UINT8(0, id); // scartato +} + +// Frame troncato (len < frame completo): deve essere scartato senza leggere +// oltre il buffer. +void test_decode_rejects_truncated(void) { + uint8_t buf[8] = {0}; + buf[0] = 0xC8; buf[1] = 17; buf[2] = GPS_ID; // dichiara 17 ma len=8 + uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf)); + TEST_ASSERT_EQUAL_UINT8(0, id); } void setup() { @@ -125,10 +131,11 @@ void setup() { UNITY_BEGIN(); RUN_TEST(test_decode_gps); RUN_TEST(test_decode_attitude); - RUN_TEST(test_decode_battery_documents_upstream_fuel_drawn_quirk); + RUN_TEST(test_decode_battery); RUN_TEST(test_decode_link_statistics); RUN_TEST(test_decode_flight_mode); - RUN_TEST(test_decode_does_not_validate_sync_or_crc); + RUN_TEST(test_decode_rejects_bad_crc); + RUN_TEST(test_decode_rejects_truncated); UNITY_END(); }