TWI.C is blocking other interrupts while waiting for stop conditions

[andreobi]

Hi,
this is not a real error, but I believe it is an improvement.
I would like to propose the following change: insert here an sei() would enable other interrupts while waiting for the stop condtion. Typicly it wouldn't hurt just to wait a few us but having an I2C chip which uses clock stretching like the BNO055 could cause an interupt blocking for more than 100us. Please consider my proposal. The risk to do the sei is when you use the twi inside an interrupt. so it could come to an dead lock because the new write would wait for the TWI_READY.

void twi_stop(void)
{
  // send stop condition
  TWCR = _BV(TWEN) | _BV(TWIE) | _BV(TWEA) | _BV(TWINT) | _BV(TWSTO);

// change!!
  sei();
//
  // wait for stop condition to be exectued on bus
  // TWINT is not set after a stop condition!
  while(TWCR & _BV(TWSTO)){
    continue;
  }

  // update twi state
  twi_state = TWI_READY;
}

If you are interested, I made a lot of changes to the I2C section. My solution will only work as a master (but could also expanded to a slave), couldn't be used in an interrupt, but the good things are it doesn't need any aditional buffer, has an simpler interface:

    void	begin(void);
	void	setClock(uint32_t speed);
	void	setWaitMode(uint8_t mode); // wait after write or just return and let the interupt state machine do the job
	bool	getSendActive(void); // check if the write job is done
	uint8_t	getError(void);
    uint8_t	sendTo(uint8_t chipAdr, uint8_t regAdr, uint8_t *txBuffer, uint8_t length=1, bool stop=true);	// max length =254!!!
	uint8_t	requestFrom(uint8_t chipAdr, uint8_t regAdr, uint8_t *rxBuffer, uint8_t length=1, bool adrStop=true, bool stop=true);

and is in some parts faster.
regards
Andre

[matthijskooijman]

I've transfered this issue to the ArduinoCore-avr repository, since it is about the AVR-specific Wire library included with this core.

This request actually ties in with a discussion about adding timeouts to the Wire library that has recently flared up again, see #42 and #107. Your input there might also be useful.

In particular, the approach you mention has briefly been mentioned in this discussion as well: #107 (comment)

Typically, re-enabling interrupts inside an ISR is considered fragile and tricky to get right. I take it you've been using this approach for a while and it works well? Any insights to share about how and why this works reliably?