-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Is there any way to ensure 100% accurate i2c interfacing ? #811
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
Comments
@euquiq a couple of us, @lonerzzz and I are working to improve the I2C subsystem. @lonerzzz has implement some fixes for bigger block handling. I am working on changing to an interrupt driven model from the current polling version. As you intimate, there is an interaction with the RTOS background process that is interfering with the polling model. I hope to achieve my goal. I an currently having success with big block reads, up to 32768 byte continuous transfers. Alas, I still have to implement Writes, onRequest() and onReceive(). Chuck. |
@euquiq I have a question: Chuck. |
Thanks @stickbreaker for your fast eyes-up response! I am glad you and @lonerzzz are working on it. Good luck on it, and I am eagerly following your posts on i2c topic. For now, I catch the return value of Wire.endTransmission() and Wire.requestFrom() and if I get an error or the wrong number of bytes, I retry, previously doing a Wire.reset() ... it seems to work ok. BUT ... Wire.endTransmission() sometimes returns error code 5 ... and I cannot find a definition for such error. Probably I am looking into the wrong places. Do you or anyone can enlighten me what error 5 would mean ? Regards, Enrique |
@stickbreaker About ReStart ... it uses Wire.endTransmission(false) in several places. //Pixel Corrections Also, just in case anyone wants to comment (or if this is useful to anyone), this is what I am doing now for catching errors: //Pixel Corrections |
@stickbreaker thx for the offering. This code is part of a POC device ("proof of concept") which I am presenting to a third party over the following hours / days, so right now I am not inclined to tinker with it much more. Over the next hours / days, I got yet to build like 6 more devices, but there is a ton of soldering and 3D printing ahead, so I know I will not be able to deliver the attention to testing your function deserves ... not at least until / after I get the next "batch" ready for tinkering and got time avail to resume coding. As I understand, you propose a Wire.transact() function which will "take over" all the Wire.write() and Wire.endTransmission() work, fixing the i2c timeout due to issuing a At this time, with the error catching you told me a couple of days ago on another post, placed as in my above code, all over my i2c data fetching / writing routines I am not getting the timeout errors "all the time". Right now my sensor has been polled once per second, for the last 3 hours () and the i2c errors detection / correction triggered only seven times over that time ... Which, given my timeline - sleep deprived situation, is bearable. Yes, I want to eliminate every i2c error altogether. I want the same i2c behavior I am currently experiencing on my ESP8266 based devices, which has been working with no error catching and no false readings for weeks by now. So I think I will need to test your proposed solution, and dive deeper into this, but three or four days from now, if that is ok with you ? |
That would be fine. If it is enough of a problem that you want to spend the time chasing it drop me a note and I'll provide the code. Chuck. |
@stickbreaker Update: I keep receiving an unacceptable number of corrupt values from Wire.read() even after catching the Wire.endTransmission() and checking for the correct number of bytes received by Wire.read(). The MLX90621 sensor is being polled once per second. After 10 hours or so, I received at least 3 wrong values from the thermal array (Declaring weird temperatures on one -random- pixel). I got several sensors, and several ESP32 and ESP8266: As far as I could test, this behavior does NOT occur on the ESP8266. AFAIK, I do not have a reliable way of detecting such errors from the Arduino framework itself. Under my (mostly uninformed and rushed) eye, I suspect this may also be a consequence from bad timing / "multitasking" breaks while Arduino ESP32 deals with the Wire interface ? For now, I am adding (more) extra code order for my program to detect such corrupt values, but I am concerned about the whole thing. If Arduino had some mechanism / command to freeze the RTOS "multitasking" and dedicated 100% of the core for executing the Wire transaction, and then resuming, maybe it would reach the desired stability ? Do you think your function / modifications would improve the stability, considering my scenario (where bytes are being corrupted when read) ? If so, please, share it :) |
I am in the midst of reverse engineering the I2C subsystem. The ESP-IDF, Espressif documentation are not very conclusive. What I have discovered so far, is that the I2C statemachine is no very forgiving. The I2C ReStart support is very time sensitive. In the current implementation of I wrote a But, as you can see from this excuse, I do not have a working pull to offer you at this time. I have continued my exploration of the I2C subsystem, currently I am in the middle of a major rewrite to support background interrupt driven data handling. My goal is to develop SLAVE mode support. I have successfully created and tested MASTER mode unlimited block size sending and receiving. I have spend the last week trying to figure out error recovery methods that would allow me to reset the I2C hardware without re initializing it all the time. Sometimes the reinit sequence creates glitches on the bus that cause other devices to become confused and fail to respond to commands. I had assumed by your prior message that your interests were divergent from mine, so I continued on my path. It will probably take me a week or two to compete my ISR work (if I don't fail 😄) As soon as I get a working version I will message you. I have had another thought about the possible coding for this restart support. Arduino support for ReStart sequences was a bolt on addition to the The ReStart function on the I2C bus allows multiple read/write transactions to be preformed without loosing ownership of the I2C bus. In a Multi-MASTER environment a STOP command allows other MASTERs to start a new transaction. The ReStart holds the ownership, but allows multiple command sequences that are contiguous. What do you think about a Wire.endTransmission(false) returning ERROR_CONTINUE on the ESP32? To use the existing Wire.beginTransmission(ID);
Wire.write(addr);
uint8_t err=Wire.endTransmission(false); //initiate ReStart operation,
// on the ESP32, the Write data is just stored until the requestFrom() command is executed,
// then the combined START->WRITE->ReStart->READ->STOP sequence is actually executed.
// this delayed write may fail during the requestFrom() command. So error Checking is different.
if(err==ERROR_CONTINUE){ // write command has been queued.
err=Wire.requestFrom(id,numRead,true); //actually preform the sequence
// possible Error return value from requestFrom are 0 or numRead. This is not sufficient
// my version returns ActualNumberRead
// to check for errors I created a new function in `Wire.h`
err = Wire.lastError();
// lastError() returns the standard I2C error Values plus
// ERROR_WRITE_ADDR_NAK // NAK during device address of a stand alone write operation, or as part of transact()
// ERROR_READ_ADDR_NAK// NAK during device address of a stand alone read operation, or as part of transact()
// ERROR_DATA_NAK // only possible during Write Operations
// ERROR_ARBITRATION_LOST // in a Multi-Master environment, the Other master gained ownership of the bus, this transaction failed. Error recovery is left to the app.
while(Wire.available()){
Wire.read();
}
} Another Question: Does your code require this sequence of I2C commands? Wire.beginTransmission(ID);
Wire.write(value);
Wire.endTransmission(false); // would return ERROR_CONTINUE
Wire.beginTransmission(ID);
Wire.write(value);
Wire.endTransmission(false); // Would Return ERROR_CONTINUE
Wire.beginTransmission(ID);
Wire.write(value);
Wire.endTransmission(); // or Wire.endTransmission(true); would return an Actual Error Value. The reason I ask, is that all of these three sequences need to be embodied into ONE I2C sequence to operate properly. If I need to support these type of sequences, I'll have to do implement a more complex multi sequence queue. Chuck. |
Hi Chuck! Thanks for your time. My knowledge about i2c is flimsy, so I am re-reading your last post, while digesting it. I will comment about it a bit later. About your last question, I cannot see that specific kind of commands sequence on the code. The mlx90621 sensor needs several i2c functions in order to configure / receive data. Such functions can be seen, i.e. in here: https://github.com/robinvanemden/MLX90621_Arduino_Processing/blob/master/readTemperatures/MLX90621.cpp I get errors, at random on each of these functions inside ESP32… Most of them include the dreaded Wire.endTransmission(false) ! By now I modified every function with error catching and after reading temperatures, I do also check for inconsistent values and retry until I am sure the values read are ok. Such mechanisms saved the day, final user-wise. But I am not happy with all the extra code and time used in order to police around the Wire problem. In case you are interested / curious about each function needed, when interfacing with the MLX90621 sensor thru i2c there is interaction with an EEPROM (at address 0x50) and the sensor pixel array computer itself (0x60), thru the following: readIR() Reads 64 2-byte values from the sensor, fetching each 16bit temperature value from each cell readPTAT() It reads the ambient temperature readCPIX() Reads a 2 byte value (uInt16) (value used later for thermal pixel corrections, whatever that should mean). readConfig() It fetches two bytes with config flag-bits inside writeTrimmingValue() Writes trimming value for the internal oscillator. readEEPROM() it reads 64 byte with factory calculated compensations for each pixel, specific for each sensor pixel. setConfiguration() it writes the user configuration for the sensor (i.e. update speed ). |
@stickbreaker Thanks Chuck ! Brilliant ! I will create a copy of my current code and implement all i2c as you deviced, and will come back with my results :) Thanks a lot, your investigation and work in invaluable ! |
@stickbreaker Hi there ! I am working on adapting your code. I had to acommodate it into my sensor's functions and add a Wire.endTransaction() at the end of each of each one. I had other problems, I am slowly fixing them. I will post my overall experience ASAP. Overall, It seems to be working for me, but with some "quirks". I am yet to get it 100% in working condition. The interesting part is that there seems to be NO timeout errors, as you said :) Thanks again ! |
@euquiq What kind of code changes have you had to make? I would be interested in seeing a copy of your before and after conversion. Chuck. |
@stickbreaker I think I've found a bug: I implemented your i2c version on all i2c related functions and I got weird temperature values. Checking the data being polled from i2c, with your implementation some bytes get consistently lost and replaced by "255" value. I.E. Function that reads 256 bytes from the eeprom of my MLX90621 sensor. It reads it in chunks of 32 bytes. As a reference, this is the code you can find on several repositories, valid for ESP8266 with no error correction whatsoever and it works like a charm in them, taken from: https://github.com/GarageProto/GP-004-01/blob/master/MLX90621.cpp
The following is my "old" (standard ESP32 i2c) code with the error catching stuff added to it, which "gets the work done" catching tons of errors:
The above code produces the following 256 values byte array:
And with your i2c version, this is the code, which I tested both with the
The above code with your i2c version produces the following output on the 256 byte array:
And subsequently all other i2c reads (and writes ?) seems to be somewhat garbled, since I get really weird temperatures as a result. It is as if your i2c implementation is missing reading some bytes. Hopefully this helps. Enrique. |
@euquiq try this code: void MLX90621::readEEPROM() { // Read in blocks of 32 bytes to accomodate Wire library
Wire.beginTransmission(0x50);
Wire.write((uint8_t)0); // set address pointer to zero,
int err =Wire.transact(eepromData,256); //grab 256 byte directly into eepromData
if(err != 256){
Serial.printf("Transact failed =%d, bytes Received =%d",Wire.lastError(),err);
}
}
Chuck. |
@stickbreaker I am trying the above codes in a second ESP32, (other brand) and another mlx90621 sensor, in order to rule out hardware problems. Again, weird data read with your i2c implementation. Consider this is a second MLX90621 sensor, and the eeprom data is the precalibration at factory time, so bytes are different from the first sensor: This result is the correct one, with the standard i2c implementation:
And now with your i2c implementation, as seen on my post above:
|
@stickbreaker I posted my last comment before reading your suggestion about placing the eeprom data directly inside the byte array. I will try your suggestion tomorrow as now I am so asleep I am afraid I will make a mistake. I will post the result ASAP. |
@stickbreaker sorry, I could not go to sleep without trying: Congrats ! When reading the whole eeprom content directly into the array, it read OK. Problem may still be present inside all other i2c functions that the MLX90621 uses. I will dive into it tomorrow. In the meantime, hopefully you can use this info.
|
ok, I can see from the two data blocks, the second on is skipping a value every read, here is the code from my test function: Wire.beginTransmission(ID);
if((ID >= 0x50)&&(ID <= 0x57)) Wire.write(highByte(addr)); // 16bit device addressing
Wire.write(lowByte(addr));
if((err=Wire.endTransmission())!=0) {
Serial.printf(" EndTransmission=%d",err);
if(err!=2) {
Serial.printf(", resetting\n");
Wire.reset();
return;
}
else {
Serial.printf(", No Device presend, aborting\n");
return;
}
}
err = Wire.requestFrom((uint8_t)ID,(size_t)BlockLen,true);
Serial.printf("@0x%02x(0x%04x)=%d ->",ID,addr,err);
addr += BlockLen;
if(!((ID >= 0x50)&&(ID <= 0x57))) addr %= 256; // byte addressed devices
uint8_t b=0;
char buf[100];
uint16_t blen=0;
while(Wire.available()){
char a=Wire.read();
if((a<32)||(a>127)) Serial.print('.');
else Serial.print(a);
blen+=sprintf(&buf[blen]," %02x",a);
b++;
if((b%32)==0){
Serial.println(buf);
blen=0;
}
}
if(!((b%32)==0)) Serial.println(buf);
} for your device the two address range Chuck. |
@euquiq Duplicated your problem, Am researching it. Should have solution by morning 😬 Chuck. |
@euquiq , found it, I did not set txQueued back to zero after the requestFrom() at about line 174 in /libraries/Wire/src/Wire.cpp at txQueued =0; Sorry! |
@stickbreaker thanks a lot, Chuck ! I just edited Wire.cpp and added the code line you posted. My device compiles OK and seems to be working fine !! I will give it a non/stop run over the weekend and log any errors / hiccups, but I am quite confident that errors are tamed / null with your code. I would encourage you and @me-no-dev to consider your additions / changes for the main arduino ESP32 repository :) since as it is, changes for earlier implementation i2c code are nill / minimal, but it adds on stability and new features. Bugs -if still present- will be ironed out faster if more people try it ! Cheers, |
Sounds great, I found another related issue that would only crop up if multiple consecutive:
Blocks were present. My code would keep summing the total bytes written with each newly queued You did not encounter it because your code only queued ONE command before the queue was processed. I pushed another fix to my repo. Wire.cpp:334 I hope you have a Happy and Productive Weekend! Chuck. |
@stickbreaker There might be another bug: This happened at one "beta testing" location for my hardware: when turned on, it had yet to be configured for working with their WIFI network, so it was set up in AP mode. Weirdly enough, the ESP32 returned garbage from the i2c sensor. We tried several cycling / AP reconnecting instances, and the behavior was consistent. When I configured the wifi ssid and password and the ESP32 started working on wifi station mode, the i2c started working normally. This should be pretty much easily duplicated, I will try it at home in a couple of hours, but wanted to give you a heads up, in the event that you where around at this time and wanted to check it for yourself. Regards, Enrique |
@euquiq off the top of my head, I have no clues. About interaction between the WiFi code and my i2c, Unless the Wifi configures/ uses the i2c during its init? My i2c code assumes exclusive access to I2C0 Chuck. |
@euquiq If you are using multiple tasks, you should pin the one using @stickbreaker 's code to CORE1 otherwise you will have collisions with the WiFi interrupt. Just figured this out. |
@lonerzzz Where would the best place to note this Wifi/i2c interaction. The necessity of pinning? Should we add some file in the |
@stickbreaker We should ask @me-no-dev if there are issues with enabling the Wiki because otherwise the question will likely come up often. |
@me-no-dev nudge, nudge |
@me-no-dev @lonerzzz I have been thinking about creating a branch in my repo to contain a cleanup merge candidate. Should it be Here espressif/arduino-esp32 or stickbreaker/arduino-esp32? |
@stickbreaker @lonerzzz Hi again ... I was not able to replicate that weird i2c behavior in my other ESP32 / MLX90621 sensors back here at home . I got only one task (send email) which was pinned at core 0 and seldom used, but even triggering an email send in order for it to work, I am not getting any problems (although I just moved it into core 1, just in case ...). I am trying to think on every combination of action I could have made at the installation time in order to garble i2c in such a way for it to throw me the wrong bytes when read while on AP mode. Unless I can get to trigger the weird values again, I am inclined of thinking on some other spurious situation (static, bad manipulation from the person that was installing it, etc.) Again, Kudos for this i2c implementation ! |
@euquiq Keep testing, It is has only been tested on a narrow range of hardware and there could be other hardware/software interactions. But, I'm still happy that it works for you! Chuck. |
someone poking me? :D I am all for enabling the Wiki ;) |
@stickbreaker @lonerzzz let's figure out a more "real-time" way to discuss this. How about gitter? |
@me-no-dev , Now you want me to figure out an totally new website, 😬 Ok, I'll try. |
@stickbreaker Hi there! I am updating the github ESP32 main branch, so then I can get into your repository for the i2c related changes. But correct me if I am wrong, now your repository is a branch of the whole esp32 master repository, instead of just the files you changed ? Or was it always like this ? I am asking because I think that the master now includes some updates to IDF and other commits that your fork seems to be missing. Sorry for the questions / Eyes-up but I am still trying to get an understanding at github way of work. Regards, Enrique |
@euquiq , I'm going to try to merge from here into my repo, wish me luck! Grab the latest from here espressif/arduino-esp32 then replace these 5 files. \libraries\Wire\scr\Wire.h from my repo. -log.h is not necessary right now, but in a couple of hours I am committing a update that will require it. I have been working to clean up and organize my code. Trying to make it more polite. I have changed a couple more: In the Wire library I have added some documentation, and examples, updated the libraries.properties and the keywords.txt Chuck. |
@euquiq Ok, I think my repo is current. Now you will need the I have been using my I2c without any fails for the last week. I'm starting on something else. If you have a issues mention me. I have been adding to a wiki on my repo I2C thoughts |
@euquiq What is your status? Are you using my I2C? is it working? If something else crops up open a new issue in stickbreaker/arduino-esp32 Chuck. |
Hi @stickbreaker ... yeps no problems whatsoever. I really think your code should be integrated to esp32's main branch ! Congratulations for your fine and exhaustive work. |
@stickbreaker I am still facing an error. I guess it will help you to debug more. Problem seems to be only causing if delay between calling button.tick() is less than 200ms I get following error: [E][esp32-hal-i2c.c:609] i2cDumpI2c(): i2c=0x3ffc1624 Simran |
@harji2130 This debug output is showing the I2C bus is in a Busy state before this Something is causing the I2C bus to hang. It is not this call. The Busy state is probably initiated in a prior Also, capture earlier debug output. The debug you posted is after the problem occurred. Chuck. |
@stickbreaker thank you for doing the branch. I was having drop outs with the i2c after i started wifi and interfacing with a bnO055. Taking your branch has solved it so now I can control my little robot over wifi and run the BNO YAY! |
@carbonadam Always good to hear it works! Thanks. Chuck. |
Just wanted to inform everyone that yesterday I tried to compile with Platformio's platform = https://github.com/platformio/platform-espressif32.git#feature/stage |
[W][esp32-hal-i2c.c:334] i2cRead(): Ack Error! Addr: 40 |
@stickbreaker Hi Chuck! I've been trying to contact you. I ** think ** I wrote you on your facebook page. Hopefully you can read it :) Thanks in advance! |
I came to realize that i2c timing is critical and since Arduino layer is on top os RTOS, interfacing with espressif IDF, depending on my code, it may be very difficult to maintain such i2c strict timing requirements.
I cannot get 100% clean data from my Melexis MLX90621 termal array sensor. It communicates through i2c with my ESP32 and sooner or later, data gets garbled. I already posted this issue before. #659
On the esp8266 everything works smoothly, no errors after days of running my code. On the esp32, sadly, the i2c communication is at most, unstable.
I get:
[E][esp32-hal-i2c.c:313] i2cRead(): Timeout! Addr: 60, index 32
[E][esp32-hal-i2c.c:161] i2cWrite(): Busy Timeout! Addr: 60
[W][esp32-hal-i2c.c:280] i2cRead(): Busy Timeout! Addr: 60
By the dozen each minute.
I did follow user stickbreaker recommendation, stating "The I2C subsystem is timing dependent. If any of the core error messages are generated it causes a chain reaction of additional timeout/ack failures. Disable the core logging and test the return codes in your application. uint8_t err=Wire.endTransmission() uint8_t err=Wire.requestFrom(slaveId,count);"
I can see the correct number of bytes being returned by my sensor, and no error after wire.endTransmission() ... until the errors start.
I can catch this and retry the reading / writing to i2c, or even first RESETTING the wire interface and then retry.
Right now I am doing just that. But it does not feel right (unless it is the only way to cope with it) ... hence the question:
Is there any way to tell Arduino "suspend all RTOS activities while I execute this code" ?
Regards, Enrique
The text was updated successfully, but these errors were encountered: