I2C sending int between arduino problem

I am having an issue where when I read a temperature from a sensor on a slave device and send it to another arduino (both unos), the value that is received on the other end is not expected.
Here's my slave code:

#include <Wire.h>
const int sensorPin = A0;
byte len = 2;
byte temps[2];
char ch[5];
uint16_t temp;

void setup() {
    Wire.begin(8);
    Wire.onRequest( requestEvent );  //similar to interrupt. executes "requestEvent method when slave receives request
    Serial.begin(9600);
}

void loop() {
    delay(1000); //not sure why but in examples
//    temp = getTemp();
//    Serial.println(temp);
//    byte msb = temp >> 8;
//    byte lsb = temp & 0x00FF;
//    Serial.println("msb:  " + String( msb ));
//    Serial.println("lsb:  " + String( lsb ));
}

void requestEvent() {
    temp = getTemp();         //doesnt' communicate through i2c correctly
//    uint16_t temp = 72;     //communicates through i2c correctly
    byte msb = (temp) >> 8;
    byte lsb = temp & 0x00FF;
    Wire.write( msb );
    Wire.write( lsb );
}


uint16_t getTemp( ) {
    int sensorVal = analogRead( sensorPin );
    float voltage = (sensorVal /1024.0) *5.0; //fraction of 10 bit value compared to 5V
    temp = (voltage - .5) * 100.0; // equation given by arduino book;
    //return int(temp);
    temp = temp*1.8 + 32.0;
    return temp;
}

And here's my master code:

//Code for master device --- attached to Arduino Due
#include <Wire.h>
const int sensorPin = A1;

void setup() {
  // put your setup code here, to run once:
  Wire.begin(); //master device
  Serial.begin(9600); //will give feedback on serial monitor
}

void loop() {
  // put your main code here, to run repeatedly:
  union {
    uint16_t intVal;
    byte b[2];
  };
  
  Wire.requestFrom(8, 2); //request 2 bytes from address #8 
  while(Wire.available() < 2); //wait for 2 bytes to be on wire -- could loop inf
  if( Wire.available() >= 2 ) {
    for(int i = 1; i >= 0; i--) {
      b[i] = Wire.read();
    }
  }
  uint16_t result = intVal;
  Serial.println("result from slave: ");
  Serial.print("HEX: ");
  Serial.print(intVal, HEX);
  Serial.print("   ");
  Serial.print("DEC: ");
  Serial.println(intVal);
  uint16_t readVal = getTemp();
  Serial.println("result from sensor: ");
  Serial.print("HEX: ");
  Serial.print(readVal, HEX);
  Serial.print("   ");
  Serial.print("DEC: ");
  Serial.println(String(readVal));
  Serial.println("");
  delay(1000);
}


uint16_t getTemp() {
  int sensorVal = analogRead( sensorPin );
  float voltage = float(sensorVal)/1024.0*5.0;
  float temp = (voltage - .5)*100.0;
  temp = temp*1.8+32.0;
  return uint16_t(temp);
}

The sensor reading is correct when I print it straight to the serial monitor so I am fairly confident that it is an issue with the communication. That being said, the weird part is that if I just let the temp var be equal to say, 72 arbitrarily and send it, it works as expected. Attached is an example of the serial monitor showing it not working:

My one guess is that it could have to do with pointer type stuff, but the number that I'm getting doesn't really look like an address and I'm not very familiar with c/c++ pointers.

Let me know if you need any more information! thanks for your help!

Your requestEvent() routine is called when the master addresses your Arduino (so it's ID is sent as the first byte's upper 7bit after a start condition). It then has to respond as soon as the master starts to clock it (by alternating the SCL signal). Although the slave might stretch the clock signal for a very short time, it is not allowed to do that for the time it needs to read a analog value. So you have to read the analog value continuously and store the value in a volatile variable so that the interrupt routine (requestEvent) can send it immediately if asked to do so.

So declare your temp variable volatile:

volatile uint16_t temp;

remove the comment from the code in the loop() (so that the temperature is constantly read) and remove the delay() call from the loop. Then remove the getTemp() call from requestEvent().

I am having an issue where when I read a temperature from a sensor on a slave device and send it to another arduino (both unos), the value that is received on the other end is not expected.

//Code for master device --- attached to Arduino Due

What Arduinos are you actually using?

