Merge pull request #13 from Sensirion/leak-fix

Fix memory leak in I2C scanning

src/I2CAutoDetector.cpp

@@ -28,70 +28,117 @@
 #include "SensorWrappers/Svm4x.h"
 #endif
 
-byte I2CAutoDetector::probeAddress(const byte& address) {
+void I2CAutoDetector::findSensors(SensorList& sensorList) {
+    for (byte address = 1; address < 127; address++) {
+        auto st = probeAddress(address);
+        if (st == SensorType::UNDEFINED) {
+            continue;
+        }
+        if (sensorList.containsSensor(st)) {
+            continue;
+        }
+
+        ISensor* pSensor = createSensorFromSensorType(st);
+        if (!pSensor) {
+            continue;
+        }
+        sensorList.addSensor(pSensor);
+    }
+}
+
+/**
+ * Private methods
+ */
+
+SensorType I2CAutoDetector::probeAddress(const byte& address) {
     _wire.beginTransmission(address);
     byte error = _wire.endTransmission();
-    return error;
+    if (error){
+        return SensorType::UNDEFINED;
+    }
+    return getSensorTypeFromAddress(address);
+}
+
+ISensor* I2CAutoDetector::createSensorFromSensorType(SensorType sensor_type) {
+    switch (sensor_type) {
+        case (SensorType::SCD30): {
+            return new Scd30(_wire);
+        }
+        case (SensorType::SCD4X): {
+            return new Scd4x(_wire);
+        }
+
+        case (SensorType::SEN5X): {
+            return new Sen5x(_wire);
+        }
+        case (SensorType::SFA3X): {
+            return new Sfa3x(_wire);
+        }
+        case (SensorType::SGP41): {
+            return new Sgp41(_wire);
+        }
+        case (SensorType::SHT4X): {
+            return new Sht4x(_wire);
+        }
+        case (SensorType::STC3X): {
+            return new Stc3x(_wire);
+        }
+        case (SensorType::SVM4X): {
+            return new Svm4x(_wire);
+        }
+        default: {
+            return nullptr;
+        }
+    }
 }
 
-ISensor* I2CAutoDetector::createSensorFromAddress(const byte& address) {
+
+SensorType I2CAutoDetector::getSensorTypeFromAddress(const byte address) {
     switch (address) {
 #ifdef INCLUDE_SCD30_DRIVER
         case (Scd30::I2C_ADDRESS): {
-            return new Scd30(_wire);
+            return SensorType::SCD30;
         }
 #endif
 #ifdef INCLUDE_SCD4X_DRIVER
         case (Scd4x::I2C_ADDRESS): {
-            return new Scd4x(_wire);
+            return SensorType::SCD4X;
         }
 #endif
 #ifdef INCLUDE_SEN5X_DRIVER
         case (Sen5x::I2C_ADDRESS): {
-            return new Sen5x(_wire);
+            return SensorType::SEN5X;
         }
 #endif
 #ifdef INCLUDE_SFA3X_DRIVER
         case (Sfa3x::I2C_ADDRESS): {
-            return new Sfa3x(_wire);
+            return SensorType::SFA3X;
         }
 #endif
 #ifdef INCLUDE_SGP4X_DRIVER
         case (Sgp41::I2C_ADDRESS): {
-            return new Sgp41(_wire);
+            return SensorType::SGP41;
         }
 #endif
 #ifdef INCLUDE_SHT4X_DRIVER
         case (Sht4x::I2C_ADDRESS): {
-            return new Sht4x(_wire);
+            return SensorType::SHT4X;
         }
 #endif
 #ifdef INCLUDE_STC3X_DRIVER
         case (Stc3x::I2C_ADDRESS): {
-            return new Stc3x(_wire);
+            return SensorType::STC3X;
         }
 #endif
 #ifdef INCLUDE_SVM4X_DRIVER
         case (Svm4x::I2C_ADDRESS): {
-            return new Svm4x(_wire);
+            return SensorType::SVM4X;
         }
 #endif
         default: {
-            return nullptr;
+            return SensorType::UNDEFINED;
         }
     }
 }
 
-void I2CAutoDetector::findSensors(SensorList& sensorList) {
-    for (byte address = 1; address < 127; address++) {
-        byte probeFailed = probeAddress(address);
-        if (probeFailed) {
-            continue;
-        }
-        ISensor* pSensor = createSensorFromAddress(address);
-        if (!pSensor) {
-            continue;
-        }
-        sensorList.addSensor(pSensor);
-    }
-}
+

src/I2CAutoDetector.h

@@ -14,11 +14,12 @@ class I2CAutoDetector : public IAutoDetector {
      * @param SensorList& SensorList to which add the found sensors
      */
     void findSensors(SensorList& sensorList) override;
-
+    
   private:
     TwoWire& _wire;
-    byte probeAddress(const byte& address);
-    ISensor* createSensorFromAddress(const byte& address);
+    SensorType probeAddress(const byte& address);
+    ISensor* createSensorFromSensorType(SensorType sensor_type);
+    SensorType getSensorTypeFromAddress(const byte address);
 };
 
 #endif /* _I2C_AUTO_DETECTOR_H_ */

src/SensorList.cpp

@@ -1,5 +1,7 @@
 #include "SensorList.h"
 
+static const char* TAG = "SensorList";
+
 size_t SensorList::_hashSensorType(SensorType sensorType) const {
     SensorHash sensorHash = SensorHash::_UNDEFINED;
     switch (sensorType) {
@@ -41,8 +43,7 @@ size_t SensorList::_hashSensorType(SensorType sensorType) const {
     }
 
     if (sensorHash == SensorHash::_UNDEFINED) {
-        Serial.printf("Error: invalid sensor type %s encountered. Aborting!\n",
-                      sensorLabel(sensorType));
+        ESP_LOGE(TAG, "Error: invalid sensor type %s encountered. Aborting!", sensorLabel(sensorType));
         assert(0);
     }
 
@@ -63,19 +64,13 @@ SensorList::~SensorList() {
 }
 
 void SensorList::addSensor(ISensor* pSensor) {
-    // Check if we already have it
-    SensorType newSensorType = pSensor->getSensorType();
-    if (containsSensor(newSensorType)) {
-        return;
-    }
     // Hash
-    size_t hashIndex = _hashSensorType(newSensorType);
+    size_t hashIndex = _hashSensorType(pSensor->getSensorType());
     // Insert
     if (_sensors[hashIndex] == nullptr) {
         _sensors[hashIndex] = new SensorStateMachine(pSensor);
         return;
     }
-
     return;
 }
 

src/SensorManager.cpp

@@ -1,5 +1,7 @@
 #include "SensorManager.h"
 
+static const char* TAG = "SensorManager";
+
 void SensorManager::refreshConnectedSensors() {
     _sensorList.removeLostSensors();
     _detector.findSensors(_sensorList);
@@ -14,20 +16,14 @@ void SensorManager::executeSensorCommunication() {
                 sensorLabel(ssm->getSensor()->getSensorType());
             switch (error) {
                 case I2C_ERROR:
-                    Serial.printf("An I2C error occurred while attempting to "
-                                  "execute a command on sensor %s.\n",
-                                  sensorName);
+                    ESP_LOGW(TAG, "An I2C error occurred while attempting to "
+                                  "execute a command on sensor %s.", sensorName);
                     break;
                 case LOST_SENSOR_ERROR:
-                    Serial.printf(
-                        "Sensor %s was removed from list of active sensors.\n",
-                        sensorName);
+                    ESP_LOGI(TAG, "Sensor %s was removed from list of active sensors.", sensorName);
                     break;
                 case SENSOR_READY_STATE_DECAYED_ERROR:
-                    Serial.printf(
-                        "AutoDetect refresh rate too low: sensor %s "
-                        "conditioning deprecated. Decrease update interval.\n",
-                        sensorName);
+                    ESP_LOGW(TAG, "AutoDetect refresh rate too low: sensor %s conditioning deprecated. Decrease update interval.", sensorName);
                     break;
                 case NO_ERROR:
                 default:

src/SensorStateMachine.cpp

@@ -1,5 +1,7 @@
 #include "SensorStateMachine.h"
 
+static const char* TAG = "SensorStateMachine";
+
 bool timeIntervalPassed(const uint32_t interval,
                         const uint32_t currentTimeStamp,
                         const uint32_t latestUpdateTimeStamp) {
@@ -22,7 +24,7 @@ AutoDetectorError SensorStateMachine::_initialize() {
     if (error) {
         char errorMsg[256];
         errorToString(error, errorMsg, 256);
-        Serial.printf("Failed to perform initialization step: %s\n", errorMsg);
+        ESP_LOGE(TAG, "Failed to perform initialization step of sensor %s: %s", sensorLabel(_sensor->getSensorType()), errorMsg);
         return I2C_ERROR;
     }
 
@@ -109,8 +111,7 @@ AutoDetectorError SensorStateMachine::_readSignals() {
     if (error) {
         char errorMsg[256];
         errorToString(error, errorMsg, 256);
-        Serial.printf("Failed to read measurements for sensor %s: %s\n",
-                      sensorLabel(_sensor->getSensorType()), errorMsg);
+        ESP_LOGE(TAG, "Failed to read measurements for sensor %s: %s", sensorLabel(_sensor->getSensorType()), errorMsg);
         return I2C_ERROR;
     }
 

src/SensorStateMachine.h

@@ -40,7 +40,7 @@ class SensorStateMachine {
      * to UNINITIALIZED
      *
      * @return  I2C_ERROR if ISensor::initializationStep() fails (in which case
-     *            the driver error is decoded and printed to console)
+     *            the driver error is decoded and printed to logs)
      *          NO_ERROR on success
      */
     AutoDetectorError _initialize();
@@ -60,7 +60,7 @@ class SensorStateMachine {
      * short, or too long.
      *
      * @return  I2C_ERROR if _readSignals() fails (in which case the driver
-     *            error is decoded and printed to console)
+     *            error is decoded and printed to logs)
      *          SENSOR_READY_STATE_DECAYED_ERROR if too much time has elapsed
      *            since last measurement was performed NO_ERROR on success
      */
@@ -70,7 +70,7 @@ class SensorStateMachine {
      * @brief Query sensor for new signals
      *
      * @return  I2C_ERROR if ISensor::measureAndWrite() fails (in which case the
-     *            error is decoded and printed to console)
+     *            error is decoded and printed to logs)
      *          NO_ERROR on success
      */
     AutoDetectorError _readSignals();
@@ -110,7 +110,7 @@ class SensorStateMachine {
      * Serial, but such errors may not be fatal.
      *
      * @return  I2C_ERROR if bus communication fails (in which case the
-     *            driver error is decoded and printed to console)
+     *            driver error is decoded and printed to logs)
      *          SENSOR_LOST_ERROR if allowable number of consecutive operation
      *            errors was exceeded during update
      *          SENSOR_READY_STATE_DECAYED_ERROR if too much time has elapsed