change of global variable in Wire.onRequest handler sometimes fails

Projects Networking, Protocols, and Devices
1

Hi,
I have made I2C communication between ATMega644(16MHz) and ATMega328(16MHz)
according Arduino Wire library examples ("Master Reader" and "Slave Sender").
But I have a problem in I2C slave in its Wire.onRequest handler.
If I change some global variable in this handler,
afterwards this global variable sometimes contains wrong value.

Here is the code of the I2C master (ATMega644):

#include <Wire.h>

#define TWI_RESPONSE_LENGTH   30

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

void loop()
{
  Wire.requestFrom(1, TWI_RESPONSE_LENGTH);
  while(Wire.available())    
  { 
    char c = Wire.receive(); 
    Serial.print(c);         
  };
  delay(500);
}

I want to know number of program cycles between two calls of Wire.onRequest handler
in the slave. This number I will send to master.
Due to I set "cycles_a=0" "cycles_b=0" inside of Wire.onRequest handler.
But this setting makes problems.

Here is the code of the I2C slave (ATMega328):

#include <Wire.h>

#define TWI_RESPONSE_LENGTH      30

long int cycles_a = 0;
long int cycles_b = 0;

//-------------------------------
void twiRequestHandler() 
//-------------------------------
{
  char sTwiResponse[TWI_RESPONSE_LENGTH + 1];
  sTwiResponse[0] = '\0';
  //--
  char buf[20];
  //-- write cycles_a
  ltoa(cycles_a, buf, 10); 
  strcat(sTwiResponse, buf);
  //---
  strcat(sTwiResponse, ":");
  //-- write cycles_b
  ltoa(cycles_b, buf, 10); 
  strcat(sTwiResponse, buf);
  //-- fill in remainder by spaces
  int countOfSpaces = TWI_RESPONSE_LENGTH - strlen(sTwiResponse); 
  for (int i = strlen(sTwiResponse); i < TWI_RESPONSE_LENGTH; ++i) {
    sTwiResponse[i] = ' ';
  };
  sTwiResponse[TWI_RESPONSE_LENGTH] = '\0';
  //--
  Wire.send(sTwiResponse);
  //--
  cycles_a = 0;   //problematic instruction
  cycles_b = 0;   //problematic instruction
}

//-------------------------------
void setup() 
//-------------------------------
{
  cycles_a = 0;
  cycles_b = 0;
  Wire.begin(1);
  Wire.onRequest(twiRequestHandler);
}


//-------------------------------
void loop() 
//-------------------------------
{
  float f = sqrt(micros());  //due to slow down main loop
  f = micros()*micros();     //due to slow down main loop
  f = micros()/f;            //due to slow down main loop
  ++cycles_a;
  ++cycles_b;
}

Result from the master which I get is e.g. this:

cycles_a:cycles_b
-----------------
35543:35543                   
35543:35543                   
35542:35542                   
35543:35542  <- difference                 
35541:71084  <- difference                 
35542:35542                   
35542:35542                   
35757:35543  <- difference                  
35542:35542                   
35542:35542                   
35541:35541                   
35542:35542                   
35543:35542  <- difference                   
35541:35542  <- difference                   
35541:35541                   
35543:35543                   
35543:35543                   
35542:35542                   
35541:35541                   
35542:35542                   
35542:35542                   
35543:35543                   
35543:35543                   
35542:35542                   
35543:35543                   
35542:35542                   
35542:35542                   
35542:35542                   
35756:35542  <- difference                   
35543:35543                   
35543:35543                   
35541:35541

I have discovered, that in the Wire.onRequest handler is possible only reading
of global variables, but write access sometimes fails.
When I null "cycles_a=0" "cycles_b=0" in Wire.onRequest handler,
result is sometimes wrong.

Could somebody to explain me why ?
Is it possible to change global variables in Wire.onRequest handler ?

Thank you for your answer.
Vojtech

2

When I null "cycles_a=0" "cycles_b=0" in Wire.onRequest handler,
result is sometimes wrong.

Could somebody to explain me why ?

You have interrupts happening. The variables being accessed by the interrupt handler and the other code (loop() and setup()) are not declared volatile. They should be.

In setup() and loop(), you need to disable interrupts prior to modifying non-byte sized values (such as cycles_a and cycles_b) and re-enable them afterwards. Otherwise, an interrupt in the middle of changing the multi-byte value could occur.

Is it possible to change global variables in Wire.onRequest handler ?

Of course.

3

Hi Paul,
thank you very much for your help !
Vojta

4

This post is also important for me, but the answer is not clear. I cannot modify my code by adding "Of course" to it... Sad.

5

Lobobo:
This post is also important for me, but the answer is not clear. I cannot modify my code by adding "Of course" to it... Sad.

The compiler optimizes variable storage for efficient access, some times a variable is stored in the cpu's internal registers while being accessed in a function. If an interrupt routine is triggered, these optimizations can cause the current memory value of a global variable to be stale(the current value is in one of the cpu registers). Since the ISR(interrupt service routine) assumes the current value of a global variable is the value stored in RAM, it loads this value, changes it, then stores this updated value back to RAM. Then the ISR exits, returning to the interrupted function (which has the global value store in a cpu register), the foreground function uses the register value of the global variable, then copies the 'updated' value into RAM, overwriting the value changed by the ISR.

To solve this problem you need to tell the compiler to always reload the value of all global variable that are accessed in a ISR.

volatile byte myGlobalByte;
volatile uint16_t myGlobalWord;

Now, there is another possible problem. any time you change the value of one of these 'volatile globals', you should make sure your changes are not overwritten, by the ISR

noInterrupts();
myGlobalWord = myGlobalWord+2;
interrupts();

noInterrupts(), interrupts() disable the triggering of interrupts in code the needs to execute atomically.

Let say my word variable contained 510, so, the value of it's high byte was 1 and it's low byte was 254. Now lets assume inside my ISR I have the code:

myGlobalWord = myGlobalWord +4;

If I did not put the noInterrupts(),interrupts(), calls around my foreground increment, and the ISR triggered in the middle of the increment, the value could change in unexpected ways.

510 // original value loaded into CPU registers for increment
514 // changed in ISR
512 // saved back into RAM after the foreground increment executed

so, in this case the ISR update is lost.

Chuck

6

Thanks Chuck. It all make sense and is much clearly now, except myGlobalWord = myGlobalWord+2;. I do investigating my I2C transfer problems in another post: Wire.onReceive doesn't report global variables or program freezes. I2C problems - Programming Questions - Arduino Forum
I tried to put your examples, but it doesn't help. Is this code:

noInterrupts();
myGlobalWord = myGlobalWord+2;
interrupts();

must be inside: Wire.onRequest(requestEvent); routine?
Thanks.

7

Is this code:
Code: [Select]

noInterrupts();
myGlobalWord = myGlobalWord+2;
interrupts();

must be inside: Wire.onRequest(requestEvent); routine?

No. During an interrupt handler, interrupts are already disabled. Re-enabling them is NOT something YOU should be doing.