Wire.readbytes() error with volatile directive

I try to compile this for esp32 but the compiler throws error but only with volatile directive why?

Error message:

no matching function for call to 'readBytes(volatile char [4], unsigned int)'

Code,

#include <Wire.h>

struct sms {
  int a;
};
volatile sms sms1;
volatile char  messageBuffer[sizeof(sms1)];

void setup() {
}

void loop() {
}
volatile void readSubModule() {
  Wire.beginTransmission(0x50);
  Wire.write(0x01);
  Wire.endTransmission();
  Wire.requestFrom(0x50, sizeof(sms1));    
  if (Wire.available()) {
    Wire.readBytes(messageBuffer , sizeof(sms1));
    memcpy(&sms1, & messageBuffer , sizeof(sms1));
  }
}

I tried to cast to byte

  Wire.readBytes((byte *)messageBuffer , sizeof(sms1));

now i get a new error.

memcpy(&sms1, &messageBuffer , sizeof(sms1));
invalid conversion from 'volatile void*' to 'void*' [-fpermissive]

How can i fix this?

This compiles without error.

#include <Wire.h>

struct sms {
 volatile int a;
};
sms sms1;
char  messageBuffer[sizeof(sms1)];

void setup() {
}

void loop() {
}
volatile void readSubModule() {
  Wire.beginTransmission(0x50);
  Wire.write(0x01);
  Wire.endTransmission();
  Wire.requestFrom(0x50, sizeof(sms1));
  if (Wire.available()) {
    Wire.readBytes(messageBuffer , sizeof(sms1));
    memcpy(&sms1, & messageBuffer , sizeof(sms1));
  }
}#include <Wire.h>

struct sms {
 volatile int a;
};
sms sms1;
char  messageBuffer[sizeof(sms1)];

void setup() {
}

void loop() {
}
volatile void readSubModule() {
  Wire.beginTransmission(0x50);
  Wire.write(0x01);
  Wire.endTransmission();
  Wire.requestFrom(0x50, sizeof(sms1));
  if (Wire.available()) {
    Wire.readBytes(messageBuffer , sizeof(sms1));
    memcpy(&sms1, & messageBuffer , sizeof(sms1));
  }
}

Does writeing to messageBuffer count as accessing outside the ISR? i cannot figure out why i cant declare it as volatile. Also i cannot declare the struct as volatile, is it okay to just declare each member of the struct as volatile instead of the whole struct. How do i declare all members of struct at once

Why do you think a volatile variable? everything is synchronous

#include <Wire.h>

int  smsValue;
byte messageBuffer[sizeof(smsValue)];

void setup() {}
void loop() {}

void readSubModule() {
  Wire.beginTransmission(0x50);
  Wire.write(0x01);
  Wire.endTransmission();
  
  Wire.requestFrom(0x50, sizeof(smsValue));
  if (Wire.available() == sizeof(smsValue)) {
    Wire.readBytes(messageBuffer , sizeof(smsValue)); // careful on timeout etc.. .should be fine but...
    memcpy(&smsValue, messageBuffer , sizeof(smsValue));
  }
}

The .readBytes() is from the stream class and it is not meant for I2C data. It can be used, but I would add a extra safety check.
The .readBytes() together with a memcpy() is a extra copy that is not needed.

  Wire.beginTransmission(0x50);
  Wire.write(0x01);
  Wire.endTransmission();     // <-- a "false" as parameter is here often used

  Wire.requestFrom(0x50, sizeof(sms1));
  if (Wire.available() == sizeof(sms1))    // is all the data received ?
    Wire.readBytes((byte *) &sms1 , sizeof(sms1));

I did not check if the cast to (byte *) works.

1 Like

You should checkout Wire.h:

class TwoWire : public Stream

and at the end:

extern TwoWire Wire;

