Fixes on SCD4X admin comands (#9607)

* Fixes on SCD4X admin comands

* Minor fix in logs for SEN5X

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>

src/modules/Telemetry/Sensor/SCD4XSensor.cpp

@@ -333,12 +333,6 @@ bool SCD4XSensor::setASC(bool ascEnabled)
         return false;
     }
 
-    if (ascActive) {
-        LOG_INFO("%s: ASC is enabled", sensorName);
-    } else {
-        LOG_INFO("%s: ASC is disabled", sensorName);
-    }
-
     return true;
 }
 
@@ -358,7 +352,7 @@ bool SCD4XSensor::setASC(bool ascEnabled)
 bool SCD4XSensor::setASCBaseline(uint32_t targetCO2)
 {
     // TODO - Remove?
-    //  Available in library, but not described in datasheet.
+    // Available in library, but not described in datasheet.
     uint16_t error;
     LOG_INFO("%s: Setting ASC baseline to: %u", sensorName, targetCO2);
 
@@ -540,6 +534,7 @@ bool SCD4XSensor::setAltitude(uint32_t altitude)
     if (!stopMeasurement()) {
         return false;
     }
+    LOG_INFO("%s: setting altitude at %um", sensorName, altitude);
 
     error = scd4x.setSensorAltitude(altitude);
 
@@ -548,11 +543,15 @@ bool SCD4XSensor::setAltitude(uint32_t altitude)
         return false;
     }
 
-    error = scd4x.persistSettings();
+    // NOTE: this gives an error if issued. Sensirion's library
-    if (error != SCD4X_NO_ERROR) {
+    // doesn't indicate it's needed.
-        LOG_ERROR("%s: Unable to make settings persistent. Error code: %u", sensorName, error);
+    // error = scd4x.persistSettings();
-        return false;
+    // if (error != SCD4X_NO_ERROR) {
-    }
+    //     LOG_ERROR("%s: Unable to make settings persistent. Error code: %u", sensorName, error);
+    //     return false;
+    // }
+
+    LOG_INFO("%s: altitude set", sensorName);
 
     return true;
 }
@@ -575,6 +574,8 @@ bool SCD4XSensor::setAmbientPressure(uint32_t ambientPressure)
 {
     uint16_t error;
 
+    LOG_INFO("%s: setting ambient pressure at %u Pa", sensorName, ambientPressure);
+
     error = scd4x.setAmbientPressure(ambientPressure);
 
     if (error != SCD4X_NO_ERROR) {
@@ -589,6 +590,8 @@ bool SCD4XSensor::setAmbientPressure(uint32_t ambientPressure)
         return false;
     }
 
+    LOG_INFO("%s: ambient pressure set set", sensorName);
+
     return true;
 }
 
