co2_sensor: process only reliable measurements

Log every measurement to the serial console, but
return only true in `processData` (thus allowing
further processing like CSV, MQTT, LORAWAN)
if the data is reliable, i.e.

* CO2 measurements have stabilized after startup
* CO2 != 0
* not preparing calibration (with reduced measurement
timesteps)

ampel-firmware/co2_sensor.cpp

@@ -201,9 +201,10 @@ namespace sensor {
     current_state = new_state;
   }
 
-  void switchStateForCurrentPPM() {
+  bool switchStateForCurrentPPM() {
+  // Return true if the measurement is reliable for further processing
     if (current_state == BOOTUP) {
-      if (!hasSensorSettled()) return;
+      if (!hasSensorSettled()) return false;
       switchState(READY);
       Serial.println(F("Sensor acclimatization finished."));
       Serial.print(F("Setting SCD30 timestep to "));
@@ -212,11 +213,16 @@ namespace sensor {
       scd30.setMeasurementInterval(config::measurement_timestep); // [s]
       switchStateForCurrentPPM(); // Check all other conditions again
     }
+    // These 'else if' constructs are, strictly speaking, unnecessary,
+    // because they are not reached if the previous condition was fulfilled.
+    // However, the order is important here and using 'else if' prevents
+    // them from unintended reordering.
     if (co2 == 0) {
       // NOTE: Data is available, but it's sometimes erroneous: the sensor outputs
       // zero ppm but non-zero temperature and non-zero humidity.
       Serial.println(F("Invalid sensor data - CO2 concentration supposedly 0 ppm"));
       switchState(INVALID);
+      return false;
     } else if ((current_state == PREPARE_CALIBRATION_UNSTABLE) || (current_state == PREPARE_CALIBRATION_STABLE)) {
       // Check for pre-calibration states first, because we do not want to
       // leave them before calibration is done.
@@ -224,11 +230,14 @@ namespace sensor {
       if (ready_for_calibration) {
         calibrateAndRestart();
       }
+      return false;
     } else if (co2 < 250) {
       // Sensor should be calibrated.
       switchState(NEEDS_CALIBRATION);
+      return true;
     } else {
       switchState(READY);
+      return true;
     }
   }
 
@@ -279,6 +288,7 @@ namespace sensor {
    */
   bool processData() {
     bool freshData = scd30.dataAvailable();
+    bool isReliable = false;
 
     if (freshData) {
       // checkTimerDeviation();
@@ -287,7 +297,7 @@ namespace sensor {
       temperature = scd30.getTemperature();
       humidity = scd30.getHumidity();
 
-      switchStateForCurrentPPM();
+      isReliable = switchStateForCurrentPPM();
 
       // Log every time fresh data is available.
       logToSerial();
@@ -295,7 +305,7 @@ namespace sensor {
 
     showState();
 
-    return freshData;
+    return (freshData && isReliable);
   }
 
   /*****************************************************************

[Eric Duminil]

Hmm. The goal is good, but the implementation looks a bit complex IMHO, and is basically what you wanted to avoid by adding States in the first place. I'll think about an alternate way to implement it.

[Käppler]

Do you mean the whole MR is too complex or this "feedback mechanism" for the main loop()? I'm asking because I would not invest time in addressing the comments if you're going to write everything from scratch.

[Eric Duminil]

I was referring to this specific commit. Don't worry, I won't rewrite anything from scratch. Your branch is good enough, and just needs to be polished before it's merged.

What about:

• don't return anything from switchStateForCurrentPPM()
• only send data via MQTT/LoRa/CSV if the sensor is in READY or NEED_CALIBRATION state. I thought about only sending data in READY mode, but you're right, it should be sent in NEED_CALIBRATION state too, as a warning that something should be done.
• So processData would return `freshData && (current_state == READY || current_state == NEEDS_CALIBRATION);
• If it's too long, a new method interestingData() could be defined as current_state == READY || current_state == NEEDS_CALIBRATION.

[Käppler]

Sounds good. 👍🏼