From 068a4a55470f9d36a7c87cb31dc7a307185e2bc7 Mon Sep 17 00:00:00 2001 From: Quentin Date: Mon, 28 Oct 2024 10:32:40 +0100 Subject: [PATCH 1/3] Fix leak in I2C scanning --- src/I2CAutoDetector.cpp | 74 ++++++++++++++++++++++++++++++++--------- src/I2CAutoDetector.h | 7 ++-- src/SensorList.cpp | 8 +---- 3 files changed, 63 insertions(+), 26 deletions(-) diff --git a/src/I2CAutoDetector.cpp b/src/I2CAutoDetector.cpp index 119f889..4216cbb 100644 --- a/src/I2CAutoDetector.cpp +++ b/src/I2CAutoDetector.cpp @@ -28,70 +28,112 @@ #include "SensorWrappers/Svm4x.h" #endif -byte I2CAutoDetector::probeAddress(const byte& address) { +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::createSensorFromAddress(const byte& 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; + } + } +} + + +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) { + auto st = probeAddress(address); + if (st == SensorType::UNDEFINED) { continue; } - ISensor* pSensor = createSensorFromAddress(address); + if (sensorList.containsSensor(st)) { + continue; + } + + ISensor* pSensor = createSensorFromSensorType(st); if (!pSensor) { continue; } sensorList.addSensor(pSensor); } -} +} \ No newline at end of file diff --git a/src/I2CAutoDetector.h b/src/I2CAutoDetector.h index 80f2660..9b754c0 100644 --- a/src/I2CAutoDetector.h +++ b/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_ */ diff --git a/src/SensorList.cpp b/src/SensorList.cpp index 1e53695..c71a18c 100644 --- a/src/SensorList.cpp +++ b/src/SensorList.cpp @@ -63,19 +63,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; } From 196887f7dafcfacd5e94eb6dcfa7bb71d096c6d9 Mon Sep 17 00:00:00 2001 From: Quentin Date: Mon, 28 Oct 2024 10:34:51 +0100 Subject: [PATCH 2/3] reorder according to header --- src/I2CAutoDetector.cpp | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/I2CAutoDetector.cpp b/src/I2CAutoDetector.cpp index 4216cbb..c90c522 100644 --- a/src/I2CAutoDetector.cpp +++ b/src/I2CAutoDetector.cpp @@ -28,6 +28,28 @@ #include "SensorWrappers/Svm4x.h" #endif +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(); @@ -120,20 +142,3 @@ SensorType I2CAutoDetector::getSensorTypeFromAddress(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); - } -} \ No newline at end of file From 2a527bcfe22b5250995ce4d13de289161d68df16 Mon Sep 17 00:00:00 2001 From: Quentin Date: Mon, 28 Oct 2024 10:48:27 +0100 Subject: [PATCH 3/3] replaced prints by ESP logs in library code --- src/SensorList.cpp | 5 +++-- src/SensorManager.cpp | 16 ++++++---------- src/SensorStateMachine.cpp | 7 ++++--- src/SensorStateMachine.h | 8 ++++---- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/SensorList.cpp b/src/SensorList.cpp index c71a18c..870d4e5 100644 --- a/src/SensorList.cpp +++ b/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); } diff --git a/src/SensorManager.cpp b/src/SensorManager.cpp index 3b62dca..d6e3061 100644 --- a/src/SensorManager.cpp +++ b/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: diff --git a/src/SensorStateMachine.cpp b/src/SensorStateMachine.cpp index c1c8215..256669e 100644 --- a/src/SensorStateMachine.cpp +++ b/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; } diff --git a/src/SensorStateMachine.h b/src/SensorStateMachine.h index c9a7996..351e00c 100644 --- a/src/SensorStateMachine.h +++ b/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