I2C ReWrite

[stickbreaker]

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.

[me-no-dev]

@stickbreaker why not PR this so we can just switch to it and give it a go?

[stickbreaker]

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.

[everslick]

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.

[stickbreaker]

@everslick Yes they do, in certain cases, If an Arduino sketch or library uses Wire() and Wire.endTransmission(false) and tests the return code: uint8_t err = Wire.endTransmission(false); if( err!=0) {//some Failure occurred just substituting my fix will trigger the if( err !=0 ) {. Because My code queues the i2c Write transaction until (maybe forever) the next i2c transaction that does NOT set sendStop=false; and returns and error code noting that queue: I2C_ERROR_CONTINUE==7;
The same response is generated by Wire.requestFrom(id,length,false);.

If the written code does not test the return codes(sloppy code), then most WRITE operations will work. But, the requestFrom(false) won't. Because NO data is transferred until the NEXT i2c transaction that sets sendStop=true;

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 Wire.endTransmission() is a failure code, That failure could have been generated by ANY of the queued commands. If the result code is success (I2c_ERROR_OK) all of the queued commands succeeded. My code is designed to allow inspection of each queued commands results, but, for simplicity I have not exposed it through the Wire() library.
After this review and it passes I will add this 'feature'.

Chuck.

[everslick]

@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?

[me-no-dev]

@everslick I have started going through the new code and brainstorming on how we can make it all work ;) we might have success!

[stickbreaker]

@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 Wire library?

Chuck.

[me-no-dev]

@stickbreaker here is what I have vaguely on my mind: Rework your code to expose a functions like i2c_err_t i2cTransmission(uint8_t addr, const uint8_t * in, size_t inLen, uint8_t * out, size_t outLen) which can then be plugged into the Wire wrapper and execute either at entTransmission(true) or requestFrom(len).

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

[lonerzzz]

@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 [E][esp32-hal-i2c.c:1104] dumpI2c(): i2c=0x3ffc10cc [E][esp32-hal-i2c.c:1105] dumpI2c(): dev=0x60013000 [E][esp32-hal-i2c.c:1106] dumpI2c(): lock=0x3ffd142c [E][esp32-hal-i2c.c:1107] dumpI2c(): num=0 [E][esp32-hal-i2c.c:1108] dumpI2c(): mode=1 [E][esp32-hal-i2c.c:1109] dumpI2c(): stage=3 [E][esp32-hal-i2c.c:1110] dumpI2c(): error=1 [E][esp32-hal-i2c.c:1111] dumpI2c(): event=0x3ffe317c bits=110 [E][esp32-hal-i2c.c:1112] dumpI2c(): intr_handle=0x3ffe3598 [E][esp32-hal-i2c.c:1113] dumpI2c(): dq=0x3ffe3f78 [E][esp32-hal-i2c.c:1114] dumpI2c(): queueCount=1 [E][esp32-hal-i2c.c:1115] dumpI2c(): queuePos=0 [E][esp32-hal-i2c.c:1116] dumpI2c(): byteCnt=2 [E][esp32-hal-i2c.c:1121] dumpI2c(): [0] 67 R STOP buf@=0x3ffd58a8, len=1, pos=1, eventH=0x0 bits=0

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 { // I2C_NONE=0, I2C_OK=1, I2C_ERROR, I2C_ADDR_NAK, I2C_DATA_NAK, I2C_ARBITRATION, I2C_TIMEOUT }I2C_ERROR_t;

[PhilColbert]

Will this work as an component, just trying to compile it now, getting loads of errors, must be missing something, thanks for the work ! :)

[lonerzzz]

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.

[lonerzzz]

@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.

