twi can be blocking in case of not correctly functioning I2C device

[jantje]

I have had multiple compass modules (HMC5883) which get confused after some time of running. Resetting the arduino made the arduino to stop working from time to time.
This article helped me to confirm that twi can get in an endless loop. http://www.megunolink.com/how-to-detect-lockups-using-the-arduino-watchdog/
Basically I compiled my code with a watchdog which dumped the address and then avr-objdump is used to find the watchdog triggering code.
This method (details below) show that following code is running in an endless loop.

  // wait for write operation to complete
  while(wait && (TWI_MTX == twi_state)){
    continue;
  }

I changed the code to this

void delayMicroseconds(unsigned int us); //declare this method from wiring.c
....
...
  uint8_t counter=0;
  // wait for write operation to complete
  while(wait && (TWI_MTX == twi_state)){
    delayMicroseconds(100);
    if(counter++>10)
        {
        //TODO write correct error handling here
        return 5;//error timing out
        }
  }

But as the code states there is some extra error handling needed as otherwise all consequent wire actions fail. I'm also not confident that the timing used is even closely correct as at the time of writing no I2C devices worked in my test setup.

More details on how I know the twi code is the problem
This is objdump of the code
// wait for write operation to complete
while(wait && (TWI_MTX == twi_state)){
4920: 22 23 and r18, r18
4922: 21 f0 breq .+8 ; 0x492c <twi_writeTo+0x78>
4924: 80 91 ab 0d lds r24, 0x0DAB
4928: 82 30 cpi r24, 0x02 ; 2
492a: e1 f3 breq .-8 ; 0x4924 <twi_writeTo+0x70>
continue;
}
This is the report generated by the code mentioned above

Application Monitor

Saved reports: 10
Next report: 7
0: word-address=0x2492: byte-address=0x4924, data=0x0
1: word-address=0x2494: byte-address=0x4928, data=0x0
2: word-address=0x2494: byte-address=0x4928, data=0x0
3: word-address=0x2494: byte-address=0x4928, data=0x0
4: word-address=0x2495: byte-address=0x492A, data=0x0
5: word-address=0x2494: byte-address=0x4928, data=0x0
6: word-address=0x2494: byte-address=0x4928, data=0x0
7: word-address=0x2492: byte-address=0x4924, data=0x0
8: word-address=0x2494: byte-address=0x4928, data=0x0
9: word-address=0x2492: byte-address=0x4924, data=0x0

[jantje]

I found why the I2C bus got messed up. More reads from a I2C device than the device can handle made the device go in a bad state. Subsequent calls made the twi hang.
Still I think a graceful exit would be nice
The code I ended up with allows for other I2C devices to continue working -in this particular situation- and is as follows.

void delayMicroseconds(unsigned int us);
uint8_t twi_writeTo(uint8_t address, uint8_t* data, uint8_t length, uint8_t wait, uint8_t sendStop)
{
...
  uint16_t counter=0;
  // wait for write operation to complete
  while(wait && (TWI_MTX == twi_state)){
    delayMicroseconds(10);
    if(counter++>2000)
        {
        //TODO write correct error handling here
        twi_releaseBus();
        return 5;//error timing out
        }
  }

I know there are more potential endless loops in twi but I don't feel confident to handle them.

[jantje]

I guess I should have closed this one a long time ago

[ermtl]

It's been 9 [expletive deleted] YEARS since that bug was first discussed (2011), countless people pulled their hair trying to understand why their Arduino was freezing, why would it work normally, then suddenly stop, a dozen of times, it's been raised in here, dozens of times people had been told to get lost, use something else, etc ...
#7328
#2418
#1842
#5249
arduino/ArduinoCore-avr#42
Google shows pages after pages of people getting started and who get discouraged by this elusive bug they don't understand. This runs totally contrary of what arduino is supposed to stand for.

If the fix was difficult, if it compromised compatibility or added other problem, it would be understandable, but it's not the case, all that [expletive deleted] is caused by those 2 damn lines of code that obviously can create an infinite loop if for some reason the read operation does not complete :

// wait for read operation to complete
while(TWI_MRX == twi_state){
continue;
}

Yes, I know, this state is not supposed to happen according to the I2C protocol, but guess what ? electrical glitches didn't get the memo.

Countless people, after losing hours or days kind of solved the issue either by making a modified version for themselves, or switching to another library, so there are several implementations of a timeout that are simple and would easily solve the issue.

But arduino developpers stubbornly refuse to fix it !

You think I'm rude ? After 9 years of giving the finger on this issue to the whole community for absolutely no reason, I couldn't care less.

How long before you close it again ?
Just be honest, and put a "[expletive deleted] you all, WONTFIX" label on it ...

[jantje]

@per1234 you marked this issue as a duplicate but didn't mention the "open" issue to track this problem.

[per1234]

Duplicate of arduino/ArduinoCore-avr#42