abrauchli
commented
4 years ago Hi @monkeytronics! Many thanks for your interest in our sensor and driver.
Hi guys! I was using your excellent library to run up my SCD30 on ESP32 with Amazon FreeRTOS. I found a few little gotchas that I thought I'd mention.
Thanks, your feedback is most appreciated
Firstly, I was getting a full on kernel panic when I tried to read data. I tracked it down to a problem in the scd30_read_measurement function in scd30.c. The problem is when it calls sensirion_i2c_read_bytes, it gives it the wrong number of words. It should be 6 (defined as the number of words in the CO2, Temp & Hum excluding CRC bytes (as these are accounted for in the function). I think it was evaluating 12 instead from sizeof(data). Hence, it was asking to go and get 24 bytes and stick them into a defined variable only long enough for 12. I guess that's where the overflow came from. My humidity pointer was getting overwritten with a random address outside the allowable range.
Great catch - that function sensirion_i2c_read_bytes is misleading and needs a new name. The parameter name hints at it and the doc specifies it, but glancing over the code it's all too easy to overlook that it take a number of words and not bytes.
Also, I think (although I may be mixing and matching the order in which I debugged things), that you need to do a soft reset after initialising the i2c driver. May be ESP32 specific, so not a bug in any case, just an observation.
Could you elaborate on that?
Lastly, because I'm using an ESP32, I had to define the timeout specifically. Obviously not a bug either, but perhaps useful for others to know if they are using the ESP32.
Could you post your sample implementation (sensirion_hw_i2c_implementation.c) or submit a merge request for inclusion against embedded-common? https://github.com/Sensirion/embedded-common/tree/master/hw_i2c/sample-implementations
Kind regards, Andreas
rnestler
Hi guys! I was using your excellent library to run up my SCD30 on ESP32 with Amazon FreeRTOS. I found a few little gotchas that I thought I'd mention.
Firstly, I was getting a full on kernel panic when I tried to read data. I tracked it down to a problem in the scd30_read_measurement function in scd30.c. The problem is when it calls sensirion_i2c_read_bytes, it gives it the wrong number of words. It should be 6 (defined as the number of words in the CO2, Temp & Hum excluding CRC bytes (as these are accounted for in the function). I think it was evaluating 12 instead from sizeof(data). Hence, it was asking to go and get 24 bytes and stick them into a defined variable only long enough for 12. I guess that's where the overflow came from. My humidity pointer was getting overwritten with a random address outside the allowable range.
Also, I think (although I may be mixing and matching the order in which I debugged things), that you need to do a soft reset after initialising the i2c driver. May be ESP32 specific, so not a bug in any case, just an observation.
Lastly, because I'm using an ESP32, I had to define the timeout specifically. Obviously not a bug either, but perhaps useful for others to know if they are using the ESP32.