letscontrolit / ESPEasy

Easy MultiSensor device based on ESP8266/ESP32
http://www.espeasy.com
Other
3.28k stars 2.22k forks source link

Central helper class for I2C communication #473

Closed TD-er closed 2 years ago

TD-er commented 7 years ago

One of my test setups using a BMP280 sensor had stopped communicating with the sensor, so I was looking into the code for this sensor and I noticed there was a lot of "standard" code in there for basic I2C communication. A simple find showed similar code in: P028_BME280 , P032_MS5611 and P047_i2c_soil_moisture_sensor I think it is making things easier and more clean when there is a central helper class for these basic I2C interactions.

This will also make it more stable and using less code is always a good idea in a space constrained environment. Also I was thinking of adding support for HDC1080 (high precision humidity/temperature sensor) and CCS811, a volatile organic compounds (VOCs) "eCO2" sensor. (available here) These communicate also via I2C

Is there a good reason not to introduce such a helper class?

If it is considered a good idea, I would be glad to implement it. But then there are some simple unknowns like:

TD-er commented 7 years ago

I have had a second look into both BMP/BME-280 module classes. Both have a lot of code duplication. So perhaps it is even better to merge both into one and then look at the device-ID to automatically detect the device and thus the presence of humidity data.

In the resources used, a combined codebase would mean the a reduction in size equal to the size of the BMP280 currently used:

module                        |cache IRAM |init RAM   |r.o. RAM   |uninit RAM |Flash ROM  
src/_P030_BMP280.ino          |128        |0          |40         |64         |2848       
src/_P028_BME280.ino          |128        |0          |40         |80         |3280       
psy0rz commented 7 years ago

combining the BMP/BME sounds like a good idea.

Also we could really use a good i2c helper, i also didnt like all that code duplication when i first saw it.

This stuff happens when projects grow and people copy and modify a plugin. Its not a big problem but sometimes refactoring is needed.

I'm not sure about all the details, but I think error handling is important. And it shouldn't be a plugin. Just a library with a class. (The compiler will automagicaclly compile it when a plugin needs it)

I think this should go in version 2.1.0. (2.0.0 needs to be releases asap)

tonhuisman commented 2 years ago

This suggested plugin-merge has been completed some time ago, so this issue can be closed.