I can not confirm any problem with your code using two Uno's.

I opened two instances of the ide and ran both codes with the Serial monitor. I modified the slave code for a print out when the requestEvent interrupt handler was triggered. The slave and the master were printing the same value from the slave.

SLAVE

#include <Wire.h>
const int sensorPin = A0;
byte len = 2;
byte temps[2];
char ch[5];
//volatile uint16_t temp; //should be volatile but is not the cause of your problem
uint16_t temp;
volatile boolean flag = true;

void setup() {
  Wire.begin(8);
  Wire.onRequest( requestEvent );  //similar to interrupt. executes "requestEvent method when slave receives request
  Serial.begin(9600);
}

void loop() {
  //delay(1000); //not sure why but in examples
  if (flag)
  {
    Serial.print("RequestEvent Temp from A0  ");
    Serial.println(temp);
    flag = false;
  }
}

void requestEvent() {
  flag = true;
  temp = getTemp();         //doesnt' communicate through i2c correctly
  //    uint16_t temp = 72;     //communicates through i2c correctly
  byte msb = (temp) >> 8;
  byte lsb = temp & 0x00FF;
  Wire.write( msb );
  Wire.write( lsb );

}

uint16_t getTemp( ) {
  int sensorVal = analogRead( sensorPin );
  float voltage = (sensorVal / 1024.0) * 5.0; //fraction of 10 bit value compared to 5V
  float voltage1 = float(sensorVal) / 1024.0 * 5.0;
  temp = (voltage - .5) * 100.0; // equation given by arduino book;
  //return int(temp);
  temp = temp * 1.8 + 32.0;
  return temp;
}

MASTER

#include <Wire.h>

void setup() {
  // put your setup code here, to run once:
  Wire.begin(); //master device
  Serial.begin(9600); //will give feedback on serial monitor
}

void loop() {
  // put your main code here, to run repeatedly:
  union {
    uint16_t intVal;
    byte b[2];
  };
  
  Wire.requestFrom(8, 2); //request 2 bytes from address #8 
  while(Wire.available() < 2); //wait for 2 bytes to be on wire -- could loop inf
  if( Wire.available() >= 2 ) {
    for(int i = 1; i >= 0; i--) {
      b[i] = Wire.read();
    }
  }
  uint16_t result = intVal;
  Serial.println("result from slave: ");
  Serial.print("HEX: ");
  Serial.print(intVal, HEX);
  Serial.print("   ");
  Serial.print("DEC: ");
  Serial.println(intVal);
  delay(1000);
}

EDIT:
What version of the IDE are you using. This code wit multiple writes from the slave will not be correct for versions older than 1.6.10 I believe.

Wire.write( msb );
  Wire.write( lsb );

Yuck :stuck_out_tongue_closed_eyes: Please remove this line: while(Wire.available() < 2);Explanation: Common mistake number 1.

I assume that you are testing with two Arduino Uno boards and use the newest version of the Arduino IDE.
Looking at the code, the I2C communication is probably okay. @cattledog has confirmed that.

However, you better combine the two bytes to 'intval' in the same way as the integer was split into two parts.

Your Slave is sending a 16-bit unsigned integer with MSB first. Then that is what you should do in the Master as well.

uint16_t tempSlave = word( b[0], b[1]);  // MSB is the first received byte

You can test it by sending a fixed unsigned integer. For example 0xABCD.

When the communication fails, you still print variables, and those variables don't have valid data.
I suggest to print an error message when the communication fails:

  Wire.requestFrom(8, 2); //request 2 bytes from address #8
  if( Wire.available() == 2 ) {   // 2 bytes received ?
    for(int i = 1; i >= 0; i--) {
      b[i] = Wire.read();
    }
    Serial.println("result from slave: ");
    ...
  } else {
    Serial.println("Error, no data received");
  }

@pylon wants you to put the analogRead() outside the interrupt routine (the requestEvent). It will work with the analogRead() inside the interrupt routine, but you might run into problems. Once it is working, you should improve your sketch by putting that analogRead() in the loop(), outside the interrupt routine.

Wire.requestFrom () is a looping/waiting instruction until ACK comes from slave. Therefore, the following codes should be enough to collect/build temperature data from the queued buffer of the slave.

byte x = Wire.read ();
byte y = Wire.read();
temp = x << 8 | y;

