From f960c84f5bf73894fcfd67ba7e201fa3e8d590be Mon Sep 17 00:00:00 2001 From: Andrew Yong Date: Wed, 12 Aug 2026 14:29:52 +0800 Subject: [PATCH 1/8] fix(stm32wl): reset instead of hanging on faults (#11420) * fix(stm32wl): reset instead of hanging on faults Reset instead of hanging forever on three unrecoverable faults, each of which previously required a manual power cycle to recover: - HardFault_Handler_C: blinked SOS forever with no debugger attached; now resets once the fault registers are printed. - __wrap___assert_func: silently hung on an assert failure; now prints file/line/func/expr via debug_printf, then resets. - earlyBootCheck: silently hung if the jump into the bootloader ROM failed to take; now calls the bare NVIC_SystemReset(), not the HAL wrapper, since it runs pre-HAL_Init() and MSP/VTOR are already repointed at the bootloader by this point, so a return would unwind through a corrupted stack frame. Assisted-by: Claude Sonnet 5 Signed-off-by: Andrew Yong * refactor(stm32wl): group fault-handling code Group the fault-handling code together and drop incidental cruft: - Move __wrap___assert_func next to HardFault_Handler_C and the other fault-reporting helpers. - Add banner comments separating linker-hack wrappers from fault-handling/recovery code, matching the existing Bootloader redirect banner. - Drop the forward declaration for debug_printf, no longer needed now that __wrap___assert_func sits below its definition. - Trim the Bootloader redirect banner comment to 1-3 lines. No behavior change. Assisted-by: Claude Sonnet 5 Signed-off-by: Andrew Yong --------- Signed-off-by: Andrew Yong --- src/platform/stm32wl/main-stm32wl.cpp | 80 ++++++--------------------- 1 file changed, 16 insertions(+), 64 deletions(-) diff --git a/src/platform/stm32wl/main-stm32wl.cpp b/src/platform/stm32wl/main-stm32wl.cpp index a2fed83573..50f7ae3d81 100644 --- a/src/platform/stm32wl/main-stm32wl.cpp +++ b/src/platform/stm32wl/main-stm32wl.cpp @@ -18,20 +18,9 @@ static bool stm32wlRtcValid = false; #endif // ─── Bootloader redirect ────────────────────────────────────────────────────── -// -// Why .noinit + constructor instead of TAMP backup registers: -// -// The STM32duino startup sequence initialises clocks which may call -// __HAL_RCC_BACKUPRESET_FORCE/RELEASE when configuring the LSE oscillator, -// wiping the entire backup domain (including TAMP->BKP0R) before setup() -// ever runs. The backup-register approach therefore cannot reliably survive -// a soft reset in this toolchain. -// -// Solution: store the magic in a .noinit SRAM variable. -// - NVIC_SystemReset() does NOT clear SRAM. -// - The linker script skips zero-init for .noinit sections. -// - __attribute__((constructor)) fires before main()/HAL_Init(), so we can -// intercept and jump before anything disturbs peripheral state. +// Uses .noinit SRAM instead of TAMP backup registers: STM32duino's clock init can wipe the +// backup domain via __HAL_RCC_BACKUPRESET_FORCE/RELEASE before setup() runs, but .noinit +// survives NVIC_SystemReset() and this constructor fires before HAL_Init() touches anything. #define BOOTLOADER_MAGIC 0xD00DB007UL #define SYS_MEM_BASE 0x1FFF0000UL @@ -58,8 +47,10 @@ __attribute__((constructor(101), used)) static void earlyBootCheck(void) SCB->VTOR = SYS_MEM_BASE; __set_MSP(*(volatile uint32_t *)SYS_MEM_BASE); ((void (*)(void))(*(volatile uint32_t *)(SYS_MEM_BASE + 4)))(); - while (1) - ; + // Should never be reached: the bootloader ROM does not return. A bare reset + // (rather than returning normally) avoids unwinding through this function's + // epilogue, which would restore registers relative to the now-repointed MSP. + NVIC_SystemReset(); } void enterDfuMode() @@ -169,15 +160,7 @@ void cpuDeepSleep(uint32_t msecToWake) #endif } -// Hacks to force more code and data out. - -// By default __assert_func uses fiprintf which pulls in stdio. -extern "C" void __wrap___assert_func(const char *, int, const char *, const char *) -{ - while (true) - ; - return; -} +// ─── Linker hacks to reduce code size ───────────────────────────────────────── // By default strerror has a lot of strings we probably don't use. Make it return an empty string instead. char empty = 0; @@ -197,6 +180,8 @@ extern "C" void __wrap__tzset_unlocked_r(struct _reent *reent_ptr) } #endif +// ─── Fault handling & recovery ──────────────────────────────────────────────── + // Taken from https://interrupt.memfault.com/blog/cortex-m-hardfault-debug typedef struct __attribute__((packed)) ContextStateFrame { uint32_t r0; @@ -233,32 +218,11 @@ static void debug_printf(const char *format, ...) uart_debug_write((uint8_t *)hardfault_message_buffer, min((unsigned int)length, sizeof(hardfault_message_buffer) - 1)); } -// N picked by guessing -#define DOT_TIME 1200000 -static void dot() +// By default __assert_func uses fiprintf which pulls in stdio. +extern "C" void __wrap___assert_func(const char *file, int line, const char *func, const char *failedexpr) { - digitalWrite(LED_POWER, LED_STATE_ON); - for (volatile int i = 0; i < DOT_TIME; i++) { /* busy wait */ - } - digitalWrite(LED_POWER, LED_STATE_OFF); - for (volatile int i = 0; i < DOT_TIME; i++) { /* busy wait */ - } -} - -static void dash() -{ - digitalWrite(LED_POWER, LED_STATE_ON); - for (volatile int i = 0; i < (DOT_TIME * 3); i++) { /* busy wait */ - } - digitalWrite(LED_POWER, LED_STATE_OFF); - for (volatile int i = 0; i < DOT_TIME; i++) { /* busy wait */ - } -} - -static void space() -{ - for (volatile int i = 0; i < (DOT_TIME * 3); i++) { /* busy wait */ - } + debug_printf("assert: %s:%d in %s: %s\r\n", file, line, func, failedexpr); + HAL_NVIC_SystemReset(); } // Disable optimizations for this function so "frame" argument @@ -277,17 +241,5 @@ extern "C" __attribute__((optimize("O0"))) void HardFault_Handler_C(sContextStat HALT_IF_DEBUGGING(); - // blink SOS forever - while (1) { - dot(); - dot(); - dot(); - dash(); - dash(); - dash(); - dot(); - dot(); - dot(); - space(); - } -} \ No newline at end of file + HAL_NVIC_SystemReset(); +} From b68de08c6bd5f74607324e1bf2010cb240519884 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 12 Aug 2026 08:34:44 +0200 Subject: [PATCH 2/8] chore(deps): update esp32-ch390 to v1.1.1 (#11398) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- variants/esp32s3/ELECROW-ThinkNode-G3/platformio.ini | 2 +- variants/esp32s3/ELECROW-ThinkNode-M7/platformio.ini | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/variants/esp32s3/ELECROW-ThinkNode-G3/platformio.ini b/variants/esp32s3/ELECROW-ThinkNode-G3/platformio.ini index f3d5e0f3b8..92236f638d 100644 --- a/variants/esp32s3/ELECROW-ThinkNode-G3/platformio.ini +++ b/variants/esp32s3/ELECROW-ThinkNode-G3/platformio.ini @@ -28,4 +28,4 @@ build_src_filter = lib_deps = ${esp32s3_base.lib_deps} # renovate: datasource=github-tags depName=ESP32-CH390 packageName=meshtastic/ESP32-CH390 - https://github.com/meshtastic/ESP32-CH390/archive/refs/tags/v1.1.0.zip + https://github.com/meshtastic/ESP32-CH390/archive/v1.1.1.zip diff --git a/variants/esp32s3/ELECROW-ThinkNode-M7/platformio.ini b/variants/esp32s3/ELECROW-ThinkNode-M7/platformio.ini index bfa69ab6c0..c1a872f89e 100644 --- a/variants/esp32s3/ELECROW-ThinkNode-M7/platformio.ini +++ b/variants/esp32s3/ELECROW-ThinkNode-M7/platformio.ini @@ -28,4 +28,4 @@ build_src_filter = lib_deps = ${esp32s3_base.lib_deps} # renovate: datasource=github-tags depName=ESP32-CH390 packageName=meshtastic/ESP32-CH390 - https://github.com/meshtastic/ESP32-CH390/archive/refs/tags/v1.1.0.zip + https://github.com/meshtastic/ESP32-CH390/archive/v1.1.1.zip From d5d5bad97cdca77da4ad77fd5fde9f84bba34d14 Mon Sep 17 00:00:00 2001 From: Michael Mohr Date: Tue, 11 Aug 2026 23:56:46 -0700 Subject: [PATCH 3/8] SEN5X: fix version parsing, VOC index reporting, and read-buffer handling (#11114) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * SEN5X: validate read lengths and initialize read buffers readBuffer() returns the number of data bytes written (0 on error). Check the return value against the number of bytes each caller consumes before parsing, and zero-initialize the destination buffers: findModel (5), getMeasurements (2), readValues (16), readPNValues (20), and vocStateFromSensor (SEN5X_VOC_STATE_BUFFER_SIZE). This also resolves maybe-uninitialized compiler warnings. Small simplifications in the same area: - Assign the converted measurement values directly; the isnan() checks on integer intermediates always took the conversion branch, so this preserves behavior. - Fold a redundant state comparison in wakeUp() that immediately followed the assignment of the same value. - Add an explicit 'return false' to the non-FSCom branches of loadState() and saveState(). Co-Authored-By: Claude Fable 5 * SEN5X: correct version parsing, VOC index gating, and cleaning wait - getVersion() requested 3 raw I2C bytes (2 data bytes) but parsed versionBuffer[0..6], so the hardware and protocol versions came from the buffer's initialized-but-unwritten tail. Request the full 12-byte reply (8 data bytes, the layout used by Sensirion's embedded-i2c-sen5x driver) and validate the received length before parsing. Also make the error message specific to the version read. - Use floating-point division when deriving major.minor version numbers so minor versions below 10 are preserved (integer division reported e.g. firmware 2.2 as 2.00). - Gate pm_voc_idx on vocIndex rather than noxIndex, so SEN54 devices (VOC but no NOx) report their VOC index. - Widen the millis() snapshot in startCleaning() to uint32_t so the 10-second fan-cleaning wait always measures elapsed time correctly. Co-Authored-By: Claude Fable 5 * SEN5X: add size checks to I2C helpers, stage VOC state, handle unavailable readings - readBuffer(): accept only request sizes that are a multiple of 3 (2 data bytes + 1 CRC per group), keeping the read loop's size arithmetic in bounds for any future caller. Current callers all comply. - sendCommand(): likewise accept only even payload sizes on the write side. - vocStateFromSensor(): read into a staging buffer and copy to vocState only after the full transfer verifies, so the stored state stays consistent if a read fails partway through. - readValues()/readPNValues(): the sensor reports unavailable values as 0xFFFF (unsigned) / 0x7FFF (signed); map these to the UINT16_MAX / UINT32_MAX / FLT_MAX sentinels that getMetrics() checks, so unavailable channels are omitted from telemetry rather than scaled into numeric readings. Guard the cumulative-to-binned PN subtraction so the sentinels are preserved. - readPNValues(): convert #/cm3 to #/0.1l as raw * 10, retaining the 0.1-resolution digit that dividing before multiplying discarded. Co-Authored-By: Claude Fable 5 * SEN5X: size read buffers in data bytes and document I2C helper conventions readBuffer()'s size parameter is the raw I2C transfer size including CRC bytes, while only the verified data bytes (2/3 of the request) are written to the destination. Two call sites sized their buffers in raw units (findModel: 48 for 32 data bytes; getMeasurements: 3 for 2); both were safe over-allocations. Size them in data bytes so every call site reflects the same convention, and document the raw-vs-data contracts on the readBuffer() and sendCommand() declarations. No functional change. Co-Authored-By: Claude Fable 5 * SEN5X: use named defines for I2C reply buffer sizes Follow the SEN5X_VOC_STATE_BUFFER_SIZE pattern for all reply reads, per review feedback: define each reply's payload size in data bytes, size the destination buffer with it, request + / 2 raw bytes, and compare the received count against the same define. The version and product-name guards now compare against the full reply size rather than the bytes parsed (previously 7 and 5); readBuffer() returns either 0 or the full data count, so the conditions accept and reject the same transfers. Co-Authored-By: Claude Fable 5 * SEN5X: document I2C helper size requirements instead of checking at runtime Per review feedback: drop the runtime even-size and multiple-of-3 checks from sendCommand()/readBuffer() and state the requirements in @brief/@param documentation on the declarations. All callers pass sizes derived from the SEN5X_*_BUFFER_SIZE defines, which satisfy both requirements. Co-Authored-By: Claude Fable 5 * SEN5X: name the sensor's invalid-value constants Per review feedback, define SEN5X_UINT_INVALID (0xFFFF) and SEN5X_INT_INVALID (0x7FFF) for the values the sensor reports when a reading is unavailable, and use them in the readValues()/readPNValues() conversions in place of the numeric literals. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 Co-authored-by: oscgonfer Co-authored-by: Thomas Göttgens --- src/modules/Telemetry/Sensor/SEN5XSensor.cpp | 114 +++++++++++-------- src/modules/Telemetry/Sensor/SEN5XSensor.h | 29 ++++- 2 files changed, 92 insertions(+), 51 deletions(-) diff --git a/src/modules/Telemetry/Sensor/SEN5XSensor.cpp b/src/modules/Telemetry/Sensor/SEN5XSensor.cpp index 7a721433c0..37df1204be 100644 --- a/src/modules/Telemetry/Sensor/SEN5XSensor.cpp +++ b/src/modules/Telemetry/Sensor/SEN5XSensor.cpp @@ -22,16 +22,18 @@ bool SEN5XSensor::getVersion() } delay(20); // From Sensirion Datasheet - uint8_t versionBuffer[12]{}; - size_t charNumber = readBuffer(&versionBuffer[0], 3); - if (charNumber == 0) { - LOG_ERROR("%s: Error getting data ready flag value", sensorName); + // Version reply layout: fw major/minor, fw debug, hw major/minor, + // protocol major/minor, padding + uint8_t versionBuffer[SEN5X_VERSION_BUFFER_SIZE]{}; + size_t charNumber = readBuffer(&versionBuffer[0], SEN5X_VERSION_BUFFER_SIZE + (SEN5X_VERSION_BUFFER_SIZE / 2)); + if (charNumber < SEN5X_VERSION_BUFFER_SIZE) { + LOG_ERROR("%s: Error getting device version value", sensorName); return false; } - firmwareVer = versionBuffer[0] + (versionBuffer[1] / 10); - hardwareVer = versionBuffer[3] + (versionBuffer[4] / 10); - protocolVer = versionBuffer[5] + (versionBuffer[6] / 10); + firmwareVer = versionBuffer[0] + (versionBuffer[1] / 10.0f); + hardwareVer = versionBuffer[3] + (versionBuffer[4] / 10.0f); + protocolVer = versionBuffer[5] + (versionBuffer[6] / 10.0f); LOG_INFO("%s: Firmware Version: %0.2f", sensorName, firmwareVer); LOG_INFO("%s: Hardware Version: %0.2f", sensorName, hardwareVer); @@ -48,12 +50,11 @@ bool SEN5XSensor::findModel() } delay(50); // From Sensirion Datasheet - const uint8_t nameSize = 48; - uint8_t name[nameSize]; - size_t charNumber = readBuffer(&name[0], nameSize); + uint8_t name[SEN5X_PRODUCT_NAME_BUFFER_SIZE]{}; + size_t charNumber = readBuffer(&name[0], SEN5X_PRODUCT_NAME_BUFFER_SIZE + (SEN5X_PRODUCT_NAME_BUFFER_SIZE / 2)); bool foundModel = false; - if (charNumber == 0) { + if (charNumber < SEN5X_PRODUCT_NAME_BUFFER_SIZE) { LOG_ERROR("%s: Error getting device name", sensorName); return foundModel; } @@ -361,15 +362,18 @@ bool SEN5XSensor::vocStateFromSensor() delay(20); // From Sensirion Datasheet - // Retrieve the data - // Allocate buffer to account for CRC - size_t receivedNumber = readBuffer(&vocState[0], SEN5X_VOC_STATE_BUFFER_SIZE + (SEN5X_VOC_STATE_BUFFER_SIZE / 2)); + // Retrieve the data into a staging buffer so a partial read (e.g. a CRC + // failure halfway through) cannot corrupt the current vocState. + // The requested size accounts for the CRC bytes + uint8_t stateBuffer[SEN5X_VOC_STATE_BUFFER_SIZE]{}; + size_t receivedNumber = readBuffer(&stateBuffer[0], SEN5X_VOC_STATE_BUFFER_SIZE + (SEN5X_VOC_STATE_BUFFER_SIZE / 2)); delay(20); // From Sensirion Datasheet - if (receivedNumber == 0) { + if (receivedNumber < SEN5X_VOC_STATE_BUFFER_SIZE) { LOG_DEBUG("%s: Error getting VOC's state", sensorName); return false; } + memcpy(vocState, stateBuffer, SEN5X_VOC_STATE_BUFFER_SIZE); // Print the state (if debug is on) LOG_DEBUG("%s: VOC state from sensor: [%u, %u, %u, %u, %u, %u, %u, %u]", sensorName, vocState[0], vocState[1], vocState[2], @@ -427,6 +431,7 @@ bool SEN5XSensor::loadState() return okay; #else LOG_ERROR("%s: Filesystem not implemented", sensorName); + return false; #endif } @@ -472,6 +477,7 @@ bool SEN5XSensor::saveState() return okay; #else LOG_ERROR("%s: Filesystem not implemented", sensorName); + return false; #endif } @@ -497,8 +503,7 @@ uint32_t SEN5XSensor::wakeUp() // keep track of how long it has passed pmMeasureStarted = getTime(); state = SEN5X_MEASUREMENT; - if (state == SEN5X_MEASUREMENT) - LOG_INFO("%s: Started measurement mode", sensorName); + LOG_INFO("%s: Started measurement mode", sensorName); return SEN5X_WARMUP_MS_1; } @@ -533,7 +538,7 @@ bool SEN5XSensor::startCleaning() // This message will be always printed so the user knows the device it's not hung LOG_INFO("%s: Started fan cleaning (10 sec)", sensorName); - uint16_t started = millis(); + uint32_t started = millis(); while (millis() - started < 10500) { delay(500); } @@ -658,9 +663,9 @@ bool SEN5XSensor::readValues() LOG_TRACE("%s: Reading PM Values", sensorName); delay(20); // From Sensirion Datasheet - uint8_t dataBuffer[16]{}; - size_t receivedNumber = readBuffer(&dataBuffer[0], 24); - if (receivedNumber == 0) { + uint8_t dataBuffer[SEN5X_READ_VALUES_BUFFER_SIZE]{}; + size_t receivedNumber = readBuffer(&dataBuffer[0], SEN5X_READ_VALUES_BUFFER_SIZE + (SEN5X_READ_VALUES_BUFFER_SIZE / 2)); + if (receivedNumber < SEN5X_READ_VALUES_BUFFER_SIZE) { LOG_ERROR("%s: Error getting values", sensorName); return false; } @@ -676,15 +681,17 @@ bool SEN5XSensor::readValues() int16_t int_vocIndex = static_cast((dataBuffer[12] << 8) | dataBuffer[13]); int16_t int_noxIndex = static_cast((dataBuffer[14] << 8) | dataBuffer[15]); - // Convert values based on Sensirion Arduino lib - sen5xmeasurement.pM1p0 = !isnan(uint_pM1p0) ? uint_pM1p0 / 10 : UINT16_MAX; - sen5xmeasurement.pM2p5 = !isnan(uint_pM2p5) ? uint_pM2p5 / 10 : UINT16_MAX; - sen5xmeasurement.pM4p0 = !isnan(uint_pM4p0) ? uint_pM4p0 / 10 : UINT16_MAX; - sen5xmeasurement.pM10p0 = !isnan(uint_pM10p0) ? uint_pM10p0 / 10 : UINT16_MAX; - sen5xmeasurement.humidity = !isnan(int_humidity) ? int_humidity / 100.0f : FLT_MAX; - sen5xmeasurement.temperature = !isnan(int_temperature) ? int_temperature / 200.0f : FLT_MAX; - sen5xmeasurement.vocIndex = !isnan(int_vocIndex) ? int_vocIndex / 10.0f : FLT_MAX; - sen5xmeasurement.noxIndex = !isnan(int_noxIndex) ? int_noxIndex / 10.0f : FLT_MAX; + // Convert values based on Sensirion Arduino lib. + // Map values the sensor reports as unavailable (SEN5X_UINT_INVALID / + // SEN5X_INT_INVALID) to the sentinels getMetrics() checks for + sen5xmeasurement.pM1p0 = (uint_pM1p0 != SEN5X_UINT_INVALID) ? (uint_pM1p0 / 10) : UINT16_MAX; + sen5xmeasurement.pM2p5 = (uint_pM2p5 != SEN5X_UINT_INVALID) ? (uint_pM2p5 / 10) : UINT16_MAX; + sen5xmeasurement.pM4p0 = (uint_pM4p0 != SEN5X_UINT_INVALID) ? (uint_pM4p0 / 10) : UINT16_MAX; + sen5xmeasurement.pM10p0 = (uint_pM10p0 != SEN5X_UINT_INVALID) ? (uint_pM10p0 / 10) : UINT16_MAX; + sen5xmeasurement.humidity = (int_humidity != SEN5X_INT_INVALID) ? (int_humidity / 100.0f) : FLT_MAX; + sen5xmeasurement.temperature = (int_temperature != SEN5X_INT_INVALID) ? (int_temperature / 200.0f) : FLT_MAX; + sen5xmeasurement.vocIndex = (int_vocIndex != SEN5X_INT_INVALID) ? (int_vocIndex / 10.0f) : FLT_MAX; + sen5xmeasurement.noxIndex = (int_noxIndex != SEN5X_INT_INVALID) ? (int_noxIndex / 10.0f) : FLT_MAX; LOG_TRACE("%s: Got readings: pM1p0=%u, pM2p5=%u, pM4p0=%u, pM10p0=%u", sensorName, sen5xmeasurement.pM1p0, sen5xmeasurement.pM2p5, sen5xmeasurement.pM4p0, sen5xmeasurement.pM10p0); @@ -711,9 +718,9 @@ bool SEN5XSensor::readPNValues(bool cumulative) LOG_TRACE("%s: Reading PN Values", sensorName); delay(20); // From Sensirion Datasheet - uint8_t dataBuffer[20]{}; - size_t receivedNumber = readBuffer(&dataBuffer[0], 30); - if (receivedNumber == 0) { + uint8_t dataBuffer[SEN5X_READ_PM_BUFFER_SIZE]{}; + size_t receivedNumber = readBuffer(&dataBuffer[0], SEN5X_READ_PM_BUFFER_SIZE + (SEN5X_READ_PM_BUFFER_SIZE / 2)); + if (receivedNumber < SEN5X_READ_PM_BUFFER_SIZE) { LOG_ERROR("%s: Error getting PN values", sensorName); return false; } @@ -730,22 +737,29 @@ bool SEN5XSensor::readPNValues(bool cumulative) uint16_t uint_pN10p0 = static_cast((dataBuffer[16] << 8) | dataBuffer[17]); uint16_t uint_tSize = static_cast((dataBuffer[18] << 8) | dataBuffer[19]); - // Convert values based on Sensirion Arduino lib - // Multiply by 100 for converting from #/cm3 to #/0.1l for PN values - sen5xmeasurement.pN0p5 = !isnan(uint_pN0p5) ? uint_pN0p5 / 10 * 100 : UINT32_MAX; - sen5xmeasurement.pN1p0 = !isnan(uint_pN1p0) ? uint_pN1p0 / 10 * 100 : UINT32_MAX; - sen5xmeasurement.pN2p5 = !isnan(uint_pN2p5) ? uint_pN2p5 / 10 * 100 : UINT32_MAX; - sen5xmeasurement.pN4p0 = !isnan(uint_pN4p0) ? uint_pN4p0 / 10 * 100 : UINT32_MAX; - sen5xmeasurement.pN10p0 = !isnan(uint_pN10p0) ? uint_pN10p0 / 10 * 100 : UINT32_MAX; - sen5xmeasurement.tSize = !isnan(uint_tSize) ? uint_tSize / 1000.0f : FLT_MAX; + // Convert values based on Sensirion Arduino lib. + // Raw PN values are #/cm3 with 0.1 resolution; multiplying by 10 + // converts to #/0.1l without the truncation of dividing first. + // Map values the sensor reports as unavailable (SEN5X_UINT_INVALID) to the + // sentinels getMetrics() checks for + sen5xmeasurement.pN0p5 = (uint_pN0p5 != SEN5X_UINT_INVALID) ? ((uint32_t)uint_pN0p5 * 10) : UINT32_MAX; + sen5xmeasurement.pN1p0 = (uint_pN1p0 != SEN5X_UINT_INVALID) ? ((uint32_t)uint_pN1p0 * 10) : UINT32_MAX; + sen5xmeasurement.pN2p5 = (uint_pN2p5 != SEN5X_UINT_INVALID) ? ((uint32_t)uint_pN2p5 * 10) : UINT32_MAX; + sen5xmeasurement.pN4p0 = (uint_pN4p0 != SEN5X_UINT_INVALID) ? ((uint32_t)uint_pN4p0 * 10) : UINT32_MAX; + sen5xmeasurement.pN10p0 = (uint_pN10p0 != SEN5X_UINT_INVALID) ? ((uint32_t)uint_pN10p0 * 10) : UINT32_MAX; + sen5xmeasurement.tSize = (uint_tSize != SEN5X_UINT_INVALID) ? (uint_tSize / 1000.0f) : FLT_MAX; // Remove accumuluative values: // https://github.com/fablabbcn/smartcitizen-kit-2x/issues/85 if (!cumulative) { - sen5xmeasurement.pN10p0 -= sen5xmeasurement.pN4p0; - sen5xmeasurement.pN4p0 -= sen5xmeasurement.pN2p5; - sen5xmeasurement.pN2p5 -= sen5xmeasurement.pN1p0; - sen5xmeasurement.pN1p0 -= sen5xmeasurement.pN0p5; + if (sen5xmeasurement.pN10p0 != UINT32_MAX && sen5xmeasurement.pN4p0 != UINT32_MAX) + sen5xmeasurement.pN10p0 -= sen5xmeasurement.pN4p0; + if (sen5xmeasurement.pN4p0 != UINT32_MAX && sen5xmeasurement.pN2p5 != UINT32_MAX) + sen5xmeasurement.pN4p0 -= sen5xmeasurement.pN2p5; + if (sen5xmeasurement.pN2p5 != UINT32_MAX && sen5xmeasurement.pN1p0 != UINT32_MAX) + sen5xmeasurement.pN2p5 -= sen5xmeasurement.pN1p0; + if (sen5xmeasurement.pN1p0 != UINT32_MAX && sen5xmeasurement.pN0p5 != UINT32_MAX) + sen5xmeasurement.pN1p0 -= sen5xmeasurement.pN0p5; } LOG_TRACE("%s: Got readings: pN0p5=%u, pN1p0=%u, pN2p5=%u, pN4p0=%u, pN10p0=%u, tSize=%.2f", sensorName, @@ -767,10 +781,10 @@ uint8_t SEN5XSensor::getMeasurements() } delay(20); // From Sensirion Datasheet - uint8_t dataReadyBuffer[3]; - size_t charNumber = readBuffer(&dataReadyBuffer[0], 3); - if (charNumber == 0) { - LOG_ERROR("%s: Error getting device version value", sensorName); + uint8_t dataReadyBuffer[SEN5X_DATA_READY_BUFFER_SIZE]{}; + size_t charNumber = readBuffer(&dataReadyBuffer[0], SEN5X_DATA_READY_BUFFER_SIZE + (SEN5X_DATA_READY_BUFFER_SIZE / 2)); + if (charNumber < SEN5X_DATA_READY_BUFFER_SIZE) { + LOG_ERROR("%s: Error getting data ready flag value", sensorName); return 2; } @@ -909,7 +923,7 @@ bool SEN5XSensor::getMetrics(meshtastic_Telemetry *measurement) measurement->variant.air_quality_metrics.has_pm_temperature = true; measurement->variant.air_quality_metrics.pm_temperature = sen5xmeasurement.temperature; } - if (sen5xmeasurement.noxIndex != FLT_MAX) { + if (sen5xmeasurement.vocIndex != FLT_MAX) { measurement->variant.air_quality_metrics.has_pm_voc_idx = true; measurement->variant.air_quality_metrics.pm_voc_idx = sen5xmeasurement.vocIndex; } diff --git a/src/modules/Telemetry/Sensor/SEN5XSensor.h b/src/modules/Telemetry/Sensor/SEN5XSensor.h index 5d84b89169..eeebbd3735 100644 --- a/src/modules/Telemetry/Sensor/SEN5XSensor.h +++ b/src/modules/Telemetry/Sensor/SEN5XSensor.h @@ -86,6 +86,18 @@ class SEN5XSensor : public TelemetrySensor #define SEN5X_READ_RAW_VALUES 0x03D2 #define SEN5X_READ_PM_VALUES 0x0413 +// Values the sensor reports when a reading is unavailable +#define SEN5X_UINT_INVALID 0xFFFF +#define SEN5X_INT_INVALID 0x7FFF + +// Reply payload sizes in data bytes; the raw I2C transfer adds one CRC byte +// per 2-byte group, so requests are + / 2 raw bytes +#define SEN5X_VERSION_BUFFER_SIZE 8 +#define SEN5X_PRODUCT_NAME_BUFFER_SIZE 32 +#define SEN5X_DATA_READY_BUFFER_SIZE 2 +#define SEN5X_READ_VALUES_BUFFER_SIZE 16 +#define SEN5X_READ_PM_BUFFER_SIZE 20 + #define SEN5X_VOC_VALID_TIME 600 #define SEN5X_VOC_VALID_DATE 1514764800 @@ -114,8 +126,23 @@ See: https://sensirion.com/resource/application_note/low_power_mode/sen5x #define SEN5X_PN4P0_CONC_THD 100 bool sendCommand(uint16_t command); + /** + * @brief Send a command word followed by a data payload; a CRC byte is + * computed and inserted on the wire after every 2-byte pair. + * @param command 16-bit command code, sent big-endian + * @param buffer payload data bytes, without CRCs + * @param byteNumber payload size in data bytes; must be even + * @return true when the full transfer is written and acknowledged + */ bool sendCommand(uint16_t command, uint8_t *buffer, uint8_t byteNumber = 0); - uint8_t readBuffer(uint8_t *buffer, uint8_t byteNumber); // Return number of bytes received + /** + * @brief Read a reply, verifying and stripping the interleaved CRC bytes. + * @param buffer destination for the data bytes (byteNumber * 2 / 3 of them) + * @param byteNumber raw transfer size including CRCs; must be a multiple + * of 3 (2 data bytes + 1 CRC per group) + * @return the number of data bytes written to buffer, or 0 on any error + */ + uint8_t readBuffer(uint8_t *buffer, uint8_t byteNumber); uint8_t sen5xCRC(const uint8_t *buffer); bool startCleaning(); uint8_t getMeasurements(); From 9199e6b663a9d10644c7596f7c312c679ebd4ae4 Mon Sep 17 00:00:00 2001 From: Tadayoshi MIURA <11958457+t-miura@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:02:56 +0900 Subject: [PATCH 4/8] fix(stm32wl): smaller MAX_RX_TOPHONE and PACKETHISTORY_MAX on stm32wl (#11400) * fixes for stm32: memory optimization and constraints tuning * fold stm32wl elif to existing define * revert changes for packet pool --- src/mesh/mesh-pb-constants.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/mesh/mesh-pb-constants.h b/src/mesh/mesh-pb-constants.h index 1c376818af..aa41c16952 100644 --- a/src/mesh/mesh-pb-constants.h +++ b/src/mesh/mesh-pb-constants.h @@ -26,7 +26,7 @@ // FIXME - max_count is actually 32 but we save/load this as one long string of preencoded MeshPacket bytes - not a big array in // RAM #define MAX_RX_TOPHONE (member_size(DeviceState, receive_queue) / member_size(DeviceState, receive_queue[0])) #ifndef MAX_RX_TOPHONE -#if defined(ARCH_ESP32) && !(defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32S3)) +#if defined(ARCH_STM32WL) || (defined(ARCH_ESP32) && !(defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32S3))) #define MAX_RX_TOPHONE 8 #elif defined(NRF52840_XXAA) // Each slot is a ~340 B MeshPacket in the static pool (Router.cpp MAX_PACKETS_STATIC), so 32 slots @@ -34,10 +34,8 @@ // the 8 classic ESP32 has shipped with for years; drops start when a stalled phone/serial client has // 16 packets queued. #define MAX_RX_TOPHONE 16 -#elif MESHTASTIC_MEM_CLASS >= MEM_CLASS_MEDIUM || defined(ARCH_RP2040) || defined(CONFIG_IDF_TARGET_ESP32C3) || \ - defined(ARCH_STM32WL) -// RP2040/RP2350, ESP32-C3 and STM32WL keep their historical 32 (no field pressure to cut them; -// STM32WL's pool is dynamic, so the constant only bounds in-flight packets there). +#elif MESHTASTIC_MEM_CLASS >= MEM_CLASS_MEDIUM || defined(ARCH_RP2040) || defined(CONFIG_IDF_TARGET_ESP32C3) +// RP2040/RP2350 and ESP32-C3 keep their historical 32. #define MAX_RX_TOPHONE 32 #else #define MAX_RX_TOPHONE 16 // unclassified small parts: fail safe-small @@ -131,8 +129,12 @@ static inline int get_max_num_nodes() /// full mesh, floored at 100. Shared by PacketHistory's constructor clamp and /// the boot-cache budget assert below so the two cannot drift. #ifndef PACKETHISTORY_MAX +#if defined(ARCH_STM32WL) +#define PACKETHISTORY_MAX (MAX_NUM_NODES * 2) // 20 entries for 10-node STM32WL +#else #define PACKETHISTORY_MAX (MAX_NUM_NODES * 2 > 100 ? (uint32_t)(MAX_NUM_NODES * 2) : (uint32_t)100) #endif +#endif /// Per-map cap (position/telemetry/environment/status): only the freshest /// MAX_SATELLITE_NODES nodes keep satellite payloads, the rest just the From 2f6906974e723843bbd8cd4be9d970e9b558f916 Mon Sep 17 00:00:00 2001 From: Andrew Yong Date: Wed, 12 Aug 2026 18:27:36 +0800 Subject: [PATCH 5/8] gps: replace GeoCoord::latLongToMeter's spherical trig with equirectangular approximation (#11184) --- src/configuration.h | 6 + src/gps/GeoCoord.cpp | 39 +++++ test/test_geocoord_distance/test_main.cpp | 165 ++++++++++++++++++++++ 3 files changed, 210 insertions(+) create mode 100644 test/test_geocoord_distance/test_main.cpp diff --git a/src/configuration.h b/src/configuration.h index f9be2fc46c..0ed4dd893d 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -88,6 +88,12 @@ along with this program. If not, see . #define MESHTASTIC_PREHOP_DROP 1 #endif +// Use polynomial approximations for trigonometric functions to save flash. +// Override with -D MESHTASTIC_TRIG_APPROX=0 for exact trig for special use cases e.g. close to Earth's poles. +#ifndef MESHTASTIC_TRIG_APPROX +#define MESHTASTIC_TRIG_APPROX 1 +#endif + // Debug/test only: let a wired client (serial/TCP) inject frames into the RX pipeline as if they had // arrived over LoRa - a SIMULATOR_APP ToRadio packet is delivered through the real receive path on real // hardware (see MeshService::injectAsReceived). This forges over-the-air traffic, so it MUST stay 0 in diff --git a/src/gps/GeoCoord.cpp b/src/gps/GeoCoord.cpp index 4afae9394d..1fc60c3049 100644 --- a/src/gps/GeoCoord.cpp +++ b/src/gps/GeoCoord.cpp @@ -1,4 +1,5 @@ #include "GeoCoord.h" +#include "configuration.h" #include // Narrow a UTM meter value to its unsigned field, clamping non-finite/negative/oversized inputs: an @@ -433,6 +434,43 @@ void GeoCoord::convertWGS84ToOSGB36(const double lat, const double lon, double & //(airyA*airyA/(airyA / sqrt(1 - airyEcc*sin(osgb.latitude)*sin(osgb.latitude)))); // Not used, no OSTN data } +#if MESHTASTIC_TRIG_APPROX +// cos(x) minimax approx for x in [-pi/2, pi/2] ("cos_52"): https://www.ganssle.com/approx.htm +static double cosLatitudeApprox(double latRad) +{ + constexpr double c1 = 0.9999932946, c2 = -0.4999124376, c3 = 0.0414877472, c4 = -0.0012712095; + double x2 = latRad * latRad; + return c1 + x2 * (c2 + x2 * (c3 + c4 * x2)); +} + +/// Approximate distance in meters via equirectangular projection (not exact spherical trig). +/// <1% error to ~500km, degrading near the poles at long range (see test_geocoord_distance). +float GeoCoord::latLongToMeter(double lat_a, double lng_a, double lat_b, double lng_b) +{ + // Don't do math if the points are the same + if (lat_a == lat_b && lng_a == lng_b) + return 0.0; + + double a1 = lat_a / DEG_CONVERT; + double a2 = lng_a / DEG_CONVERT; + double b1 = lat_b / DEG_CONVERT; + double b2 = lng_b / DEG_CONVERT; + + double meanLat = (a1 + b1) / 2; + double dLng = b2 - a2; + // Wrap to [-PI, PI]: unlike cos()/sin(), a raw longitude difference doesn't handle points that + // straddle the antimeridian (e.g. 179.9 and -179.9 are ~0.2 degrees apart, not ~360). + if (dLng > PI) + dLng -= 2 * PI; + else if (dLng < -PI) + dLng += 2 * PI; + double x = dLng * cosLatitudeApprox(meanLat); + double y = b1 - a1; + double tt = sqrt(x * x + y * y); + + return (float)(6366000 * tt); +} +#else /// Ported from my old java code, returns distance in meters along the globe /// surface (by Haversine formula) float GeoCoord::latLongToMeter(double lat_a, double lng_a, double lat_b, double lng_b) @@ -456,6 +494,7 @@ float GeoCoord::latLongToMeter(double lat_a, double lng_a, double lat_b, double return (float)(6366000 * tt); } +#endif /** * Computes the bearing in degrees between two points on Earth. Ported from my diff --git a/test/test_geocoord_distance/test_main.cpp b/test/test_geocoord_distance/test_main.cpp new file mode 100644 index 0000000000..de3430f1c3 --- /dev/null +++ b/test/test_geocoord_distance/test_main.cpp @@ -0,0 +1,165 @@ +#include "configuration.h" +#include "gps/GeoCoord.h" +#include +#include +#include + +void setUp(void) {} +void tearDown(void) {} + +// Pins latLongToMeter()'s equirectangular-approximation accuracy against the original spherical +// law of cosines, so a future change can't silently regress it. + +static constexpr double kPi = 3.14159265358979323846; + +static double referenceSphericalLawOfCosines(double lat_a, double lng_a, double lat_b, double lng_b) +{ + double a1 = lat_a * kPi / 180.0; + double a2 = lng_a * kPi / 180.0; + double b1 = lat_b * kPi / 180.0; + double b2 = lng_b * kPi / 180.0; + double t1 = std::cos(a1) * std::cos(a2) * std::cos(b1) * std::cos(b2); + double t2 = std::cos(a1) * std::sin(a2) * std::cos(b1) * std::sin(b2); + double t3 = std::sin(a1) * std::sin(b1); + double arg = t1 + t2 + t3; + if (arg > 1.0) + arg = 1.0; + if (arg < -1.0) + arg = -1.0; + return 6366000 * std::acos(arg); +} + +// Below ~1m, relative error is dominated by rounding noise rather than the formula itself, so +// assert an absolute bound instead (still catches a badly-broken implementation). +static constexpr double kNearZeroAbsoluteToleranceMeters = 0.5; + +// An order of magnitude above what the implementation currently produces per group - tight enough to +// catch a regression, loose enough not to track float rounding. Groups differ because +// equirectangular error grows with both separation and latitude. +static constexpr double kLocalTolerancePercent = 0.01; +static constexpr double kRegionalTolerancePercent = 0.1; +static constexpr double kHighLatitudeTolerancePercent = 0.2; +static constexpr double kAntimeridianTolerancePercent = 0.01; + +static void assertWithinPercent(double expected, double actual, double pct, const char *msg) +{ + if (expected < 1.0) { + if (std::fabs(actual - expected) > kNearZeroAbsoluteToleranceMeters) { + char buf[160]; + snprintf(buf, sizeof(buf), "%s: expected=%.3f actual=%.3f (near-zero, limit %.1fm absolute)", msg, expected, actual, + kNearZeroAbsoluteToleranceMeters); + TEST_FAIL_MESSAGE(buf); + } + return; + } + double err = std::fabs(actual - expected) / expected * 100.0; + if (err > pct) { + char buf[160]; + snprintf(buf, sizeof(buf), "%s: expected=%.1f actual=%.1f err=%.2f%% (limit %.2f%%)", msg, expected, actual, err, pct); + TEST_FAIL_MESSAGE(buf); + } +} + +static void test_identical_points_is_zero(void) +{ + TEST_ASSERT_EQUAL_FLOAT(0.0f, GeoCoord::latLongToMeter(51.5, -0.1, 51.5, -0.1)); +} + +static void test_local_distances(void) +{ + // Movement-threshold scale (meters to a few km) - the most common real usage. + struct { + double la, lo, lb, lob; + } cases[] = { + {51.5074, -0.1278, 51.5080, -0.1278}, // ~67m north + {51.5074, -0.1278, 51.5074, -0.1200}, // ~540m east at London's latitude + {0.0, 0.0, 0.001, 0.001}, // ~157m near the equator + {65.0, 25.0, 65.001, 25.002}, // high-ish latitude, small delta + {-33.87, 151.21, -33.865, 151.215}, // Sydney, southern hemisphere + }; + for (auto &c : cases) { + double expected = referenceSphericalLawOfCosines(c.la, c.lo, c.lb, c.lob); + double actual = GeoCoord::latLongToMeter(c.la, c.lo, c.lb, c.lob); + assertWithinPercent(expected, actual, kLocalTolerancePercent, "local distance"); + } +} + +static void test_regional_distances(void) +{ + // City-to-city scale (tens to ~500km) below 60 degrees; see test_high_latitude_distances. + struct { + double la, lo, lb, lob; + } cases[] = { + {51.5074, -0.1278, 48.8566, 2.3522}, // London to Paris, ~344km + {40.7128, -74.0060, 42.3601, -71.0589}, // NYC to Boston, ~306km + {35.6762, 139.6503, 34.6937, 135.5023}, // Tokyo to Osaka, ~400km + {-33.8688, 151.2093, -37.8136, 144.9631}, // Sydney to Melbourne, ~714km + }; + for (auto &c : cases) { + double expected = referenceSphericalLawOfCosines(c.la, c.lo, c.lb, c.lob); + double actual = GeoCoord::latLongToMeter(c.la, c.lo, c.lb, c.lob); + assertWithinPercent(expected, actual, kRegionalTolerancePercent, "regional distance"); + } +} + +static void test_high_latitude_distances(void) +{ + // Regional scale above 60 degrees, where equirectangular error grows fastest - a 500km pair at + // 80 degrees already exceeds 1%. + struct { + double la, lo, lb, lob; + } cases[] = { + {69.6492, 18.9553, 67.2804, 14.4049}, // Tromso to Bodo, ~322km + {64.8378, -147.7164, 61.2181, -149.9003}, // Fairbanks to Anchorage, ~417km + {78.2232, 15.6469, 78.9230, 11.9219}, // Longyearbyen to Ny-Alesund, ~113km + }; + for (auto &c : cases) { + double expected = referenceSphericalLawOfCosines(c.la, c.lo, c.lb, c.lob); + double actual = GeoCoord::latLongToMeter(c.la, c.lo, c.lb, c.lob); + assertWithinPercent(expected, actual, kHighLatitudeTolerancePercent, "high-latitude distance"); + } +} + +static void test_antimeridian_wraparound(void) +{ + // Two points ~22km apart straddling the 180th meridian - regression case for the antimeridian + // wraparound fix (a naive b2-a2 would compute this as ~40,000km). + double expected = referenceSphericalLawOfCosines(0.0, 179.9, 0.0, -179.9); + double actual = GeoCoord::latLongToMeter(0.0, 179.9, 0.0, -179.9); + assertWithinPercent(expected, actual, kAntimeridianTolerancePercent, "antimeridian distance"); + TEST_ASSERT_LESS_THAN_FLOAT(1000000.0f, actual); // sanity: nowhere near the naive-bug's ~40,000km +} + +static void test_symmetry(void) +{ + // distance(a,b) should equal distance(b,a) + double d1 = GeoCoord::latLongToMeter(51.5074, -0.1278, 48.8566, 2.3522); + double d2 = GeoCoord::latLongToMeter(48.8566, 2.3522, 51.5074, -0.1278); + TEST_ASSERT_FLOAT_WITHIN(0.01f, d1, d2); +} + +static void test_no_nan_at_extreme_latitudes(void) +{ + float d1 = GeoCoord::latLongToMeter(90.0, 0.0, -90.0, 0.0); + float d2 = GeoCoord::latLongToMeter(89.9, 10.0, 89.9, -170.0); + float d3 = GeoCoord::latLongToMeter(-89.9, 45.0, -89.9, -135.0); + TEST_ASSERT_FALSE(std::isnan(d1)); + TEST_ASSERT_FALSE(std::isnan(d2)); + TEST_ASSERT_FALSE(std::isnan(d3)); + TEST_ASSERT_TRUE(d1 > 0); +} + +void setup() +{ + UNITY_BEGIN(); + RUN_TEST(test_identical_points_is_zero); + RUN_TEST(test_local_distances); + RUN_TEST(test_regional_distances); + RUN_TEST(test_high_latitude_distances); + RUN_TEST(test_antimeridian_wraparound); + RUN_TEST(test_symmetry); + RUN_TEST(test_no_nan_at_extreme_latitudes); + exit(UNITY_END()); +} + +void loop() {} From 579d26e1b2f21bc3c9153cfda6b8b1ad4073973e Mon Sep 17 00:00:00 2001 From: Clive Blackledge Date: Wed, 12 Aug 2026 05:22:53 -0700 Subject: [PATCH 6/8] fix(NodeDB): don't let an empty contact key erase a stored public key (#11432) Clients send `add_contact` before every text-message DM, because a phone often holds a larger contact database (with public keys) than the radio can keep. That makes `addFromContact()` the highest-volume key-write path on the device - and it had no protection against key erasure. Its only key guard covered the manually-verified case: if the local entry was marked manually verified and the incoming contact was not, a key mismatch aborted the update. Every ordinary entry fell straight through to `CopyUserToNodeInfoLite()`, which assigns `public_key` unconditionally. So a SharedContact with `has_user` set and an empty `public_key` overwrote a peer's stored, XEdDSA-proven key with zeros - and `addFromContact()` calls `saveNodeDatabaseToDisk()`, so the erasure survived a reboot. Subsequent DMs to that peer then failed with PKI_SEND_FAIL_PUBLIC_KEY, with no way to recover until the peer's NodeInfo was re-exchanged. `public_key` is a singular (non-optional) bytes field, so "absent" and "empty" both decode to size 0; a client that simply has no key for a contact is indistinguishable on the wire from one asking to clear it. The fix is deliberately narrow: keep the stored key when the entry already holds a full 32-byte key and the incoming contact does not. A well-formed 32-byte contact key still updates the entry exactly as before. Deliberately NOT changed here: - `updateUser()`'s first-key-wins pin is not applied to this path. Clients legitimately use add_contact to supply keys the radio never had and to update them (QR-code contact sharing); a blanket pin would break that documented flow. Only erasure is refused. - `CopyUserToNodeInfoLite()` itself is untouched - it has many other callers (self-record refresh, updateUser, warm-tier rehydration), so the guard lives at this call site. - The manually-verified branch is unchanged. - Node-number validation (reserved/broadcast/self) on this path remains open and is tracked separately. Co-authored-by: Claude Opus 5 --- src/mesh/NodeDB.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index e6c1ac67e6..d24473125b 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -3482,7 +3482,16 @@ void NodeDB::addFromContact(meshtastic_SharedContact contact) } } info->num = contact.node_num; + // CopyUserToNodeInfoLite assigns public_key unconditionally, and clients send add_contact before every + // DM - often from an entry that carries no key at all. A contact may still supply or update a full + // 32-byte key (that's what add_contact is for), but it must never *erase* a key we already hold, which + // would be persisted below and break subsequent DMs with PKI_SEND_FAIL_PUBLIC_KEY. + const meshtastic_NodeInfoLite_public_key_t storedKey = info->public_key; TypeConversions::CopyUserToNodeInfoLite(info, contact.user); + if (storedKey.size == 32 && info->public_key.size != 32) { + LOG_INFO("Contact 0x%08x has no key, keep the stored one", contact.node_num); + info->public_key = storedKey; + } if (contact.should_ignore) { // Block the contact and drop its rich satellite data, but keep the // public key copied above - an ignored peer keeps a usable identity From 54d6ce833e3b65380dc2d2e818f5a872c65a4730 Mon Sep 17 00:00:00 2001 From: Andrew Yong Date: Wed, 12 Aug 2026 20:42:19 +0800 Subject: [PATCH 7/8] gps: avoid pow() in GPS_HARDSLEEP threshold heuristic (#11179) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * gps: avoid pow() in GPS_HARDSLEEP threshold heuristic GPS::down() used pow(seconds, 1.22) to pick between GPS_SOFTSLEEP and GPS_HARDSLEEP - a curve fit the surrounding comment already describes as "not particularly accurate". On flash-constrained builds where this was the only pow() call site (e.g. wio-e5), it single-handedly pulled in the full double-precision libm pow/rem_pio2 chain for a heuristic threshold decision. Replaces it with gpsHardsleepThresholdMs(), a piecewise-linear lookup over the same curve, sampled at 16 points and verified to track the original formula within ~0.5% for inputs >=10s and ~1.6% for 5-10s (worse only in relative terms below 5s, where the absolute difference is at most a couple of seconds - negligible against update intervals measured in tens of seconds to hours). Signed-off-by: Andrew Yong Assisted-by: Claude Sonnet 5 * gps: trim comments to repo convention (1-2 lines) Addresses a CodeRabbit nitpick: the explanatory comments in GPSUpdateScheduling.cpp and test_gps_update_scheduling/test_main.cpp had grown into multi-line blocks with provenance detail that belongs in the commit message, not inline. Trims each to 1-2 lines, keeping only the essential rationale/bounds. Signed-off-by: Andrew Yong Assisted-by: Claude Sonnet 5 * Refactor main function to setup and loop for tests Signed-off-by: Thomas Göttgens * gps: extend the hardsleep threshold table below 5s and tighten its tests The 0s-to-5s chord read 42% high at 1s, 22% at 2s and 12% at 3s, against the ~1.6% the comment claimed. Adding 1s, 2s and 3s sample points brings the worst error below 10s to 1.60% at 7s. Above 10s it is 0.55% at 728s, unchanged. Tests: sample off-breakpoint values only, including both worst-error inputs (7s and 728s). Replace the 3000ms absolute floor, which made the 1s assertion unfalsifiable given a true value of 2750ms, with 2% and 0.75% bounds. Add breakpoint-exactness and clamp-boundary coverage. --------- Signed-off-by: Andrew Yong Signed-off-by: Thomas Göttgens Co-authored-by: Austin Co-authored-by: Thomas Göttgens --- src/gps/GPS.cpp | 6 +- src/gps/GPSUpdateScheduling.cpp | 25 +++++ src/gps/GPSUpdateScheduling.h | 4 + test/test_gps_update_scheduling/test_main.cpp | 91 +++++++++++++++++++ 4 files changed, 121 insertions(+), 5 deletions(-) create mode 100644 test/test_gps_update_scheduling/test_main.cpp diff --git a/src/gps/GPS.cpp b/src/gps/GPS.cpp index fd5be417e7..2ca1d86d19 100644 --- a/src/gps/GPS.cpp +++ b/src/gps/GPS.cpp @@ -1389,11 +1389,7 @@ void GPS::down() #endif if (softsleepSupported) { - // How long does gps_update_interval need to be, for GPS_HARDSLEEP to become more efficient than - // GPS_SOFTSLEEP? Heuristic equation. A compromise manually fitted to power observations from U-blox NEO-6M - // and M10050 https://www.desmos.com/calculator/6gvjghoumr This is not particularly accurate, but probably an - // improvement over a single, fixed threshold - uint32_t hardsleepThreshold = (2750 * pow(predictedSearchDuration / 1000, 1.22)); + uint32_t hardsleepThreshold = gpsHardsleepThresholdMs(predictedSearchDuration / 1000); LOG_DEBUG("gps_update_interval >= %us needed for hardsleep", hardsleepThreshold / 1000); // If update interval too short: softsleep (if supported by hardware) diff --git a/src/gps/GPSUpdateScheduling.cpp b/src/gps/GPSUpdateScheduling.cpp index a19d9c7d57..fe2c3ae78a 100644 --- a/src/gps/GPSUpdateScheduling.cpp +++ b/src/gps/GPSUpdateScheduling.cpp @@ -2,6 +2,31 @@ #include "Default.h" +// Sampled from the original `2750 * seconds^1.22` curve. Interpolation tracks it within 0.6% for +// inputs >=10s and 1.7% below that; the 1s/2s/3s points keep the convex first segment from +// overshooting (a 0s-to-5s chord reads 42% high at 1s). +static constexpr uint32_t kThresholdCurveSecs[] = {0, 1, 2, 3, 5, 10, 15, 20, 30, 45, 60, 90, 120, 180, 240, 300, 450, 600, 900}; +static constexpr uint32_t kThresholdCurveMs[] = {0, 2750, 6406, 10506, 19592, 45639, 74845, + 106314, 174350, 285925, 406141, 666053, 946093, 1551548, + 2203893, 2893481, 4745172, 6740269, 11053722}; +static constexpr size_t kThresholdCurvePoints = sizeof(kThresholdCurveSecs) / sizeof(kThresholdCurveSecs[0]); + +// How long does gps_update_interval need to be, for GPS_HARDSLEEP to become more efficient than +// GPS_SOFTSLEEP? Avoids pow() so this heuristic doesn't pull double-precision libm into the image. +uint32_t gpsHardsleepThresholdMs(uint32_t predictedSearchSecs) +{ + if (predictedSearchSecs >= kThresholdCurveSecs[kThresholdCurvePoints - 1]) + return kThresholdCurveMs[kThresholdCurvePoints - 1]; + + size_t i = 1; + while (kThresholdCurveSecs[i] < predictedSearchSecs) + i++; + + uint32_t x0 = kThresholdCurveSecs[i - 1], x1 = kThresholdCurveSecs[i]; + uint32_t y0 = kThresholdCurveMs[i - 1], y1 = kThresholdCurveMs[i]; + return y0 + (uint32_t)((uint64_t)(y1 - y0) * (predictedSearchSecs - x0) / (x1 - x0)); +} + // Mark the time when searching for GPS position begins void GPSUpdateScheduling::informSearching() { diff --git a/src/gps/GPSUpdateScheduling.h b/src/gps/GPSUpdateScheduling.h index 120605c4ef..d7609d704e 100644 --- a/src/gps/GPSUpdateScheduling.h +++ b/src/gps/GPSUpdateScheduling.h @@ -2,6 +2,10 @@ #include "configuration.h" +// Approximates the GPS_HARDSLEEP/GPS_SOFTSLEEP crossover curve without pow(); see .cpp for the +// sampled reference values it interpolates between. +uint32_t gpsHardsleepThresholdMs(uint32_t predictedSearchSecs); + // Encapsulates code responsible for the timing of GPS updates class GPSUpdateScheduling { diff --git a/test/test_gps_update_scheduling/test_main.cpp b/test/test_gps_update_scheduling/test_main.cpp new file mode 100644 index 0000000000..00c01c0f60 --- /dev/null +++ b/test/test_gps_update_scheduling/test_main.cpp @@ -0,0 +1,91 @@ +#include "Arduino.h" +#include "TestUtil.h" +#include "gps/GPSUpdateScheduling.h" +#include +#include +#include + +void setUp(void) {} +void tearDown(void) {} + +// Confirms gpsHardsleepThresholdMs()'s pow()-free lookup table tracks the original +// `2750 * pow(seconds, 1.22)` curve closely. +static double originalFormula(uint32_t seconds) +{ + return 2750.0 * std::pow((double)seconds, 1.22); +} + +static void test_matches_original_formula_at_sampled_points(void) +{ + // Off-breakpoint values only - a breakpoint interpolates exactly by construction, so it would + // test nothing here (test_exact_at_table_breakpoints covers those). Includes both worst-error + // inputs: 7s (1.60%) and 728s (0.55%). Capped at 900s, the pre-existing 15-minute search clamp. + const uint32_t samples[] = {4, 6, 7, 8, 9, 33, 100, 150, 500, 728, 899}; + for (uint32_t s : samples) { + double expected = originalFormula(s); + uint32_t actual = gpsHardsleepThresholdMs(s); + // Pure integer arithmetic, so results are bit-identical everywhere - no float noise to + // leave headroom for, and these sit just above the measured worst cases. + double tolerance = expected * (s < 10 ? 0.02 : 0.0075); + TEST_ASSERT_DOUBLE_WITHIN(tolerance, expected, (double)actual); + } +} + +static void test_zero_seconds_is_zero(void) +{ + TEST_ASSERT_EQUAL_UINT32(0, gpsHardsleepThresholdMs(0)); +} + +static void test_monotonically_nondecreasing(void) +{ + uint32_t prev = gpsHardsleepThresholdMs(0); + for (uint32_t s = 1; s <= 1200; s += 7) { + uint32_t cur = gpsHardsleepThresholdMs(s); + TEST_ASSERT_GREATER_OR_EQUAL_UINT32(prev, cur); + prev = cur; + } +} + +static void test_exact_at_table_breakpoints(void) +{ + // Every breakpoint must return its own sampled value. Catches an off-by-one in the segment + // scan, which a percentage bound on interpolated points would absorb. + const uint32_t breakpoints[] = {0, 1, 2, 3, 5, 10, 15, 20, 30, 45, 60, 90, 120, 180, 240, 300, 450, 600, 900}; + for (uint32_t s : breakpoints) { + char msg[64]; + snprintf(msg, sizeof(msg), "breakpoint %us", s); + // Within 2ms, not exact: the 30s entry is rounded 1ms high, and pow() can differ by an ULP + // across libm implementations. A real off-by-one in the scan misses by thousands. + TEST_ASSERT_UINT32_WITHIN_MESSAGE(2, (uint32_t)(originalFormula(s) + 0.5), gpsHardsleepThresholdMs(s), msg); + } +} + +static void test_clamps_above_table_range(void) +{ + uint32_t atMax = gpsHardsleepThresholdMs(900); + TEST_ASSERT_EQUAL_UINT32(atMax, gpsHardsleepThresholdMs(2000)); + TEST_ASSERT_EQUAL_UINT32(atMax, gpsHardsleepThresholdMs(UINT32_MAX)); +} + +static void test_clamp_boundary(void) +{ + // The clamp must engage exactly at the last table point, not before or after it. + TEST_ASSERT_LESS_THAN_UINT32(gpsHardsleepThresholdMs(900), gpsHardsleepThresholdMs(899)); + TEST_ASSERT_EQUAL_UINT32(gpsHardsleepThresholdMs(900), gpsHardsleepThresholdMs(901)); +} + +void setup() +{ + delay(10); + initializeTestEnvironment(); + UNITY_BEGIN(); + RUN_TEST(test_matches_original_formula_at_sampled_points); + RUN_TEST(test_zero_seconds_is_zero); + RUN_TEST(test_monotonically_nondecreasing); + RUN_TEST(test_exact_at_table_breakpoints); + RUN_TEST(test_clamps_above_table_range); + RUN_TEST(test_clamp_boundary); + exit(UNITY_END()); +} + +void loop() {} From db3eb91015f1e09234a713cd68be5d41fae1c758 Mon Sep 17 00:00:00 2001 From: Clive Blackledge Date: Wed, 12 Aug 2026 06:05:20 -0700 Subject: [PATCH 8/8] fix(security): never log the X25519 identity private key (#11435) installDefaultConfig() restores a preserved identity key when the config is reset with preserveKey=true. On that path it called: printBytes("Restored key", config.security.private_key.bytes, config.security.private_key.size); printBytes() hex-dumps the buffer to LOG_DEBUG, so this emitted all 32 bytes of the raw X25519 identity private key to the serial/BLE debug log. Debug logs are not a private channel. They are routinely captured over serial or BLE and pasted verbatim into GitHub issues, Discord threads and support requests. Anyone who reads such a log recovers the node's identity private key, and can then impersonate the node and decrypt every PKI direct message addressed to it -- past messages included, since the key is long-lived and the DH shared secret is static per node pair. There is no revocation story short of generating a new identity. Replaced with a LOG_DEBUG that records that a key was restored and contains no key-derived bytes. The restore/no-restore signal is the genuinely useful diagnostic here ("did my key survive the reset?"), and it costs nothing to keep; the bytes were never what made the line useful. Log level is unchanged -- printBytes() already logged at LOG_DEBUG. Deliberately NOT a truncated prefix or a hash. A prefix is still key material: it hands an attacker free bytes and shrinks the search space. A hash is a confirmation oracle -- it lets anyone holding a candidate key verify it against the log, which is exactly the check an attacker needs. Neither is a compromise; both leak. If a log line survives at all it must carry zero key-derived bytes. Sites changed: - src/mesh/NodeDB.cpp:988 -- the only full private-key dump in src/. Audited and deliberately left alone: - NodeDB.cpp:3552,3604 ("Incoming Pubkey", "Saved Pubkey") -- public keys, published to the mesh by design; not secret. - CryptoEngine.cpp:245,285 -- nonces, not key material. - CryptoEngine.cpp:246,286 -- first 8 bytes of the derived shared_key, and AdminModule.cpp:2006,2013,2014 -- the 8-byte admin session passkey. Both are secrets rather than public values, but neither is the identity private key and both are out of scope for this fix; noted for follow-up. No unused-variable fallout: private_key_temp is still read by the memcpy above, and printBytes() is still used by the two pubkey sites, so the meshUtils.h include is still required. Co-authored-by: Claude Opus 5 --- src/mesh/NodeDB.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index d24473125b..e31df4faa1 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -985,7 +985,8 @@ void NodeDB::installDefaultConfig(bool preserveKey = false) if (shouldPreserveKey) { config.security.private_key.size = 32; memcpy(config.security.private_key.bytes, private_key_temp, config.security.private_key.size); - printBytes("Restored key", config.security.private_key.bytes, config.security.private_key.size); + // Never log the key bytes: debug logs get pasted into public bug reports. + LOG_DEBUG("Restored preserved private key"); } else { config.security.private_key.size = 0; }