I2C requestEvent handle failing

Projects Networking, Protocols, and Devices
1

Hello, this is my first post here, I am no expert with arduino/coding but I am studying mechatronics engineering and I have this project where I am sending temperature information to a master Arduino uno through another slave Arduino uno via I2C using the Wire library. The temperature sensor I am using is DS18B20 which uses one wire so I am using Teensy's OneWire library OneWire Arduino Library, connecting 1-wire devices (DS18S20, etc) to Teensy and also milesurton's Dallas Temperature library GitHub - milesburton/Arduino-Temperature-Control-Library: Arduino Temperature Library. In addition to this I have a motion sensor HC-SR501 that which outputs a high when detecting motion and I use as an interrupt in pin 2 to turn on a LED on pin 13 for 10 seconds.

I started by placing the temperature measurements in my slave inside void loop () as you can see below in my code and it went very well, my master prints the slave number and the temperature measurement received by the slave every 5 seconds, it works properly and also my motion sensor interruption works fine.
This is what I get in my console:
MASTER READER NODE
Maximum Slave Nodes: 6
Payload size: 5

1
25.13

1
25.13

1
25.13

Then I realised I only wanted to measure the temperature when asked by the master, which is why I thought putting all measurement code inside the requestEvent handle would be a good idea. However when I place my measurement code inside the requestEvent handle I don't get the correct temperature information sent to my master, some information gets sent at the beginning 3-4 random numbers and then it stops. And not only that, also my motion sensor interrupt gets stuck. It looks as if all code went wrong when doing this. This leads me to think the problem is mainly in the slave's code, since both the temperature measurements and motion interrupt stop working properly. Maybe master nad slave get desynchronized or something?
This is what I get in my console:
MASTER READER NODE
Maximum Slave Nodes: 6
Payload size: 5

135
0.00

97
0.00

0
nan

I am thinking maybe it's just that I can't be much time inside my requestEvent handle and that's why it works when the temperature measurement is done outside of it, but maybe I am missing something else and it is possible to do the measurement inside the handle? This is my starting point on the project and I want to do the measurement only when requested because the end goal is to save as much energy as possible (I will be implementing power management in the near future using a solar panel and a battery, and some other tricks I learned in other sites to reduce power consumption in arduino. I need to measure temperature just once per hour but I am printing every 5 seconds now so that I can check it is working). I'd appreciate if someone could advise me on this. Maybe I am using the wrong approach to my problem.

Thanks in advance!

Master

#include <Wire.h>

#define PAYLOAD_SIZE 5 // how many bytes to expect from each I2C salve node
#define NODE_MAX 6 // maximum number of slave nodes (I2C addresses) to probe
#define START_NODE 1 // The starting I2C address of slave nodes
#define NODE_READ_DELAY 5000 // Some delay between I2C node reads

typedef union
{
 float number;
 byte bytes[3];
} FLOATUNION_t; 
FLOATUNION_t myFloat;
int nodePayload[PAYLOAD_SIZE];

void setup()
{
  Serial.begin(9600);  
  Serial.println("MASTER READER NODE");
  Serial.print("Maximum Slave Nodes: ");
  Serial.println(NODE_MAX);
  Serial.print("Payload size: ");
  Serial.println(PAYLOAD_SIZE);
  Serial.println("***********************");
  
  Wire.begin();        // Activate I2C link
}

void loop()
{
  for (int nodeAddress = START_NODE; nodeAddress <= NODE_MAX; nodeAddress++) { // we are starting from Node address 2
    Wire.requestFrom(nodeAddress, PAYLOAD_SIZE);    // request data from node#
    if(Wire.available() == PAYLOAD_SIZE) {  // if data size is avaliable from nodes
      for (int i = 0; i < PAYLOAD_SIZE; i++) nodePayload[i] = Wire.read();  // get nodes data
      for (int j = 0; j < PAYLOAD_SIZE; j++)  
      myFloat.bytes[0] = nodePayload[1];
      myFloat.bytes[1] = nodePayload[2];
      myFloat.bytes[2] = nodePayload[3];
      myFloat.bytes[3] = nodePayload[4];
      Serial.println(nodePayload[0]);
      Serial.println(myFloat.number);
      Serial.println("*************************");      
      }
      
    }
    delay(NODE_READ_DELAY);
}

Slave with temperature measurement inside void loop () working fine

#include <Wire.h>
#include <OneWire.h>
#include <DallasTemperature.h>
#include <elapsedMillis.h>
 
#define ONE_WIRE_BUS 7  // Data wire is plugged into pin 7 on the Arduino
#define NODE_ADDRESS 1  // Change this unique address for each I2C slave node
#define PAYLOAD_SIZE 5 // Number of bytes  expected to be received by the master I2C node

typedef union
{
 float number;
 byte bytes[3];
} FLOATUNION_t; 

FLOATUNION_t myFloat;
byte nodePayload[PAYLOAD_SIZE];

// Setup a oneWire instance to communicate with any OneWire devices 
// (not just Maxim/Dallas temperature ICs)
OneWire oneWire(ONE_WIRE_BUS);
 
// Pass our oneWire reference to Dallas Temperature.
DallasTemperature sensors(&oneWire);

const byte ledPin = 13;
const byte interruptPin = 2;
volatile byte state = LOW;
elapsedMillis move_begin;

void setup()
{
  Serial.begin(9600);  
  Serial.println("SLAVE SENDER NODE");
  Serial.print("Node address: ");
  Serial.println(NODE_ADDRESS);
  Serial.print("Payload size: ");
  Serial.println(PAYLOAD_SIZE);
  Serial.println("***********************");

  // Start up the library
  sensors.begin();
  Wire.begin(NODE_ADDRESS);  // Activate I2C network
  Wire.onRequest(requestEvent); // Request attention of master node

  pinMode(ledPin, OUTPUT);
  pinMode(interruptPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(interruptPin), blink, RISING);
}