@@ -824,15 +827,28 @@ AdminMessageHandleResult SCD4XSensor::handleAdminMessage(const meshtastic_MeshPa
 
         if (request->sensor_config.scd4x_config.has_factory_reset) {
             LOG_DEBUG("%s: Requested factory reset", sensorName);
-            this->factoryReset();
+            if (!this->factoryReset()) {
+                result = AdminMessageHandleResult::NOT_HANDLED;
+                break;
+            }
         } else {
-
             if (request->sensor_config.scd4x_config.has_set_asc) {
-                this->setASC(request->sensor_config.scd4x_config.set_asc);
+                getASC(ascActive);
+                bool currentASC = ascActive;
                 if (request->sensor_config.scd4x_config.set_asc == false) {
                     LOG_DEBUG("%s: Request for FRC", sensorName);
                     if (request->sensor_config.scd4x_config.has_set_target_co2_conc) {
-                        this->performFRC(request->sensor_config.scd4x_config.set_target_co2_conc);
+                        if (this->setASC(request->sensor_config.scd4x_config.set_asc)) {
+                            if (!this->performFRC(request->sensor_config.scd4x_config.set_target_co2_conc)) {
+                                result = AdminMessageHandleResult::NOT_HANDLED;
+                                // Set it back to ASC if failed
+                                setASC(currentASC);
+                                break;
+                            };
+                        } else {
+                            result = AdminMessageHandleResult::NOT_HANDLED;
+                            break;
+                        }
                     } else {
                         // FRC requested but no target CO2 provided
                         LOG_ERROR("%s: target CO2 not provided", sensorName);
@@ -841,12 +857,17 @@ AdminMessageHandleResult SCD4XSensor::handleAdminMessage(const meshtastic_MeshPa
                     }
                 } else {
                     LOG_DEBUG("%s: Request for ASC", sensorName);
-                    if (request->sensor_config.scd4x_config.has_set_target_co2_conc) {
+                    if (this->setASC(request->sensor_config.scd4x_config.set_asc)) {
-                        LOG_DEBUG("%s: Request has target CO2", sensorName);
+                        if (request->sensor_config.scd4x_config.has_set_target_co2_conc) {
-                        // TODO - Remove? see setASCBaseline function
+                            LOG_DEBUG("%s: Request has target CO2", sensorName);
-                        this->setASCBaseline(request->sensor_config.scd4x_config.set_target_co2_conc);
+                            this->setASCBaseline(request->sensor_config.scd4x_config.set_target_co2_conc);
+                            // NOTE - in this situation, if we set ASC, but baseline set fails, we stay on ASC
+                        } else {
+                            LOG_DEBUG("%s: Request doesn't have target CO2", sensorName);
+                        }
                     } else {
-                        LOG_DEBUG("%s: Request doesn't have target CO2", sensorName);
+                        result = AdminMessageHandleResult::NOT_HANDLED;
+                        break;
                     }
                 }
             }
@@ -855,27 +876,36 @@ AdminMessageHandleResult SCD4XSensor::handleAdminMessage(const meshtastic_MeshPa
             // NOTE: this requires to have a sensor working on stable environment
             // And to make it between readings
             if (request->sensor_config.scd4x_config.has_set_temperature) {
-                this->setTemperature(request->sensor_config.scd4x_config.set_temperature);
+                if (!this->setTemperature(request->sensor_config.scd4x_config.set_temperature)) {
+                    result = AdminMessageHandleResult::NOT_HANDLED;
+                    break;
+                }
             }
 
             // Check for altitude or pressure offset
             if (request->sensor_config.scd4x_config.has_set_altitude) {
-                this->setAltitude(request->sensor_config.scd4x_config.set_altitude);
+                if (!this->setAltitude(request->sensor_config.scd4x_config.set_altitude)) {
+                    result = AdminMessageHandleResult::NOT_HANDLED;
+                    break;
+                }
             } else if (request->sensor_config.scd4x_config.has_set_ambient_pressure) {
-                this->setAmbientPressure(request->sensor_config.scd4x_config.set_ambient_pressure);
+                if (!this->setAmbientPressure(request->sensor_config.scd4x_config.set_ambient_pressure)) {
+                    result = AdminMessageHandleResult::NOT_HANDLED;
+                    break;
+                }
             }
 
             // Check for low power mode
             // NOTE: to switch from one mode to another do:
             // setPowerMode -> startMeasurement
             if (request->sensor_config.scd4x_config.has_set_power_mode) {
-                this->setPowerMode(request->sensor_config.scd4x_config.set_power_mode);
+                if (!this->setPowerMode(request->sensor_config.scd4x_config.set_power_mode)) {
+                    result = AdminMessageHandleResult::NOT_HANDLED;
+                    break;
+                }
             }
         }
 
-        // Start measurement mode
-        this->startMeasurement();
-
         result = AdminMessageHandleResult::HANDLED;
         break;
 
@@ -883,6 +913,9 @@ AdminMessageHandleResult SCD4XSensor::handleAdminMessage(const meshtastic_MeshPa
         result = AdminMessageHandleResult::NOT_HANDLED;
     }
 
+    // Start measurement mode
+    this->startMeasurement();
+
 #if defined(SCD4X_I2C_CLOCK_SPEED) && defined(CAN_RECLOCK_I2C)
     reClockI2C(currentClock, _bus, false);
 #endif

src/modules/Telemetry/Sensor/SEN5XSensor.cpp

@@ -919,6 +919,11 @@ bool SEN5XSensor::getMetrics(meshtastic_Telemetry *measurement)
 void SEN5XSensor::setMode(bool setOneShot)
 {
     oneShotMode = setOneShot;
+    if (oneShotMode) {
+        LOG_INFO("%s setting mode to one shot mode", sensorName);
+    } else {
+        LOG_INFO("%s setting mode to continuous mode", sensorName);
+    }
 }
 
 AdminMessageHandleResult SEN5XSensor::handleAdminMessage(const meshtastic_MeshPacket &mp, meshtastic_AdminMessage *request,
@@ -934,16 +939,22 @@ AdminMessageHandleResult SEN5XSensor::handleAdminMessage(const meshtastic_MeshPa
             break;
         }
 
-        // TODO - Add admin command to set temperature offset
+        // Check for one-shot/continuous mode request
+        if (request->sensor_config.sen5x_config.has_set_one_shot_mode) {
+            this->setMode(request->sensor_config.sen5x_config.set_one_shot_mode);
+        }
+
+        // TODO - Add admin command to set temperature offset?
         // Check for temperature offset
         // if (request->sensor_config.sen5x_config.has_set_temperature) {
         //     this->setTemperature(request->sensor_config.sen5x_config.set_temperature);
         // }
 
+        // TODO - Add admin command to trigger fan cleaning?
         // Check for one-shot/continuous mode request
-        if (request->sensor_config.sen5x_config.has_set_one_shot_mode) {
+        // if (request->sensor_config.sen5x_config.has_fan_cleaning && request->sensor_config.sen5x_config.fan_cleaning) {
-            this->setMode(request->sensor_config.sen5x_config.set_one_shot_mode);
+        //     this->startCleaning();
-        }
+        // }
 
         result = AdminMessageHandleResult::HANDLED;
         break;