-
Notifications
You must be signed in to change notification settings - Fork 7.6k
I2C ReWrite #839
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
@stickbreaker why not PR this so we can just switch to it and give it a go? |
It would break everyone's partially functioning code. Have you read the README.md in my fork? I explain it detail what the drawback would be. If you want a pull I'm all for it, but I thought the cascade of "It BROKE my CODE" would be deafening! Chuck. |
From the README I gather, that users of Wire::* don't need to change their code, do they? So I dropped your fork in and I can confirm that it fixes ds3231 issues i had before with at least 2 modules, and it did not break my at24c32 eeproms, yet. i will do more tests. |
@everslick Yes they do, in certain cases, If the written code does not test the return codes(sloppy code), then most WRITE operations will work. But, the Wire.beginTransmision(ID);
Wire.write(stuff);
Wire.endTransmission(false); // command is just queued, no i2c activity happened
Wire.requestFrom(ID,len,false); //command is just queued, NO i2c activity happened
while(Wire.available()){ // will ALWAY be FALSE because no i2c activity happened to fill the READ buffer
Serial.print((char)Wire.read());
}
Wire.beginTransmission(ID);
Wire.write(stuff);
Wire.endTransmission(); // this command cause all QUEUED i2c command to be ran, sendStop=true is default.
//Now
while(Wire.available()){ // could be true, if the First Write i2c transaction succeeded and requestFrom()
// succeeded then the READ buffer will contain len characters
Serial.print((char)Wire.read());
} If the result code from the last Chuck. |
@stickbreaker: Thank you very much, for the clarification, your work is very much appreciated! So i will migrate all my I2C com to your Wire library and report back. do you plan to sync your fork of arduino-esp32 with upstream from time to time? if yes I will stay on your branch for the time being. @me-no-dev: with the incompatibilities to existing sensors in mind, maybe we should add chuck's fork as a new ESPWire library and replace the current Wire lib with a pure software bitbanging lib? |
@everslick I have started going through the new code and brainstorming on how we can make it all work ;) we might have success! |
@everslick My plan such as it is, is to get enough testing to prove my idea works. If it works, then I would like to Clean it up and fold it into the official espressif/arduino-esp32. I do not wish to carry this forward. It is just a development branch that will be pruned after all the useful fiber has been gleaned. Your comment about a 'ESPWire' library is not as desirable. It develops duplicate code that becomes cumbersome over time. I cannot see a better answer than the one you posed. But a bit bang solution that is compatible to the I2C standard and Arduino compatible would be very difficult if not impossible. I think complete Arduino compatibility with the ESP32 hardware is unobtainable. This queued implementation is the best I can think of. @me-no-dev you seem to be the guiding influence for this project, do you think this approach would be acceptable as the Chuck. |
@stickbreaker here is what I have vaguely on my mind: Rework your code to expose a functions like From what I can gather in my mind there are a few cases of use of I2C: // Only writing data to slave
Wire.beginTransmission(addr);
Wire.write(data);
//........ more writes
Wire.endTransmission(true);
// Writing Then Reading from Slave
Wire.beginTransmission(addr);
Wire.write(data);
//........ more writes
Wire.endTransmission(false);
Wire.requestFrom(len, true);
Wire.read();
//........ more reads till empty
// Only Reading from Slave
Wire.requestFrom(len, true);
Wire.read();
//........ more reads till empty Are there any other realistic cases? Very important is to implement locks so another thread does not use I2C while you are between Start and the RX Buffer being empty or endTransmission(true) if only writing. If I am correct about the possible cases, then we can abstract the i2cTransmission through the Wire API and ensure that the I2C bus is touched only once per Start-Stop |
@stickbreaker Do you actually sleep? :) Thanks for all the updates. In testing, I have found that the current code can get itself into a stuck state where it is constantly calling dumpI2c and looks to be self triggering errors. After reset, the issue goes away for a while. I am trying to find the cause. THis is the dump output that I keep getting: [E][esp32-hal-i2c.c:1103] dumpI2c(): loc=4 I added the loc just to indicate where the method was being called from in the code. Am I correct in understanding that the error field is actually indicating that the error is I2C_OK according to this enum? typedef enum { |
Will this work as an component, just trying to compile it now, getting loads of errors, must be missing something, thanks for the work ! :) |
Nothing has been done to specifically make this work as a component. There are 6 files that are involved. The Wire.h/.cpp, the esp32-hal-i2c.c/.h and the i2c_reg.h/i2c_struct.h deep under tools. |
@stickbreaker I noticed that you set the timeout to 400000 from 1048575 but also that you were commenting on another issue about timing. Is there a reason you found that prevents using the longer timeout? I ask because I am getting timeouts as my most regular error. |
Sorry, am new to all this .... just tried using the entire thing as a component and tried just copying the 6 new files in..... loads of compile errors for example C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:585:2: error: unknown type name 'I2C_COMMAND_t' Any ideas? ta |
I saws these errors when I didn't have the updated files i2c_reg.h/i2c_struct.h in place deep under tools.../soc/soc. |
Thanks, but definitely have the right files there - created a new project and copied the fork in, also did it with just copying the 6 new files, same errors :( Not my day ! ha |
@stickbreaker Part of the issue that I am seeing is due to a WiFi/Wire conflict. Due to another known WiFi problem, an AUTH_FAIL is happening on the WiFi connection and once that happens, there are no issues at all with the Wire code running, it runs and runs. So we may have to investigate interrupt priorities between the WiFi code and Wire code. |
@lonerzzz on your dump post, Decoding the dq(dataQueue) record come out as a // from esp32-hal-i2c.h
// i2c_event bits
#define EVENT_ERROR_NAK (BIT(0))
#define EVENT_ERROR (BIT(1))
#define EVENT_RUNNING (BIT(3))
#define EVENT_DONE (BIT(4))
#define EVENT_IN_END (BIT(5))
#define EVENT_ERROR_PREV (BIT(6))
#define EVENT_ERROR_TIMEOUT (BIT(7))
#define EVENT_ERROR_ARBITRATION (BIT(8))
#define EVENT_ERROR_DATA_NAK (BIT(9)) I'll have to Chuck. |
@lonerzzz WiFi is on Core 0 and I2C is on Core 1. Their interrupts should not interact? |
@stickbreaker I am running with multiple threads, but not multiple masters. One thread has the I2C access so I would be surprised with seeing arbitration errors. However, based on my last post, the WiFi may be the source of the problem. |
@me-no-dev It is strange but I have reproduced it three times. Once I get the AUTH_FAIL, there are no longer any I2C errors and communication is quite clean. I am running multiple threads so I will try locking my thread using I2C onto Core1. |
@lonerzzz are we seeing a WiFi power injection into the i2c bus? |
@stickbreaker Going to try locking I2C onto Core 1 and see if the problem persists. If it does then we have cross talk at some level. |
@lonerzzz do you have a 'scope? What are your pullup values? |
@stickbreaker Using a scope and my edges are quite clean. However, I am noticing a difference in the variance of low time duration when WiFi is working. I am using an I2C driver to keep the transitions fast. |
@PhilColbert Just those six files, Verify their locations. Base on you error messages, I would say that |
@pieterlagrange That's good to hear. Chuck. |
Looks like it is working for everyone that has tried it. I'm closings this issue. If you have any more comments open an issue in stickbreaker/arduino-esp32 Chuck. |
i2c known issue: espressif/arduino-esp32#839
Why is this issue closed? It's not fixed in this repo, is it? With https://github.com/stickbreaker/arduino-esp32 I am able to reliable read my RTC, but without it only works for a short time. |
This Issue was opened to introduce my fork. It was closed after multiple comments that confirmed my fork was functional and useful. @me-no-dev was working on adapting my fork to merge, My coding style differs greatly from the 'standard' for this repo. I haven't heard any response from @me-no-dev since Late November 2017 or Early December 2017. I haven't merged any of my new work into my fork since he stated he was working on a merge. I don't know what the current status of this repo is, nothing has been added, changed in almost a month. I'm kind of worried about the silence. Chuck. |
Chuck--
What is the name of your branch?
David
…On Sat, Jan 13, 2018 at 11:58 AM, chuck todd ***@***.***> wrote:
This Issue was opened to introduce my fork. It was closed after multiple
comments that confirmed my fork was functional and useful. @me-no-dev
<https://github.com/me-no-dev> was working on adapting my fork to merge,
My coding style differs greatly from the 'standard' for this repo. I
haven't heard any response from @me-no-dev <https://github.com/me-no-dev>
since Late November 2017 or Early December 2017. I haven't merged any of my
new work into my fork since he stated he was working on a merge.
I don't know what the current status of this repo is, nothing has been
added, changed in almost a month.
I'm kind of worried about the silence.
Chuck.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#839 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAg4SosyxBOrMbqG8CsBSohzApanqQWMks5tKQragaJpZM4Qg1h7>
.
|
@dpharris my fork is at stickbreaker/arduino-esp32 It is almost up to date with main branch. My last sync was ~ 28Nov2017 Chuck. |
Thanks!
David
…On Sat, Jan 13, 2018 at 2:00 PM, chuck todd ***@***.***> wrote:
@dpharris <https://github.com/dpharris> my fork is at
stickbreaker/arduino-esp32 <https://github.com/stickbreaker/arduino-esp32>
It is almost up to date with main branch. My last sync was ~ 28Nov2017
Chuck.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#839 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAg4Sp2HxfFUiYdzOVAKr-FmwVtMWqwqks5tKSdzgaJpZM4Qg1h7>
.
|
Hi! Using esp32 arduino framework through platformio. I have BME280 and MLX90614 connected. And have the same i2c issues - I2C timeout after ~1 minute of readings. I replaced the files from stickbreaker's fork and merged them into platformio's esp32 arduino framework directory. And it got worse, here are the log file: Perhaps the problem is with the platformio's esp32 arduino framework fork, which can't be used together with stickbreaker's updated I2C files? |
Ok, my code uses esp-now and if I disable everything related to esp-now, it works with stickbreaker's updated files. Have to see how long and then will try to find out where the problem lies in relation to the esp-now or the code I use for esp-now functionality. |
@stickbreaker had to move away from Arduino to finish some other tasks |
Just wanted to inform everyone that yesterday I tried to compile with Platformio's platform = https://github.com/platformio/platform-espressif32.git#feature/stage And it seems that with the latest Platformio's Arduino fork (without stickbreaker's mods) the I2C with my BME280 and MLX90614 parallel readings is working without problems! |
I tried using stickbreaker's mods (5 files) with an mpu6050. But getting the following error. I am assuming the error is the same as earlier, but with more debug logs by @stickbreaker .
|
Do you know how long your device is expected to take? You are getting a timeout so either your device is not responding at all or is responding after the 50ms timeout period. If it is a longer response time that you need, that can be set in the API. If not, you need to use a scope and test to see if something is getting sent. |
Hi All... I will be more than happy to get some help in this issue... a code sample how to read from the MLX90614 will be very helpful. Eldad |
Hi kristsm... maybe you can help in this issue ? (I am using ESP-IDF)... |
@lonerzzz , The device is an MPU6050. The delay isn't 50 ms . |
If you cannot figure it out, try inserting a low value resistor (100 to 300 ohms) in series on the SDA line, one on the MPU and the other on the ESP with the pull-up on the main line between . Then use your scope and capture the signal on either side of the resistor for each of the MPU and ESP. Where you see a small voltage difference is the device that is pulling low. Only the pull-up should be pulling the line up. |
oops. This involves cutting a trace within the inner layer of a PCB. I couldn't find another way to do this. Checking the scope shows that the MPU respond within few milliseconds. Could the delay be the issue ? I can increase the timeout in API. Do you know which API that is ? |
setTimeout is what you need. Cutting a trace is often possible too because you can scrape the solder resist off on either side of the cut and solder in a surface mount capacitor if you have some tweezers to hold it while you solder. When done, you can put a 0 ohm jump in place of your resistor. |
@lonerzzz off subject, I think we (@ESP32DE and I) have solved the Spikes on SDA and SCL check out this PR on my fork proposal to i2c @echoGee the timeout function is
This error, the i2c hardware has either detected a START from another master (SDA going low while SCL is high) or a prior communication has stalled with the Slave device extending SCL (holding it low) to delay communications while the Slave is processing the last command. The ESP32's i2c hardware has a maximum timeout of about 13.1ms between events(can't be sure, but at least between entire bytes, but hopefully between bits too).
This part of the debug output show that:
To attempt to recover you should call Chuck. |
@stickbreaker thanks for the work you put into this. What do you think the future of the I2C on the arduino-esp32 will be? Maintaining your separate repo and keeping it up to date will cause you a lot of work and leaves the original issue in place (in the esspressif's repo). I've read through the relevant threads to educate myself on the issue an it seem like I2C without your changes is very problematic right now. Am I missing something? Is this some small niche problem that only a couple of us have? Thanks for the answers! |
@balzss I don't know what is going to happen, I created my fork because I believe that an interrupt driven I2C subsystem is better than a polling system. I thought if that my proof of concept actually worked, it would be incorporated into the main branch. I don't plan on maintaining a separate fork. I am working on SLAVE mode, I hope that by the time I complete it the main branch be using a interrupt based driver. As you have concluded, the main branch's i2c is very unstable. Since it handles repeated STARTS (ReSTART) operations in a lackadaisical manner, The success of i2c operations is highly dependent on the how the app is coded. For Example: Serial.begin(9600); // slow baud, just to delay execution
uint16_t addr=0x1234; // memory address for 24lc512 EEPROM
Wire.beginTransmission(i2cDev);
Wire.write( highByte(add)r);
Wire.write(lowByte(addr));
Wire.endTransmission(false); // ReStart, instead of STop
Serial.println("I have just set the internal address pointer of my 24lc512 EEProm to 0x1234,"
" and now I am uselessly delaying between execution of a 'repeated Start' operation and the "
"Next I2c command,\n"
" if this delay between the endTransaction() and requestFrom() exeeds 13.1 ms the I2C"
" hardware will fall into a timeout cascade error that should not happen");
Wire.requestfrom(i2cDev,20);
while(Wire.available()){
Serial.print(Wire.read(),DEC);
Serial.print(' ');
}
Serial.println(); This code should work without any issues; But, because I fill up the Serial output buffer, and I am sending data slowly (960 characters a second) the println() call hangs until it can queue all of it's data. since this wait will exceed 13.1ms the i2c hardware will fail into a timeout Cascade. My fork correctly handles ReSTART operations. My fork was branched November 2017, It is falling behind I don't know if I should level up or let it wither away. Currently it can be manually merged by overwriting about five files. So it is not much of a hardship begin on the main branch. If you need functional I2C you just manually replace the I2C subsystem with my files. Chuck. |
I was having I2C stability issues in my current ESP32 project, but this rewrite solved them. |
With latest Arduino-Core + 'stickbreaker's 5 file fork' I2C is pretty much unusable. The last 0-3 bytes of reads are sometimes (often enough) garbage or 0xff. I will dig into it deeper tomorrow. Just wanted to note it somewhere, just in case ... |
@everslick Are you getting any error messages? Do you have Debug level set to at least Error? Yours is the first report of problems. Chuck. |
Not yet, I only synced my fork with latest code from you and me-no-dev
yesterday and my at24c32 stopped doing anything useful. But I cannot rule
out an error on my side ither at this point in time. I will report back
either way.
|
I have published a rewrite of the I2C subsystem that should solve the I2C stability Issues, It is NOT a drop-in replacement for
Wire()
. There are hardware differences between the AVR and ESP32 that require a different strategy.It is a ALPHA TEST version, DO NOT use it in ANY PRODUCTION ENVIRONMENT.
To try it you can simply download the zip and OVERWRITE your arduino-esp32 sdk.
This fork is base off of espressif/master from 11/11/2017
It changes:
cores\esp32\esp32-hal-i2c.c
cores\esp32\esp32-hal-i2c.h
tools\sdk\include\soc\soc\i2c_reg.h
tools\sdk\include\soc\soc\i2c_struct.h
libraries\Wire\src\Wire.cpp
libraries\Wire\src\Wire.h
README.md
stickbreaker:arduino-esp32
Chuck.
The text was updated successfully, but these errors were encountered: