From 392a396fb1eab25a88b983aab8430b8969cf5650 Mon Sep 17 00:00:00 2001 From: Strycher Date: Tue, 8 Sep 2026 17:25:22 -0400 Subject: [PATCH 01/10] feat(#289): I2C dropout back-off and re-probe for every bus device A device that stops answering cost a driver timeout per loop pass and an error line per read: on 2026-09-08 the touch controller and then the IMU breakout went silent for hours, the loop fell from 44 Hz to 12-27 Hz and the log filled with a quarter of a million identical errors. i2c_health: per-device consecutive-failure count; after 5 the device is dropped (its reads stop, logged once), re-probed every 10 s (probe, then a 9-clock bus recovery and a second probe), and re-initialised through its normal init when it answers. Only devices present at boot are re-probed. The failure signal per library, checked in the pinned sources: SHT4x getEvent() is false on a NACK; BME68x leaves a negative sensor.status, sampled on the 3 s BSEC cadence; MAX17048 isDeviceReady() reads the version register; LSM6DS and LIS3MDL getEvent() return true unconditionally and FT6206 touched() has no error path, so those are address-probed (per read pass for the IMU, every 1 s for touch). The boot log now states the reset reason, so a crash boot is attributable from the next boot's log. Builds: um_feathers3 RAM 22.7% Flash 27.6%; feather_s3 RAM 31.6% Flash 64.7%. Co-Authored-By: Claude Fable 5.1 --- src/battery.cpp | 21 ++++++++++ src/battery.h | 1 + src/env.cpp | 41 +++++++++++++++++++- src/env.h | 2 + src/i2c_health.cpp | 95 ++++++++++++++++++++++++++++++++++++++++++++++ src/i2c_health.h | 53 ++++++++++++++++++++++++++ src/imu.cpp | 46 ++++++++++++++++++++-- src/imu.h | 1 + src/src.ino | 11 ++++++ src/touch.cpp | 23 +++++++++++ src/touch.h | 1 + 11 files changed, 289 insertions(+), 6 deletions(-) create mode 100644 src/i2c_health.cpp create mode 100644 src/i2c_health.h diff --git a/src/battery.cpp b/src/battery.cpp index e485ecb..038ae43 100644 --- a/src/battery.cpp +++ b/src/battery.cpp @@ -2,10 +2,18 @@ #include "battery.h" #include "fram.h" #include "logging.h" +#include "i2c_health.h" Adafruit_MAX17048 battery; bool batteryAvailable = false; +// Dropout tracking (#289). The gauge is read on a 10 s timer and by the +// settings screen and /diags, never in a tight loop, so a health check every +// BATT_CHECK_MS is enough. isDeviceReady() reads the version register and is +// false on a NACK; every cellVoltage()/cellPercent() call already gates on it. +static I2CDevice battDev = {"MAX17048", 0x36, &batteryAvailable}; +#define BATT_CHECK_MS 2000 + void initBattery() { logPrint("Initializing MAX17048... "); @@ -15,9 +23,22 @@ void initBattery() { } batteryAvailable = true; + i2cNoteOk(battDev); logPrintln("OK"); } +// Called every loop pass (#289). +void serviceBattery() { + if (!batteryAvailable) { + if (i2cReprobeDue(battDev)) initBattery(); + return; + } + static unsigned long lastCheck = 0; + if (millis() - lastCheck < BATT_CHECK_MS) return; + lastCheck = millis(); + if (battery.isDeviceReady()) i2cNoteOk(battDev); else i2cNoteFail(battDev); +} + // Log battery data to SD card for analysis void logBatteryToSD() { if (!sdHealth.available || !batteryAvailable) return; diff --git a/src/battery.h b/src/battery.h index c376fb8..acb255a 100644 --- a/src/battery.h +++ b/src/battery.h @@ -9,6 +9,7 @@ extern Adafruit_MAX17048 battery; extern bool batteryAvailable; void initBattery(); +void serviceBattery(); // health check; re-probe a dropped gauge and re-init it (#289) void logBatteryToSD(); void logBatteryToFRAM(); bool isBatteryConnected(); diff --git a/src/env.cpp b/src/env.cpp index a5b741c..89789d2 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -2,6 +2,7 @@ #include "env.h" #include "fram.h" #include "logging.h" +#include "i2c_health.h" // BSEC2 IAQ config for BME680/688 at 3.3V, 3-second sample rate, 4-day calibration const uint8_t bsec2_config[] = { @@ -12,6 +13,15 @@ Bsec2 envSensor; Adafruit_SHT4x sht4 = Adafruit_SHT4x(); // SHT41 temp/humidity (#48) bool bmeAvailable = false; bool shtAvailable = false; // SHT41 temp/humidity (#48) + +// Dropout tracking (#289). The BME688 address is whichever answered in +// initBME688(); the SHT41 has only one. +static I2CDevice shtDev = {"SHT41", 0x44, &shtAvailable}; +static I2CDevice bmeDev = {"BME688", 0x77, &bmeAvailable}; +// The BME688 is touched once per BSEC sample (LP mode = 3 s) and its status +// persists until the next access, so its health is sampled on that cadence; +// sampling every loop pass would count one failed read five times over. +#define BME_HEALTH_MS 3000 static uint8_t bsecState[BSEC_MAX_STATE_BLOB_SIZE]; static unsigned long lastBsecStateSave = 0; bool bsecStateLoaded = false; @@ -88,14 +98,18 @@ void initBME688() { }; // Try primary address (0x77), then secondary (0x76) - if (!envSensor.begin(0x77, Wire)) { - if (!envSensor.begin(0x76, Wire)) { + uint8_t addr = 0x77; + if (!envSensor.begin(addr, Wire)) { + addr = 0x76; + if (!envSensor.begin(addr, Wire)) { logPrintln("NOT FOUND"); logPrintf(" BSEC status: %d\n", envSensor.status); logPrintf(" Sensor status: %d\n", envSensor.sensor.status); return; } } + bmeDev.addr = addr; + i2cNoteOk(bmeDev); // Load BSEC2 IAQ config if (!envSensor.setConfig(bsec2_config)) { @@ -261,6 +275,9 @@ void readSHT41() { if (sht4.getEvent(&humEv, &tempEv)) { shtData.temperature = tempEv.temperature; shtData.humidity = humEv.relative_humidity; + i2cNoteOk(shtDev); + } else { + i2cNoteFail(shtDev); // false on a NACK (before the measurement delay) or a bad CRC } } @@ -272,6 +289,26 @@ void readBME688() { LOG_ERROR("BSEC error: %d", envSensor.status); } } + + // Bus health (#289): the BME68x driver checks endTransmission() and leaves + // BME68X_E_COM_FAIL in sensor.status when the chip stops answering. + static unsigned long lastHealth = 0; + if (millis() - lastHealth >= BME_HEALTH_MS) { + lastHealth = millis(); + if (envSensor.sensor.status < BME68X_OK) i2cNoteFail(bmeDev); else i2cNoteOk(bmeDev); + } +} + +// Called every loop pass (#289): re-probe a dropped sensor on the slow timer +// and run its normal init when it answers. +void serviceSHT41() { + if (!shtAvailable && i2cReprobeDue(shtDev)) initSHT41(); +} + +void serviceBME688() { + if (bmeAvailable || !i2cReprobeDue(bmeDev)) return; + initBME688(); // fresh BSEC instance: restore its state the way setup() does + if (bmeAvailable && !loadBsecFromFRAM() && sdHealth.available) loadBsecState(); } // Helper function to get IAQ accuracy as short text diff --git a/src/env.h b/src/env.h index fd9ad36..bac9f73 100644 --- a/src/env.h +++ b/src/env.h @@ -41,6 +41,8 @@ bool saveBsecToFRAM(); bool loadBsecFromFRAM(); void readSHT41(); void readBME688(); +void serviceSHT41(); // re-probe a dropped sensor and re-init it (#289) +void serviceBME688(); const char* getIaqAccuracyText(uint8_t accuracy); const char* getIaqQualityText(float iaq); float hPaToInHg(float hPa); diff --git a/src/i2c_health.cpp b/src/i2c_health.cpp new file mode 100644 index 0000000..7a10f1e --- /dev/null +++ b/src/i2c_health.cpp @@ -0,0 +1,95 @@ +// i2c_health.cpp -- per-device I2C failure tracking, back-off and re-probe (#289). +#include "i2c_health.h" +#include "logging.h" +#include +#include + +void i2cNoteOk(I2CDevice& d) { + d.fails = 0; +} + +bool i2cNoteFail(I2CDevice& d) { + d.totalFails++; + if (d.fails < 255) d.fails++; + if (!*d.available) return false; // already dropped + if (d.fails < I2C_FAIL_LIMIT) return false; + *d.available = false; + d.drops++; + d.droppedAt = millis(); + d.lastProbe = d.droppedAt; // first re-probe one interval from now + logPrintf("[I2C] %s (0x%02X) stopped answering: %u consecutive failures, dropped; re-probing every %d s\n", + d.name, d.addr, (unsigned)d.fails, I2C_REPROBE_MS / 1000); + return true; +} + +bool i2cProbe(uint8_t addr) { + // Zero-length write: the core turns it into i2c_master_probe (address and + // ACK only), the same transaction scanI2CBus() uses. + Wire.beginTransmission(addr); + return Wire.endTransmission() == 0; +} + +// A slave left mid-transaction by a glitch can hold SDA low until it sees +// enough clocks to finish its byte and then a STOP. Clock it out with the +// pins as plain GPIO, then hand the pins back to the I2C peripheral. Same +// end()/begin() pair the #283 bus diagnostic uses, verified on both boards. +void i2cBusRecover() { + Wire.end(); + digitalWrite(SDA, HIGH); // output latch high before the mode change, + digitalWrite(SCL, HIGH); // so neither line dips when it becomes GPIO + pinMode(SDA, INPUT_PULLUP); + pinMode(SCL, OUTPUT_OPEN_DRAIN); + delayMicroseconds(5); + for (int i = 0; i < 9; i++) { + digitalWrite(SCL, LOW); + delayMicroseconds(5); + digitalWrite(SCL, HIGH); + delayMicroseconds(5); + if (digitalRead(SDA) == HIGH) break; // slave released the line + } + // START then STOP with no address: every slave's state machine resets on + // the STOP. SDA falling while SCL is high is the START. + pinMode(SDA, OUTPUT_OPEN_DRAIN); + digitalWrite(SDA, LOW); + delayMicroseconds(5); + digitalWrite(SDA, HIGH); + delayMicroseconds(5); + Wire.begin(); // variant SDA/SCL at the default clock, as in setup() +} + +bool i2cReprobeDue(I2CDevice& d) { + if (*d.available || d.drops == 0) return false; + unsigned long now = millis(); + if (now - d.lastProbe < I2C_REPROBE_MS) return false; + d.lastProbe = now; + if (!i2cProbe(d.addr)) { + i2cBusRecover(); + if (!i2cProbe(d.addr)) return false; + } + logPrintf("[I2C] %s (0x%02X) answers again after %lu s (drop #%lu, %lu failed reads so far); re-initialising\n", + d.name, d.addr, (now - d.droppedAt) / 1000, (unsigned long)d.drops, (unsigned long)d.totalFails); + d.fails = 0; + return true; +} + +void logResetReason() { + esp_reset_reason_t r = esp_reset_reason(); + const char* s; + switch (r) { + case ESP_RST_POWERON: s = "power-on"; break; + case ESP_RST_EXT: s = "external pin"; break; + case ESP_RST_SW: s = "software (esp_restart)"; break; + case ESP_RST_PANIC: s = "PANIC -- the previous run crashed"; break; + case ESP_RST_INT_WDT: s = "INTERRUPT WATCHDOG"; break; + case ESP_RST_TASK_WDT: s = "TASK WATCHDOG -- loop() stalled"; break; + case ESP_RST_WDT: s = "other watchdog"; break; + case ESP_RST_DEEPSLEEP: s = "deep-sleep wake"; break; + case ESP_RST_BROWNOUT: s = "BROWNOUT -- supply dipped"; break; + case ESP_RST_SDIO: s = "SDIO"; break; + case ESP_RST_USB: s = "USB peripheral"; break; + case ESP_RST_JTAG: s = "JTAG"; break; + case ESP_RST_EFUSE: s = "efuse error"; break; + default: s = "unknown"; break; + } + logPrintf("[BOOT] Reset reason: %s (%d)\n", s, (int)r); +} diff --git a/src/i2c_health.h b/src/i2c_health.h new file mode 100644 index 0000000..732c412 --- /dev/null +++ b/src/i2c_health.h @@ -0,0 +1,53 @@ +#pragma once +// i2c_health.h -- per-device I2C failure tracking, back-off and re-probe (#289). +// +// Why: on 2026-09-08 the touch controller, and later the IMU breakout, went +// silent on the bus for hours. Every reader retried its device on every loop +// pass, each retry costing a driver timeout, so the loop fell from 44 Hz to +// 12-27 Hz and the log filled with a quarter of a million identical errors. +// A device that stops answering must cost nothing per pass and come back on +// its own when it answers again. +// +// Pattern for a unit that owns an I2C device: +// static I2CDevice dev = {"LSM6DSOX", 0x6A, &imuAvailable}; +// read path: ok ? i2cNoteOk(dev) : i2cNoteFail(dev); +// service path: if (!imuAvailable && i2cReprobeDue(dev)) initIMU(); +// i2cNoteFail() clears *available after I2C_FAIL_LIMIT consecutive failures +// and logs once. i2cReprobeDue() runs at most every I2C_REPROBE_MS: it probes +// the address; on no answer it clears the bus (nine SCL clocks with SDA +// released, then START+STOP), re-opens Wire and probes once more. It returns +// true only when the device ACKs, so the caller can run its normal init. +// Only devices that were present at boot are re-probed: this is dropout +// recovery, not hot-plug. Transitions are logged once each. +// +// Where the failure signal comes from, per library (checked in the pinned +// sources under .pio/libdeps): SHT4x getEvent() returns false on a NACK; the +// BME68x driver leaves a negative sensor.status; MAX17048 isDeviceReady() +// reads the version register. LSM6DS and LIS3MDL getEvent() return true +// unconditionally and FT6206 touched() has no error path, so those are +// covered by an address probe (i2cProbe) instead. +#include + +#define I2C_FAIL_LIMIT 5 // consecutive failures before a device is dropped +#define I2C_REPROBE_MS 10000 // how often a dropped device is probed again + +struct I2CDevice { + const char* name; + uint8_t addr; + bool* available; + uint8_t fails = 0; // consecutive; reset by i2cNoteOk + uint32_t totalFails = 0; + uint32_t drops = 0; + unsigned long droppedAt = 0; + unsigned long lastProbe = 0; +}; + +void i2cNoteOk(I2CDevice& d); +bool i2cNoteFail(I2CDevice& d); // true on the call that drops the device +bool i2cReprobeDue(I2CDevice& d); // true when a dropped device answers again +bool i2cProbe(uint8_t addr); // address ACK test, one transaction +void i2cBusRecover(); // clock out a stuck slave, re-open Wire + +// Boot: print why the chip reset (power-on, panic, watchdog, brownout...). +// Without it a crash boot is unattributable from the next boot's log. +void logResetReason(); diff --git a/src/imu.cpp b/src/imu.cpp index 7aa03fb..e912853 100644 --- a/src/imu.cpp +++ b/src/imu.cpp @@ -1,11 +1,18 @@ // imu.cpp -- extracted from src.ino by scripts/extract_unit.py (E4). #include "imu.h" #include "logging.h" +#include "i2c_health.h" Adafruit_LSM6DSOX lsm; Adafruit_LIS3MDL lis; bool imuAvailable = false; bool magAvailable = false; + +// Dropout tracking (#289). Both chips sit on one breakout, so in practice +// they fail together; they are still tracked by address because they are +// separate I2C devices. The address is whichever one answered in initIMU(). +static I2CDevice imuDev = {"LSM6DSOX", 0x6A, &imuAvailable}; +static I2CDevice magDev = {"LIS3MDL", 0x1C, &magAvailable}; float magOffsetX = 0, magOffsetY = 0, magOffsetZ = 0; bool magCalibrated = false; bool magCalibrating = false; @@ -24,12 +31,15 @@ void magLogPrintln(const char* msg) { void initIMU() { logPrint("Initializing LSM6DSOX... "); - if (!lsm.begin_I2C(0x6A)) { - if (!lsm.begin_I2C(0x6B)) { + uint8_t addr = 0x6A; + if (!lsm.begin_I2C(addr)) { + addr = 0x6B; + if (!lsm.begin_I2C(addr)) { logPrintln("NOT FOUND"); return; } } + imuDev.addr = addr; lsm.setAccelRange(LSM6DS_ACCEL_RANGE_4_G); lsm.setGyroRange(LSM6DS_GYRO_RANGE_500_DPS); @@ -37,16 +47,20 @@ void initIMU() { lsm.setGyroDataRate(LSM6DS_RATE_104_HZ); imuAvailable = true; + i2cNoteOk(imuDev); logPrintln("OK"); logPrint("Initializing LIS3MDL... "); - if (!lis.begin_I2C(0x1C)) { - if (!lis.begin_I2C(0x1E)) { + addr = 0x1C; + if (!lis.begin_I2C(addr)) { + addr = 0x1E; + if (!lis.begin_I2C(addr)) { logPrintln("NOT FOUND"); return; } } + magDev.addr = addr; lis.setPerformanceMode(LIS3MDL_MEDIUMMODE); lis.setOperationMode(LIS3MDL_CONTINUOUSMODE); @@ -54,9 +68,21 @@ void initIMU() { lis.setRange(LIS3MDL_RANGE_4_GAUSS); magAvailable = true; + i2cNoteOk(magDev); logPrintln("OK"); } +// Called every loop pass (#289). Nothing to do while both chips answer; once +// one has been dropped, re-probe it on the slow timer and run the normal +// init when it answers. initIMU() re-initialises both chips, which is what a +// breakout that lost power or its bus needs anyway. +void serviceIMU() { + if ((!imuAvailable && i2cReprobeDue(imuDev)) || + (!magAvailable && i2cReprobeDue(magDev))) { + initIMU(); + } +} + // Load magnetometer calibration from SD card void loadMagCal() { if (!sdHealth.available) return; @@ -114,6 +140,18 @@ void saveMagCal() { void readIMU() { sensors_event_t accel, gyro, temp, mag; + // Both getEvent() calls return true unconditionally (checked in the pinned + // Adafruit_LSM6DS / Adafruit_LIS3MDL sources) and read into an uninitialised + // buffer on a NACK, so the bus failure has to be caught before the read: + // one address probe per chip per pass, ~0.1 ms each on a healthy bus. On a + // dead chip the probe fails fast, and after I2C_FAIL_LIMIT passes the loop + // gate (imuAvailable && magAvailable) stops calling this until serviceIMU() + // brings the breakout back (#289). + if (!i2cProbe(imuDev.addr)) { i2cNoteFail(imuDev); return; } + i2cNoteOk(imuDev); + if (!i2cProbe(magDev.addr)) { i2cNoteFail(magDev); return; } + i2cNoteOk(magDev); + lsm.getEvent(&accel, &gyro, &temp); lis.getEvent(&mag); diff --git a/src/imu.h b/src/imu.h index e2df607..966f4b6 100644 --- a/src/imu.h +++ b/src/imu.h @@ -33,3 +33,4 @@ void initIMU(); void loadMagCal(); void saveMagCal(); void readIMU(); +void serviceIMU(); // re-probe a dropped chip and re-init it (#289) diff --git a/src/src.ino b/src/src.ino index ba7907b..1476cab 100644 --- a/src/src.ino +++ b/src/src.ino @@ -69,6 +69,7 @@ #include "display.h" #include "ui_state.h" #include "touch.h" +#include "i2c_health.h" // reset reason at boot; the units own their device records (#289) #include "web.h" #include "lvgl_port.h" #include "oled_screens.h" @@ -134,6 +135,7 @@ void setup() { char banner[128]; snprintf(banner, sizeof(banner), "=================================\nField Compass Dual %s\n=================================\n\n", FW_VERSION); logPrintf("%s", banner); + logResetReason(); // panic / watchdog / brownout / power-on -- so a crash boot is attributable (#289) #ifdef FC_LDO2_PIN // UM FeatherS3 only (#283): the second LDO feeds the vertical STEMMA QT @@ -310,6 +312,15 @@ void loop() { if (shtAvailable) readSHT41(); // SHT41 temp/humidity (#48) if (imuAvailable && magAvailable) readIMU(); + // I2C health (#289): a device that stops answering is dropped after + // I2C_FAIL_LIMIT consecutive failures (its reads above stop), re-probed + // every I2C_REPROBE_MS, and re-initialised when it answers again. + serviceIMU(); + serviceSHT41(); + serviceBME688(); + serviceBattery(); + serviceTouch(); + // Weather logging (every 5 minutes) if (sdAvailable && bmeAvailable && (millis() - lastWeatherLog > WEATHER_LOG_INTERVAL)) { logWeatherReading(); diff --git a/src/touch.cpp b/src/touch.cpp index f58e34e..dd198c0 100644 --- a/src/touch.cpp +++ b/src/touch.cpp @@ -2,10 +2,33 @@ #include "touch.h" #include "fc_config.h" #include "logging.h" +#include "i2c_health.h" Adafruit_FT6206 ctp = Adafruit_FT6206(); bool touchAvailable = false; // FT6336U capacitive touch +// Dropout tracking (#289). The LVGL indev reads the chip at 30 Hz through +// touched(), which has no error path (a NACK reads as "not touched"), so the +// health check is an address probe on its own timer. TOUCH_CHECK_MS x +// I2C_FAIL_LIMIT is how long a dead chip keeps being read: 5 s, ~150 failed +// reads, against the 250k of 2026-09-08. Dropping touchAvailable stops both +// the indev callback and the wake poll. +static I2CDevice touchDev = {"FT6336U", 0x38, &touchAvailable}; +#define TOUCH_CHECK_MS 1000 + +// Called every loop pass (#289). initTouch() re-runs begin() and re-attaches +// the CTP_INT interrupt (the core replaces the handler on re-attach). +void serviceTouch() { + if (!touchAvailable) { + if (i2cReprobeDue(touchDev)) initTouch(); + return; + } + static unsigned long lastCheck = 0; + if (millis() - lastCheck < TOUCH_CHECK_MS) return; + lastCheck = millis(); + if (i2cProbe(touchDev.addr)) i2cNoteOk(touchDev); else i2cNoteFail(touchDev); +} + // CTP_INT is active-low: the FT6336U pulls it low for the duration of a // touch. The interrupt is attached FALLING (initTouch), so the ISR runs once // per finger-down and does nothing but set the flag -- no pin read, no call diff --git a/src/touch.h b/src/touch.h index 54293d2..c187198 100644 --- a/src/touch.h +++ b/src/touch.h @@ -7,6 +7,7 @@ extern bool touchAvailable; void IRAM_ATTR touchISR(); void initTouch(); +void serviceTouch(); // health probe; re-probe a dropped chip and re-init it (#289) // Wake-from-sleep path (#290). While the TFT sleeps loop() skips // lv_timer_handler(), so the LVGL indev callback -- the only reader of the From a2746b19751335a006eadaeca90cc4853708949e Mon Sep 17 00:00:00 2001 From: Strycher Date: Tue, 8 Sep 2026 17:30:46 -0400 Subject: [PATCH 02/10] fix(#289): BME688 health counts only BME68X_E_COM_FAIL Any negative sensor.status was being counted as a bus failure. Null-pointer, self-test and invalid-length codes are not bus faults and a re-probe would not clear them, so a chip with an internal fault would have been dropped and re-initialised every 10 s for nothing (Gemini finding on #289). Both envs rebuilt, sizes unchanged. Co-Authored-By: Claude Fable 5.1 --- src/env.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/env.cpp b/src/env.cpp index 89789d2..f32c628 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -291,11 +291,17 @@ void readBME688() { } // Bus health (#289): the BME68x driver checks endTransmission() and leaves - // BME68X_E_COM_FAIL in sensor.status when the chip stops answering. + // BME68X_E_COM_FAIL in sensor.status when the chip stops answering. Only + // that code counts: the other negative codes (null pointer, self-test, + // invalid length) are not bus faults and a re-probe would not clear them + // (review finding on #289). BSEC touches the chip in one burst per sample + // (setTPH, heater profile, forced mode, then fetchData), so a status + // sampled on the same 3 s cadence sees at most one or two samples per + // burst: one transient failure cannot reach I2C_FAIL_LIMIT. static unsigned long lastHealth = 0; if (millis() - lastHealth >= BME_HEALTH_MS) { lastHealth = millis(); - if (envSensor.sensor.status < BME68X_OK) i2cNoteFail(bmeDev); else i2cNoteOk(bmeDev); + if (envSensor.sensor.status == BME68X_E_COM_FAIL) i2cNoteFail(bmeDev); else i2cNoteOk(bmeDev); } } From bca36a4e9d6883fd9e26f219605efcf4d8de5ec5 Mon Sep 17 00:00:00 2001 From: Strycher Date: Tue, 8 Sep 2026 17:57:20 -0400 Subject: [PATCH 03/10] fix(#289): health check is a chip-ID register read, not an address probe The live bench disproved the first design while it was under review. The LIS3MDL on the 4517 breakout has been NACKing nearly every data byte for hours, yet the boot bus scan lists 0x1C on every boot: the chip ACKs its address and fails the transaction after it. An address-only probe would have called it healthy all night. At the 17:38 boot its WHO_AM_I read succeeded, every configuration write after it NACKed, and begin_I2C() still returned true. A reset did not clear the fault; the LSM6DSOX on the same board and cable reads fine, so it is the chip, not the wiring. - i2cReadReg(): write register, repeated start, read one byte, on Wire directly (the drivers' own transaction shape). i2cCheck() compares the chip-ID register to the expected value: LSM6DSOX 0x0F=0x6C, LIS3MDL 0x0F=0x3D, FT6336U 0xA8=0x11, BME688 0xD0=0x61. SHT41 and MAX17048 have no ID register and keep the address probe for the re-probe only. - IMU check moves off the read path onto a 200 ms timer in serviceIMU(). - Two-address chips record the alternate; the re-probe tries it after the bus recovery and swaps addr if the chip answers there. - Drop on 5 consecutive failures OR 8 of the last 16 checks, so a chip that answers one read in ten cannot keep resetting the count (review pass 2, finding 2). - initIMU() clears both flags first and reads one configuration register back on each chip; a chip whose configuration writes were NACKed is refused instead of declared available while powered down (review pass 2, finding 3; the 17:38 boot did exactly that). Builds: um_feathers3 RAM 22.7% Flash 27.6%; feather_s3 RAM 31.6% Flash 64.7%. Co-Authored-By: Claude Fable 5.1 --- src/env.cpp | 8 +++-- src/i2c_health.cpp | 43 +++++++++++++++++++++---- src/i2c_health.h | 57 ++++++++++++++++++++------------- src/imu.cpp | 80 ++++++++++++++++++++++++++++++++-------------- src/touch.cpp | 13 ++++---- 5 files changed, 141 insertions(+), 60 deletions(-) diff --git a/src/env.cpp b/src/env.cpp index f32c628..6600c13 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -15,9 +15,12 @@ bool bmeAvailable = false; bool shtAvailable = false; // SHT41 temp/humidity (#48) // Dropout tracking (#289). The BME688 address is whichever answered in -// initBME688(); the SHT41 has only one. +// initBME688(), the other is the alternate the re-probe also tries, and its +// chip-ID register is the re-probe check. The SHT41 has one address and no +// ID register, so its re-probe is an address probe; initSHT41() (a serial +// number read) is the real test when it answers. static I2CDevice shtDev = {"SHT41", 0x44, &shtAvailable}; -static I2CDevice bmeDev = {"BME688", 0x77, &bmeAvailable}; +static I2CDevice bmeDev = {"BME688", 0x77, &bmeAvailable, BME68X_REG_CHIP_ID, BME68X_CHIP_ID, 0x76}; // The BME688 is touched once per BSEC sample (LP mode = 3 s) and its status // persists until the next access, so its health is sampled on that cadence; // sampling every loop pass would count one failed read five times over. @@ -109,6 +112,7 @@ void initBME688() { } } bmeDev.addr = addr; + bmeDev.altAddr = (addr == 0x77) ? 0x76 : 0x77; i2cNoteOk(bmeDev); // Load BSEC2 IAQ config diff --git a/src/i2c_health.cpp b/src/i2c_health.cpp index 7a10f1e..9064b66 100644 --- a/src/i2c_health.cpp +++ b/src/i2c_health.cpp @@ -6,19 +6,21 @@ void i2cNoteOk(I2CDevice& d) { d.fails = 0; + d.recent <<= 1; } bool i2cNoteFail(I2CDevice& d) { d.totalFails++; if (d.fails < 255) d.fails++; + d.recent = (d.recent << 1) | 1; if (!*d.available) return false; // already dropped - if (d.fails < I2C_FAIL_LIMIT) return false; + if (d.fails < I2C_FAIL_LIMIT && __builtin_popcount(d.recent) < I2C_FAIL_WINDOW) return false; *d.available = false; d.drops++; d.droppedAt = millis(); d.lastProbe = d.droppedAt; // first re-probe one interval from now - logPrintf("[I2C] %s (0x%02X) stopped answering: %u consecutive failures, dropped; re-probing every %d s\n", - d.name, d.addr, (unsigned)d.fails, I2C_REPROBE_MS / 1000); + logPrintf("[I2C] %s (0x%02X) stopped answering: %u consecutive failures, %d of the last 16 checks; dropped, re-probing every %d s\n", + d.name, d.addr, (unsigned)d.fails, __builtin_popcount(d.recent), I2C_REPROBE_MS / 1000); return true; } @@ -29,6 +31,28 @@ bool i2cProbe(uint8_t addr) { return Wire.endTransmission() == 0; } +bool i2cReadReg(uint8_t addr, uint8_t reg, uint8_t* val) { + // The same write-then-read every Adafruit driver uses. endTransmission(false) + // only queues the register byte on this core; the combined transaction runs + // in requestFrom(), which returns 0 bytes when any part of it is NACKed. + Wire.beginTransmission(addr); + Wire.write(reg); + Wire.endTransmission(false); + if (Wire.requestFrom((uint8_t)addr, (uint8_t)1) != 1) return false; + *val = Wire.read(); + return true; +} + +static bool checkAt(uint8_t addr, const I2CDevice& d) { + if (d.idReg == I2C_NO_REG) return i2cProbe(addr); + uint8_t v; + return i2cReadReg(addr, d.idReg, &v) && v == d.idVal; +} + +bool i2cCheck(const I2CDevice& d) { + return checkAt(d.addr, d); +} + // A slave left mid-transaction by a glitch can hold SDA low until it sees // enough clocks to finish its byte and then a STOP. Clock it out with the // pins as plain GPIO, then hand the pins back to the I2C peripheral. Same @@ -62,13 +86,20 @@ bool i2cReprobeDue(I2CDevice& d) { unsigned long now = millis(); if (now - d.lastProbe < I2C_REPROBE_MS) return false; d.lastProbe = now; - if (!i2cProbe(d.addr)) { + if (!checkAt(d.addr, d)) { i2cBusRecover(); - if (!i2cProbe(d.addr)) return false; + if (!checkAt(d.addr, d)) { + if (!d.altAddr || !checkAt(d.altAddr, d)) return false; + logPrintf("[I2C] %s answers at 0x%02X now, not 0x%02X\n", d.name, d.altAddr, d.addr); + uint8_t was = d.addr; + d.addr = d.altAddr; + d.altAddr = was; + } } - logPrintf("[I2C] %s (0x%02X) answers again after %lu s (drop #%lu, %lu failed reads so far); re-initialising\n", + logPrintf("[I2C] %s (0x%02X) answers again after %lu s (drop #%lu, %lu failed checks so far); re-initialising\n", d.name, d.addr, (now - d.droppedAt) / 1000, (unsigned long)d.drops, (unsigned long)d.totalFails); d.fails = 0; + d.recent = 0; // a fresh window, or the old one drops it on its first miss return true; } diff --git a/src/i2c_health.h b/src/i2c_health.h index 732c412..e322461 100644 --- a/src/i2c_health.h +++ b/src/i2c_health.h @@ -1,41 +1,52 @@ #pragma once // i2c_health.h -- per-device I2C failure tracking, back-off and re-probe (#289). // -// Why: on 2026-09-08 the touch controller, and later the IMU breakout, went -// silent on the bus for hours. Every reader retried its device on every loop -// pass, each retry costing a driver timeout, so the loop fell from 44 Hz to -// 12-27 Hz and the log filled with a quarter of a million identical errors. +// Why: on 2026-09-08 the touch controller, and later the LIS3MDL, went silent +// on the bus for hours. Every reader retried its device on every loop pass, +// each retry costing two error lines to serial and SD, so the loop fell from +// 44 Hz to 12-25 Hz and the log filled with a million identical errors. // A device that stops answering must cost nothing per pass and come back on // its own when it answers again. // // Pattern for a unit that owns an I2C device: -// static I2CDevice dev = {"LSM6DSOX", 0x6A, &imuAvailable}; -// read path: ok ? i2cNoteOk(dev) : i2cNoteFail(dev); +// static I2CDevice dev = {"LSM6DSOX", 0x6A, &imuAvailable, LSM6DS_WHOAMI, LSM6DSOX_CHIP_ID, 0x6B}; +// health path: i2cCheck(dev) ? i2cNoteOk(dev) : i2cNoteFail(dev); (on a slow timer) +// read path: a library call that reports failure feeds the same two // service path: if (!imuAvailable && i2cReprobeDue(dev)) initIMU(); -// i2cNoteFail() clears *available after I2C_FAIL_LIMIT consecutive failures -// and logs once. i2cReprobeDue() runs at most every I2C_REPROBE_MS: it probes -// the address; on no answer it clears the bus (nine SCL clocks with SDA -// released, then START+STOP), re-opens Wire and probes once more. It returns -// true only when the device ACKs, so the caller can run its normal init. -// Only devices that were present at boot are re-probed: this is dropout -// recovery, not hot-plug. Transitions are logged once each. // -// Where the failure signal comes from, per library (checked in the pinned -// sources under .pio/libdeps): SHT4x getEvent() returns false on a NACK; the -// BME68x driver leaves a negative sensor.status; MAX17048 isDeviceReady() -// reads the version register. LSM6DS and LIS3MDL getEvent() return true -// unconditionally and FT6206 touched() has no error path, so those are -// covered by an address probe (i2cProbe) instead. +// What counts as "answering": a chip-ID register read that returns the +// expected value. Not an address probe. The bench case that shaped this: +// the LIS3MDL kept ACKing its address (the boot scan listed 0x1C every time) +// while NACKing nearly every data byte, so an address-only probe would have +// called it healthy all night. Devices with no ID register (SHT41, MAX17048) +// fall back to the probe; their own init is the real test when they return. +// +// i2cNoteFail() clears *available after I2C_FAIL_LIMIT consecutive failures +// and logs once. i2cReprobeDue() runs at most every I2C_REPROBE_MS for a +// dropped device: check at its address; if that fails, clear the bus (nine +// SCL clocks with SDA released, then START+STOP), re-open Wire and check +// again, at the alternate address too when the chip has one (a floating +// address-select pad moves a chip between its two addresses). It returns +// true only when the chip answers, so the caller runs its normal init. Only +// devices present at boot are re-probed: dropout recovery, not hot-plug. #include -#define I2C_FAIL_LIMIT 5 // consecutive failures before a device is dropped +#define I2C_FAIL_LIMIT 5 // consecutive failures before a device is dropped... +#define I2C_FAIL_WINDOW 8 // ...or this many of the last 16 checks: an intermittent + // chip (the bench LIS3MDL answered ~1 read in 100) must + // not keep resetting the count with the odd success #define I2C_REPROBE_MS 10000 // how often a dropped device is probed again +#define I2C_NO_REG 0xFF // idReg value for a chip without an ID register struct I2CDevice { const char* name; uint8_t addr; bool* available; + uint8_t idReg = I2C_NO_REG; // chip-ID register, or I2C_NO_REG for an address probe + uint8_t idVal = 0; // the value that register must return + uint8_t altAddr = 0; // the chip's other address, 0 if it has one only uint8_t fails = 0; // consecutive; reset by i2cNoteOk + uint16_t recent = 0; // last 16 results, 1 = failed, newest in bit 0 uint32_t totalFails = 0; uint32_t drops = 0; unsigned long droppedAt = 0; @@ -44,8 +55,10 @@ struct I2CDevice { void i2cNoteOk(I2CDevice& d); bool i2cNoteFail(I2CDevice& d); // true on the call that drops the device -bool i2cReprobeDue(I2CDevice& d); // true when a dropped device answers again -bool i2cProbe(uint8_t addr); // address ACK test, one transaction +bool i2cReprobeDue(I2CDevice& d); // true when a dropped device answers again (d.addr updated if it moved) +bool i2cCheck(const I2CDevice& d); // one real transaction at d.addr: ID register read, or probe +bool i2cReadReg(uint8_t addr, uint8_t reg, uint8_t* val); // write reg, repeated start, read one byte +bool i2cProbe(uint8_t addr); // address ACK only, one transaction void i2cBusRecover(); // clock out a stuck slave, re-open Wire // Boot: print why the chip reset (power-on, panic, watchdog, brownout...). diff --git a/src/imu.cpp b/src/imu.cpp index e912853..770923b 100644 --- a/src/imu.cpp +++ b/src/imu.cpp @@ -8,11 +8,20 @@ Adafruit_LIS3MDL lis; bool imuAvailable = false; bool magAvailable = false; -// Dropout tracking (#289). Both chips sit on one breakout, so in practice -// they fail together; they are still tracked by address because they are -// separate I2C devices. The address is whichever one answered in initIMU(). -static I2CDevice imuDev = {"LSM6DSOX", 0x6A, &imuAvailable}; -static I2CDevice magDev = {"LIS3MDL", 0x1C, &magAvailable}; +// Dropout tracking (#289). Both chips sit on one breakout (Adafruit 4517) but +// are separate I2C devices, and on the bench they failed separately: the +// LIS3MDL NACKed nearly every data byte for hours while the LSM6DSOX beside +// it read fine. Each has two possible addresses; the one that answered in +// initIMU() is recorded, the other is the alternate the re-probe also tries. +// The health check is the WHO_AM_I read: the LIS3MDL kept ACKing its address +// (the boot scan listed 0x1C throughout) while failing every data read, so +// an address probe is not a signal for these chips. +static I2CDevice imuDev = {"LSM6DSOX", 0x6A, &imuAvailable, LSM6DS_WHOAMI, LSM6DSOX_CHIP_ID, 0x6B}; +static I2CDevice magDev = {"LIS3MDL", 0x1C, &magAvailable, LIS3MDL_REG_WHO_AM_I, 0x3D, 0x1E}; +// getEvent() on both chips returns true unconditionally, so the read path +// carries no failure signal; the checks run on this timer instead. Worst +// case a dead chip is read for IMU_CHECK_MS x I2C_FAIL_LIMIT = 1 s. +#define IMU_CHECK_MS 200 float magOffsetX = 0, magOffsetY = 0, magOffsetZ = 0; bool magCalibrated = false; bool magCalibrating = false; @@ -29,6 +38,11 @@ void magLogPrintln(const char* msg) { } void initIMU() { + // Re-runnable (#289): a re-init that fails must not leave the flag from + // the previous success standing. + imuAvailable = false; + magAvailable = false; + logPrint("Initializing LSM6DSOX... "); uint8_t addr = 0x6A; @@ -40,12 +54,23 @@ void initIMU() { } } imuDev.addr = addr; + imuDev.altAddr = (addr == 0x6A) ? 0x6B : 0x6A; lsm.setAccelRange(LSM6DS_ACCEL_RANGE_4_G); lsm.setGyroRange(LSM6DS_GYRO_RANGE_500_DPS); lsm.setAccelDataRate(LSM6DS_RATE_104_HZ); lsm.setGyroDataRate(LSM6DS_RATE_104_HZ); + // Read one setting back. begin_I2C() checks only WHO_AM_I and the setters + // are void, so a chip that NACKs its configuration writes would be declared + // available while misconfigured or powered down. The 17:38 boot on + // 2026-09-08 did exactly that on the LIS3MDL: WHO_AM_I read, every write + // after it NACKed, "OK". A failed read-back returns all-ones, never 104 Hz. + if (lsm.getAccelDataRate() != LSM6DS_RATE_104_HZ) { + logPrintln("CONFIG NOT ACCEPTED (read-back mismatch)"); + return; + } + imuAvailable = true; i2cNoteOk(imuDev); logPrintln("OK"); @@ -61,26 +86,41 @@ void initIMU() { } } magDev.addr = addr; + magDev.altAddr = (addr == 0x1C) ? 0x1E : 0x1C; lis.setPerformanceMode(LIS3MDL_MEDIUMMODE); lis.setOperationMode(LIS3MDL_CONTINUOUSMODE); lis.setDataRate(LIS3MDL_DATARATE_155_HZ); lis.setRange(LIS3MDL_RANGE_4_GAUSS); + if (lis.getDataRate() != LIS3MDL_DATARATE_155_HZ) { // same read-back as the LSM6DSOX above + logPrintln("CONFIG NOT ACCEPTED (read-back mismatch)"); + return; + } + magAvailable = true; i2cNoteOk(magDev); logPrintln("OK"); } -// Called every loop pass (#289). Nothing to do while both chips answer; once -// one has been dropped, re-probe it on the slow timer and run the normal -// init when it answers. initIMU() re-initialises both chips, which is what a -// breakout that lost power or its bus needs anyway. +// Called every loop pass (#289). While both chips are up, read each WHO_AM_I +// every IMU_CHECK_MS and count the result. Once either has been dropped, +// re-probe it on the slow timer and run the normal init when it answers; +// initIMU() re-initialises both chips, which a breakout that lost power or +// its bus needs anyway, and tries both addresses of each. void serviceIMU() { - if ((!imuAvailable && i2cReprobeDue(imuDev)) || - (!magAvailable && i2cReprobeDue(magDev))) { - initIMU(); + if (!imuAvailable || !magAvailable) { + if ((!imuAvailable && i2cReprobeDue(imuDev)) || + (!magAvailable && i2cReprobeDue(magDev))) { + initIMU(); + } + return; } + static unsigned long lastCheck = 0; + if (millis() - lastCheck < IMU_CHECK_MS) return; + lastCheck = millis(); + if (i2cCheck(imuDev)) i2cNoteOk(imuDev); else i2cNoteFail(imuDev); + if (i2cCheck(magDev)) i2cNoteOk(magDev); else i2cNoteFail(magDev); } // Load magnetometer calibration from SD card @@ -140,18 +180,10 @@ void saveMagCal() { void readIMU() { sensors_event_t accel, gyro, temp, mag; - // Both getEvent() calls return true unconditionally (checked in the pinned - // Adafruit_LSM6DS / Adafruit_LIS3MDL sources) and read into an uninitialised - // buffer on a NACK, so the bus failure has to be caught before the read: - // one address probe per chip per pass, ~0.1 ms each on a healthy bus. On a - // dead chip the probe fails fast, and after I2C_FAIL_LIMIT passes the loop - // gate (imuAvailable && magAvailable) stops calling this until serviceIMU() - // brings the breakout back (#289). - if (!i2cProbe(imuDev.addr)) { i2cNoteFail(imuDev); return; } - i2cNoteOk(imuDev); - if (!i2cProbe(magDev.addr)) { i2cNoteFail(magDev); return; } - i2cNoteOk(magDev); - + // Neither getEvent() reports a failed read (they return true and leave the + // buffer uninitialised on a NACK); serviceIMU() watches the chips instead + // and the loop gate (imuAvailable && magAvailable) stops this once one is + // dropped (#289). lsm.getEvent(&accel, &gyro, &temp); lis.getEvent(&mag); diff --git a/src/touch.cpp b/src/touch.cpp index dd198c0..1d4676c 100644 --- a/src/touch.cpp +++ b/src/touch.cpp @@ -9,11 +9,12 @@ bool touchAvailable = false; // FT6336U capacitive touch // Dropout tracking (#289). The LVGL indev reads the chip at 30 Hz through // touched(), which has no error path (a NACK reads as "not touched"), so the -// health check is an address probe on its own timer. TOUCH_CHECK_MS x -// I2C_FAIL_LIMIT is how long a dead chip keeps being read: 5 s, ~150 failed -// reads, against the 250k of 2026-09-08. Dropping touchAvailable stops both -// the indev callback and the wake poll. -static I2CDevice touchDev = {"FT6336U", 0x38, &touchAvailable}; +// health check is a vendor-ID register read on its own timer (the same +// register begin() verifies). TOUCH_CHECK_MS x I2C_FAIL_LIMIT is how long a +// dead chip keeps being read: 5 s, ~150 failed reads, against the 250k of +// 2026-09-08. Dropping touchAvailable stops both the indev callback and the +// wake poll. +static I2CDevice touchDev = {"FT6336U", 0x38, &touchAvailable, FT62XX_REG_VENDID, FT62XX_VENDID}; #define TOUCH_CHECK_MS 1000 // Called every loop pass (#289). initTouch() re-runs begin() and re-attaches @@ -26,7 +27,7 @@ void serviceTouch() { static unsigned long lastCheck = 0; if (millis() - lastCheck < TOUCH_CHECK_MS) return; lastCheck = millis(); - if (i2cProbe(touchDev.addr)) i2cNoteOk(touchDev); else i2cNoteFail(touchDev); + if (i2cCheck(touchDev)) i2cNoteOk(touchDev); else i2cNoteFail(touchDev); } // CTP_INT is active-low: the FT6336U pulls it low for the duration of a From 3b0ff2fe1a6931928135a24a296dde592bf4bedc Mon Sep 17 00:00:00 2001 From: Strycher Date: Tue, 8 Sep 2026 18:03:20 -0400 Subject: [PATCH 04/10] fix(#289): init clears the failure history; LIS3MDL read-back checks the operation mode Third review pass. The re-probe cleared a device's failure window on a good WHO_AM_I read before its init had run, so a chip that passes the probe but fails its configuration forgot it was flaky; now only a unit's init clears the window, through i2cNoteReady(), when it succeeds. The LIS3MDL read-back moved from the data-rate bits to the operation mode, the setting whose failure yields zero data and whose register default and failed-read value are both power-down. Both envs rebuilt, sizes unchanged. Co-Authored-By: Claude Fable 5.1 --- src/battery.cpp | 2 +- src/env.cpp | 3 ++- src/i2c_health.cpp | 9 ++++++--- src/i2c_health.h | 3 +++ src/imu.cpp | 9 ++++++--- src/touch.cpp | 1 + 6 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/battery.cpp b/src/battery.cpp index 038ae43..b3c1d41 100644 --- a/src/battery.cpp +++ b/src/battery.cpp @@ -23,7 +23,7 @@ void initBattery() { } batteryAvailable = true; - i2cNoteOk(battDev); + i2cNoteReady(battDev); logPrintln("OK"); } diff --git a/src/env.cpp b/src/env.cpp index 6600c13..e0c0a1c 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -113,7 +113,7 @@ void initBME688() { } bmeDev.addr = addr; bmeDev.altAddr = (addr == 0x77) ? 0x76 : 0x77; - i2cNoteOk(bmeDev); + i2cNoteReady(bmeDev); // Load BSEC2 IAQ config if (!envSensor.setConfig(bsec2_config)) { @@ -155,6 +155,7 @@ void initSHT41() { sht4.setHeater(SHT4X_NO_HEATER); shtAvailable = true; + i2cNoteReady(shtDev); logPrintln("OK (0x44)"); // Read initial values immediately diff --git a/src/i2c_health.cpp b/src/i2c_health.cpp index 9064b66..b2cb0e2 100644 --- a/src/i2c_health.cpp +++ b/src/i2c_health.cpp @@ -9,6 +9,11 @@ void i2cNoteOk(I2CDevice& d) { d.recent <<= 1; } +void i2cNoteReady(I2CDevice& d) { + d.fails = 0; + d.recent = 0; +} + bool i2cNoteFail(I2CDevice& d) { d.totalFails++; if (d.fails < 255) d.fails++; @@ -98,9 +103,7 @@ bool i2cReprobeDue(I2CDevice& d) { } logPrintf("[I2C] %s (0x%02X) answers again after %lu s (drop #%lu, %lu failed checks so far); re-initialising\n", d.name, d.addr, (now - d.droppedAt) / 1000, (unsigned long)d.drops, (unsigned long)d.totalFails); - d.fails = 0; - d.recent = 0; // a fresh window, or the old one drops it on its first miss - return true; + return true; // the caller's init clears the history via i2cNoteReady() if it succeeds } void logResetReason() { diff --git a/src/i2c_health.h b/src/i2c_health.h index e322461..8f3d3a3 100644 --- a/src/i2c_health.h +++ b/src/i2c_health.h @@ -55,6 +55,9 @@ struct I2CDevice { void i2cNoteOk(I2CDevice& d); bool i2cNoteFail(I2CDevice& d); // true on the call that drops the device +void i2cNoteReady(I2CDevice& d); // a unit's init calls this on success: clears the count and the window. + // Nothing else clears the window, so a chip that passes the re-probe + // but fails its init keeps its history (review finding on #289) bool i2cReprobeDue(I2CDevice& d); // true when a dropped device answers again (d.addr updated if it moved) bool i2cCheck(const I2CDevice& d); // one real transaction at d.addr: ID register read, or probe bool i2cReadReg(uint8_t addr, uint8_t reg, uint8_t* val); // write reg, repeated start, read one byte diff --git a/src/imu.cpp b/src/imu.cpp index 770923b..3d94524 100644 --- a/src/imu.cpp +++ b/src/imu.cpp @@ -72,7 +72,7 @@ void initIMU() { } imuAvailable = true; - i2cNoteOk(imuDev); + i2cNoteReady(imuDev); logPrintln("OK"); logPrint("Initializing LIS3MDL... "); @@ -93,13 +93,16 @@ void initIMU() { lis.setDataRate(LIS3MDL_DATARATE_155_HZ); lis.setRange(LIS3MDL_RANGE_4_GAUSS); - if (lis.getDataRate() != LIS3MDL_DATARATE_155_HZ) { // same read-back as the LSM6DSOX above + // Same read-back as the LSM6DSOX above, on the setting that matters: a + // chip left in power-down (the register default, and what a failed read + // returns as all-ones) reports zeros for every axis. + if (lis.getOperationMode() != LIS3MDL_CONTINUOUSMODE) { logPrintln("CONFIG NOT ACCEPTED (read-back mismatch)"); return; } magAvailable = true; - i2cNoteOk(magDev); + i2cNoteReady(magDev); logPrintln("OK"); } diff --git a/src/touch.cpp b/src/touch.cpp index 1d4676c..90b7a3d 100644 --- a/src/touch.cpp +++ b/src/touch.cpp @@ -53,6 +53,7 @@ void initTouch() { } touchAvailable = true; + i2cNoteReady(touchDev); // Configure interrupt pin (CTP_INT is active-low, open-drain). FALLING: // one interrupt per finger-down, nothing on release (#290). From b09bb19a4f960001c25d101de9905b99dd4ad89c Mon Sep 17 00:00:00 2001 From: Strycher Date: Tue, 8 Sep 2026 19:00:49 -0400 Subject: [PATCH 05/10] docs(#289): bench STEMMA chain order; scan names 0x29 as the TSL2591 Chain on the bench, from the owner on 2026-09-08: Feather -> BME688 -> SHT41 -> LSM6DSOX/LIS3MDL (one 4517 board) -> TSL2591 (unused). Moving the IMU board up the chain did not change the failure rate (one failed IMU-board transaction per loop pass, 22.4/s, steady for 75 minutes either side), and the failing read moved from the LIS3MDL to the other chip on the same board with no reboot. The scan labelled 0x29 as a VEML7700; the device there is the TSL2591. Both envs rebuilt. Co-Authored-By: Claude Fable 5.1 --- docs/HARDWARE.md | 7 +++++++ src/src.ino | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/HARDWARE.md b/docs/HARDWARE.md index 9f9ac2a..0f0114b 100644 --- a/docs/HARDWARE.md +++ b/docs/HARDWARE.md @@ -378,6 +378,13 @@ Source for all of these: UM's FeatherS3D pinout card, needs an external boost from `BAT`. IO34 "5V SENSE" reads the USB net through a 2 kΩ / 3.3 kΩ divider. STEMMA1 (LDO1) = IO9 SCL / IO8 SDA; STEMMA2 (LDO2) = IO15 SCL / IO16 SDA, as the wiring page says. +- **STEMMA QT chain order on the bench** (owner, 2026-09-08 18:5x, during #289): Feather → + BME688 → SHT41 → LSM6DSOX/LIS3MDL (one 4517 board) → TSL2591 (unused). The IMU board was + moved up the chain to this position. The bus failure it was moved for did not change + with the move: one failed transaction to the IMU board per loop pass, 22.4/s, steady for + 75 minutes either side of it, and it moved from the LIS3MDL read (garbage magnetometer + data until 17:49) to the other read on the same board with no reboot. The scan's 0x29 is + that TSL2591, not a VEML7700 (label fixed in #289). - **First run results** (image 8af7ce5): TFT init and LVGL completed and the backlight lit, but the panel stayed blank, the I2C1 scan found zero devices (the stacked OLED wing included), and the first SD mount never returned. The SD hang was two SPI drivers on one diff --git a/src/src.ino b/src/src.ino index 1476cab..87c974f 100644 --- a/src/src.ino +++ b/src/src.ino @@ -492,7 +492,7 @@ static int scanI2CBus(TwoWire &bus, const char *label) { } else if (address == 0x44) { desc = " (SHT41 - Temp/Humidity)"; } else if (address == 0x29) { - desc = " (VEML7700 - Light Sensor)"; + desc = " (TSL2591 - Light Sensor, unused)"; // the bench device at 0x29 (#289); a VEML7700 lives at 0x10 } else if (address == 0x68) { desc = " (PCF8523 - RTC)"; } else if (address == 0x7E) { From 8c96702cad5c60b2a4e66517d21259112dbb9c58 Mon Sep 17 00:00:00 2001 From: Strycher Date: Tue, 8 Sep 2026 19:08:41 -0400 Subject: [PATCH 06/10] docs(#289): the 4517 board facts behind the dropout diagnosis From the product page and the pinouts guide, read 2026-09-08: one shared regulator, shared FET level shifters with 10k pull-ups, ADM/ADAG address jumpers (open = 0x1C / 0x6A), INT pins unwired. Shared parts would take both chips down together; the bench fault takes one at a time. Co-Authored-By: Claude Fable 5.1 --- docs/HARDWARE.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/HARDWARE.md b/docs/HARDWARE.md index 0f0114b..7dd3449 100644 --- a/docs/HARDWARE.md +++ b/docs/HARDWARE.md @@ -385,6 +385,13 @@ Source for all of these: UM's FeatherS3D pinout card, 75 minutes either side of it, and it moved from the LIS3MDL read (garbage magnetometer data until 17:49) to the other read on the same board with no reboot. The scan's 0x29 is that TSL2591, not a VEML7700 (label fixed in #289). + The 4517 itself (adafruit.com/product/4517 and learn.adafruit.com/st-9-dof-combo/pinouts, + read 2026-09-08): one on-board 3.3 V regulator for both chips (AP2112K, or a ≥150 mA + substitute since the 2022-06 parts shortage), FET level shifters on SDA/SCL with 10 kΩ + pull-ups, address jumpers on the back — `ADM` bridges the LIS3MDL 0x1C → 0x1E, `ADAG` + bridges the LSM6DSOX 0x6A → 0x6B, both open on the bench — and INT1/INT2/INTM/DRDY pins, + none wired. Regulator and level shifters are shared by both chips, so a fault in either + would hit both reads at once; the bench fault takes one chip at a time. - **First run results** (image 8af7ce5): TFT init and LVGL completed and the backlight lit, but the panel stayed blank, the I2C1 scan found zero devices (the stacked OLED wing included), and the first SD mount never returned. The SD hang was two SPI drivers on one From b5c84ee5a1cd454238d591677b92776c1e61b951 Mon Sep 17 00:00:00 2001 From: Strycher Date: Tue, 8 Sep 2026 19:47:58 -0400 Subject: [PATCH 07/10] docs(#289): the redirected-output flash hang and its fix pio-flash confirm hung twice at what looked like Connecting... for ten minutes. PlatformIO's echo thread died with UnicodeEncodeError (cp1252) on esptool's Unicode progress bar because stdout was a file, esptool blocked on the full pipe mid-write. PYTHONUTF8=1 PYTHONIOENCODING=utf-8 fixed it; the third run flashed and verified all four regions. Co-Authored-By: Claude Fable 5.1 --- docs/DIAGNOSTICS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/DIAGNOSTICS.md b/docs/DIAGNOSTICS.md index 7e5be76..1e63efd 100644 --- a/docs/DIAGNOSTICS.md +++ b/docs/DIAGNOSTICS.md @@ -169,6 +169,7 @@ arduino-cli upload --fqbn esp32:esp32:adafruit_feather_esp32s3 --port F | `--before usb-reset` from a running app | does not enter download mode | | bulk `read-flash` over native USB | fails repeatedly; small transactions like `chip-id` work | | holding a serial handle across a reset | port re-enumerates, handle dies | +| `pio-flash confirm` with its output redirected to a file or pipe on Windows, default code page | looks like a hang at `Connecting...` with no `Writing at` line for ten minutes. Not the board: PlatformIO's echo thread dies with `UnicodeEncodeError` (cp1252) on esptool's Unicode progress bar, esptool blocks on the full pipe **mid-write**, and the chip is left in the stub. Set `PYTHONUTF8=1 PYTHONIOENCODING=utf-8` for the wrapper call. Seen twice on 2026-09-08 (#289), clean flash on the third run with the variables set | A full flash image backup needs a UART adapter wired to RX/TX — it is not achievable over native USB. From 4a9c9aa0e5bff5bb0a7099edc0c3293f87366d9c Mon Sep 17 00:00:00 2001 From: Strycher Date: Tue, 8 Sep 2026 20:52:34 -0400 Subject: [PATCH 08/10] fix(#289): bus recovery sets the pins to GPIO before writing the latch The first live recovery (2026-09-08 20:44) logged two core refusals per run: digitalWrite() on a pin still routed to the I2C peripheral. Inputs first, latch high, then SCL to open-drain output. Both envs rebuilt. Co-Authored-By: Claude Fable 5.1 --- src/i2c_health.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/i2c_health.cpp b/src/i2c_health.cpp index b2cb0e2..fbfbebb 100644 --- a/src/i2c_health.cpp +++ b/src/i2c_health.cpp @@ -64,9 +64,14 @@ bool i2cCheck(const I2CDevice& d) { // end()/begin() pair the #283 bus diagnostic uses, verified on both boards. void i2cBusRecover() { Wire.end(); - digitalWrite(SDA, HIGH); // output latch high before the mode change, - digitalWrite(SCL, HIGH); // so neither line dips when it becomes GPIO + // Pins back to GPIO as inputs first: the core refuses digitalWrite() on a + // pin still routed to the I2C peripheral (seen in the first recovery on the + // bench, 2026-09-08 20:44). Then set the latch high while nothing drives, + // then switch SCL to open-drain output so it does not dip on the change. pinMode(SDA, INPUT_PULLUP); + pinMode(SCL, INPUT_PULLUP); + digitalWrite(SDA, HIGH); + digitalWrite(SCL, HIGH); pinMode(SCL, OUTPUT_OPEN_DRAIN); delayMicroseconds(5); for (int i = 0; i < 9; i++) { From 79b4344877046cc375120a92b31f16730876196c Mon Sep 17 00:00:00 2001 From: Strycher Date: Tue, 8 Sep 2026 20:56:27 -0400 Subject: [PATCH 09/10] feat(#289): back off the re-probe of a flapping device On the bench the LIS3MDL, whose address-select input is floating, came back at the other address every 10 s and was dropped again within a second: a re-init and a burst of failed reads every 11 s. A device that drops again within 30 s of returning now waits twice as long before the next re-probe, 10 s doubling to 320 s; a return that holds for 30 s resets it. Correction: the previous commit (4a9c9aa) said both envs were rebuilt. That build had failed, on a tool-esptoolpy package that PlatformIO reinstalled as a stub and could not complete under MSYS. Rebuilt from PowerShell: um_feathers3 RAM 22.7% Flash 27.6%, feather_s3 RAM 31.6% Flash 64.7%, and the package is whole again. Co-Authored-By: Claude Fable 5.1 --- src/i2c_health.cpp | 14 +++++++++++--- src/i2c_health.h | 10 +++++++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/i2c_health.cpp b/src/i2c_health.cpp index fbfbebb..872bbcd 100644 --- a/src/i2c_health.cpp +++ b/src/i2c_health.cpp @@ -24,8 +24,15 @@ bool i2cNoteFail(I2CDevice& d) { d.drops++; d.droppedAt = millis(); d.lastProbe = d.droppedAt; // first re-probe one interval from now - logPrintf("[I2C] %s (0x%02X) stopped answering: %u consecutive failures, %d of the last 16 checks; dropped, re-probing every %d s\n", - d.name, d.addr, (unsigned)d.fails, __builtin_popcount(d.recent), I2C_REPROBE_MS / 1000); + // Flapping: dropped again soon after it came back, so wait longer each time. + if (d.lastReturn && d.droppedAt - d.lastReturn < I2C_FLAP_MS) { + if (d.backoff < I2C_BACKOFF_MAX) d.backoff++; + } else { + d.backoff = 0; + } + logPrintf("[I2C] %s (0x%02X) stopped answering: %u consecutive failures, %d of the last 16 checks; dropped, re-probing in %lu s\n", + d.name, d.addr, (unsigned)d.fails, __builtin_popcount(d.recent), + (unsigned long)(I2C_REPROBE_MS << d.backoff) / 1000); return true; } @@ -94,7 +101,7 @@ void i2cBusRecover() { bool i2cReprobeDue(I2CDevice& d) { if (*d.available || d.drops == 0) return false; unsigned long now = millis(); - if (now - d.lastProbe < I2C_REPROBE_MS) return false; + if (now - d.lastProbe < (unsigned long)(I2C_REPROBE_MS << d.backoff)) return false; d.lastProbe = now; if (!checkAt(d.addr, d)) { i2cBusRecover(); @@ -108,6 +115,7 @@ bool i2cReprobeDue(I2CDevice& d) { } logPrintf("[I2C] %s (0x%02X) answers again after %lu s (drop #%lu, %lu failed checks so far); re-initialising\n", d.name, d.addr, (now - d.droppedAt) / 1000, (unsigned long)d.drops, (unsigned long)d.totalFails); + d.lastReturn = now; // a drop within I2C_FLAP_MS of this raises the back-off return true; // the caller's init clears the history via i2cNoteReady() if it succeeds } diff --git a/src/i2c_health.h b/src/i2c_health.h index 8f3d3a3..42bdf8b 100644 --- a/src/i2c_health.h +++ b/src/i2c_health.h @@ -35,7 +35,13 @@ #define I2C_FAIL_WINDOW 8 // ...or this many of the last 16 checks: an intermittent // chip (the bench LIS3MDL answered ~1 read in 100) must // not keep resetting the count with the odd success -#define I2C_REPROBE_MS 10000 // how often a dropped device is probed again +#define I2C_REPROBE_MS 10000 // how often a dropped device is probed again... +#define I2C_FLAP_MS 30000 // ...doubling each time it drops again within this long of a +#define I2C_BACKOFF_MAX 5 // return, up to I2C_REPROBE_MS << 5 = 320 s. The bench + // LIS3MDL with its floating address pad came back and + // dropped again every 11 s, a re-init and a burst of failed + // reads each time (2026-09-08 20:44). The first good spell + // of I2C_FLAP_MS resets it. #define I2C_NO_REG 0xFF // idReg value for a chip without an ID register struct I2CDevice { @@ -49,8 +55,10 @@ struct I2CDevice { uint16_t recent = 0; // last 16 results, 1 = failed, newest in bit 0 uint32_t totalFails = 0; uint32_t drops = 0; + uint8_t backoff = 0; // re-probe interval is I2C_REPROBE_MS << backoff unsigned long droppedAt = 0; unsigned long lastProbe = 0; + unsigned long lastReturn = 0; // when it last answered a re-probe; 0 = never dropped }; void i2cNoteOk(I2CDevice& d); From 147354af4170fbe0aa0610e892900a79053ee2a5 Mon Sep 17 00:00:00 2001 From: Strycher Date: Tue, 8 Sep 2026 21:32:32 -0400 Subject: [PATCH 10/10] fix(#289): back-off decays after 30 s of good checks Fourth review pass: the back-off ladder could only be cleared by the timing of the next drop, so a device with glitches spaced just under 30 s apart climbed to the 320 s interval with 29 s of perfect operation between them. i2cNoteOk() now clears the ladder once the device has answered for I2C_FLAP_MS since it came back. Both envs rebuilt from PowerShell, sizes unchanged. Co-Authored-By: Claude Fable 5.1 --- src/i2c_health.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/i2c_health.cpp b/src/i2c_health.cpp index 872bbcd..84fba41 100644 --- a/src/i2c_health.cpp +++ b/src/i2c_health.cpp @@ -7,6 +7,11 @@ void i2cNoteOk(I2CDevice& d) { d.fails = 0; d.recent <<= 1; + // Probation ends with good behaviour, not only at the next drop: a device + // that has answered for I2C_FLAP_MS since it came back is off the back-off + // ladder (review finding on #289: otherwise only the timing of the next + // failure could clear it). + if (d.backoff && millis() - d.lastReturn >= I2C_FLAP_MS) d.backoff = 0; } void i2cNoteReady(I2CDevice& d) {