I2C - problem

Projects Networking, Protocols, and Devices
1

Hello,
I hope I posted this in the right place.

I have made an I2C connection between an Arduino Uno and an ATMEGA328 working on a breadboard.
I want to send from slave device (ATMEGA328) an unsigned long value to the master (Arduino).
The slave code:

#include <Wire.h>

unsigned long MotPos=0;
unsigned long TimeLimit=1000;
unsigned int time;

unsigned long* MotorPosition = new unsigned long[1];
char* MotorPositionBytes=(char*)MotorPosition;

void setup()
{
Wire.begin(7);
Wire.onRequest(sendRequestedMotorPosition);
}

void loop()
{
time=millis();
if (time>=TimeLimit)
{
*MotorPosition=MotPos;
MotPos++;
TimeLimit=time+1000;
}
}

void sendRequestedMotorPosition()
{
Wire.write(MotorPositionBytes, 4);
}

Master code:
#include <Wire.h>

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

void loop()
{
unsigned long DECPosition;
Wire.requestFrom(7, 4); // request 4 bytes from slave device #7

if (Wire.available()==4)
{
//Recompose the unsigned long from 4 bytes
DECPosition = (unsigned long) Wire.read();
DECPosition = DECPosition | ((unsigned long) Wire.read() <<8 ) ;
DECPosition = DECPosition | ((unsigned long) Wire.read() << 16);
DECPosition = DECPosition | ((unsigned long) Wire.read() << 24);
Serial.println(DECPosition);
}
else
{
//transmision error
}
delay(500);
}

The connection is working but only for 50-100 requests from the master. After that the transmitted number is wrong. In serial monitor I get something like this:
350, 351, 351, 352, 352, 353, 353, 6536, 22884, 22884, 22885, 22885, 22886, 22886, 22887

I can't figure what is going wrong.

2

What I do is create a struct with the data, sometimes combined with an union. That way I don't have to do all that shifting in the Master. But it's okay, keep it as it is.

What did you try to do with the 'new' and the pointer ?

volatile unsigned long motorPosition;
...
  noInterrupts();
  motorPosition = MotPos;
  interrupts();
...
  Serial.write( (byte *) &motorPosistion, 4);
3

Thank you Peter

The motorPosition is unsigned long and I can't pass it directly to Wire.Write function. So I tried somehow to pass an array of bytes.... :smiley:

I tried with your code but the transmission works in the same way:
352,353,353, 6993, 24011, 24011, 24012 and after a while 24074, 24075, 24075, 35575, 47823, 47823, 47824 and so on. It is interesting that the wrong values are not randomly, they are always increasing.

Could you give me a hint about how could I implement a struct in master to read that unsigned long without shifting?

4

You have only one unsigned long, so there is no need to pack a number of variables into a struct.
A union can be used to declare a few bytes that make up the unsigned long.
A union sets every element to the same memory location.

I don't have my code nearby, so there might be a few mistakes. I also don't know if the msb is transmitted first. Is the msb at the lowest address of an unsigned long ?

union
{
  unsigned long ul;           // this could be a struct with a few variables
  byte by[4];                    // match the size of the normal data
} myDataPack;

myDataPack.by[0] = Wire.read();
myDataPack.by[1] = Wire.read();
myDataPack.by[2] = Wire.read();
myDataPack.by[3] = Wire.read();

Serial.println( myDataPack.ul);

An other option is the Wire.readBytes.

  Wire.readBytes( (byte *) &MotorPosition, 4);

I think the way you use time, millis() and TimeLimit in the Slave is not okay. After about a minute something will go wrong.
There is a way that is used by most Arduino users that will work.

int interval = 1000;           // internval in ms. int or unsigned int or unsigned long, all okay.
unsigned long prevMillis;   // must be unsigned long.
...
void setup()
{
  ...
  // at the end of setup(), set prevMillis.
  prevMillis = millis();
}

void loop()
{
  if( millis() - prevMillis >= interval)
  {
    prevMillis += interval;          // or use currentMillis
    ...
  }
}
5

It's working!

The problem was indeed related to the timing procedure. As you commented the code line which declares the time variable, this should be unsigned long. In my code it was unsigned int.
Of course, in the real code I will not use at all millis() - this was only to see that I2C connection works properly and also to see some changing numbers.

Also, your example for "assembling" the unsigned long with a union works very well and I think is better than shifting bytes.
The first byte transmitted is LSB and your code works as it is - without reversing the bytes.

Thank you very much.