SparkFun_CCS811_Arduino_Library icon indicating copy to clipboard operation
SparkFun_CCS811_Arduino_Library copied to clipboard

Timing Issues Likely Related to Clock Holding

Open mnewman401 opened this issue 4 years ago • 4 comments

I found a number of timing issues on the ESP8266 and ESP32 where the simple examples would not work with the CCS811. These are likely related to the fact that some I2C libraries (including the ESP32) do not support clock stretching. I have not verified that this is actually the reason for the failure.

In the existing libraries there was no error checking in the interactions with the device. There were a number of device interactions that did not always work. I added retries to work around this issue. I added error checking and perform retries on errors.

After a great deal of trial and error I was able to repair the library so it would work correctly on the SparkFun ESP32 Thing, the ESP32 Dev Module and my LOGGER ESP8266 board. The repaired code attached uses delays to make the interactions work. It should replace the existing modules cleanly. Error handling and retries are performed. Error reporting and some test code is included but turned off.

Also included is an enhanced simple test .INO that does a bit more reporting. The library fixes are important this test is not.

test.zip

mnewman401 avatar Sep 13 '21 23:09 mnewman401

Thank you very much for reporting and providing the extensive changes within test.zip. Is there a specific piece of code you can provide that shows failure? The current release of the library is working for me on the ESP32 without issue.

Can you provide a link to 'do not support clock stretching' claim? We've been using the ESP32 on many a device that use I2C clock stretching without error.

I've looked through your zip and there's a lot of changes. Some are simple and easily approved (CSS typo fix), some I'd rather avoid (lots of Serial.prints added). If you can create a series of PRs, this would help us more easily sort through the changes rather than having to dissect and approve a single chunk of code.

Again, thanks for the improvements, we just need it in smaller bite sized pieces that can be checked and tested against.

nseidle avatar Oct 11 '21 21:10 nseidle

I will take a look tomorrow at test cases. My recollection is that the first example failed for me after I added error checks and reports. My impression was that ignoring the errors gave a false impression of proper operation.

I should be able to find the clock stretching discussion too.

Not sure what a PR is. Can you give a reference so I can respond in a helpful way?

The printouts are all under conditionals and were necessary to figure out what was going on. The add no other value except to report errors that were ignored.

On Mon, Oct 11, 2021, 5:02 PM Nathan Seidle @.***> wrote:

Thank you very much for reporting and providing the extensive changes within test.zip. Is there a specific piece of code you can provide that shows failure? The current release of the library is working for me on the ESP32 without issue.

Can you provide a link to 'do not support clock stretching' claim? We've been using the ESP32 on many a device that use I2C clock stretching without error.

I've looked through your zip and there's a lot of changes. Some are simple and easily approved (CSS typo fix), some I'd rather avoid (lots of Serial.prints added). If you can create a series of PRs, this would help us more easily sort through the changes rather than having to dissect and approve a single chunk of code.

Again, thanks for the improvements, we just need it in smaller bite sized pieces that can be checked and tested against.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sparkfun/SparkFun_CCS811_Arduino_Library/issues/22#issuecomment-940437174, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABUAGI5XDQBQFCMWOVFVAJLUGNGF3ANCNFSM5EWLLCFQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

mnewman401 avatar Oct 12 '21 01:10 mnewman401

A PR or pull request is a way of adding new code to this library. How to create a pull request can be helpful. It will seem daunting at first but bear with us, PRs are a key to code collaborations.

nseidle avatar Oct 12 '21 15:10 nseidle

Test case test.ino

I was building code that worked on multiple platforms. I found that use of the I2C libraries in the SparkFun code did not not check for error returns in many cases. When I added these checks and reported errors on readRegister and readMultiregister I found that errors occurred which would not be noticed without the checks. The typical result of ignoring an error in the I2C operation was that an old value would be quietly used or that nothing was read from a register. This lead to a cascade of changes, fixing problems that showed up as ignored errors. I also found that the SparkFun Libraries had 'reliability' issues on other platforms (my hardware and the ESP-32-WROOM). Without the retries the error checks I added caused failures and reported what appeared to be real errors.