//.............................

Wire.receiveEvent() is an event handler which is called upon once request arrivess from the Master. So, it should be totally legal to execute the analogRead() instruction in this handler.

@pylon says

Although the slave might stretch the clock signal for a very short time, it is not allowed to do that for the time it needs to read a analog value. So you have to read the analog value continuously and store the value in a volatile variable so that the interrupt routine (requestEvent) can send it immediately if asked to do so.

@pylon wants you to put the analogRead() outside the interrupt routine (the requestEvent). It will work with the analogRead() inside the interrupt routine, but you might run into problems.

Is the slave holding SCL low for several hundred microseconds really an issue? Analog readings are quite common inside onRequest handlers. Indeed Nick Gammon's i2c tutorial shows an analogReading following a request from. Reading an external eeprom with i2c is also quite slow, and I've seen benchmarks of over 500us to read a byte.

As general practice ISR's should be kept short, but are there specific clock stretching issues with lengthy ISR's with the wire library or is there a specific issue with the Due?

I don't know. Many sensors have specifications for the maximum timing, but the Arduino Uno does not really have those. I don't know if the Due has timing specifications for I2C.

With a logic analyzer the I2C clock pulse stretching while the requestEvent() is busy can be seen.
An analogRead() takes 112 µs with a Arduino Uno. That is not very bad, but it is better to avoid it.

This is not the same as a slow I2C EEPROM. When the Arduino is used as a Slave and it stretches the clock, that is during a I2C transaction. Such things do not happen with I2C EEPROM and sensors, only when a microcontroller or processor is the Slave.

Assume that the Master requests data many times from an Arduino as a Slave. Assume there is also Serial communication in the Slave, and the interrupts are sometimes turned off by NeoPixels, DHT22 and DS18B20 libraries. Then there is also the Timer0 interrupt for millis(). In such a situation, an requestEvent() that takes a long time will make it worse... just kidding :wink: this situation will not work at all :o

It is therefor important to stay away from troubles.
Beside the I2C bus, interrupt routines should be as short as possible in the first place.

cattledog:
What Arduinos are you actually using?

I can not confirm any problem with your code using two Uno's.

I opened two instances of the ide and ran both codes with the Serial monitor. I modified the slave code for a print out when the requestEvent interrupt handler was triggered. The slave and the master were printing the same value from the slave.

I am using two arduino unos. That's the weird part. I can't replicate the issue either if all that I do is set temp to some specific value however, when I set temp to the value that I read from the sensor, the slave is consistently off by about 27 or 28 (in decimal) according to the master. When I read the value from the sensor and print it straight to the serial monitor from the slave, I get expected values so I don't think that it is an issue with the sensor.

I also tried changing temp to volatile uint16_t and repeatedly measuring the temperature in the loop instead of the requestEvent and that did not work.

Koepel:
However, you better combine the two bytes to 'intval' in the same way as the integer was split into two parts.

Your Slave is sending a 16-bit unsigned integer with MSB first. Then that is what you should do in the Master as well.

uint16_t tempSlave = word( b[0], b[1]);  // MSB is the first received byte

How is this different than the union that I use in the beginning of my master loop? The union, too, is reading 2 individual bytes as a 16 bit unsigned integer.

Do you know if there is a list that shows of all Arduino boards which is big-endian or little-endian ?

When you have two Arduino Uno boards and you don't change that, then it is better to use a union for both or split and combine in the same way for both.

I think the Arduino Uno puts the lowest byte on the first address location and that makes it little-endian ?

Question: What is wrong in the above sentence ?
Answer: The ATmega328P is not little-endian and not big-endian, since it can only operate on bytes, nothing else. The compiler makes it little-endian. This was not correct, as GolamMostafa writes below.

I am using two arduino unos. That's the weird part. I can't replicate the issue either if all that I do is set temp to some specific value however, when I set temp to the value that I read from the sensor, the slave is consistently off by about 27 or 28 (in decimal) according to the master.

How do you know the slave temperature reading is "consistently off"? Are you reading the same sensor with two different Arduinos, or are there two similar sensors which should somehow give the same answer.

What is the sensor(s),what are you measuring, and how are the sensors and Arduino's powered and wired together.

My test sent an analogRead() value taken on one uno to another uno.

