Arduino to Arduino - i2c - Master requesting a string of values to Slaves

Dear all,
I would like to make communicate two arduinos using I2C.
The master triggers an update from the slave and receive the answer, a string of characters variable in length.

Please find below the code I wrote. It does not work, I think that I did some mistakes with the string sent and the conversion, since the output displayed by the master in the serial monitor is not readable.

I would also like to know if the logic is correct or, for my case, there is a better way to code this.

Furthermore I don't understand what are the implication of using the quantity number in the Wire.requestFrom(slave, 15); (Master), what happens if the Slave replies with a shorter message other than 15?

Thank you for your support,
dk

NOTE: I still not defied the format of the string sent back by the slave but it could be something like values separated by comma with a starting and terminating char such as: "!10,21,5,100?". The string will be then parsed by the Master to extract numbers 10, 21, 5, and 100. Is there a better way to do this?

// MASTER
#include <Wire.h>

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

void loop()
{
  // [...]
  // Request data from slave #2
  Serial.println(requestValues(2));
  delay(10000);
}

String requestValues(int slave){
  String sValues;
  int i = 0;
  
  // Request value from slave
  Wire.requestFrom(slave, 15);
  
  while(0 < Wire.available())
  {
    sValues[i] = Wire.read();
    i++;
  }
  
  sValues[i] = '\0';
   
  return sValues; 
}
// SLAVE
#include <Wire.h>

int led = 13;

void setup()
{
  pinMode(led, OUTPUT);
  Wire.begin(2);
  Wire.onRequest(requestEvent);
  ledStatus(1000);
}
 
void loop()
{
  //do something
  delay(100);
}
 
void requestEvent()
{
  String sSensor = "Text value: " + sensorRead(1); // size of this string can vary from 10 up to 15 
  char buffer[20];
  sSensor.toCharArray(buffer, 20);

  Wire.write(buffer);
}

int sensorRead(int index){
  int sensor;
  sensor = 20;
  // [...]
  
  sensor = sensor + index;

  ledStatus(500);
  return sensor;
}

void ledStatus(int t){
  digitalWrite(led, HIGH);
  delay(t);
  digitalWrite(led, LOW);
  delay(t); 
}
  //do something
  delay(100);

That is NOT doing something.

  int sensor;
  sensor = 20;
  // [...]
  
  sensor = sensor + index;

Three lines of code to do what one line could. Why?

void requestEvent()

This is an interrupt handler. You can NOT call delay in an interrupt service routine. That includes any function called by the interrupt service routine, as ledStatus() is.

Nothing in your code on either end needs Strings.

Dear PaulS,
Thank you for your reply.

PaulS:
That is NOT doing something.

Right, actually at the beginning it is supposed to be some code here.. for now can I just leave the "loop ()" of the slave empty?

PaulS:
Three lines of code to do what one line could. Why?

Right, actually there is some more code which I omitted that is used to read sensors value.. and I simplified it.

PaulS:

void requestEvent()

This is an interrupt handler. You can NOT call delay in an interrupt service routine. That includes any function called by the interrupt service routine, as ledStatus() is.

Ok, this I didn't know. I just wanted to make the led blink after reading sensors values. Hum.. I think I should find another way.

PaulS:
Nothing in your code on either end needs Strings.

Ok, I will try to use only char array using char *requestValues(int slave) and change the message returning from the slave.

Any clue for the Wire.requestFrom question?
Thank you again,
dk

for now can I just leave the "loop ()" of the slave empty?

Yes.

Hum.. I think I should find another way.

Set a flag in receiveEvent(). Test the flag in loop(). If set, blink a number of times, then clear the flag.

Ok, I will try to use only char array using char *requestValues(int slave) and change the message returning from the slave.

It's better to pass the array to populate to the function. Returning a pointer from a function is generally not a good idea.

Any clue for the Wire.requestFrom question?

You need to fix the other issues, first. Then, if there is still a problem, post the modified code. Right now, you are asking about fixing a hangnail when your arm is broken, and nearly severed in a chainsaw incident. By the time you get your arm sewn back on, and the bones set and a cast put on, the hangnail might not be an issue.

Dear PaulS,

Following your advice I have updated the code and now it seems working...

// MASTER
#include <Wire.h>

char message[25];

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

void loop()
{
  // [...]
  // Request data from slave #2
  requestValues(2);
  Serial.print("Slave #2: ");
  Serial.println(message);
  delay(5000);
}

void requestValues(int slave)
{ 
  int i = 0;
  
  // Request value from slave
  Wire.requestFrom(slave, 15);
  
  while(0 < Wire.available())
  {
    message[i] = Wire.read();
    i++;
  }
  
  message[i] = '\0';
}
// SLAVE
#include <Wire.h>

int led = 13;
int ledFlag = 0;
int test = 0;
char message[25];

void setup()
{
  pinMode(led, OUTPUT);
  Wire.begin(2);
  Wire.onRequest(requestEvent);
  ledStatus(1000);
}
 
void loop()
{
  if (ledFlag == 1)
  {
    ledStatus(500);
    ledFlag = 0;
  }
  delay(100);
}
 
void requestEvent()
{
  int s1;
  int s2;
  test++;
  
  s1 = sensorRead(1);
  s2 = sensorRead(2);
  sprintf(message, "[%d:%d]", s1, s2); // size of this string can vary from 10 up to 15
  Wire.write(message);
}

int sensorRead(int index)
{
  int sensor;
  sensor = 20;
  // Read sensors value ...
  sensor = sensor + index + test;
  
  ledFlag = 1;
  return sensor;
}

void ledStatus(int t)
{
  digitalWrite(led, HIGH);
  delay(t);
  digitalWrite(led, LOW);
  delay(t); 
}

Now, the master displays on the serial strings similar to this "Slave #2: [22:23]ÿÿÿÿÿÿÿÿ" where "ÿ" are the character to "fill" the 15 char requested Wire.requestFrom(slave, 15).
So if I correctly understood, while(0 < Wire.available()) cycles 15 times and the last 8 there is nothing to receive.
Any suggestion on how to make it adaptive? The string to receive goes from 9 (the example is only using two sensors) up to 15 characters.
The master will anyhow parse the message to identify the values, so don't think the "ÿ" would be an issue but just in case.
Thank you
dk

  while(0 < Wire.available())
  {
    message[i] = Wire.read();
    i++;
  }

This would be written more conventionally like so:

  while(Wire.available() > 0)
  {
    message[i] = Wire.read();
    i++;
  }

Most people think "while there are more than 0 characters to read, do something", not "while 0 is less than the number of characters to read, so something". Write code that reflects the way you think.

Reversing the statement makes it clear that the while loop will end as soon as all the data has been read, even when that is less than 15 characters.

The NULL is then inserted as a stop sign.

Try changing your while loop, to see if it then stops correctly.

Hi,
I have changed the loop as suggested but still get "ÿ" symbols.
I thought there was a way to make the master request a dynamic length message from slaves. I will use a parsing function to get the values from the message received using the "[" and "]" characters.

Another question that came in my mind is the connection between arduinos. On this page http://arduino.cc/en/Tutorial/MasterReader they just connect pin 4 (SCL) and pin 5 (SDA) of the master to their counterparts on the slave board.

On this project instead (https://web.archive.org/web/20190609170522/http://www.uchobby.com/index.php/2008/09/16/introduction-to-i2c/) they also use 1.5k pull-up resistances.

What is the purpose of the two resistances?

Another thing, the final version of the project will have a master arduino powered by 3.3V. How can I connect a 5V arduino slave safely via i2c?

Thank you for your kind support.
dk

EDIT: they just published a similar topic: i2c circuit - Networking, Protocols, and Devices - Arduino Forum but still missing the answer for the second question

I have a number of slave devices and am adapting the code in this topic.
When I load this sketch into the master, I do not get anything printing to the serial monitor

Master:

#include <Wire.h>

char message[15];

void setup()
{
	Wire.begin(0);                
	Wire.onReceive(receiveEvent); // register event
	Serial.begin(9600);           // start serial for output
}

void loop()
{

}

// function that executes whenever data is received from master
// this function is registered as an event, see setup()
void receiveEvent(int howmany)
{
	int i = 0;
	  while(Wire.available() > 0)
	  {
		  message[i] = Wire.read();
		  i++;
	  }
		Serial.println(message);
		  message[i] = '\0';
}

Slave sends a message which is printed correctly on the serial monitor of the slave UNO:

	  int s1;
	  int s2;
	  
	  s1 = numZone;
	  s2 = correctedTemp;
	  sprintf(message, "[%d:%d]", s1, s2); // size of this string can vary from 10 up to 15
	  Wire.write(message);
	  Serial.println(message);

I do not know what I am doing wrong.
I suspect it could be in the "void receiveEvent(int howmany)", but do not seem to find anything relating to the "(int howmany)" etc.

	  while(Wire.available() > 0)
	  {
		  message[i] = Wire.read();
		  i++;
	  }

There hardly seems a point to passing the number of characters to read to the function, when you don't use it. I wonder why the authors thought that it was necessary to do that.

Printing serial data in an interrupt service routine is a BAD idea.

Slave sends a message which is printed correctly on the serial monitor of the slave UNO:

Well, it's supposed to send that message, anyway. What proof do you have that it does?

I, possibly incorrectly, assume that if I am able to print the message to the Serial monitor, the message is send to the master.