From 25dba97c068cb81a0823ed5c55bc9a404329a5c8 Mon Sep 17 00:00:00 2001 From: Oliver Walter Date: Tue, 28 Jul 2026 20:35:59 +0200 Subject: [PATCH] Stop miscounting SHT31 bring-up and status-read failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two ways the error counters lied about sensor health. The first measurement failed on every boot, leaving a permanent i2c_errors=1 that a master polling READ_STATUS could not tell apart from a real fault. The sensor NACKs a command arriving while it is still busy with the previous one, and the bring-up sequence issued a measurement immediately after clear_status with no quiet time. Commands are now spaced by sht3x::COMMAND_SETTLE_MS. Bring-up failures stay uncounted on purpose: the counters describe operational health, and an unreadable serial already surfaces as a zero in READ_INFO. read_status() failures were discarded via `if let Ok`, which made a persistently unreadable status register indistinguishable from a genuinely clean one — both leave sensor_status at zero. They are now counted, but kept out of the consecutive-failure tally so only the measurement path drives the fault and soft-reset logic. Confirmed on hardware: a fresh boot now holds err[i2c=0 sht_crc=0 frame=0 bus_crc=0] indefinitely, where it previously showed i2c=1 from the first poll onwards. After a full bus test run the counters read exactly frame=2 bus_crc=2, accounting for the four faults the suite injects deliberately and nothing else. Co-Authored-By: Claude Opus 5 (1M context) --- core/src/sht3x.rs | 9 ++++++++ firmware/src/main.rs | 49 ++++++++++++++++++++++++++++++++++++-------- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/core/src/sht3x.rs b/core/src/sht3x.rs index 50e3114..4f4c931 100644 --- a/core/src/sht3x.rs +++ b/core/src/sht3x.rs @@ -77,6 +77,15 @@ pub const SOFT_RESET_MAX_MS: u32 = 2; /// Worst-case power-up time (data sheet: 1.5 ms). pub const POWER_UP_MAX_MS: u32 = 2; +/// Quiet time to leave between two consecutive commands. +/// +/// The data sheet gives durations for individual commands but no general +/// inter-command gap, and the sensor NACKs a command that arrives while it is +/// still busy with the previous one. Sending a measurement immediately after a +/// status-register write is enough to trigger it. 5 ms is far more than any +/// non-measurement command needs and only ever costs us during bring-up. +pub const COMMAND_SETTLE_MS: u32 = 5; + /// Split a 16-bit command into the wire byte order (MSB first). pub const fn command_bytes(cmd: u16) -> [u8; 2] { cmd.to_be_bytes() diff --git a/firmware/src/main.rs b/firmware/src/main.rs index 225d6c2..4cb6527 100644 --- a/firmware/src/main.rs +++ b/firmware/src/main.rs @@ -488,6 +488,17 @@ mod app { Mono::delay(board::Duration::millis(u64::from(sht3x::POWER_UP_MAX_MS))).await; + // Bring-up. Every command is followed by quiet time, because the sensor + // NACKs anything arriving while it is still busy with the previous one. + // Without the settle after `clear_status` the first measurement below + // failed on every boot, leaving a permanent i2c_errors=1 that a master + // polling READ_STATUS could not distinguish from a real fault. + // + // Failures here are logged but deliberately not counted: the counters + // describe operational health, and an unreadable serial already shows up + // as a zero in READ_INFO. + let settle = board::Duration::millis(u64::from(sht3x::COMMAND_SETTLE_MS)); + // Clear whatever state a warm reset left behind, so the sensor's // configuration is known regardless of how we got here. let _ = sensor.soft_reset(); @@ -500,7 +511,10 @@ mod app { } Err(e) => defmt::warn!("SHT31 serial read failed: {}", e), } + Mono::delay(settle).await; + let _ = sensor.clear_status(); + Mono::delay(settle).await; let period = board::Duration::millis(u64::from(board::SENSOR_PERIOD_MS)); let mut next = Mono::now(); @@ -561,15 +575,32 @@ mod app { // Read the status register so an unexpected sensor reset shows up in // READ_STATUS instead of silently reverting the configuration. - if let Ok(status) = sensor.read_status() { - let saw_reset = status & sht3x::status::SYSTEM_RESET_DETECTED != 0; - cx.shared.state.lock(|s| { - s.sensor_status = status; - s.sensor_reset_seen |= saw_reset; - }); - if saw_reset { - defmt::warn!("SHT31 reported a reset (status {=u16:#06x})", status); - let _ = sensor.clear_status(); + // + // Failures are counted rather than discarded. Swallowing them made a + // persistently unreadable status indistinguishable from a genuinely + // clean one, since both leave `sensor_status` at zero. They are kept + // out of the consecutive-failure count so only the measurement path + // drives the fault and reset logic. + match sensor.read_status() { + Ok(status) => { + let saw_reset = status & sht3x::status::SYSTEM_RESET_DETECTED != 0; + cx.shared.state.lock(|s| { + s.sensor_status = status; + s.sensor_reset_seen |= saw_reset; + }); + if saw_reset { + defmt::warn!("SHT31 reported a reset (status {=u16:#06x})", status); + let _ = sensor.clear_status(); + } + } + Err(e) => { + defmt::warn!("SHT31 status read failed: {}", e); + cx.shared.state.lock(|s| match e { + crate::sensor::Error::I2c => s.i2c_errors = s.i2c_errors.saturating_add(1), + crate::sensor::Error::Crc => { + s.sensor_crc_errors = s.sensor_crc_errors.saturating_add(1) + } + }); } }