cattledog:
How do you know the slave temperature reading is "consistently off"? Are you reading the same sensor with two different Arduinos, or are there two similar sensors which should somehow give the same answer.

So my test setup is that I actually have both of the arduinos reading from the sensor. The sensor is just a tmp36. So I have both arduino Unos doing an analog read on that sensor. I have the slave then sending its reading to the master. Then the master prints out the value that is sent to it, and the value that it read from the sensor itself.
The value that the master reads reads from the sensor is lower than the value that the master receives from the slave by in between 27 and 30 degrees or so (usually around 28). If I heat up the sensor, both values increase at about the same rate.
For example, if it's about 72 degrees in the room. The master will read from the sensor that it's about 72 degrees in the room and the slave if i plug into serial monitor also reads about 72 but when I send that value to the master and it prints, it prints about 100 degrees. If I put my finger on it and heat it up. it will be about 80 and 108 degrees.

I appreciate you trying to help me, please let me know if you have any other ideas or questions.

Ignoring the inter Arduino communication for a moment, can you indeed hook the sensor to both Arduinos and get the same reading from it when both are connected? You may have to phase the readings so they alternate, to test this.

Do you know how to have two instances of the serial monitor running? You have an icon on the desktop (or elsewhere) that runs an instance of the IDE. Use the icon to open another instance of the IDE. Don't use File-Open or File-New, in an already open IDE, to start a new IDE and/or load a sketch. Open a separate instance of the IDE and then load the other sketch into the new instance. Then you can have different serial ports for each open IDE. Make sure you select different ports for the two sketchs.

I will break out a tmp36 for some testing, but at this point I would be more concerned about the dual connections and reading process than the i2c communications. Are the grounds of the two Arduinos connected? Are they both powered from the same source?

For example, if it's about 72 degrees in the room. The master will read from the sensor that it's about 72 degrees in the room and the slave if i plug into serial monitor also reads about 72 but when I send that value to the master and it prints, it prints about 100 degrees. If I put my finger on it and heat it up. it will be about 80 and 108 degrees.

I would bet that one Arduino is powered by the PC's USB port and one by a power supply.

If the power supply has 5.0V and the USB bus has 4.8V a reading at 25°C (77°F) will be about 3°C (6°F) off the other one. If the difference in Vcc of the two Arduinos is even more the difference in the reading will increase too.

Koepel:
Question: What is wrong in the above sentence ?
Answer: The ATmega328P is not little-endian and not big-endian, since it can only operate on bytes, nothing else. The compiler makes it little-endian.

Does the Compiler really offer endianness to the ATmega328P architecture?

The ATmega328P architecture demands that the 16-bit instruction would only be executed if the lower-byte of the instruction is stored in the low-ordered flash byte-location and the higher byte of the instruction is stored in the higher order flash byte-location. Therefore, the ATmega328P is, by itself, of little-endian.

//----------------------------------------------------------------------------------------------------------------
From program fetch-execution point-of-view, the flash locations are words organized which means that every consecutive two-byte (16-bit) makes a location and is given an address.

From program uploading point-of-view, the flash locations are byte organized which means that every 8-bit makes a location and is given an address.

The flash capacity of the ATmega328P has been documented as: 16 kWord or 32 kbyte
//----------------------------------------------------------------------------------------------------------------

GolamMostafa:
The ATmega328P architecture demands that the 16-bit instruction would only be executed if the lower-byte of the instruction is stored in the low-ordered flash byte-location and the higher byte of the instruction is stored in the higher order flash byte-location. Therefore, the ATmega328P is, by itself, of little-endian.

Sorry, I was wrong. Thanks GolamMostafa. I should have known better. Thanks ! learned something new today :stuck_out_tongue:

An address has the lower byte at lower address location. Some registers are 16-bits, for example timer registers. Therefor it is indeed little-endian. I read in the datasheet that the General Purpose Registers are also capable of 16-bit.

Thanks @Koepel for the sincere feedback. I was a little bit worry to foresee how the message would be taken. But, I was being pressed heavily to pass this fact which I knew during the design phase of the In-system Programmer (ISP) for the CMCKIT (an ATmega32A Learning Kit using Assembly Language).

cattledog:
Are the grounds of the two Arduinos connected? Are they both powered from the same source?

The master arduino is powered by USB and the slave is powered by the 5V output of the master.

They both share a common ground.