Stop miscounting SHT31 bring-up and status-read failures
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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()
|
||||
|
||||
+40
-9
@@ -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)
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user