How to properly use Wire.onReceive

[trullock]

I've got a stream of mesasges coming into an I2C slave from its master.

How do I properly resolve the below issues?

If I do:

Wire.onReceive(handler);

void handler(int howMany) {
    byte message = Wire.read();
}

what should I do with message?
I shouldn't process it in the ISR in order to keep the interrupt slim and fast.
If I instead do:

volatile byte message;

and then set this from the ISR and read it inside loop(), I might not loop fast enough to read quickly incoming messages - thus losing messages.

So instead I can create a buffer, and push to it from inside the ISR, and then read from loop() but thats even more susceptible to race conditions - unless I disable interrupts whilst i take a local copy of the buffer before reading it - but with interrupts off I'll miss messages.

Wire has an internal rx buffer, so I'd have thought I could just call Wire.available() and Wire.read() from inside my loop, but the implementation of Wire.cpp states:

// must be called in:
// slave rx event callback
// or after requestFrom(address, numBytes)
int TwoWire::available(void);

So, whats the right way to do this? How do I handle/enqueue fast incoming i2c messages and process them all without dropping data?

Assume i can process messages faster than my rx buffer will fill up, messages will just come in bursts, so no need to care about overflows necessarilly

Thanks

[Koepel]

Is the Master also an Arduino board and did you write the sketch for the Master ?
I hope you don't send text with variable length over I2C, that is often not needed.

Yes, you can call Wire.available() in the loop() and no, that is not allowed. Well, it works, but you are not supposed to do that. I put that on top of my page "Tricks that are (almost) allowed".

You need a volatile global variable to store the data in and you need a global volatile flag to indicate the new data has arrived. You can even add some kind of handshake to that, so you know if new data has arrived and the previous data has not been processed yet.

I can not tell you what is best, because it depends on many things: how many bytes, how often, variable length or fixed length, which Arduino boards, how long does it take to process the data, and so on.

volatile myData...  // a variable or array or struct
volatile bool newData; // flag

void loop()
{
  if( newData)
  {
    process myData (sometimes that means turning interrupts off to make copy)
    newData = false;
  }
}

void receiveEvent( int howMany)
{
  if( newData)
  {
    set an error, the previous data was not yet processed
  }
  copy data into global variable myData
  newData = true;
}

Did you know that the Slave needs time to breathe ?
Scroll up on that page and read how other libraries can interfere with heavy I2C traffic.

About your concern to lose messages:
In a embedded system with a multitasking system and enough memory, there would be a (circular) buffer between the interrupt and the normal task that processes the data. If the buffer never gets full, then everything is okay.
With a Arduino Uno, it can do no more than it can do.
To keep the Arduino most responsive, keep interrupts short and process the data in the loop(). At least knowing if there are missed messages is important.

[trullock]

Thanks for your reply

Let me add some clarifying details...

Yes its my master and slave, both running on Arduino Nanos

Messages from master to slave are two bytes in length.

Whilst they will come infrequently, its likely that a few (~3) will come in quick (immediate) succession.

Handling the messages in the ISR is producing problems for me, which is why I'm refactoring and hence posting this question on best practice.

In your code, doesn't copy data into global variable myData need to be inside a mutex lock? Otherwise process myData could be midway through looping myData whilst I'm trying to push more data onto it - essentially implying i need a concurrent queue?
Thats my real question here, how do I implement such a lock in Arduino land - or is there some subtelty I'm missing?

Or do I misunderstand how interrupts are actaully implemented in Arduino?

Thanks

[Koepel]

Now I understand your questions in your first post better.
So you really need an array of 16-bit integers that can be filled by the ISR ? That is very rare. What is wrong with the Master that it gives bursts of packages ?

I think there is some confusion with: 'buffer', 'array' and 'queue'.
The Wire library has buffers, but it uses packages of data, not a stream of data. A buffer contains only one package of data from one I2C session.

An Arduino Nano has a 8-bit microcontroller, and a interrupt can split a 16-bit integer in half. Most people ignore that, but if you want to do it right then interrupts should be disabled for a very short time.

Disabling the interrupt is not bad. It will delay the I2C interrupt inside the Wire library, but no data will get lost. The Arduino as a Slave pauses the Master by keeping the SCL low when the software is slower than the hardware (clock pulse stretching).

Suppose there is no queue, but just a single unsigned 16-bit integer:

volatile unsigned int myData;
volatile bool newData;

void loop()
{
  if( newData)
  {
    // Disabling the interrupts for a very short time will do no harm.
    // Make a copy, using memcpy() is no problem
    // Release the flag 'newData' while the interrupts are off, to keep
    // that flag tied to the data.
    noInterrupts();
    unsigned int myDataCopy = myData;
    newData = false;
    interrupts();

    Serial.println( myDataCopy);   // use the copy
  }
}

Suppose your Master sketch is misbehaving and does send bursts:

#define QUEUE_SIZE 10
volatile unsigned int myData[QUEUE_SIZE]
volatile int indexIn;
volatile int indexOut;
volatile bool errorFlag;

void loop()
{
  noInterrupts();
  check if there is data, advance the indexOut
  interrupts();

  if( errorFlag)
  {
    Serial.println( "Queue full !");
    errorFlag = false;
  }
}

void onReceive( int howMany)
{
  if( howMany == 2)   // extra safety check, two bytes are expected.
  {
    check if there is still a free spot in the buffer, write data, advance indexIn
    if queue is full, set errorFlag
  }
}

If you do stress-tests, then you will find the limits of a 16MHz microcontroller with the Arduino Wire library soon enough.
Do you have a logic analyzer ? To see the SCL pulse stretching.

[trullock]

Thanks again for your response, I've read your wiki and Nick Gammon's page, theyre very helpful thanks - but I do still have some questions.

Let me show you my pseudocode:

Master

loop() {

    if(conditionA)
        sendCommand(0x01, 0x0);

    // conditionA and B are not mutually exclusive, could both be true in the same loop, causing both command 0x1 and 0x2 to be sent one after the other
    if(conditionB)
        sendCommand(0x2, 0x1);

    // ... assume there are N other command variations, just for this exercise
}

void sendCommand(byte cmd, byte arg) {
	Wire.beginTransmission(SLAVE_ADDRESS);
	Wire.write(cmd);
	Wire.write(arg);
	int error = Wire.endTransmission();

	if (error != 0)
	{
		Serial.print("Error sending command: ");
		Serial.println(error);
	}
}

Slave

volatile byte commandBuffer[32];
volatile byte commandBufferLength = 0;

Wire.onReceive(handler);

void handler(int howMany)
{
    commandBuffer[commandBufferLength] = Wire.read();
    commandBuffer[commandBufferLength + 1] = Wire.read();
    commandBufferLength += 2;
}

void loop()
{

    byte buffer[32];
    byte length;
    noInterrupts();
    memcpy(buffer, commandBuffer, 32);
    length = commandBufferLength;
    commandBufferLength = 0;
    interrupts();

    // process buffer
}

In response to your points

1. not an int16, just two bytes. A command and some related data.

2. Why do you think the master has something wrong with it because its "bursting packets"? I realise I could refactor the above code to avoid both sending A and B, or to have a delay between them - and this may be what i do eventually - for now I'm just trying to fully understand I2C on Arduino for completeness.

3. Does the above code look safe? My worry is that loop could be executing between the noInterrupts and interrupts instructions, and two I2C packets come in. My understanding is that theres only one flag per interrupt, so queued two interrupts will only cause one ISR to fire, and I dont know what Wire.available() would return in such situation

In practice I'm fine to add a small delay between command sends from the master to largely avoid this problem, I'm just trying to fully understand the best practices and functionality available as that feels like a hack

Many thanks

[GolamMostafa]

@trullock

Or do I misunderstand how interrupts are actaully implemented in Arduino?

1. Let us try to understand the mechanism of data exchange between two Arduinos using I2C Bus.

2. Let us be sure that we have the following hardware setup (Fig-1) betwee two Arduino NANOs. Here, we will send the data 0x1234 from NANO-1 (Master) to NANO-2 (Slave); where, Slave will receive the data and will show on it Serial Monitor (SM2).

Figure-1:

3. Uplaod the following sketch in NANO-1.

#include<Wire.h>
int x = 0x1234;

void setup()
{
  Serial.begin(9600);
  Wire.begin();

  Wire.beginTransmission(0x21); //roll calling for the Slave
  byte busStatus = Wire.endTransmission();
  if (busStatus != 0x00)
  {
    Serial.print("I2C Bus communication problem...!");
    while (1);  //wait for ever
  }
  Serial.println("Slave found!");
}

void loop()
{
  Wire.beginTransmission(0x21); //address byte in write mode=0100001+0=01000010= 0x42 (queued)
  Wire.write(highByte(x));  //0x12 is queued
  Wire.write(lowByte(x));   //0x34 is queued
  Wire.endTransmission();   //all queued data bytes are sent on ACK
  //-------------------
  delay(1000);        //test interval
}

4. Upload the following sketh in NANO-2.

#include<Wire.h>
volatile bool flag = false;
volatile byte myArray[2];

void setup()
{
  Serial.begin(9600);
  Wire.begin(0x21); //7-bit address = 0100001 
  Wire.onReceive(receiveEvent);
}

void loop()
{
  if (flag == true)
  {
    int x = (myArray[0]<<8)|myArray[1];
    Serial.println(x, HEX);  //shows: 1234  
    flag = false;
  }
}

void receiveEvent(int howMany)
{
  for(int i=0; i<howMany; i++)
  {
    myArray[i] = Wire.read();
  }
  flag = true;
}

5. Press RESET button on both NANOs.
6. Check that SM2 shows 1234 at 1-sec interval
7. Working Principle
(1) I2C Bus is a byte oriented system; where, data exchange takes place 1-byte at a time.
(2) Data bytes are received by the Slave from the Master on interrupt basis and are saved into the FIFO type Serial Buffer.

(3) Once the Slave has finished receiving all the data bytes from the Master, the Slave goes to receiveEvent(int howMany) routine, brings out the data bytes from FIFO I2C Serial Buffer and saves them into a user-defined array (for multiple data bytes). The argument of the routine (howMany) is always equal to the number of data bytes received from Master.

[trullock]

@GolamMostafa Thanks for your input.

If you remove the delay from the master loop so its sending packets quickly, how do you see the slave behaving?

Will receiveEvent not likely interrup the slave loop - for example's sake lets say during the Serial.println command - thus ending up in an inconsistent state, we'll lose messages as flag will just get set back to false and the latter messages wont be read

[GolamMostafa]

In I2C communication, things happen very coherently and nothing goes into chaos. You have opportunity to experiment any idea that comes in your mind rather than making any guess.

1. The delay(1000) has been marked as test interval. You can remove it and then re-run the programs to observe the behavior. Do you have the NANOs to test the sketches?

2. The Slave spends time in the loop() function. Whenever dicatation comes, it goes to the receiveEvent() handler, executes the codes, and then returns to loop() function.

[trullock]

@GolamMostafa Thanks again for your input, but it doesn't really respond to my point. IMO there are two options here, based on my ignorance of how Arduino is actaully implemented under the hood:

1. If interrupts don't actually interrupt loop(), but are executed between calls to it, then there isnt a problem - because this lack of true interruption. I don't believe this is the case - but I'll build it in hardware and test if we can't answer this here.

2. If interrupts do interrupt loop(), then we have a traditional computer science concurrency problem, which will require some form of semaphores/mutex to resolve. If this is the case, what is the established Arduino paradigm/pattern for solving this?

And on the topic of building this in hardware, I can build a test to demonstrate my point and there will be one of two outcomes: I'll see the behaviour I'm suggesting - and still not know how to fix it. Or, I wont see this behaviuour, but wont understand why - hence this theoretical discussion

[GolamMostafa]

@trullock
Have you tested my sketches? Are they working? If they are working, then you have working examples which you can analyse to get the answers to your all queries.

The sketches have been designed based on the sound principles of the I2C Bus which has been developed by the "Really" smart guys of the Arduino Development Team.

The Arduino Team had to perform a lot of register level instructions before developing the high level structure of the I2C Bus. You need to design and execute some register level programs in order to grasp the working principles of the following high level commands of Arduino Platform on I2C/TWI Bus.
1. Wire.beginTransmission()
2. Wire.endTransmission()
3. Wire.write()
4. Wire.read()
5. Wire.requestFrom()
6. Wire.onReceive()
7. Wire. onRequest()

The reading of the following short note may help you to understand the I2C Bus operation before you proceed to write register level instructions:

6.2 Understanding I2C Bus Operation by Register Level Instructions

Figure-6.4: 7-bit address format of an I2C Bus Slave

(1) IDLE State: The I2C Bus is said to be at IDLE State when both SDA and SCL lines are at LH-states (U-state in Fig-6.4) and there is no activities on the bus.

(2) START State: The START (S-state in Fig-6.4) condition is brought on the I2C Bus by bringing down the SDA line into low-state (LL-state) while the SCL line is still at high-state and then pulling down the SCL line into low-state. The event happens when the user/Master executes the following codes; where, logic-high is placed at the TWSTA-bit (TWI START Condition Bit) of TWCR Register. If the bus responses to this action, the TWINT-bit will assume logic-high state and upper 5-bit of TWSR Register will hold 00001 which is known as bus status. In order to allow the TWINT-bit assuming new value, we have to clear its content at the beginning by writing logic-high into it.

TWCR = 0b10100100; //TWINT TWEA TWSTA TWSTO TWWC TWEN X TWIE //no SCL is generated

while(bitRead(TWCR,7)!= HIGH)

; //wait until the process completes and then TWINT will assume LH-state

lcd.print((TWSR & 0b11111000),HEX); //shows: 0x08(00001000) indicates successful process;