[PhilColbert]

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' I2C_COMMAND_t cmdx; ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:586:7: error: request for member 'val' in something not a structure or union cmdx.val =i2c->dev->command[cmdIdx-1].val; ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:587:10: error: request for member 'byte_num' in something not a structure or union if(cmdx.byte_num>1){ ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:588:7: error: request for member 'byte_num' in something not a structure or union cmdx.byte_num--; ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:589:41: error: request for member 'val' in something not a structure or union i2c->dev->command[cmdIdx-1].val = cmdx.val; ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:592:7: error: request for member 'val' in something not a structure or union cmdx.val =i2c->dev->command[cmdIdx-2].val; ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:593:7: error: request for member 'byte_num' in something not a structure or union cmdx.byte_num--; ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:594:41: error: request for member 'val' in something not a structure or union i2c->dev->command[cmdIdx-2].val = cmdx.val; ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:596:6: error: request for member 'byte_num' in something not a structure or union cmdx.byte_num=1; ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:597:40: error: request for member 'val' in something not a structure or union i2c->dev->command[cmdIdx++].val = cmdx.val; ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:585:16: warning: variable 'cmdx' set but not used [-Wunused-but-set-variable] I2C_COMMAND_t cmdx;

Any ideas?

ta

[lonerzzz]

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.

[PhilColbert]

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

[lonerzzz]

@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.

[stickbreaker]

@lonerzzz on your dump post, The error is described in the EventGroup 0x110, which decodes down to EVENT_DONE meaning ISR process is complete EVENT_ERROR_ARBITRATION, Are you testing in a multiMaster environment? I haven't done any MultiMaster testing, so Either I coded it wrong? or you have an arbitration failure.

Decoding the dq(dataQueue) record come out as a Wire.requestFrom(0x1B,1,true); does that fit with your actual code? The Error message said that during the the Data reception it had an arbitration failure? The ESP32 was receiving data How could it see an Arbitration Failure?

// 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.

[me-no-dev]

@lonerzzz WiFi is on Core 0 and I2C is on Core 1. Their interrupts should not interact?

[lonerzzz]

@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.

[lonerzzz]

@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.

[stickbreaker]

@lonerzzz are we seeing a WiFi power injection into the i2c bus?

[lonerzzz]

@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.

[stickbreaker]

@lonerzzz do you have a 'scope? What are your pullup values?

[lonerzzz]

@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.

[stickbreaker]

@PhilColbert Just those six files, Verify their locations. If you just substitute them that should be all you need to do.

Base on you error messages, I would say that i2c_struct.h is not in the correct location. The compile is using the original version without my debug revisions. Chuck.

[stickbreaker]

@lonerzzz can you place a dumpInts() where your loc=4 is? It will show the last 64 different interrupts that were serviced.

[stickbreaker]

@me-no-dev there are conditional portYIELD_FROM_ISR() after every xEventGroupSetBitsFromISR() so, if the WiFi auth is caught in a loop and it has a higher pri i2C could starve.
I do no know about interrupts between cores, I have not tried to pin the ISR to any specific core.

[lonerzzz]

@me-no-dev @stickbreaker Just pinned the I2C thread on the second core (not the WiFi core) and everything is good so this probably should be made explicit in the usage documentation when we get to that. It won't be a problem if someone is doing something simple with Arduino only but in my case I needed I2C and a CLI that needed to be serviced.

[stickbreaker]

@lonerzzz How did you Pin the ISR to Core1? I haven't gotten that deep into FreeRTOS. Was it in the ISR creation options?

[lonerzzz]

@stickbreaker No, it is a task option. Right now as @me-no-dev stated, the WiFi is on Core 0 and Arduino is on Core1. In my case, I have 4 tasks running WiFi + CLI + I2C + Arduino so the CLI and I2C were floating. To pin a task, you create it with xTaskCreatePinnedToCore. The main.cpp file under cores/esp32 shows a usage example.

[stickbreaker]

@lonerzzz on the timeout value=400000. I reduced it from max to encounter timeouts. I expected to never see any. With my queued structure, there should never be any prolonged delay. That timeOut is designed to detect a SLAVE stretching SCL. 400k at 80mhz is 5ms. Max TimeOut is about 13.1ms.

With your i2c transaction only being an requestFrom(id,1); all of the I2c commands and data are preloaded before the ISR is released, and all the ISR should ever have to do is:

