Comments (4)
From @stickbreaker on August 19, 2015 1:47
I am not familiar with the DUE, but Shouldn't the I2C HW be stretching SCL, This should prohibit any new transaction before the software has completed handling the current transaction?
Chuck
from arduinocore-sam.
From @shahokun on August 19, 2015 3:47
Clock stretching in general is not necessarily an issue. In this case, however, the HW is stretching the clock for a Master Read operation, but the software incorrectly thinks it's in a Slave Send/Master Write operation and does not respond correctly. The clock is stretched indefinitely and the transaction never completes.
from arduinocore-sam.
From @stickbreaker on August 19, 2015 4:29
Ok, I think I see what you have explained. You getting an slave read interrupt while processing a slave write command.
It sounds reasonable to reconfigure before switching modes.
Chuck.
from arduinocore-sam.
From @shahokun on August 20, 2015 20:36
Testing showed that the proposed solution above did not fix the issue. After more debugging, it seems that the problem is simpler: the Master continues onto the next I2C transaction before the Slave (Arduino) has a chance to respond to the end of the first I2C transaction. This may be because the Slave is busy in a disable interrupts block, or some other rare sequence of events. The race condition goes something like this:
- The Master sends a Write operation. The Slave has
status = SLAVE_RECV
. - The I2C transaction completes. The Slave gets an interrupt for the I2C completion, but is busy elsewhere and does not service it for a bit.
- The Master continues on to the next I2C transaction. This time, it is a Read operation. The Slave still has
status = SLAVE_RECV
(Master Write), but the Arduino TWI hardware thinks it is handling a Master Read operation, and the TWI status register reflects as much. - The Slave services the I2C interrupt. Now when it reads the TWI status register (
uint32_t sr = TWI_GetStatus(twi);
), it indicates the transaction is not complete, and is in Master Read mode. - The Arduino TWI hardware stretches the clock until the slave responds, i.e. calls TWI_WriteByte(). However, since the Slave is in
status = SLAVE_RECV
, this never happens. Meanwhile, the I2C transaction never completes, freeing the Slave to switch modes, because the clock is stretched.
The root cause of this issue is that the Wire library assumes that the status register follows some predictable sequence of events. However, because it is interrupt driven, the hardware status register can change state before the software has a chance to poll the status. Essentially, the Wire driver should not assume that just because it is currently in the SLAVE_RECV state that it will stay that way. I would propose changing the Slave driver in Wire.cpp to something like this:
void TwoWire::onService(void) {
// Retrieve interrupt status
uint32_t sr = TWI_GetStatus(twi);
if (status == SLAVE_IDLE && TWI_STATUS_SVACC(sr)) {
TWI_DisableIt(twi, TWI_IDR_SVACC);
TWI_EnableIt(twi, TWI_IER_RXRDY | TWI_IER_GACC | TWI_IER_NACK
| TWI_IER_EOSACC | TWI_IER_SCL_WS | TWI_IER_TXCOMP);
srvBufferLength = 0;
srvBufferIndex = 0;
// Detect if we should go into RECV or SEND status
// SVREAD==1 means *master* reading -> SLAVE_SEND
if (!TWI_STATUS_SVREAD(sr)) {
status = SLAVE_RECV;
} else {
status = SLAVE_SEND;
// Alert calling program to generate a response ASAP
if (onRequestCallback)
onRequestCallback();
else
// create a default 1-byte response
write((uint8_t) 0);
}
}
if (status != SLAVE_IDLE) {
if (TWI_STATUS_TXCOMP(sr) && TWI_STATUS_EOSACC(sr)) {
if (status == SLAVE_RECV && onReceiveCallback) {
// Copy data into rxBuffer
// (allows to receive another packet while the
// user program reads actual data)
for (uint8_t i = 0; i < srvBufferLength; ++i)
rxBuffer[i] = srvBuffer[i];
rxBufferIndex = 0;
rxBufferLength = srvBufferLength;
// Alert calling program
onReceiveCallback( rxBufferLength);
}
// Transfer completed
//TWI_DisableIt(twi, TWI_IDR_RXRDY | TWI_IDR_GACC | TWI_IDR_NACK
// | TWI_IDR_EOSACC | TWI_IDR_SCL_WS | TWI_IER_TXCOMP);
//TWI_EnableIt(twi, TWI_SR_SVACC);
//status = SLAVE_IDLE;
// EDIT: Reversed order of operations to prevent nested
// interrupt triggering.
// This did not appear to fix the issue so may be unneeded.
status = SLAVE_IDLE;
// Transfer completed
TWI_DisableIt(twi, TWI_IDR_RXRDY | TWI_IDR_GACC | TWI_IDR_NACK
| TWI_IDR_EOSACC | TWI_IDR_SCL_WS | TWI_IER_TXCOMP);
TWI_EnableIt(twi, TWI_SR_SVACC);
}
}
if (status == SLAVE_RECV) {
if (TWI_STATUS_RXRDY(sr)) {
if (srvBufferLength < BUFFER_LENGTH)
srvBuffer[srvBufferLength++] = TWI_ReadByte(twi);
}
// EDIT: Write a dummy byte if the TWI peripheral ends up in a
// different mode than you expected, so it doesn't get stuck indefinitely.
else if (TWI_STATUS_SVREAD(sr) == 1 && TWI_STATUS_TXRDY(sr) && !TWI_STATUS_NACK(sr)) {
TWI_WriteByte(twi, 0);
}
}
if (status == SLAVE_SEND) {
if (TWI_STATUS_TXRDY(sr) && !TWI_STATUS_NACK(sr)) {
uint8_t c = 'x';
if (srvBufferIndex < srvBufferLength)
c = srvBuffer[srvBufferIndex++];
TWI_WriteByte(twi, c);
}
// EDIT: Read a dummy byte if the TWI peripheral ends up in a
// different mode than you expected, so it doesn't get stuck indefinitely.
else if (TWI_STATUS_SVREAD(sr) == 0 && TWI_STATUS_RXRDY(sr)) {
TWI_ReadByte(twi);
}
}
}
This may not be the best solution depending on how you want to handle I2C error conditions. However, I think that this issue should be at least be warned about in the Wire documentation. Yes, it results because of a combination of the Master sending I2C messages too often, and the Slave not handling I2C interrupts quickly enough, but the software driver should not be able to get stuck in this unsynchronized state.
from arduinocore-sam.
Related Issues (20)
- Serial3.setTimeout(...) is not expiring with the rx line always idle.
- Enabling ADC 14, 15 on Due
- If interrupt attached to 2 or more pins of a port, detachInterrupt() doesn't work correctly HOT 1
- Print/Stream API is old. availableForWrite is missing HOT 1
- Serial.readBytesUntil() is either misleading or buggy
- Wire does not play nicely with Adafruit I2C FRAM breakout board HOT 1
- IPAddress missing definition of operator!=()
- adc_set_resolution bug
- adc_configure_trigger bug
- Support for USART2? HOT 1
- HID.sendReport - Stack overflow
- Offset millis count "_dwTickCount" to desired value for runtime retentation from eeprom HOT 2
- _sbrk has no bounds checking HOT 2
- Wire.setWireTimeout() not supported SAM3X
- Insufficient initialization in Serial1.begin()
- No Support for C++ Standard Libraries HOT 1
- Hope to support at least C++14 soon
- <iostream> not usable
- std::locale [con/de]structor undefined
- Unexpected extra characters appeared in the IDE Serial Monitor when write to Serial in loop()
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from arduinocore-sam.