Coming back to question wether 8-bit assignments are ato ... well, ok, only single instructions are atomic, and if a write to RAM requires two instructions to do so, the C++ - code may look atomic, but it probably isn't.
Even though I still struggle with this view. Provided there are no intermediate results written to RAM, then it would be a quite stupid HW design if a memory access on the ATmega328P would require actually TWO instructions... And, ... ahm, no, I simply don't know enough about AVR architecture.
I have cleaned-up my program, and removed all stuff not dealing with I2C bus. The remaining code is still compiled without errors, and does the Serial.Print.
You will see no volatile, no enable/disable of interrupts.
With respect to volatile:
Is it needed? Buffer itself is somehow "protected" by indicies readIdx and writeIdx - they are just a byte long and accessed once(!). So why volatile then.
With respect to enable/disable interrupts:
I have simply no idea where an interrupt would disturb the execution of loop() significantly. Latest data may not be taken, ok, but next loop of loop() it will.
So, code is working properly. My question is just: Is there a hidden trap because reading/writing of indicies may cause strange side effects (which I haven't seen yet) ... so I'm curious, I try to better understand.
Your comments are welcome. And here is the code:
#include <Arduino.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
I2CBuffer i2cBuffer = {{},0,0,false,true}; // No Overflow, received is fine
// ================================================================================================
void receiveEvent(int byteNum) { // receiveEvent just takes care for receiving data
byte cnt = 0; // no byte received yet
byte message[I2C_BUFFER_DEPTH]; // Intermediate buffer for I2C-message
byte next; // next value buffer.writeIdx, calculate in advance
next = ((i2cBuffer.writeIdx + 1) & I2C_BUFFER_MASK); // cycling [ 0 ... I2C_BUFFER_SIZE-1 ]
while (Wire.available()) {
byte in = Wire.read(); // always read, until buffer is empty, but take ...
if (cnt < I2C_BUFFER_DEPTH) { // ... only the number expected fitting to buffer
message[cnt] = in;
}
cnt++;
}
if (byteNum == I2C_BUFFER_DEPTH) { // Number of received bytes as expected?
if (i2cBuffer.readIdx == next) { // Y: Overflow of buffer? (one row remains unused)
i2cBuffer.Overflow = true; // Y: just indicate, do nothing more here
}
else { // N: Data seems ok, so fill Buffer with new data
for (byte col = 0; col < I2C_BUFFER_DEPTH; col++) {
i2cBuffer.data[i2cBuffer.writeIdx][col] = message[col];
}
i2cBuffer.writeIdx = next; // Update writeIdx ro next empty row, atomic!
}
}
else {
i2cBuffer.receiveOk = false; // N: 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
byte i2cWriteIdx = i2cBuffer.writeIdx; // ISR-changed, but reading just a byte always fine
byte i2cReadIdx = i2cBuffer.readIdx; // locally here updated
byte i2cReadIdxNext = ((i2cReadIdx + 1) & I2C_BUFFER_MASK); // Calculate in advance
boolean i2cOverflow = i2cBuffer.Overflow; // True, if overflow has occured
boolean i2cReceiveOk = i2cBuffer.receiveOk; // Ture, if Number of reveived byte is as expected
// ================================================================================================
// 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
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;
}
}