//int hex,  desc, tx to fifo, rx from fifo
0x2, TX_EMPTY, 1, 0 /// add the ID byte
0x200, TransStart, 0,0 // basic housekeeping
0x40, byteMove, 0,0 // each byte, in or out is counted, queue index adjusted, tx_enabled if necessary
0x40, byteMove, 0,0 // each byte, in or out is counted, queue index adjusted, tx_enabled if necessary
0x80, STOP detected, 0,1 // the stop results in a forced rxFifo empty

I had read a warning about only being able to release an ISR handle from the core it originated on, and something about wandering execution. But it didn't sink in.

Chuck

[lonerzzz]

@stickbreaker The concern on timeout from my end was because of the longer processing intervals that I was seeing before locking the I2C task to another core. With your code in place, it is now rather unexciting and just works :) without issues. That 400000 timeout was in the code before so we can probably leave it.

[stickbreaker]

@me-no-dev

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

I have no reasonable Idea why Wire.endTransmission(false); exists, the only purpose is to hold ownership(arbitration) of the bus. I have never seen any hardware that requires a ReSTART after a read, only after a write.

I Understand the Idea of mutexlocks, but are you saying we need to atomize all Wire accesses?

I had envisioned a lower level. I can see were all of my Queue manipulations should be behind a lock. If they are correctly implemented, multiple threads could have queue entries coexist during one block of ISR service.

Locking Wire from beginTransmission() to (endTransmission(true) || (requestFrom(true)&&!available())) ?

I can see implementing a thread safe Queued version, but the simplistic Wire.beginTransmission() ... could create many, many opportunities for deadlocks.

The oneshot description is basically how this queue works, a single valid i2c operation is condensed from multiple parts and only executed when it is deterministic. No waiting for a STOP.

Chuck.

[stickbreaker]

@lonerzzz I REALLY REALLY ❤️ your:

With your code in place, it is now rather unexciting and just works :) without issues.

Chuck.

[lonerzzz]

@stickbreaker @me-no-dev I think that the HAL I2C code should be pushed into the idf device driver in the idf project as it is much more solid than what is there. Then we could migrate the HAL code to reference that driver. Since Arduino is the more simplified programming approach, we could put locks in it and allow anyone wanting deeper access to access the HAL layer.

[stickbreaker]

@lonerzzz That would be a more important place alright. But their current code is just different, for some reason they use a ringbuffer structure for Reads? I could never figure out where the ringBuffer was ever filled. It just magically appeared. I did not spend enough time to go thru their example code to understand the flow. It just appeared to me to be obfuscated for no reason.

I can see were a driver base on app local buffers would coexist with a multiprocess/processor model. The i2c bus would be happy as long as all transaction have a START -> STOP flow. But, alas, I can see were a START WRITE (ID)(addr) ReSTART READ (ID) STOP would need to be atomic.

There is another Issue I have been contemplating about 10bit access. I have not verified the H/W accurately follow the protocol. The READ 10bit sequence is a kind of odd ball. To allow co-existence with 7bit devices it is like this:

START WRITE(0b11110 a9 a8 0)(0b a7 a6 a5 a4 a3 a2 a1 a0) ReStart READ(0b11110 a9 a8 1) (read data) STOP

When I get a chance Slave Mode is my next project, with 10bit support hopefully.

Chuck.

[lonerzzz]

@stickbreaker I know the code is quite different, but once you have your code cleaned up, we should still try to float it with the IDF folks. They may be able to use your code as is to replace what is there now or work with us to align it while keeping your capabilities in place. It is really a question of how tied they are to their code base. Since they still cannot demonstrate the stability that you have achieved, they can definitely make use of it.

[stickbreaker]

Well, before we submit it to IDF It needs a lot of cleaning, and Locking. There is no code to support SlaveMode co-existence, locking around queue maintenance, secondary I2C1 hardware, bus busy.

My current ISR assumes all the dq entries are atomic, and will cascade the first error through all successive entries. If the ISR needs to support multiple processes( or tasks) maybe each dq entry needs some affinity value so that only dq entries from the same tasks receives the error cascade. I'm thinking about NAK's mainly. But, should the ISR retry a Arbitration failure? By definition it is not a permanent error? An address NAK can mean "I'm busy" or "I'm Dead", an Arbitration just means try again.