The I2C bus is not a stream of data, it is packages of data.
The .readBytes() is from the Stream class and has a timeout with millis() because it is used for a stream. That timeout can get in the way when using the Wire library.
My solution is as shown, check the number of bytes that are available and then use .readBytes() to read those bytes. That avoids that the timeout kicks in. If you disagree, can we do that discussion somewhere else ? I don't want to confuse notsolowki.

1 Like

Variable a is of type int.
Why is the messageBuffer[] array is of type char instead of byte?

So your solution is to not use memcpy? I had not realized this,

Wire.readBytes((byte *) &sms1 , sizeof(sms1));

Would do the same thing as memcpy in my case. When you mention the timeout, do you mean If if memcpy took too long it would have caused problems?

As I understand it I think by default i2c send 32 byte packets at a time?

But it makes a lot of sense to receive the bytes right into that location

I think would be really bad practice but would it matter if was byte or char? Atleast in arduino?

Reaching just from my memory.. i think char is a byte, so it would work either way?

They are both 8 bit, but byte is unsigned (0 to 255) and char is signed (-128 to 127).

2 Likes

I think i was definitely wrong about i2c sending 32byte packets. After checking again. I discovered they are 8 bit packets and each are followed by an ack.

There is also unsigned char data type with range: 0 to 255.

Test sketch:

void setup() 
{
  Serial.begin(9600);
  unsigned char y1 = 0x80;  //range: 0 to 255
  Serial.println(y1, DEC);

  char y2 = 0x80;  //range: -128 to 127
  Serial.println(y2, DEC);

  byte y3 = 0x80;  //range: 0 to 255
  Serial.println(y3, DEC);
}

void loop() {}

Output:

128
-128
128
1 Like

In this case, technically no because Wire.requestFrom(0x50, sizeof(sms1)); is synchronous.

When you return from that call, the data is already in the Wire buffer so timeout won't be an issue there. You could actually just check the returned value of Wire.requestFrom() and if it's not 0 then assume all was right

if (Wire.requestFrom(0x50, sizeof(sms1))) {
  Wire.readBytes((byte *) &sms1 , sizeof sms1);
  ...
}

(checking that you actually got what you asked for is not really a security as you get 0 or the length you asked for)

1 Like

By synchronous i think you mean "Wire" is going to wait for response so everything else will be blocked anyways?

I was under the impression that if you access a global variable outside/during the ISR at any point that it would need to be volatile. But it appears my logic is wrong. At what point would the members of the struct need to be declared volatile?

yes. if that was not the case you could not read the data with just one if (Wire.available()) you would need a while loop to extract all the expected data

volatile is the safe way to ensure the compiler does not do funny optimisations and keep the data in a register whilst you read it from memory elsewhere. ➜ all reads and writes actually go to memory and any cache is not assumed valid

volatile does not guarantee atomicity though, so when working with multi-byte variables and an ISR, you need a critical section where you copy or handle the data.

here you pass a buffer to the Wire library, which indeed uses interrupts to get the data back but your main code does not do anything until the data has been read, and once read the ISR has no chance to run and mess with the buffer (until you ask for it). So there's no need for volatile or critical section.

to your question, nothing in the code calls readSubModule() so the optimiser just gets rid of it :slight_smile:

1 Like

You assume that every (compatible) Wire library follows that undocumented rule and that it will be the same in the future. I don't.

If I ask for three oranges and someone gives me three oranges, then it is okay.

No. It is never used in an interrupt service routine.
In fact, most of your code is largely unused, and there is no code that is executed in an interrupt context.

The volatile here applies to the void return type, it has no effect on the readSubModule function itself. A return type of volatile void is meaningless.

Okay then im afraid i still have some confusion. i read what volatile does to the compiler and i understand that. But some how i dont see the logic of using it.

For example flow sensor code,

uint32_t flow2 () // Interrupt function
{
  flow_frequency++;
}

the above function called by and interrupt. Then flow_frequency need to be volatile?

Yes.