I2C reclock guard - avoid gazillion calls to reclock on SENXX sensors (#11412)

* Add SEN6X

* Adds new SENXX class for SEN5X and SEN6X
* Adds CO2 sensor calibration class to be shared among othre CO2 sensors

* Make existing CO2 sensor draw from CO2Sensor class

* Minor coment for CO2 sensor class

* Move away from getRTC in SENXX class to keep track of time changes.

* Change all sensors to millis for tracking time, instead of using getRTC

* Add comments regarding VOC state

* Avoid storing non-valid RTC

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Avoid CO2 sensor warm-up time to be below PM measured started

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Fix limits in CO2 sensor calibration

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Add pragma once on headers

* Avoid non-working ASC commands

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Fix data poll

* Move pm measure started before warmup check

* Make cleaning non-blocking

* Restore previous state if cleaning fails. Fix data ready condition.

* Fix CO2 sensor checks for calibration

* Add ReClockI2C guard to simplify calls to Reclock. Make SENXX calls to reclock outside of readBuffer, to avoid bizillion calls

* Add new reClockGuard to all sensor classes that require it

* Make clock guard store values on each construction and restore them directly

* Reduce log messages

* Update ADS1X15 to new guard

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

src/detect/ReClockI2C.h

@@ -13,7 +13,8 @@
     https://github.com/sandeepmistry/arduino-nRF5/blob/master/libraries/Wire/Wire.h#L50
     https://github.com/earlephilhower/arduino-pico/blob/master/libraries/Wire/src/Wire.h#L60
     https://github.com/stm32duino/Arduino_Core_STM32/blob/main/libraries/Wire/src/Wire.h#L103
-    For cases when I2C speed is different to the ones defined by sensors (see defines in sensor classes)
+    For cases when I2C speed is different to the ones defined by sensors
+    (see defines in sensor classes)
     we need to reclock I2C and set it back to the previous established speed.
     Only for cases where we can know it (ESP32 or known screen) we can do this.
 */
@@ -27,10 +28,16 @@ class ReClockI2C
     {
         this->i2cBus = i2cBus;
         this->port = port;
-        this->previousClock = 0;
     }
 
-    bool setClock(uint32_t desiredClock)
+    // Sets the I2C clock to desiredClock and returns whatever clock was active
+    // beforehand, so the caller can hand it back to restoreClock() later. The
+    // previous clock is returned rather than stored on this object, so callers
+    // that nest calls (see ReClockI2CGuard) each keep their own restoration
+    // value instead of clobbering a single shared one.
+    // Returns 0 if the clock was already at desiredClock, or if the previous
+    // clock couldn't be determined - in both cases there's nothing to restore.
+    uint32_t setClock(uint32_t desiredClock)
     {
         uint32_t currentClock = this->getClock();
 
@@ -41,36 +48,27 @@ class ReClockI2C
         if (currentClock != desiredClock) {
             LOG_TRACE("Changing I2C clock to %uHz", desiredClock);
             this->i2cBus->setClock(desiredClock);
-            // If the clock is 0Hz, we still store it
-            // We'll check in restoreClock function
-            setPreviousClock(currentClock);
-            LOG_TRACE("Stored previous clock I2C clock: %uHz", this->previousClock);
-            return true;
+            LOG_TRACE("Previous I2C clock: %uHz", currentClock);
+            return currentClock;
         }
 
         LOG_TRACE("I2C clock was already %uHz. Skipping", desiredClock);
-        setPreviousClock(0);
-        return false;
+        return 0;
     }
 
-    bool restoreClock()
+    void restoreClock(uint32_t previousClock)
     {
-        if (this->previousClock) {
-            LOG_TRACE("Restoring I2C clock to %uHz", this->previousClock);
-            i2cBus->setClock(this->previousClock);
-            setPreviousClock(0);
-            return true;
+        if (previousClock) {
+            LOG_TRACE("Restoring I2C clock to %uHz", previousClock);
+            i2cBus->setClock(previousClock);
+            return;
         }
         LOG_TRACE("I2C clock was unknown. Not restored");
-        return false;
     }
 
   private:
     TwoWire *i2cBus{};
     ScanI2C::I2CPort port{};
-    uint32_t previousClock = 0;
-
-    void setPreviousClock(uint32_t clock) { this->previousClock = clock; }
 
     uint32_t getClock()
     {
@@ -95,4 +93,23 @@ class ReClockI2C
     }
 };
 
+/* Helper for ReClockI2C: sets the clock on construction and restores it on
+   destruction, so a caller with multiple early-return paths doesn't need to
+   remember to call restoreClock() on each one.
+ */
+class ReClockI2CGuard
+{
+  public:
+    ReClockI2CGuard(ReClockI2C &reClock, uint32_t desiredClock) : reClock(reClock), previousClock(reClock.setClock(desiredClock))
+    {
+    }
+    ~ReClockI2CGuard() { reClock.restoreClock(previousClock); }
+    ReClockI2CGuard(const ReClockI2CGuard &) = delete;
+    ReClockI2CGuard &operator=(const ReClockI2CGuard &) = delete;
+
+  private:
+    ReClockI2C &reClock;
+    uint32_t previousClock;
+};
+
 #endif

src/modules/Telemetry/Sensor/ADS1X15Sensor.cpp

@@ -14,21 +14,18 @@ bool ADS1X15Sensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
     LOG_INFO("Init sensor: %s (address: 0x%x)", sensorName, dev->address.address);
 
     _bus = bus;
-    _port = dev->address.port;
+
     _address = dev->address.address;
     _deviceType = dev->type;
 
 #ifdef ADS1X15_I2C_CLOCK_SPEED
+    _port = dev->address.port;
     reClockI2C.setup(_bus, _port);
-    reClockI2C.setClock(ADS1X15_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, ADS1X15_I2C_CLOCK_SPEED);
 #endif /* ADS1X15_I2C_CLOCK_SPEED */
 
     status = ads1x15.begin(_address, _bus);
 
-#ifdef ADS1X15_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* ADS1X15_I2C_CLOCK_SPEED */
-
     initI2CSensor();
 
     return status;
@@ -104,15 +101,11 @@ bool ADS1X15Sensor::getMetrics(meshtastic_Telemetry *measurement)
 {
     // Done here and not in getMeasurements to avoid the back-and-forth 4-8 times one after the other
 #ifdef ADS1X15_I2C_CLOCK_SPEED
-    reClockI2C.setClock(ADS1X15_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, ADS1X15_I2C_CLOCK_SPEED);
 #endif /* ADS1X15_I2C_CLOCK_SPEED */
 
     struct _ADS1X15Measurements m = getMeasurements();
 
-#ifdef ADS1X15_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* ADS1X15_I2C_CLOCK_SPEED */
-
     switch (_deviceType) {
     case ScanI2C::DeviceType::ADS1X15: {
         measurement->variant.environment_metrics.has_adc_voltage_ch0 = true;

src/modules/Telemetry/Sensor/DS248XSensor.cpp

@@ -63,13 +63,11 @@ bool DS248XSensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
 
 #ifdef DS248X_I2C_CLOCK_SPEED
     reClockI2C.setup(_bus, _port);
-    reClockI2C.setClock(DS248X_I2C_CLOCK_SPEED);
+    LOG_INFO("%s: reclock speed %uHz", sensorName, DS248X_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, DS248X_I2C_CLOCK_SPEED);
 #endif /* DS248X_I2C_CLOCK_SPEED */
 
     if (!ds248x.begin(bus, _address)) {
-#ifdef DS248X_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* DS248X_I2C_CLOCK_SPEED */
         return false;
     }
 
@@ -151,9 +149,6 @@ bool DS248XSensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
         }
 
         if (initError && retry == numRetries) {
-#ifdef DS248X_I2C_CLOCK_SPEED
-            reClockI2C.restoreClock();
-#endif /* DS248X_I2C_CLOCK_SPEED */
             LOG_ERROR("%s: Max retries for one-wire init (%u/%u). Aborting", sensorName, retry, numRetries);
             return false;
         }
@@ -173,10 +168,6 @@ bool DS248XSensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
         delay(500);
     }
 
-#ifdef DS248X_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* DS248X_I2C_CLOCK_SPEED */
-
     initI2CSensor();
     return status;
 }
@@ -190,7 +181,8 @@ bool DS248XSensor::isValidROM(const uint8_t *rom)
 float DS248XSensor::readTemperatureROM(const uint8_t *rom)
 {
 #ifdef DS248X_I2C_CLOCK_SPEED
-    reClockI2C.setClock(DS248X_I2C_CLOCK_SPEED);
+    LOG_DEBUG("%s: reclock speed %uHz", sensorName, DS248X_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, DS248X_I2C_CLOCK_SPEED);
 #endif /* DS248X_I2C_CLOCK_SPEED */
 
     uint8_t data[9]{};
@@ -219,10 +211,6 @@ float DS248XSensor::readTemperatureROM(const uint8_t *rom)
         }
     }
 