How smart does the ISR need to be?

Chuck.

[lonerzzz]

@stickbreaker I wouldn't assume the ISR needs to be smart since that is likely to create more compatability issues than it helps. We would be better off leaving the Wire implementation simple with locks to protect those wanting Arduino simplicity while allowing the flexibility in the HAL and providing some documentation guidance. Anyone who gets into the issues you are describing will have to set things up according to their application needs which I doubt we can predict.

As for retries and the like, I think bubbling up such errors to the application layers lets the user handle these scenarios according to their requirements. If, over time, we find a need for additional handling in the I2C layer, it can still be added. I just wouldn't try to handle too much only to find it breaks someone's use case. My 2 cents.

[stickbreaker]

@lonerzzz That sounds about right. I've implemented a tinyFAT file system on i2c EEPROMs currently it works with AVR Arduino's 32byte buffers, I'm thinking about what I would like under the ESP32, with it's larger RAM, bigger blocks are easier to handle, the WritePage issues of different EEPROMs is a headache. I'm tossing around the idea of a library that handles it without App intervention.

• Segments the Writes into the Writepage blocks
• Program each block with NAK polled Program Cycle timing

Since you have looked at the code, What do you think about my idea of using EventGroup bits to propagate error status for each dq element? There is a NULL parameter on the addQueue() functions that will contain a EventGroup handle. So each element is independent and could possibly have a task hanging in an xEventGroupWaitBits() for the ISR to transact its buffer.

Chuck.

[PhilColbert]

Sorry to be slow on this, compiling it as a component, you need to move the i2c_struct.h and i2c_reg.h into the esp-idf compent directory - obviously ....

compiling from command line brings back all these new errors :-1:

any ideas?