Using this simple test case I now find a difference between library versions 1.0.7 and 2.0.1. Using the attached test.ino and a SparkFun Thing board the earlier library fails every time with:

CCS811 Basic Example MJN Fails with library SparkFun CCS811 1.0.7 CCS811 error. Please check wiring. Freezing...

tst.ino is a slightly modified version of Example1_BasicReadings. Significant changes are to formally include a library vs a local file by using <xxx.h> instead of "xxx.h", to add messages that I changed as I changed the version of the library so I could make sure what I was using and to add a test for success on reads. The newer library offers a proper test for a successful read which is supplied in test.ino but turned off since the return value symbol involved is not in the old library.

FYI CAUTION: I observed a case when I downgraded the library where I did not get the change in library version until I closed all Arduino windows and restarted the Arduino environment. This does not repeat every time. I have never seen this problem when I upgraded the library. I may have had multiple Arduino windows open and did not close all the others when I changed the library and saw this.

My Test Code test2.ino

Most of my testing was with my own code. I attach a second test test2.ino that is representative of my test code. It is a bit of a bastard with code pulled from a number of my libraries and simply pasted in here. One thing I found well after I started updating the library is that some register reads leave the error bit set (NTC and ENV_DATA) these spurious error bits could have created confusion for me and caused more checking and improvements than necessary. test2.ino notices this, reports and clears the problem. With library 2.0.1 test2.ino now works as expected without needing retries on the Sparkfun ESP32 Thing.

Clock Stretching

I observed logic analyzer traces that were consistent with failures to observe clock stretching rules.

I have found only incomplete info on clock stretching reported on line for the ESP32 so far. I remember seeing a discussion which said clock stretching was not implemented at all in the ESP32 libraries which I have not found. Here are some discussions:

https://github.com/espressif/esp-idf/issues/4173 https://forum.pycom.io/topic/331/i2c-clock-stretching/13

It does seem that the ESP32 has limits on how long clock stretching can occur and that other configuration parameters change the limit of maximum time that a stretch can go.

PRs For Changes

The first thing I would do is to address the spurious error bit issue for NTC and ENV_DATA register reads. This is not well documented if at all in the chip data sheets and needs a dialog with the chip manufacturer.

Second I would add error checks for all calls to the library in all of the examples. I would also add error checks to all calls internal to the library and return errors as appropriate instead of ignoring the possibility of errors.

I looked at the web discussion and am still a bit out of my depth. I may look further at doing this, but it will take a lower priority.

On Mon, Oct 11, 2021 at 9:52 PM Michael Newman @.***> wrote:

I will take a look tomorrow at test cases. My recollection is that the first example failed for me after I added error checks and reports. My impression was that ignoring the errors gave a false impression of proper operation.

I should be able to find the clock stretching discussion too.

Not sure what a PR is. Can you give a reference so I can respond in a helpful way?

The printouts are all under conditionals and were necessary to figure out what was going on. The add no other value except to report errors that were ignored.

On Mon, Oct 11, 2021, 5:02 PM Nathan Seidle @.***> wrote:

Thank you very much for reporting and providing the extensive changes within test.zip. Is there a specific piece of code you can provide that shows failure? The current release of the library is working for me on the ESP32 without issue.

Can you provide a link to 'do not support clock stretching' claim? We've been using the ESP32 on many a device that use I2C clock stretching without error.

I've looked through your zip and there's a lot of changes. Some are simple and easily approved (CSS typo fix), some I'd rather avoid (lots of Serial.prints added). If you can create a series of PRs, this would help us more easily sort through the changes rather than having to dissect and approve a single chunk of code.

Again, thanks for the improvements, we just need it in smaller bite sized pieces that can be checked and tested against.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sparkfun/SparkFun_CCS811_Arduino_Library/issues/22#issuecomment-940437174, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABUAGI5XDQBQFCMWOVFVAJLUGNGF3ANCNFSM5EWLLCFQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Michael Newman Dragonnorth Group www.dragonnorth.com

Phone: 617 566-7975 Cell: 617 821-4608

16 Nason Hill Lane Sherborn, MA 01770

mnewman401 avatar Oct 12 '21 18:10 mnewman401