Only if another thread (ISR...) can write the same variable at any time.
Ok. I guess its because a call of ISR is not along the linear execution of the code, because, well, as you have written "any time". So to prevent the compiler to run some optimization that may lead, e.g., loose the connection to the updated value.
I have rewritten my code. Except that there is no flag. The flag is part of the struct
// Define type for I2CBuffer
struct I2CBuffer {
byte data[I2C_BUFFER_SIZE][I2C_BUFFER_DEPTH];
byte readIdx; // Index into oldest row with data
byte writeIdx; // Index into empty / writeable row of data
boolean Overflow; // Will be true if overflow has occured
boolean receiveOk; // Will be false if received data is not ok
};
// Declare I2CBuffer
volatile I2CBuffer i2cBuffer = {{},0,0,false,true}; // No Overflow, received is fine
So, I think, as I do the reading to readIdx, writeIdx, Overflow, and receiveOk while interrupts are disabled, I get a consistent set of data (readIdx, writeIdx, ...) and thus a valid new (if any) set of received data will be extracted from data because readIdx (and all other) are valid, and the ISR does not mess with this read-index around.
So, all in all I guess all done 
:
//#include <Arduino.h>
#include <util/atomic.h>
#include <Wire.h>
// Some enumeration to make life easier
enum class OpMode : byte {Sleep, WakeUp, Awake, LightOn, FallASleep, ClearBuffer};
enum Mode : byte {Normal, Configure};
enum Cmd : byte {PowerOff, PowerOn, LightOn};
// I2C BUS
#define I2C_BUFFER_DEPTH 6 // There will be always 6 bytes send
#define I2C_BUFFER_SIZE 16 // Buffer can up to (SIZE-1) messages, shall be 2^n
#define I2C_BUFFER_MASK (I2C_BUFFER_SIZE - 1) // Used in buffer code for fast implementation
#define POWER_ADDRESS 0b0001000 // I2C-Adress of PowerStage
// Define type for I2CBuffer
struct I2CBuffer {
byte data[I2C_BUFFER_SIZE][I2C_BUFFER_DEPTH];
byte readIdx; // Index into oldest row with data
byte writeIdx; // Index into empty / writeable row of data
boolean Overflow; // Will be true if overflow has occured
boolean receiveOk; // Will be false if received data is not ok
};
// Declare I2CBuffer
volatile I2CBuffer i2cBuffer = {{},0,0,false,true}; // No Overflow, received is fine
// ================================================================================================
void receiveEvent(int byteNum) {
if (byteNum == I2C_BUFFER_DEPTH) {
byte next = ((i2cBuffer.writeIdx + 1) & I2C_BUFFER_MASK); // cycling [ 0 : I2C_BUFFER_SIZE-1 ]
if (i2cBuffer.readIdx == next) { // Y: Overflow of buffer? (one row remains unused)
i2cBuffer.Overflow = true; // --> indicate it
}
else { // N: Data seems ok, so fill Buffer with new data
Wire.readBytes( (byte*) i2cBuffer.data[i2cBuffer.writeIdx], byteNum);
i2cBuffer.writeIdx = next; // Update writeIdx ro next empty row, atomic!
i2cBuffer.Overflow = false; // No overflow
}
i2cBuffer.receiveOk = true; // N: Message not as expected, no copy but report
}
else {
i2cBuffer.receiveOk = false; // Message not as expected, no copy but report
}
}
// ================================================================================================
void setup() {
// put your setup code here, to run once:
Serial.begin(115200);
// I2C-Bus
Wire.begin(POWER_ADDRESS);
Wire.onReceive(receiveEvent);
}
// ================================================================================================
void loop() {
// Data flow control for state machine
static OpMode opMode = OpMode::Sleep; // Sleep after big-bang
static OpMode opModeLast = OpMode::Sleep; // dito
// Power LED commands
static Mode cmdMode = Normal; // Commanded Mode of CtrlUnit (=Normal,Configure,)
static Cmd cmdParameter = PowerOff; // and associated commands
static int cmdFilterTau = 0; // Normal: time constant of 1st order filter
static int cmdBrightFac = 0; // normalized brightness
static int cmdColorT = 0; // reference colortemperature
// I2C Buffer
noInterrupts();
byte i2cWriteIdx = i2cBuffer.writeIdx; // ISR-changed, but reading just a byte always fine
byte i2cReadIdx = i2cBuffer.readIdx; // locally here updated
boolean i2cOverflow = i2cBuffer.Overflow; // True, if overflow has occured
boolean i2cReceiveOk = i2cBuffer.receiveOk; // Ture, if #bytes received are as expected
interrupts();
byte i2cReadIdxNext = ((i2cReadIdx + 1) & I2C_BUFFER_MASK); // Calculate in advance
// ================================================================================================
// R E A D I N P U T D A T A
// ================================================================================================
if ((!i2cOverflow) && i2cReceiveOk) { // No errors on bus in receiving data?
if (i2cWriteIdx != i2cReadIdx) { // New message received (indices are different then)
switch (i2cBuffer.data[i2cReadIdx][0] >> 4) { // What mode the Powerstage shall work with?
case 0: // Normal Mode
cmdMode = Normal; // Y: read ALL remaining data
switch (i2cBuffer.data[i2cReadIdx][0] & 0x0F) { // Translate cmd into local meaning (enum)
case 0: cmdParameter = PowerOff; break;
case 8: cmdParameter = PowerOn; break;
case 12: cmdParameter = LightOn; break;
default: cmdParameter = PowerOff; break;
}
cmdFilterTau = i2cBuffer.data[i2cReadIdx][1];
cmdColorT = ((int)i2cBuffer.data[i2cReadIdx][2] << 8) | // ColorT spread on two bytes
((int)i2cBuffer.data[i2cReadIdx][3]);
cmdBrightFac = ((int)i2cBuffer.data[i2cReadIdx][4] << 8) | // BrightFac spread on two bytes
((int)i2cBuffer.data[i2cReadIdx][5]);
break;
case 1: // Configuration Mode. To come ...
default:
break;
}
// Now update readIdx for I2C-Buffer for all Modes of Powerstage
ATOMIC_BLOCK (ATOMIC_RESTORESTATE) {
i2cBuffer.readIdx = i2cReadIdxNext; // Writing back to buffer. Should be fine for a byte
}
// DEBUGGING ONLY: Show powerCMD to evaluate at beginning of loop()
if (cmdMode == Normal) {
byte freeMem = (i2cReadIdx > i2cWriteIdx) ? (i2cReadIdx - i2cWriteIdx) :
(I2C_BUFFER_SIZE + i2cReadIdx - i2cWriteIdx - 1);
Serial.print(F("Mode\t\t: ")); Serial.println(byte(cmdMode));
Serial.print(F("Parameter\t: ")); Serial.println(cmdParameter);
Serial.print(F("Delay\t\t: ")); Serial.println(cmdFilterTau);
Serial.print(F("colorT\t\t: ")); Serial.println(cmdColorT);
Serial.print(F("brightFac\t: ")); Serial.println(cmdBrightFac);
Serial.print(F("FreeMemory\t: ")); Serial.println(freeMem);
if (opModeLast != opMode) {
Serial.print(F("Last Operating Mode\t:"));
if (opModeLast == OpMode::Sleep) Serial.print(F("Sleep"));
else if (opModeLast == OpMode::WakeUp) Serial.print(F("WakeUp"));
else if (opModeLast == OpMode::Awake ) Serial.print(F("Awake"));
else if (opModeLast == OpMode::LightOn) Serial.print(F("LightOn"));
else if (opModeLast == OpMode::FallASleep) Serial.print(F("FallASleep"));
else if (opModeLast == OpMode::ClearBuffer) Serial.print(F("ClearBuffer"));
Serial.print("\t Acutal Mode \t:");
if (opModeLast == OpMode::Sleep) Serial.print(F("Sleep"));
else if (opMode == OpMode::WakeUp) Serial.print(F("WakeUp"));
else if (opMode == OpMode::Awake ) Serial.print(F("Awake"));
else if (opMode == OpMode::LightOn) Serial.print(F("LightOn"));
else if (opMode == OpMode::FallASleep) Serial.print(F("FallASleep"));
else if (opMode == OpMode::ClearBuffer) Serial.print(F("ClearBuffer"));
}
Serial.println(" ");
}
}
}
else {
cmdMode = Normal; // Shut down in normal Modus, create feasible
cmdParameter = PowerOff; // ... command to shut down
cmdFilterTau = 100; // ... if needed, or directly. State machine below
cmdColorT = 0; // ... will take care of it
cmdBrightFac = 0;
}
// ================================================================================================
// O P E R A T E L I G H T C O N T R O L
// ================================================================================================
switch (opMode) {
case OpMode::Sleep:
if (cmdMode == Normal) {
}
break;
case OpMode::WakeUp:
if (cmdMode == Normal) {
}
break;
case OpMode::FallASleep:
if (cmdMode == Normal) {
}
break;
case OpMode::Awake:
if (cmdMode == Normal) {
}
break;
case OpMode::LightOn:
if (cmdMode == Normal) {
}
break;
case OpMode::ClearBuffer:
opModeLast = opMode;
opMode = OpMode::Sleep;
break;
default:
break;
}
}By the way:
-
is there a difference between
cli()/sei()and noInterrupts()/interrupts()? -
Where is the documentation for
Wire.readBytes()?. I just could find something for ESP32, which makes me believe it is not a standard function ofwire.h? https://www.arduino.cc/en/Reference/Wire is not mention that function either ...
What is level of "pounds the slave"? 100 times per second, 1000, 10000? Or much slower is the receiveEvent in comparsion to the transmission (i.e. sending the message the slave receives)? 2 times slower? 5 times? Do you have a value at hand? Assuming both are ATmega328Ps ...
I meant continuously filling the I2C bus by the Master. The ATmega328P has no buffer for the Serial input and also no buffer for the I2C data. Therefor the interrupt (inside the Wire library) runs for every byte, that takes some time. From what I remember from my test, a delay of 1 millisecond was enough between the I2C sessions from the Master or a few milliseconds delay if the Master has nothing else to do.
Suppose a button bounces, and that information is send over I2C without any debouncing. Then things can go hairy.
Another thing is that some libraries turn off the interrupts for some time (DHT, Neopixel, FastLED, OneWire, and so on), other libraries take over the Arduino (SoftwareSerial). The Arduino as a Slave holds down the SCL signal, that puts the Master on hold, so it should be no problem in theory, but with a stress test for the I2C bus it is not hard to get into trouble.
There is more to this.
A Arduino as a Slave does not wait for something. A Arduino as a Master waits during Wire.endTransmission() and during Wire.requestFrom() and waits as long as the Slave keeps SCL low. That means that in some situations, the Master gets into trouble before the Slave.
Thanks for this information. And, actually, good news for me, because milliseconds to do not count for my Master ... 
Consider this topic as closed, even though there would quite a lot of answers (thanks again!) that contain the answers ... so I dont't know which to mark; I leave it unchecked, and hope for your understanding.
This forum is open, it is not a strict question and answer forum.
We give background information; different users show different ways to deal with a problem; sometimes we get carried away. In the end we hope that you can extract the information to proceed.
You started this atomic / interrupt / volatile thing, so you could expect something was coming 
This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.