-#ifdef DS248X_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* DS248X_I2C_CLOCK_SPEED */
-
     if (!ok) {
         LOG_WARN("%s: One-wire transaction failed", sensorName);
         return DS248X_INVALID_TEMPERATURE;

src/modules/Telemetry/Sensor/HM330XSensor.cpp

@@ -17,21 +17,16 @@ bool HM330XSensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
 #ifdef HM330X_I2C_CLOCK_SPEED
     _port = dev->address.port;
     reClockI2C.setup(_bus, _port);
-    reClockI2C.setClock(HM330X_I2C_CLOCK_SPEED);
+
+    LOG_INFO("%s: reclock speed %uHz", sensorName, HM330X_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, HM330X_I2C_CLOCK_SPEED);
 #endif /* HM330X_I2C_CLOCK_SPEED */
 
     if (hm330x.init(_bus) != HM330XErrorCode::NO_ERROR) {
-#ifdef HM330X_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* HM330X_I2C_CLOCK_SPEED */
         LOG_WARN("%s error in sensor init", sensorName);
         return false;
     }
 
-#ifdef HM330X_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* HM330X_I2C_CLOCK_SPEED */
-
     status = 1;
     LOG_INFO("%s Enabled", sensorName);
 
@@ -77,21 +72,15 @@ int32_t HM330XSensor::pendingForReadyMs()
 bool HM330XSensor::getMetrics(meshtastic_Telemetry *measurement)
 {
 #ifdef HM330X_I2C_CLOCK_SPEED
-    reClockI2C.setClock(HM330X_I2C_CLOCK_SPEED);
+    LOG_DEBUG("%s: reclock speed %uHz", sensorName, HM330X_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, HM330X_I2C_CLOCK_SPEED);
 #endif /* HM330X_I2C_CLOCK_SPEED */
 
     if (hm330x.read_sensor_value(buffer, 29)) {
         LOG_WARN("%s: read result failed", sensorName);
-#ifdef HM330X_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* HM330X_I2C_CLOCK_SPEED */
         return false;
     }
 
-#ifdef HM330X_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* HM330X_I2C_CLOCK_SPEED */
-
     if (hm330x.checksum_calc(buffer) != HM330XErrorCode::NO_ERROR) {
         LOG_ERROR("%s: Checksum error", sensorName);
         return false;

src/modules/Telemetry/Sensor/PMSA003ISensor.cpp

@@ -24,23 +24,18 @@ bool PMSA003ISensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
 #ifdef PMSA003I_I2C_CLOCK_SPEED
     _port = dev->address.port;
     reClockI2C.setup(_bus, _port);
-    reClockI2C.setClock(PMSA003I_I2C_CLOCK_SPEED);
+
+    LOG_INFO("%s: reclock speed %uHz", sensorName, PMSA003I_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, PMSA003I_I2C_CLOCK_SPEED);
 #endif /* PMSA003I_I2C_CLOCK_SPEED */
 
     _bus->beginTransmission(_address);
     if (_bus->endTransmission() != 0) {
         LOG_WARN("%s not found on I2C at 0x12", sensorName);
-#ifdef PMSA003I_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* PMSA003I_I2C_CLOCK_SPEED */
         sleep();
         return false;
     }
 
-#ifdef PMSA003I_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* PMSA003I_I2C_CLOCK_SPEED */
-
     status = 1;
     LOG_INFO("%s: Enabled", sensorName);
     sleep();
@@ -57,15 +52,13 @@ bool PMSA003ISensor::getMetrics(meshtastic_Telemetry *measurement)
     }
 
 #ifdef PMSA003I_I2C_CLOCK_SPEED
-    reClockI2C.setClock(PMSA003I_I2C_CLOCK_SPEED);
+    LOG_DEBUG("%s: reclock speed %uHz", sensorName, PMSA003I_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, PMSA003I_I2C_CLOCK_SPEED);
 #endif /* PMSA003I_I2C_CLOCK_SPEED */
 
     _bus->requestFrom(_address, (uint8_t)PMSA003I_FRAME_LENGTH);
     if (_bus->available() < PMSA003I_FRAME_LENGTH) {
         LOG_WARN("%s: read failed: incomplete data (%d bytes)", sensorName, _bus->available());
-#ifdef PMSA003I_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* PMSA003I_I2C_CLOCK_SPEED */
         return false;
     }
 
@@ -73,10 +66,6 @@ bool PMSA003ISensor::getMetrics(meshtastic_Telemetry *measurement)
         buffer[i] = _bus->read();
     }
 
-#ifdef PMSA003I_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* PMSA003I_I2C_CLOCK_SPEED */
-
     if (buffer[0] != 0x42 || buffer[1] != 0x4D) {
         LOG_WARN("%s: frame header invalid: 0x%02X 0x%02X", sensorName, buffer[0], buffer[1]);
         return false;

src/modules/Telemetry/Sensor/SCD30Sensor.cpp

@@ -18,16 +18,15 @@ bool SCD30Sensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
 #ifdef SCD30_I2C_CLOCK_SPEED
     _port = dev->address.port;
     reClockI2C.setup(_bus, _port);
-    reClockI2C.setClock(SCD30_I2C_CLOCK_SPEED);
+
+    LOG_INFO("%s: reclock speed %uHz", sensorName, SCD30_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SCD30_I2C_CLOCK_SPEED);
 #endif /* SCD30_I2C_CLOCK_SPEED */
 
     scd30.begin(*_bus, _address);
 
     if (!startMeasurement()) {
-        LOG_ERROR("%s: Periodic measurement start failed", sensorName);
-#ifdef SCD30_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* SCD30_I2C_CLOCK_SPEED */
+        LOG_ERROR("%s: Failed to start periodic measurement", sensorName);
         return false;
     }
 
@@ -35,10 +34,6 @@ bool SCD30Sensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
         LOG_WARN("%s: Can't determine ASC state", sensorName);
     }
 
-#ifdef SCD30_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SCD30_I2C_CLOCK_SPEED */
-
     if (state == SCD30_MEASUREMENT) {
         status = 1;
     } else {
@@ -55,21 +50,15 @@ bool SCD30Sensor::getMetrics(meshtastic_Telemetry *measurement)
     float co2, temperature, humidity;
 
 #ifdef SCD30_I2C_CLOCK_SPEED
-    reClockI2C.setClock(SCD30_I2C_CLOCK_SPEED);
+    LOG_DEBUG("%s: reclock speed %uHz", sensorName, SCD30_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SCD30_I2C_CLOCK_SPEED);
 #endif /* SCD30_I2C_CLOCK_SPEED */
 
     if (scd30.readMeasurementData(co2, temperature, humidity) != SCD30_NO_ERROR) {
-        LOG_ERROR("%s: Measurement read failed", sensorName);
-#ifdef SCD30_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* SCD30_I2C_CLOCK_SPEED */
+        LOG_ERROR("%s: Failed to read measurement data", sensorName);
         return false;
     }
 
-#ifdef SCD30_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SCD30_I2C_CLOCK_SPEED */
-
     if (co2 == 0) {
         LOG_ERROR("%s: Invalid CO₂ reading", sensorName);
         return false;
@@ -359,15 +348,12 @@ bool SCD30Sensor::isActive()
 uint32_t SCD30Sensor::wakeUp()
 {
 #ifdef SCD30_I2C_CLOCK_SPEED
-    reClockI2C.setClock(SCD30_I2C_CLOCK_SPEED);
+    LOG_INFO("%s: reclock speed %uHz", sensorName, SCD30_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SCD30_I2C_CLOCK_SPEED);
 #endif /* SCD30_I2C_CLOCK_SPEED */
 
     startMeasurement();
 
-#ifdef SCD30_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SCD30_I2C_CLOCK_SPEED */
-
     return 0;
 }
 
@@ -378,14 +364,11 @@ uint32_t SCD30Sensor::wakeUp()
 void SCD30Sensor::sleep()
 {
 #ifdef SCD30_I2C_CLOCK_SPEED
-    reClockI2C.setClock(SCD30_I2C_CLOCK_SPEED);
+    LOG_INFO("%s: reclock speed %uHz", sensorName, SCD30_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SCD30_I2C_CLOCK_SPEED);
 #endif /* SCD30_I2C_CLOCK_SPEED */
 
     stopMeasurement();
-
-#ifdef SCD30_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SCD30_I2C_CLOCK_SPEED */
 }
 
 bool SCD30Sensor::canSleep()
@@ -409,7 +392,8 @@ AdminMessageHandleResult SCD30Sensor::handleAdminMessage(const meshtastic_MeshPa
     AdminMessageHandleResult result;
 
 #ifdef SCD30_I2C_CLOCK_SPEED
-    reClockI2C.setClock(SCD30_I2C_CLOCK_SPEED);
+    LOG_INFO("%s: reclock speed %uHz", sensorName, SCD30_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SCD30_I2C_CLOCK_SPEED);
 #endif /* SCD30_I2C_CLOCK_SPEED */
 
     switch (request->which_payload_variant) {
@@ -462,10 +446,6 @@ AdminMessageHandleResult SCD30Sensor::handleAdminMessage(const meshtastic_MeshPa
         result = AdminMessageHandleResult::NOT_HANDLED;
     }
 
-#ifdef SCD30_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SCD30_I2C_CLOCK_SPEED */
-
     return result;
 }
 

src/modules/Telemetry/Sensor/SCD4XSensor.cpp

@@ -19,7 +19,8 @@ bool SCD4XSensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
 #ifdef SCD4X_I2C_CLOCK_SPEED
     _port = dev->address.port;
     reClockI2C.setup(_bus, _port);
-    reClockI2C.setClock(SCD4X_I2C_CLOCK_SPEED);
+    LOG_INFO("%s: reclock speed %uHz", sensorName, SCD4X_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SCD4X_I2C_CLOCK_SPEED);
 #endif /* SCD4X_I2C_CLOCK_SPEED */
 
     scd4x.begin(*_bus, _address);
@@ -29,9 +30,6 @@ bool SCD4XSensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
 
     // Stop periodic measurement
     if (!stopMeasurement()) {
-#ifdef SCD4X_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
         return false;
     }
 
@@ -41,35 +39,22 @@ bool SCD4XSensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
     if (sensorVariant == SCD4X_SENSOR_VARIANT_SCD41) {
         LOG_INFO("%s: Found SCD41", sensorName);
         if (!powerUp()) {
-            LOG_ERROR("%s: powerUp() failed", sensorName);
-#ifdef SCD4X_I2C_CLOCK_SPEED
-            reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
+            LOG_ERROR("%s: Error trying to execute powerUp()", sensorName);
             return false;
         }
     }
 
     if (!getASC(ascActive)) {
-        LOG_ERROR("%s: Can't check if ASC enabled", sensorName);
-#ifdef SCD4X_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
+        LOG_ERROR("%s: Unable to check if ASC is enabled", sensorName);
         return false;
     }
 
     // Start measurement in selected power mode (low power by default)
     if (!startMeasurement()) {
-        LOG_ERROR("%s: Can't start measurement", sensorName);
-#ifdef SCD4X_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
+        LOG_ERROR("%s: Couldn't start measurement", sensorName);
         return false;
     }
 
-#ifdef SCD4X_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
-
     if (state == SCD4X_MEASUREMENT) {
         status = 1;
     } else {
@@ -93,7 +78,8 @@ bool SCD4XSensor::getMetrics(meshtastic_Telemetry *measurement)
     float temperature, humidity;
 
 #ifdef SCD4X_I2C_CLOCK_SPEED
-    reClockI2C.setClock(SCD4X_I2C_CLOCK_SPEED);
+    LOG_DEBUG("%s: reclock speed %uHz", sensorName, SCD4X_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SCD4X_I2C_CLOCK_SPEED);
 #endif /* SCD4X_I2C_CLOCK_SPEED */
 
     bool dataReady = false;
@@ -109,19 +95,12 @@ bool SCD4XSensor::getMetrics(meshtastic_Telemetry *measurement)
     }
 
     if (error != SCD4X_NO_ERROR || !dataReady) {
-#ifdef SCD4X_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
         LOG_ERROR("SCD4X: Data is not ready");
         return false;
     }
 
     error = scd4x.readMeasurement(co2, temperature, humidity);
 
-#ifdef SCD4X_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
-
     LOG_DEBUG("Got %s readings: co2=%u, co2_temp=%.2f, co2_hum%.2f", sensorName, co2, temperature, humidity);
     if (error != SCD4X_NO_ERROR) {
         LOG_DEBUG("%s: Error getting measurements: %u", sensorName, error);
@@ -634,28 +613,19 @@ bool SCD4XSensor::powerDown()
     }
 
 #ifdef SCD4X_I2C_CLOCK_SPEED
-    reClockI2C.setClock(SCD4X_I2C_CLOCK_SPEED);
+    LOG_INFO("%s: reclock speed %uHz", sensorName, SCD4X_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SCD4X_I2C_CLOCK_SPEED);
 #endif /* SCD4X_I2C_CLOCK_SPEED */
 
     if (!stopMeasurement()) {
-#ifdef SCD4X_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
         return false;
     }
 
     if (scd4x.powerDown() != SCD4X_NO_ERROR) {
-        LOG_ERROR("%s: sleep() failed", sensorName);
-#ifdef SCD4X_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
+        LOG_ERROR("%s: Error trying to execute sleep()", sensorName);
         return false;
     }
 
-#ifdef SCD4X_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
-
     state = SCD4X_OFF;
     return true;
 }
@@ -701,21 +671,15 @@ uint32_t SCD4XSensor::wakeUp()
 {
 
 #ifdef SCD4X_I2C_CLOCK_SPEED
-    reClockI2C.setClock(SCD4X_I2C_CLOCK_SPEED);
+    LOG_INFO("%s: reclock speed %uHz", sensorName, SCD4X_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SCD4X_I2C_CLOCK_SPEED);
 #endif /* SCD4X_I2C_CLOCK_SPEED */
 
     if (startMeasurement()) {
         co2MeasureStarted = millis();
-#ifdef SCD4X_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
         return SCD4X_WARMUP_MS;
     }
 
-#ifdef SCD4X_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
-
     return 0;
 }
 
@@ -726,14 +690,11 @@ uint32_t SCD4XSensor::wakeUp()
 void SCD4XSensor::sleep()
 {
 #ifdef SCD4X_I2C_CLOCK_SPEED
-    reClockI2C.setClock(SCD4X_I2C_CLOCK_SPEED);
+    LOG_INFO("%s: reclock speed %uHz", sensorName, SCD4X_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SCD4X_I2C_CLOCK_SPEED);
 #endif /* SCD4X_I2C_CLOCK_SPEED */
 
     stopMeasurement();
-
-#ifdef SCD4X_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
 }
 
 /**
@@ -771,7 +732,8 @@ AdminMessageHandleResult SCD4XSensor::handleAdminMessage(const meshtastic_MeshPa
     AdminMessageHandleResult result;
 
 #ifdef SCD4X_I2C_CLOCK_SPEED
-    reClockI2C.setClock(SCD4X_I2C_CLOCK_SPEED);
+    LOG_INFO("%s: reclock speed %uHz", sensorName, SCD4X_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SCD4X_I2C_CLOCK_SPEED);
 #endif /* SCD4X_I2C_CLOCK_SPEED */
 
     // TODO: potentially add selftest command?
@@ -837,10 +799,6 @@ AdminMessageHandleResult SCD4XSensor::handleAdminMessage(const meshtastic_MeshPa
     // Start measurement mode
     this->startMeasurement();
 
-#ifdef SCD4X_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SCD4X_I2C_CLOCK_SPEED */
-
     return result;
 }
 

src/modules/Telemetry/Sensor/SENXXSensor.cpp

@@ -174,6 +174,7 @@ bool SENXXSensor::probe(TwoWire *bus, uint8_t address, ScanI2C::I2CPort port)
 #ifdef SENXX_I2C_CLOCK_SPEED
     _port = port;
     reClockI2C.setup(_bus, _port);
+    ReClockI2CGuard clockGuard(reClockI2C, SENXX_I2C_CLOCK_SPEED);
 #endif /* SENXX_I2C_CLOCK_SPEED */
 
     if (!findModel()) {
@@ -215,23 +216,12 @@ bool SENXXSensor::sendCommand(uint16_t command, uint8_t *buffer, uint8_t byteNum
         }
     }
 
-#ifdef SENXX_I2C_CLOCK_SPEED
-    LOG_DEBUG("%s: Attempting to reclock speed to %uHz", sensorName, SENXX_I2C_CLOCK_SPEED);
-    reClockI2C.setClock(SENXX_I2C_CLOCK_SPEED);
-#endif /* SENXX_I2C_CLOCK_SPEED */
-
-    // Transmit the data
     // Note: this delay is necessary to allow for long-buffers
     delay(20);
     _bus->beginTransmission(_address);
     size_t writtenBytes = _bus->write(toSend, bufferSize);
     uint8_t i2c_error = _bus->endTransmission();
 
-#ifdef SENXX_I2C_CLOCK_SPEED
-    LOG_DEBUG("%s: restoring clock speed", sensorName);
-    reClockI2C.restoreClock();
-#endif /* SENXX_I2C_CLOCK_SPEED */
-
     if (writtenBytes != bufferSize) {
         LOG_ERROR("%s: Error writing on I2C bus", sensorName);
         return false;
@@ -246,18 +236,9 @@ bool SENXXSensor::sendCommand(uint16_t command, uint8_t *buffer, uint8_t byteNum
 
 uint8_t SENXXSensor::readBuffer(uint8_t *buffer, uint8_t byteNumber)
 {
-#ifdef SENXX_I2C_CLOCK_SPEED
-    LOG_DEBUG("%s: Attempting to reclock speed to %uHz", sensorName, SENXX_I2C_CLOCK_SPEED);
-    reClockI2C.setClock(SENXX_I2C_CLOCK_SPEED);
-#endif /* SENXX_I2C_CLOCK_SPEED */
-
     size_t readBytes = _bus->requestFrom(_address, byteNumber);
     if (readBytes != byteNumber) {
         LOG_ERROR("%s: Error reading I2C bus", sensorName);
-#ifdef SENXX_I2C_CLOCK_SPEED
-        LOG_DEBUG("%s: restoring clock speed", sensorName);
-        reClockI2C.restoreClock();
-#endif /* SENXX_I2C_CLOCK_SPEED */
         return 0;
     }
 
@@ -270,21 +251,12 @@ uint8_t SENXXSensor::readBuffer(uint8_t *buffer, uint8_t byteNumber)
         uint8_t calcCRC = senxxCRC(&buffer[i - 2]);
         if (recvCRC != calcCRC) {
             LOG_ERROR("%s: Checksum error while receiving msg", sensorName);
-#ifdef SENXX_I2C_CLOCK_SPEED
-            LOG_DEBUG("%s: restoring clock speed", sensorName);
-            reClockI2C.restoreClock();
-#endif /* SENXX_I2C_CLOCK_SPEED */
             return 0;
         }
         readBytes -= 3;
         receivedBytes += 2;
     }
 
-#ifdef SENXX_I2C_CLOCK_SPEED
-    LOG_DEBUG("%s: restoring clock speed", sensorName);
-    reClockI2C.restoreClock();
-#endif /* SENXX_I2C_CLOCK_SPEED */
-
     return receivedBytes;
 }
 
@@ -320,6 +292,9 @@ void SENXXSensor::sleep()
         LOG_INFO("%s: Not going to sleep, fan cleaning is in progress", sensorName);
         return;
     }
+#ifdef SENXX_I2C_CLOCK_SPEED
+    ReClockI2CGuard clockGuard(reClockI2C, SENXX_I2C_CLOCK_SPEED);
+#endif /* SENXX_I2C_CLOCK_SPEED */
     idle(true);
 }
 
@@ -690,6 +665,14 @@ void SENXXSensor::reconcileTimeDependentState(uint32_t now)
 }
 
 uint32_t SENXXSensor::wakeUp()
+{
+#ifdef SENXX_I2C_CLOCK_SPEED
+    ReClockI2CGuard clockGuard(reClockI2C, SENXX_I2C_CLOCK_SPEED);
+#endif /* SENXX_I2C_CLOCK_SPEED */
+    return wakeUpInternal();
+}
+
+uint32_t SENXXSensor::wakeUpInternal()
 {
 
     LOG_DEBUG("%s: Waking up sensor", sensorName);
@@ -789,6 +772,7 @@ bool SENXXSensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
 #ifdef SENXX_I2C_CLOCK_SPEED
     _port = dev->address.port;
     reClockI2C.setup(_bus, _port);
+    ReClockI2CGuard clockGuard(reClockI2C, SENXX_I2C_CLOCK_SPEED);
 #endif /* SENXX_I2C_CLOCK_SPEED */
 
     delay(50); // without this there is an error on the deviceReset function
@@ -1141,6 +1125,12 @@ int32_t SENXXSensor::wakeUpTimeMs()
 
 int32_t SENXXSensor::pendingForReadyMs()
 {
+#ifdef SENXX_I2C_CLOCK_SPEED
+    // Only the SENXX_MEASUREMENT/SENXX_CLEANING branches below touch I2C, but this is only
+    // ever called while isActive() (i.e. one of those, or SENXX_MEASUREMENT_2, which doesn't),
+    // so bracketing unconditionally here is simpler than guarding each branch separately.
+    ReClockI2CGuard clockGuard(reClockI2C, SENXX_I2C_CLOCK_SPEED);
+#endif /* SENXX_I2C_CLOCK_SPEED */
     uint32_t now = millis();
     uint32_t sincePmMeasureStarted = now - pmMeasureStarted;
     LOG_DEBUG("%s: Since measure started: %ums", sensorName, sincePmMeasureStarted);
@@ -1203,6 +1193,10 @@ bool SENXXSensor::getMetrics(meshtastic_Telemetry *measurement)
         return false;
     }
 
+#ifdef SENXX_I2C_CLOCK_SPEED
+    ReClockI2CGuard clockGuard(reClockI2C, SENXX_I2C_CLOCK_SPEED);
+#endif /* SENXX_I2C_CLOCK_SPEED */
+
     uint8_t response;
     response = getMeasurements();
 
@@ -1525,6 +1519,9 @@ AdminMessageHandleResult SENXXSensor::handleAdminMessage(const meshtastic_MeshPa
 
     switch (request->which_payload_variant) {
     case meshtastic_AdminMessage_sensor_config_tag: {
+#ifdef SENXX_I2C_CLOCK_SPEED
+        ReClockI2CGuard clockGuard(reClockI2C, SENXX_I2C_CLOCK_SPEED);
+#endif /* SENXX_I2C_CLOCK_SPEED */
         bool ok = true;
         bool wasActive = isActive();
 
@@ -1582,7 +1579,9 @@ AdminMessageHandleResult SENXXSensor::handleAdminMessage(const meshtastic_MeshPa
                 }
 
                 if (wasActive) {
-                    this->wakeUp();
+                    // Not this->wakeUp() - we're already inside this function's own
+                    // ReClockI2CGuard, and that guard isn't reentrant (see its comment).
+                    this->wakeUpInternal();
                 }
             }
         } else {

src/modules/Telemetry/Sensor/SENXXSensor.h

@@ -255,6 +255,11 @@ class SENXXSensor : public TelemetrySensor, public CO2CalibrationSensor
     bool readPNValues(bool cumulative);
     bool readValues();
 
+    // Actual wakeUp() logic, factored out so handleAdminMessage() can resume
+    // measurement after a calibration pause without nesting a second I2C-clock
+    // guard inside its own (see ReClockI2CGuard's reentrancy note).
+    uint32_t wakeUpInternal();
+
     // Monotonic (millis()) timers for warmup/poll intervals. Deliberately not
     // wall-clock (getTime()) based: getTime() can jump discontinuously the moment the RTC
     // quality improves mid-session (see checkRTCQualityImproved()), which would corrupt

src/modules/Telemetry/Sensor/SFA30Sensor.cpp

@@ -17,33 +17,24 @@ bool SFA30Sensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
 #ifdef SFA30_I2C_CLOCK_SPEED
     _port = dev->address.port;
     reClockI2C.setup(_bus, _port);
-    reClockI2C.setClock(SFA30_I2C_CLOCK_SPEED);
+    LOG_INFO("%s: reclock speed %uHz", sensorName, SFA30_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SFA30_I2C_CLOCK_SPEED);
 #endif /* SFA30_I2C_CLOCK_SPEED */
 
     sfa30.begin(*_bus, _address);
     delay(20);
 
     if (this->isError(sfa30.deviceReset())) {
-#ifdef SFA30_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* SFA30_I2C_CLOCK_SPEED */
         return false;
     }
 
     state = State::IDLE;
     if (this->isError(sfa30.startContinuousMeasurement())) {
-#ifdef SFA30_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* SFA30_I2C_CLOCK_SPEED */
         return false;
     }
 
     LOG_INFO("%s starting measurement", sensorName);
 
-#ifdef SFA30_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SFA30_I2C_CLOCK_SPEED */
-
     status = 1;
     state = State::ACTIVE;
     measureStarted = millis();
@@ -66,7 +57,8 @@ bool SFA30Sensor::isError(uint16_t response)
 void SFA30Sensor::sleep()
 {
 #ifdef SFA30_I2C_CLOCK_SPEED
-    reClockI2C.setClock(SFA30_I2C_CLOCK_SPEED);
+    LOG_DEBUG("%s: reclock speed %uHz", sensorName, SFA30_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SFA30_I2C_CLOCK_SPEED);
 #endif /* SFA30_I2C_CLOCK_SPEED */
 
     // Note - not recommended for this sensor on a periodic basis
@@ -74,10 +66,6 @@ void SFA30Sensor::sleep()
         LOG_ERROR("%s: Can't stop measurement", sensorName);
     };
 
-#ifdef SFA30_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SFA30_I2C_CLOCK_SPEED */
-
     LOG_DEBUG("%s: stop measurement", sensorName);
     state = State::IDLE;
     measureStarted = 0;
@@ -86,21 +74,15 @@ void SFA30Sensor::sleep()
 uint32_t SFA30Sensor::wakeUp()
 {
 #ifdef SFA30_I2C_CLOCK_SPEED
-    reClockI2C.setClock(SFA30_I2C_CLOCK_SPEED);
+    LOG_DEBUG("%s: reclock speed %uHz", sensorName, SFA30_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SFA30_I2C_CLOCK_SPEED);
 #endif /* SFA30_I2C_CLOCK_SPEED */
 
     LOG_DEBUG("Waking %s", sensorName);
     if (this->isError(sfa30.startContinuousMeasurement())) {
-#ifdef SFA30_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* SFA30_I2C_CLOCK_SPEED */
         return 0;
     }
 
-#ifdef SFA30_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SFA30_I2C_CLOCK_SPEED */
-
     state = State::ACTIVE;
     measureStarted = millis();
     return SFA30_WARMUP_MS;
@@ -142,21 +124,15 @@ bool SFA30Sensor::getMetrics(meshtastic_Telemetry *measurement)
     float temperature = 0.0;
 
 #ifdef SFA30_I2C_CLOCK_SPEED
-    reClockI2C.setClock(SFA30_I2C_CLOCK_SPEED);
+    LOG_DEBUG("%s: reclock speed %uHz", sensorName, SFA30_I2C_CLOCK_SPEED);
+    ReClockI2CGuard clockGuard(reClockI2C, SFA30_I2C_CLOCK_SPEED);
 #endif /* SFA30_I2C_CLOCK_SPEED */
 
     if (this->isError(sfa30.readMeasuredValues(hcho, humidity, temperature))) {
         LOG_WARN("%s: No values", sensorName);
-#ifdef SFA30_I2C_CLOCK_SPEED
-        reClockI2C.restoreClock();
-#endif /* SFA30_I2C_CLOCK_SPEED */
         return false;
     }
 
-#ifdef SFA30_I2C_CLOCK_SPEED
-    reClockI2C.restoreClock();
-#endif /* SFA30_I2C_CLOCK_SPEED */
-
     measurement->variant.air_quality_metrics.has_form_temperature = true;
     measurement->variant.air_quality_metrics.has_form_humidity = true;
     measurement->variant.air_quality_metrics.has_form_formaldehyde = true;