(3) Process State: The Master detects the presence (V-state in Fig-6.4) of a Slave (for example: BME280 of Fig-6.1) sensor by asserting its ‘7-bit address (1110110) + data direction bit (R-W/ = 0)’ on the bus. The resultant 8-bit (11101100 = 0xEE) is known as Control Byte (Fig-6.2). To clock-in the 8-bit data of the Control Byte into the Slave, the Master automatically generates 8 SCL pulses. The 8 SCL pulses are generated when the user loads data into TWDR Register. If the Slave is present, it accepts the address bits and then generates ACK (acknowledgement) signal on the I2C bus by bringing down the SDA line. The Master generates the 9th SCL pulse (Fig-6.4) to sample/grasp the ACK-bit. The event happens when the user executes the following codes:

TWDR = 0b11101100; //Slave address (SLA) + write-bit (0) = 11101100 = 0xEE

TWCR = 0b10000100; //TWINT-bit is cleared; it will assume LH-state when process will end

while(bitRead(TWCR,7)!= HIGH)

; //wait until TWINT-bit LH-state indicating end-of-process

lcd.print((TWSR & 0b11111000),HEX); //shows: 0x18(00011000) indicates successful process;

(4) STOP State: The Master brings STOP (P-state in Fig-6.4) on the I2C Bus by bringing SDA line to LH-state and then SCL line to LH-state. The event happens when the user executes the following code where logic-high is stored into TWSTO-bit (TWI Bus Stop) of TWCR Register. As no ‘bus status’ signal will be generated during this process, there is no need to poll the TWINT-bit to see if it has assumed LH-state.

TWCR = 0b10010100; //TWINT TWEA TWSTA TWSTO TWWC TWEN X TWIE;

[trullock]

@GolamMostafa Thanks again for your really detailed reply, this is all really useful information.

So i've dug into how twi.c and Wire.cpp are implemented and in one sense they answer/ force an answer.

On each slave message receipt, the internal rx buffer is reset, so to answer my original question i definitely need to have a local buffer to capture successive messages.

This really answers my I2C question, and leaves only the concurrency problem.

twi.c is implemented in the way i've posted above, so I'm happy in as far as its probably safe for all practical purposes, albeit technically susceptible to race conditions.

[Koepel]

@GolamMostafa trullock thinks that I2C packages are incoming at a higher rate than the loop() in the Slave can process the incoming data. We have seen no proof of that, but if he wants to be 100% sure not to miss something then a buffer (array of packages) can be used between the receiveEvent() and the Slave loop().

@trullock There are no hardware buffers, maybe one byte. It is not possible that the Slave has a bunch of incoming data, because the Slave puts the Master on halt with the clock pulse stretching. The receiveEvent() is not the ISR. The ISR is inside the Wire library and I think it is called for every byte. The receiveEvent() is sometimes called from that ISR.
Your code in reply #5 is safe.
You can not apply semaphores/mutex to a interrupt. There are interrupts and there is the rest of the code, and interrupts can (temporarily) be disabled. That's all, there is nothing else. This is a well designed microcontroller, not a full operating system.

Can you give us a broader view ? What is your project about ? Why do you need two Arduino boards, and why do you want to use the I2C bus ?
Can you give us more details ? Show us a schematic, photos, the Master sketch, the Slave sketch, and so on.

For me, it helps sometimes to imagine all the processes that are going one. Let me paint a colorful picture for you:
The 16MHz Nano is like a bee, happily buzzing in a flower field. It is not slow and not fast.
The I2C is slow, it is like a rowing boat carrying fruits.
The Wire library is like a busy market on a sunny day. There is a lot going on.
The call to memset() (and copying two variables) while the interrupts are off is so fast, it travels around the world ten times when you blink your eye.
Can you combine that in a picture ? then you see that disabling the interrupts to copy variables or for memset() might delay the next interrupt, but only by a little.

Do you know what others want to do ? They call Serial functions from an ISR, but the Serial functions use interrupts themself. Or they use the String object in a ISR a lot which allocates and releases memory from the heap. Or they start a I2C session as a Master from within the receveEvent() ! Those things should be avoided.

[GolamMostafa]

@trullock

So i've dug into how twi.c and Wire.cpp are implemented and in one sense they answer/ force an answer.

This is what is called the protocol which NANO-1 and NANO-2 must agree to follow before they begin data exchange using I2C Bus.

As you can see in the low level codes of my Post#10, the TWINT bit tells the MCU whether the current process is over or not. The MCU can poll this bit or this bit can interrupt the MCU conveying the message of process completion.

You can see from Fig-6.4 of my Post#10 that the Slave generates an ACK signal by modulating the SDA line after receiving every byte of data from the Master. This feature ensures that there is no loss of information.

The Wire.h Library contains ready-made codes and routines. One can operate the I2C Bus without including the Wire.h Library in his sketch and then he has to write all the codes himself.

Thhere is really no big thing here in the I2C bus as is being anticipated. The data sheets of the MCU contains the detailed information as to the architecture and programming of the I2C/TWI Bus from which one can easily develop the protocol to operate two MCUs in Master-Slave configuration.