void loop()
{ 
  sensors.requestTemperatures(); // Send the command to get temperatures
  myFloat.number = sensors.getTempCByIndex(0);
  
  nodePayload[0] = NODE_ADDRESS;
  nodePayload[1] = myFloat.bytes[0];
  nodePayload[2] = myFloat.bytes[1];
  nodePayload[3] = myFloat.bytes[2];
  nodePayload[4] = myFloat.bytes[3];

  if (move_begin >= 10000) {
    digitalWrite(ledPin, 0);
  }
}

void requestEvent()
{
  Wire.write(nodePayload,PAYLOAD_SIZE);
}

void blink() {
  if (digitalRead(2)==1)
    move_begin = 0;
  state = HIGH;
  digitalWrite(ledPin, state);
}

Slave with temperature measurement inside requestEvent handle, not sending the correct information to the master

#include <Wire.h>
#include <OneWire.h>
#include <DallasTemperature.h>
#include <elapsedMillis.h>
 
#define ONE_WIRE_BUS 7  // Data wire is plugged into pin 7 on the Arduino
#define NODE_ADDRESS 1  // Change this unique address for each I2C slave node
#define PAYLOAD_SIZE 5 // Number of bytes  expected to be received by the master I2C node

typedef union
{
 float number;
 byte bytes[3];
} FLOATUNION_t; 

FLOATUNION_t myFloat;
byte nodePayload[PAYLOAD_SIZE];

// Setup a oneWire instance to communicate with any OneWire devices 
// (not just Maxim/Dallas temperature ICs)
OneWire oneWire(ONE_WIRE_BUS);
 
// Pass our oneWire reference to Dallas Temperature.
DallasTemperature sensors(&oneWire);

const byte ledPin = 13;
const byte interruptPin = 2;
volatile byte state = LOW;
elapsedMillis move_begin;

void setup()
{
  Serial.begin(9600);  
  Serial.println("SLAVE SENDER NODE");
  Serial.print("Node address: ");
  Serial.println(NODE_ADDRESS);
  Serial.print("Payload size: ");
  Serial.println(PAYLOAD_SIZE);
  Serial.println("***********************");

  // Start up the library
  sensors.begin();
  Wire.begin(NODE_ADDRESS);  // Activate I2C network
  Wire.onRequest(requestEvent); // Request attention of master node

  pinMode(ledPin, OUTPUT);
  pinMode(interruptPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(interruptPin), blink, RISING);
}

void loop()
{ 
  if (move_begin >= 10000) {
    digitalWrite(ledPin, 0);
  }
}

void requestEvent()
{
  sensors.requestTemperatures(); // Send the command to get temperatures
  myFloat.number = sensors.getTempCByIndex(0);
  
  nodePayload[0] = NODE_ADDRESS;
  nodePayload[1] = myFloat.bytes[0];
  nodePayload[2] = myFloat.bytes[1];
  nodePayload[3] = myFloat.bytes[2];
  nodePayload[4] = myFloat.bytes[3];
  Wire.write(nodePayload,PAYLOAD_SIZE);
}

void blink() {
  if (digitalRead(2)==1)
    move_begin = 0;
  state = HIGH;
  digitalWrite(ledPin, state);
}
2

Hi, the code is pretty good, but there is a problem with the array and the interrupt and how Arduino code is used.

An array of three elements is "int myArray[3]", that is myArray[0], myArray[1], myArray[2]. No more, just that.

A float on the Arduino Uno is 4 bytes. If you want to have a union with that, you need an array of 4 bytes: int myArray[4].

There is no need to make a union. You can transmit a 'float'. You have to take care to transmit and receive them in the same order. As far as I remember, the 'float' on a Arduino Uno is stored with the LSByte at the lowest address.
Some like the I2C_Anything: Gammon Forum : Electronics : Microprocessors : I2C - Two-Wire Peripheral Interface - for Arduino.

I often use a struct to transfer data, because I want to transmit all kind of variables. Sometimes I use an array.

You already compose the data with 5 bytes. I prefer to put that in a struct.
Perhaps the easiest way is a struct with an identifier byte and a float. Transmit it at once with Wire.write( (uint8_t *) &myStruct, sizeof( myStruct)) and read it with Wire.readBytes( (uint8_t *) &mStruct, sizeof( myStruct).

The requestEvent() handler is called from an interrupt and is therefor a interrupt routine. You should keep it as short and as fast as possible. Don't use delay(), don't use other busses like another I2C bus action or 1-Wire bus action, don't even use Serial functions.

Do you know what happens when you call "requestTemperatures()" and "getTempCByIndex()"?

Even in ASYNC mode, that is far too much to be called from an interrupt.

You already test after the Wire.requestFrom() if the same number of bytes was received that was requested. That is very good. When using a onReceive() handler, also check if the parameter 'howMany' has the correct number of bytes.
You can stress test the I2C communication by increasing the request rate, maybe up to 100 times per second. The OneWire library turns off interrupts for the 1-Wire timed protocol and that will have major consequences for the I2C bus.

That's not all, there is more:
When using a variable both in a interrupt and outside the interrupt, it should be made "volatile".
The Arduino Uno uses a 8-bit microcontroller. That means when a 'float' of 4 bytes is written or read in a interrupt, that could happen while the 4 bytes of that variable is being written or read in the loop(). That means the interrupt routine can read and write the volatile variable, but in the loop() you have to turn off the interrupts to be sure that all 4 bytes belong to each other.