terseCRSF: valida lunghezza + CRC8 in decodeTelemetry; suite di test riallineata
- decodeTelemetry() ora scarta frame incompleti o con CRC8 errata (prima non validava nulla: frame corti/corrotti da ESP-NOW venivano decodificati leggendo oltre il frame -> telemetria spazzatura, e OOB read su FLIGHT_MODE). - test_decode.cpp: vettori ricostruiti come frame reali con CRC valida (calcolata offline), battery ri-quadrato a lth=10, assert bat_fuel_drawn corretto a 350 (era 89678, pre-patch uint24), test "non valida CRC" invertito in "rigetta CRC errata", aggiunto test su frame troncato. - Emulatore (03) e backpack reale usano lo stesso framing/CRC -> passano. DA RI-ESEGUIRE A BANCO: 'pio test -e esp32dev_test' e test RF reale. Se la telemetria reale sparisce, il backpack potrebbe non rilasciare una CRC valida nel frame ESP-NOW -> in tal caso ripiegare su validazione solo-lunghezza. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -352,7 +352,18 @@ void CRSF::sendSBUS()
|
|||||||
//========================================================
|
//========================================================
|
||||||
uint8_t CRSF::decodeTelemetry(uint8_t *_buf, uint8_t len)
|
uint8_t CRSF::decodeTelemetry(uint8_t *_buf, uint8_t len)
|
||||||
{
|
{
|
||||||
|
// 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];
|
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];
|
uint8_t crsf_id = _buf[2];
|
||||||
if (crsf_id == 0)
|
if (crsf_id == 0)
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -12,6 +12,10 @@
|
|||||||
// - terseCRSF.cpp, BATTERY_ID: bat_fuel_drawn leggeva 4 byte
|
// - terseCRSF.cpp, BATTERY_ID: bat_fuel_drawn leggeva 4 byte
|
||||||
// (bytes2int32) includendo il byte "percent" come LSB e gonfiando i
|
// (bytes2int32) includendo il byte "percent" come LSB e gonfiando i
|
||||||
// mAh consumati (es. 350 -> 89678). Corretto a lettura uint24 a 3 byte.
|
// 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.
|
// Per aggiornare la lib upstream: ri-applicare queste patch a mano.
|
||||||
// ============================================================
|
// ============================================================
|
||||||
|
|||||||
@@ -1,14 +1,17 @@
|
|||||||
// ============================================================
|
// ============================================================
|
||||||
// Test on-device (Unity via USB, nessuna catena RF) per il path
|
// Test on-device (Unity via USB, nessuna catena RF) per il path
|
||||||
// di decodifica realmente usato in produzione: terseCRSF's
|
// di decodifica realmente usato in produzione: terseCRSF's
|
||||||
// CRSF::decodeTelemetry(), chiamato da mode_human.cpp/mode_ltm.cpp
|
// CRSF::decodeTelemetry(), chiamato dai modi esattamente come qui,
|
||||||
// esattamente come qui, su frame CRSF sintetici byte-esatti
|
// su frame CRSF sintetici byte-esatti (nessun ESP-NOW/radio).
|
||||||
// (nessun ESP-NOW/radio necessario).
|
|
||||||
//
|
//
|
||||||
// I vettori attesi (valori decodificati) sono stati calcolati
|
// I vettori attesi (valori decodificati) sono stati calcolati
|
||||||
// indipendentemente in Python replicando le formule di
|
// indipendentemente replicando le formule di terseCRSF.cpp
|
||||||
// terseCRSF.cpp (big-endian, *0.1, RADS2DEGS, ecc.), non copiati
|
// (big-endian, *0.1, RADS2DEGS, ecc.), non copiati dall'impl.
|
||||||
// dall'implementazione.
|
//
|
||||||
|
// 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 <Arduino.h>
|
#include <Arduino.h>
|
||||||
#include <unity.h>
|
#include <unity.h>
|
||||||
@@ -24,6 +27,7 @@ void test_decode_gps(void) {
|
|||||||
buf[0] = 0xC8; 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};
|
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));
|
memcpy(&buf[3], payload, sizeof(payload));
|
||||||
|
buf[18] = 0xB4; // CRC8 valida (type+payload)
|
||||||
|
|
||||||
uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf));
|
uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf));
|
||||||
TEST_ASSERT_EQUAL_UINT8(GPS_ID, id);
|
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;
|
buf[0] = 0xC8; buf[1] = 8; buf[2] = ATTITUDE_ID;
|
||||||
const uint8_t payload[] = {6,209, 252,151, 235,140};
|
const uint8_t payload[] = {6,209, 252,151, 235,140};
|
||||||
memcpy(&buf[3], payload, sizeof(payload));
|
memcpy(&buf[3], payload, sizeof(payload));
|
||||||
|
buf[9] = 0xF6; // CRC8 valida
|
||||||
|
|
||||||
uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf));
|
uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf));
|
||||||
TEST_ASSERT_EQUAL_UINT8(ATTITUDE_ID, id);
|
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);
|
TEST_ASSERT_FLOAT_WITHIN(0.01f, 330.0f, crsf.attiF_yaw);
|
||||||
}
|
}
|
||||||
|
|
||||||
// NB: documenta un comportamento della libreria terseCRSF vendorizzata
|
// Battery: lth corretto a 10 (type + 8 payload + crc). bat_fuel_drawn ora
|
||||||
// (non codice nostro): il commento dice "uint24_t mAh drawn" ma
|
// e' 350 (uint24 su buf[7..9]) dopo la patch locale, NON piu' 89678.
|
||||||
// bat_fuel_drawn legge bytes2int32(&_buf[7]), cioe' 4 byte (buf[7..10]),
|
void test_decode_battery(void) {
|
||||||
// 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) {
|
|
||||||
uint8_t buf[12] = {0};
|
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
|
// 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};
|
const uint8_t payload[] = {0,164, 0,12, 0,1,94, 78};
|
||||||
memcpy(&buf[3], payload, sizeof(payload));
|
memcpy(&buf[3], payload, sizeof(payload));
|
||||||
|
buf[11] = 0xC3; // CRC8 valida
|
||||||
|
|
||||||
uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf));
|
uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf));
|
||||||
TEST_ASSERT_EQUAL_UINT8(BATTERY_ID, id);
|
TEST_ASSERT_EQUAL_UINT8(BATTERY_ID, id);
|
||||||
TEST_ASSERT_FLOAT_WITHIN(0.01f, 16.4f, crsf.batF_voltage);
|
TEST_ASSERT_FLOAT_WITHIN(0.01f, 16.4f, crsf.batF_voltage);
|
||||||
TEST_ASSERT_FLOAT_WITHIN(0.01f, 1.2f, crsf.batF_current);
|
TEST_ASSERT_FLOAT_WITHIN(0.01f, 1.2f, crsf.batF_current);
|
||||||
TEST_ASSERT_EQUAL_UINT8(78, crsf.bat_remaining); // corretto
|
TEST_ASSERT_EQUAL_UINT8(78, crsf.bat_remaining);
|
||||||
TEST_ASSERT_EQUAL_UINT32(89678, crsf.bat_fuel_drawn); // NON e' 350: bug upstream
|
TEST_ASSERT_EQUAL_UINT32(350, crsf.bat_fuel_drawn); // patch uint24
|
||||||
}
|
}
|
||||||
|
|
||||||
void test_decode_link_statistics(void) {
|
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;
|
buf[0] = 0xC8; buf[1] = 12; buf[2] = LINK_ID;
|
||||||
const uint8_t payload[] = {52, 0, 100, 247, 0, 2, 3, 60, 98, 245};
|
const uint8_t payload[] = {52, 0, 100, 247, 0, 2, 3, 60, 98, 245};
|
||||||
memcpy(&buf[3], payload, sizeof(payload));
|
memcpy(&buf[3], payload, sizeof(payload));
|
||||||
|
buf[13] = 0x3D; // CRC8 valida
|
||||||
|
|
||||||
uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf));
|
uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf));
|
||||||
TEST_ASSERT_EQUAL_UINT8(LINK_ID, id);
|
TEST_ASSERT_EQUAL_UINT8(LINK_ID, id);
|
||||||
@@ -92,31 +92,37 @@ void test_decode_link_statistics(void) {
|
|||||||
void test_decode_flight_mode(void) {
|
void test_decode_flight_mode(void) {
|
||||||
uint8_t buf[10] = {0};
|
uint8_t buf[10] = {0};
|
||||||
buf[0] = 0xC8;
|
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;
|
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));
|
uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf));
|
||||||
TEST_ASSERT_EQUAL_UINT8(FLIGHT_MODE_ID, id);
|
TEST_ASSERT_EQUAL_UINT8(FLIGHT_MODE_ID, id);
|
||||||
TEST_ASSERT_EQUAL_STRING("ACRO", crsf.flightMode.c_str());
|
TEST_ASSERT_EQUAL_STRING("ACRO", crsf.flightMode.c_str());
|
||||||
}
|
}
|
||||||
|
|
||||||
// Caratterizzazione: decodeTelemetry() NON valida sync byte ne' CRC.
|
// Dalla patch locale decodeTelemetry() VALIDA la CRC: un frame con CRC
|
||||||
// La validazione CRC esiste solo in readCrsfFrame() (letture da UART),
|
// errata viene scartato (return 0). (Il sync byte resta non validato by
|
||||||
// che qui non usiamo perche' i frame arrivano da ESP-NOW. Un frame con
|
// design, ma una CRC sbagliata basta a rigettare il frame.)
|
||||||
// sync sbagliato e CRC spazzatura viene comunque decodificato.
|
void test_decode_rejects_bad_crc(void) {
|
||||||
void test_decode_does_not_validate_sync_or_crc(void) {
|
|
||||||
uint8_t buf[20] = {0};
|
uint8_t buf[20] = {0};
|
||||||
buf[0] = 0x00; // sync sbagliato (dovrebbe essere 0xC8/0xEA)
|
buf[0] = 0xC8; buf[1] = 17; buf[2] = GPS_ID;
|
||||||
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};
|
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));
|
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));
|
uint8_t id = crsf.decodeTelemetry(buf, sizeof(buf));
|
||||||
TEST_ASSERT_EQUAL_UINT8(GPS_ID, id);
|
TEST_ASSERT_EQUAL_UINT8(0, id); // scartato
|
||||||
TEST_ASSERT_FLOAT_WITHIN(0.0001f, 45.46421f, crsf.gpsF_lat);
|
}
|
||||||
|
|
||||||
|
// 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() {
|
void setup() {
|
||||||
@@ -125,10 +131,11 @@ void setup() {
|
|||||||
UNITY_BEGIN();
|
UNITY_BEGIN();
|
||||||
RUN_TEST(test_decode_gps);
|
RUN_TEST(test_decode_gps);
|
||||||
RUN_TEST(test_decode_attitude);
|
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_link_statistics);
|
||||||
RUN_TEST(test_decode_flight_mode);
|
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();
|
UNITY_END();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user