CC build/arduino/cores/esp32/esp32-hal-i2c.o C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c: In function 'fillFifo': C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:545:2: error: value computed is not used [-Werror=unused-value] index++; ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c: In function 'pollI2cRead': C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:757:19: error: pointer targets in passing argument 2 of 'emptyFifo' differ in signedness [-Werror=pointer-sign] emptyFifo(i2c,data,len,&index); ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:522:6: note: expected 'char ' but argument is of type 'uint8_t {aka unsigned char }' void emptyFifo(i2c_t i2c,char data, uint16_t len,uint16_t index){ ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:799:18: error: pointer targets in passing argument 2 of 'emptyFifo' differ in signedness [-Werror=pointer-sign] emptyFifo(i2c,data,len,&index); ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:522:6: note: expected 'char ' but argument is of type 'uint8_t {aka unsigned char }' void emptyFifo(i2c_t i2c,char data, uint16_t len,uint16_t index){ ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:804:18: error: pointer targets in passing argument 2 of 'emptyFifo' differ in signedness [-Werror=pointer-sign] emptyFifo(i2c,data,len,&index); ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:522:6: note: expected 'char ' but argument is of type 'uint8_t {aka unsigned char }' void emptyFifo(i2c_t i2c,char data, uint16_t len,uint16_t* index){ ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:654:9: warning: unused variable 'blkSize' [-Wunused-variable] uint8_t blkSize = 0; ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:652:6: warning: variable 'abort' set but not used [-Wunused-but-set-variable] bool abort = false; ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c: In function 'i2c_isr_handler_default': C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:1316:12: warning: unused variable 'oldInt' [-Wunused-variable] uint32_t oldInt =activeInt; ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c: In function 'i2cProcQueue': C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:1506:12: warning: unused variable 'ret' [-Wunused-variable] uint32_t ret=xEventGroupClearBits(i2c->i2c_event, 0xFF); ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c: In function 'i2cReleaseISR': C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:1665:13: warning: unused variable 'error' [-Wunused-variable] esp_err_t error =i2c_isr_free(i2c->intr_handle); ^ C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c: In function 'i2cFreeQueue': C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:153:1: error: control reaches end of non-void function [-Werror=return-type] } ^ cc1.exe: some warnings being treated as errors make[1]: [/esp/esp-idf/make/component_wrapper.mk:243: cores/esp32/esp32-hal-i2c.o] Error 1 make: [C:/msys32/esp/esp-idf/make/project.mk:434: component-arduino-build] Error 2

[stickbreaker]

@PhilColbert where is your arduino sketch?

This i2c rewrite is of the Arduino-esp32 Wire.h library.

It is designed to be compiled by the Arduino IDE.

#included <Wire.h>

void setup(){
Serial.begin(115200);
Wire.begin();
Serial.println("Hello World");
// is there an i2c EEPROM at 0x50
Wire.beginTransmission(0x50);
uint8_t err=Wire.endTransmission();
if(err==0) Serial.println("I found an I2C device at address 0x50");
else {
  Serial.print("I'm lonely, no one to talk to.  The Wire library said ");
  Serial.println(err,DEC);
}

void loop(){}

just copy those 6 files into the correct places, Open the Arduino IDE. compile this sketch. Download it to your ESP32

Chuck.

[PhilColbert]

Hi,

I am using it as component instead of the Arduino IDE - I think make is more strict regarding compilations and has come back with errors.

Will try moving everything to the IDE, however be nice if it compiled for both ?

[stickbreaker]

@PhilColbert I'm just guessing here, but if you tried to compile Wire.cpp that should be the component. the esp-hal-i2c.c is just a support for Wire.cpp.

Chuck.

[PhilColbert]

Ok, but some of the errors make sense ?

for example :-1:

C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c: In function 'i2cFreeQueue': C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:153:1: error: control reaches end of non-void function [-Werror=return-type }

i2c_err_t i2cFreeQueue(i2c_t * i2c){ if(i2c->dq!=NULL){ // what about EventHandle? free(i2c->dq); i2c->dq = NULL; } i2c->queueCount=0; i2c->queuePos=0; }

There is no return value in this function ?

Could you humour me and take a look at the errors for a moment just to see if they are legit ? :)

thanks

[everslick]

Afaics, those are all warnings, but the compiler is instructed to treat them as errors by the -Werror compiler switch. If you could remove that switch from the idf-component build script you would be fine . In the long run the warnings should eventually be cleaned up.

[stickbreaker]

@PhilColbert That is a mistake. I missed it. Change it to

2c_err_t i2cFreeQueue(i2c_t * i2c){
i2c_err_t rc=I2C_ERROR_OK;
if(i2c->dq!=NULL){
  // what about EventHandle?
  free(i2c->dq);
  i2c->dq = NULL;
  }
i2c->queueCount=0;
i2c->queuePos=0;
return rc;
}

as @everslick pointed out, sloppy coding on my part. I rushed this out while people were still having i2c issues. As we(or probably I) polish it, these ruff spots should disappear.

Chuck.

[PhilColbert]

Dont apologise ! :) its amazing people are helping fix this issue ... hoping it will fix my issues!

Theres this one aswell which should fix and let it compile :-1:

C:/msys32/esp/esp-idf/AirWhereVarioN/components/arduino/cores/esp32/esp32-hal-i2c.c:804:18: error: pointer targets in passing argument 2 of 'emptyFifo' differ in signedness [-Werror=pointer-sign] emptyFifo(i2c,data,len,&index);

[stickbreaker]

@PhilColbert that one is because the data parameter is defined as uint8_t * and the second parameter of emptyFifo() is defined as char *. the warning is accurate, but in this case inconsequential to the code. Before this fork is merged, all of those warning must be addressed. Also that section of code will be dropped out. It was a test case to understand the i2c StateMachine through a non-interrupt 'Polled' operation. It is left in 'just incase'.

Chuck.

[lonerzzz]

@stickbreaker Regarding the use of the EventGroup for each dq, I am curious of the use case that you are seeing for it. Are you thinking of some sort of dynamic event wiring for triggering or some sort of multiple producer/consumer model. I have other MCUs in a product that i am working on with this capability but have only had limited use cases to apply them to.