-
-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updates for new I2C sensor SHT30 #150
Updates for new I2C sensor SHT30 #150
Conversation
enelson1001
commented
Mar 27, 2021
- Tested all 6 repeatability modes
- Did minor cleanup DHT12.cpp
- Just noticed CMakeLists.txt changed selected_test_project to be i2c_sht30_test so will immediately update and change back to starter_example. I am not sure how this happened?
- The only other name I could think of for read_block was read_bytes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so it wasn't possible to combine the existing write/read functions to achieve the desired functionality. If you say that's the case then I'll take your word for it.
Let me know if you're happy with this as-is and we'll get it merged.
I2C StretchingI did more research on i2c and clock stretching and i2c-bus.org has very good explanation. https://www.i2c-bus.org/clock-stretching/ ESP32 I2C StretchingA clock stretching question was asked on espressif/esp-idf#4173 So it looks like the esp32 supports clock stretching but the maximum timeout is 13ms. What I didSo what I did was make a copy of your read function and called it read16 and implemented a 16bit slave register instead of an 8bit slave register. The read portion of the code failed. So next I read what the master i2c_timeout was set to and found it was set to a count of 8000. So then I changed the timeout to the maximum count of 1048575 (0xFFFFF) or approx. 13ms and then the new read16 worked. But it is not the best fit for the SHT30 because the High Repeatability with clock stretching (measurement duration) can be 15.5ms. I think I got lucky it worked on High Repeatability with clock stretching enabled because measurement time typically is 12.5ms and that is what I was seeing on my logic analyzer. Proposed changes to I2CMasterDevice.cpp
Proposed changes to SHT30
Changes to Test Program i2c_sht30_test.cpp
So what would you like to doLet me know how you want to proceed. |
Sorry for the late reply. Since longer clock stretching isn't supported in the esp not using that sounds like the way to go. Maybe put a note next to the enum explaining why it is off by default. |
more updates to SHT30
Tested above with SHT30 and DHT12
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also don't forget to update the contribution file.
I tried changing res operations to &= or && but log_error was never called because the resultant of these operations was always ESP_OK because ESP_OK=0. So changed back |= and | so it looks like your original code was correct. I'm not sure but I think the command link stops execution if one of the commands does not equal ESP_OK otherwise if we have multiple failures and start ORing all the results we could have a weird value at the end. I think I got all your requested updates. |
Oh, yeah. The joy of having OK = 0.... oh well. Sometime we might hcnage it to call() == ESP_OK instead. I looks good otherwise. |
I've updated to latest master (PR #151) but now have problems running CI/.build_test.sh on local PC. Errors out when trying to build test programs.
|
I can't figure out why and I'm sure I ran the tests from latest master after merging that PR. |
I ended up changing . $IDF_TOOLS_EXPORT_CMD to this -----> . $IDF_PATH/export.sh and it worked for me. Are you OK with me submitting this change? |
Let's hear if @peterus has some input as to why IDF_TOOLS_EXPORT_CMD would be empty. |
it looks like @enelson1001 is not using the newest docker image. @PerMalmberg maybe we should add some kind of version information (as the docker image tag) to the docker image. |
Yes, that's a good idea. |
deleting the local docker image did the trick and build_test.sh ran with no error. |