What could cause data to be printed correctly, then incorrectly and then repeated? CAN bus / general transmission

Question

I am attempting to send data from an MPU6050 connected to an arduino uno to another uno via CAN bus, however i don't think this is a CAN bus specific issue.

In short, i am sending two packets of 8 bytes in the following formats

Packet 1 - (uint32_t/ int16_t/ int16_t)
Packet 2 - (int16_t/ int16_t/ int16_t/ int16_t)

all converted into bytes in this sort of fashion;

int16_t byte2Int(int16_t output, int pos, uint8_t data[2]){
      for (int i = 0; i < 2; i++)
    {
      output += ((int16_t)data[i + pos]) << (i * 8);
    }
    return output;
}

where everything should make sense apart from the "int pos" part which is just where in the buffer i want to start putting the data as packet 1 and 2 are different structures.

My issue is that with just one packet, i can print the values just fine, with the second packet added in i get the following on the serial monitor;

AccX    AccY    AccZ  GyX    GyY     GyZ

16832   -816    928 -440    16832   -816
16984   -940    992 -449    16984   -940
16896   -804    988 -462    16896   -804
209 55  0   0   209 55
16900   -852    1080    -471    16900   -852
16900   -852    1080    -471    16900   -852
16984   -760    948 -481    16984   -760
16880   -824    896 -459    16880   -824
230 102 0   0   230 102
16928   -860    996 -422    16928   -860
16928   -860    996 -422    16928   -860
17004   -856    864 -466    17004   -856
17004   -856    864 -466    17004   -856
214 105 0   0   214 105
16964   -828    944 -446    16964   -828
16964   -828    944 -446    16964   -828

As you can see i get some coherent messages, then some scrambled which don't make sense, then coherent again and the cycle continues. Not sure of relevance, but when i first start the serial monitor this doesn't happen, then after varying time, but around 5 seconds or so, it starts going amiss.

I believe this is down to my programming, and me not being able to code in a reliable way to wait for the first message to complete, then send the next one and them being read correctly. Below is my embarrassing attempt at sending an ACK message to the sender to say that the message has been received and to progress to the next message. I am not a programmer by nature and i am putting up this shoddy code that i've spent days trying to sort in the hope someone can point me in the right direction.

Apologies in advance.

// CAN Send Example
//

#include <mcp_can.h>
#include <SPI.h>
#include <Wire.h>

const int MPU_addr = 0x68; // I2C address of the MPU-6050
uint32_t TimeMicro;
int16_t AccX, AccY, AccZ, Tmp, GyX, GyY, GyZ; 
int8_t data1[8], data2[8], rxBuf[8];
int Switch = 7;
long unsigned int rxId;
unsigned char len = 0;

#define CAN0_INT 2                              // Set INT to pin 2
MCP_CAN CAN0(10);     // Set CS to pin 10

void setup()
{
  Serial.begin(115200);

  // Initialize MCP2515 running at 16MHz with a baudrate of 500kb/s and the masks and filters disabled.
  if(CAN0.begin(MCP_ANY, CAN_500KBPS, MCP_16MHZ) == CAN_OK) Serial.println("MCP2515 Initialized Successfully!");
  else Serial.println("Error Initializing MCP2515...");

  CAN0.setMode(MCP_NORMAL);   // Change to normal mode to allow messages to be transmitted

  pinMode(Switch, OUTPUT);
  digitalWrite(Switch, HIGH);

  Wire.begin();
  Wire.beginTransmission(MPU_addr);
  Wire.write(0x6B);  // PWR_MGMT_1 register
  Wire.write(0);     // set to zero (wakes up the MPU-6050)
  Wire.endTransmission(true);


}

void loop()
{

  TimeMicro = millis();

  Wire.beginTransmission(MPU_addr);
  Wire.write(0x3B);  // starting with register 0x3B (ACCEL_XOUT_H)
  Wire.endTransmission(false);
  Wire.requestFrom(MPU_addr, 14, true); // request a total of 14 registers
  AccX = Wire.read() << 8 | Wire.read(); // 0x3B (ACCEL_XOUT_H) & 0x3C (ACCEL_XOUT_L)
  AccY = Wire.read() << 8 | Wire.read(); // 0x3D (ACCEL_YOUT_H) & 0x3E (ACCEL_YOUT_L)
  AccZ = Wire.read() << 8 | Wire.read(); // 0x3F (ACCEL_ZOUT_H) & 0x40 (ACCEL_ZOUT_L)
  Tmp = Wire.read() << 8 | Wire.read();
  GyX = Wire.read() << 8 | Wire.read(); // 0x43 (GYRO_XOUT_H) & 0x44 (GYRO_XOUT_L)
  GyY = Wire.read() << 8 | Wire.read(); // 0x45 (GYRO_YOUT_H) & 0x46 (GYRO_YOUT_L)
  GyZ = Wire.read() << 8 | Wire.read(); // 0x47 (GYRO_ZOUT_H) & 0x48 (GYRO_ZOUT_L)

  double2Byte(TimeMicro, 0, data1);
  int2Byte(AccX, 4, data1);
  int2Byte(AccY, 6, data1);

  int2Byte(AccZ, 0, data2);
  int2Byte(GyX, 2, data2);
  int2Byte(GyY, 4, data2);
  int2Byte(GyZ, 6, data2);  

  // send data:  ID = 0x100, Standard CAN Frame, Data length = 8 bytes, 'data' = array of data bytes to send
  byte sndStat = CAN0.sendMsgBuf(0x100, 0, 8, data1);

  if(!digitalRead(CAN0_INT))                         // If CAN0_INT pin is low, read receive buffer
  {
    CAN0.readMsgBuf(&rxId, &len, rxBuf);      // Read data: len = data length, buf = data byte(s)

    while(rxBuf[0] != 0x0A)
    {
      continue;
    }

  }
  delay(10);
  byte sndStat2 = CAN0.sendMsgBuf(0x100, 0, 8, data2);

    while(rxBuf[0] != 0x0A)
    {
      continue;
    }

  for(int i = 0; i < 8; i++)
  {
    data1[i] = 0;
    data2[i] = 0;
  }

}

//===================================================================================
//                                  FUNCTIONS                                             
//===================================================================================
uint8_t int2Byte(int16_t input, int pos, uint8_t data[2]){
  for (int i = 0; i < 2; i++)
  {
    data[i+ pos] = ((input >> (i * 8)) & 0xff);
  }
return data[2];
}

uint8_t double2Byte(uint32_t input, int pos, uint8_t data[4]){
  for (int i = 0; i < 4; i++)
  {
    data[i+ pos] = ((input >> (i * 8)) & 0xff);
  }
return data[4];
}



/*********************************************************************************************************
  END FILE
*********************************************************************************************************/

Receiver

// CAN Receive Example
//

#include <mcp_can.h>
#include <SPI.h>

long unsigned int rxId;
unsigned char len = 0;
unsigned char rxBuf1[8];
unsigned char rxBuf2[8];
unsigned char txBuf[8];
char msgString[128];                        // Array to store serial string
uint32_t TimeMicro;
int16_t AccX, AccY, AccZ, GyX, GyY, GyZ;

#define CAN0_INT 2                              // Set INT to pin 2
MCP_CAN CAN0(10);                               // Set CS to pin 10


void setup()
{
  Serial.begin(115200);

  // Initialize MCP2515 running at 16MHz with a baudrate of 500kb/s and the masks and filters disabled.
  if(CAN0.begin(MCP_ANY, CAN_500KBPS, MCP_16MHZ) == CAN_OK)
    Serial.println("MCP2515 Initialized Successfully!");
  else
    Serial.println("Error Initializing MCP2515...");

  CAN0.setMode(MCP_NORMAL);                     // Set operation mode to normal so the MCP2515 sends acks to received data.

  pinMode(CAN0_INT, INPUT);                            // Configuring pin for /INT input

  Serial.println("MCP2515 Library Receive Example...");
}

void loop()
{
  if(!digitalRead(CAN0_INT))                         // If CAN0_INT pin is low, read receive buffer
  {
    CAN0.readMsgBuf(&rxId, &len, rxBuf1);      // Read data: len = data length, buf = data byte(s)
  }

  for(int i; i < 8; i++){
    txBuf[i] = 0x0A;
  }

  byte sndStat = CAN0.sendMsgBuf(0x101, 0, 8, txBuf);

  if(!digitalRead(CAN0_INT))                         // If CAN0_INT pin is low, read receive buffer
  {
  CAN0.readMsgBuf(&rxId, &len, rxBuf1);      // Read data: len = data length, buf = data byte(s)
  }

  byte sndStat2 = CAN0.sendMsgBuf(0x101, 0, 8, txBuf);


  TimeMicro = byte2Double(TimeMicro, 0, rxBuf1);
  AccX = byte2Int(AccX, 4, rxBuf1);
  AccY = byte2Int(AccY, 6, rxBuf1);
  AccZ = byte2Int(AccZ, 0, rxBuf2);
  GyX = byte2Int(GyX, 2, rxBuf2);
  GyY = byte2Int(GyY, 4, rxBuf2);
  GyZ = byte2Int(GyZ, 6, rxBuf2);


  Serial.print(TimeMicro);
  Serial.print("\t");
  Serial.print(AccX);
  Serial.print("\t");
  Serial.print(AccY);
  Serial.print("\t");
  Serial.print(AccZ);
  Serial.print("\t");
  Serial.print(GyX);
  Serial.print("\t");
  Serial.print(GyY);
  Serial.print("\t");
  Serial.println(GyZ);



  TimeMicro = 0;
  AccX = 0;
  AccY = 0;
  AccZ = 0;
  GyX = 0;
  GyY = 0;
  GyZ = 0;


}

//===================================================================================
//                                  FUNCTIONS                                             
//===================================================================================

uint32_t byte2Double(uint32_t output, int pos, uint8_t data[4]){
      for (int i = 0; i < 4; i++)
    {
      output += ((uint32_t)rxBuf1[i + pos]) << (i * 8);
    }
    return output;
}

int16_t byte2Int(int16_t output, int pos, uint8_t data[2]){
      for (int i = 0; i < 2; i++)
    {
      output += ((int16_t)rxBuf1[i + pos]) << (i * 8);
    }
    return output;
}
/*********************************************************************************************************
  END FILE
*********************************************************************************************************/

I've tried debugging this by the following;

• check that the MPU is sending correctly (Fine)
• check if it happens with just one packet (no problems)
• check if the packets are assembled correctly on the sender ( got the data, broke it into bytes and reassembled and printed that result. did what i expected)
• removed time from being sent incase that was causing an issue (same problem as illustrated above)
• removed all serial prints and just did one value (still an issue)

So i believe that it is down to a terrible and non working attempt at sending an acknowledgment message. I have tried researching but i couldn't find an example for CAN bus, and all the others were using protocol specific interrupt routines which i tried to implement but didn't work. obviously.

Thankyou for your patience

Answer 1

DISCLAIMER: Ok, I never worked with CAN on arduino, so I may be terribly wrong, nor I have the required boards to test this, so I may make a lot of errors; if you find any issue, please point them out, so I can fix them (or if you fixed them notify me, so I can update the reply for the future readers).

Ok, from what I can read you are have two packets, but only one CAN frame. This seems to me a huge inefficiency, because how can you know which frame have you just received?

To fix this, you can either use two frames or use something in the payload that informs you which frame you have received. In my opinion, since there are a lot of free IDs, you can use the first approach.

Moreover you are performing some checks in the processing (e.g. "did I receive an ack?"), but if this fails you go on without further checking. For instance, when in the sender you write:

byte sndStat = CAN0.sendMsgBuf(0x100, 0, 8, data1);
if(!digitalRead(CAN0_INT))
{
  ...
}
delay(10);
byte sndStat2 = CAN0.sendMsgBuf(0x100, 0, 8, data2);

what happens when CAN0_INT is still high? Yes, it sends the data immediately.

I could not understand the while(rxBuf[0] != 0x0A) part; it will block your program forever...

Finally when you receive a message you never check the ID; this means you will check all the messages on CAN, and when you add another device on the net you will not be able to send any other message.

Ok, so this is what I'd try for your needs:

• The TX board will always transmit data, while the RX board will always receive it. No ack frames is needed
• There will be two frames on the network: frame 0x100 will have packet 1, while frame 0x101 will have packet 2
• The TX board will send one message every 100ms; this can be reduced, but specify a time rather than having the part send frames "continuously"
• The RX board will consider the data valid only after a 0x101 packet is received and a 0x100 packet was received before.

So, here is the code. Please note that I refactored some parts for easier understanding (putting some code in functions to have a smaller "higher level" setup and loop functions.

BTW what is the "Switch" in the sender? It is never used...

Sender:

// CAN Send Example
//

#include <mcp_can.h>
#include <SPI.h>
#include <Wire.h>

const int MPU_addr = 0x68; // I2C address of the MPU-6050

uint32_t lastTimeSent;
uint32_t currMillis;
const uint32_t intervalSendMs = 100; // Send period in milliseconds

int16_t AccX, AccY, AccZ, Tmp, GyX, GyY, GyZ; 
uint8_t data1[8], data2[8];
int Switch = 7;

#define CAN0_INT 2    // Set INT to pin 2
MCP_CAN CAN0(10);     // Set CS to pin 10

void setup()
{
    Serial.begin(115200);

    initializeMCP2515();

    pinMode(Switch, OUTPUT);
    digitalWrite(Switch, HIGH);

    initializeMPU6050();
}

void loop()
{
    currMillis = millis();

    if ((currMillis - lastTimeSent) >= intervalSendMs)
    {
        lastTimeSent += intervalSendMs;

        readFromMPU6050();

        convertDataToPackets();

        // Send the two frames; if the first sending is successful send also the second, otherwise abort
        // send data frame 1:  ID = 0x100, Standard CAN Frame, Data length = 8 bytes, 'data' = array of data bytes to send
        // send data frame 2:  ID = 0x101, Standard CAN Frame, Data length = 8 bytes, 'data' = array of data bytes to send
        if (CAN0.sendMsgBuf(0x100, 0, 8, data1) == CAN_OK)
            CAN0.sendMsgBuf(0x101, 0, 8, data2);
    }
}

//===================================================================================
//                                  FUNCTIONS                                             
//===================================================================================
void initializeMCP2515()
{
    // Initialize MCP2515 running at 16MHz with a baudrate of 500kb/s and the masks and filters disabled.
    if(CAN0.begin(MCP_ANY, CAN_500KBPS, MCP_16MHZ) == CAN_OK)
        Serial.println("MCP2515 Initialized Successfully!");
    else
        Serial.println("Error Initializing MCP2515...");

    CAN0.setMode(MCP_NORMAL);   // Change to normal mode to allow messages to be transmitted
}

void initializeMPU6050()
{
    Wire.begin();
    Wire.beginTransmission(MPU_addr);
    Wire.write(0x6B);  // PWR_MGMT_1 register
    Wire.write(0);     // set to zero (wakes up the MPU-6050)
    Wire.endTransmission(true);
}

void readFromMPU6050()
{
    Wire.beginTransmission(MPU_addr);
    Wire.write(0x3B);  // starting with register 0x3B (ACCEL_XOUT_H)
    Wire.endTransmission(false);
    Wire.requestFrom(MPU_addr, 14, true); // request a total of 14 registers
    AccX = Wire.read() << 8 | Wire.read(); // 0x3B (ACCEL_XOUT_H) & 0x3C (ACCEL_XOUT_L)
    AccY = Wire.read() << 8 | Wire.read(); // 0x3D (ACCEL_YOUT_H) & 0x3E (ACCEL_YOUT_L)
    AccZ = Wire.read() << 8 | Wire.read(); // 0x3F (ACCEL_ZOUT_H) & 0x40 (ACCEL_ZOUT_L)
    Tmp = Wire.read() << 8 | Wire.read();
    GyX = Wire.read() << 8 | Wire.read(); // 0x43 (GYRO_XOUT_H) & 0x44 (GYRO_XOUT_L)
    GyY = Wire.read() << 8 | Wire.read(); // 0x45 (GYRO_YOUT_H) & 0x46 (GYRO_YOUT_L)
    GyZ = Wire.read() << 8 | Wire.read(); // 0x47 (GYRO_ZOUT_H) & 0x48 (GYRO_ZOUT_L)
}

void convertDataToPackets()
{
    long2Byte(currMillis, 0, data1);
    int2Byte(AccX, 4, data1);
    int2Byte(AccY, 6, data1);

    int2Byte(AccZ, 0, data2);
    int2Byte(GyX, 2, data2);
    int2Byte(GyY, 4, data2);
    int2Byte(GyZ, 6, data2);
}

void int2Byte(int16_t input, int pos, uint8_t *data)
{
    for (int i = 0; i < 2; i++)
    {
        data[i + pos] = ((input >> (i * 8)) & 0xff);
    }
}

void long2Byte(uint32_t input, int pos, uint8_t *data)
{
    for (int i = 0; i < 4; i++)
    {
        data[i + pos] = ((input >> (i * 8)) & 0xff);
    }
}

/*********************************************************************************************************
  END FILE
*********************************************************************************************************/

Receiver:

// CAN Receive Example
//

#include <mcp_can.h>
#include <SPI.h>

long unsigned int rxId;
unsigned char len = 0;
unsigned char rxBuf[8];
uint8_t receivedFirstFrame;

char msgString[128];                        // Array to store serial string
uint32_t TimeMicro;
int16_t AccX, AccY, AccZ, GyX, GyY, GyZ;

#define CAN0_INT 2  // Set INT to pin 2
MCP_CAN CAN0(10);   // Set CS to pin 10


void setup()
{
    Serial.begin(115200);

    initializeMCP2515();

    Serial.println("MCP2515 Library Receive Example...");
    receivedFirstFrame = 0;
}

void loop()
{
    if(!digitalRead(CAN0_INT))
    {
        // Received a message
        if (CAN0.readMsgBuf(&rxId, &len, rxBuf) == CAN_OK)
        {
            // Note: the above line is not like in the examples, but looking at the code the
            // function may return if no message was in the queue, so I think this is safer

            // Let's check if it is one of the known messages
            switch (rxId)
            {
            case 0x100:
                { // Received first frame
                    // Decode the content and store that the first frame was received
                    decodeFirstFrame(rxBuf);
                    receivedFirstFrame = 1;
                }
                break;
            case 0x101:
                if (receivedFirstFrame)
                { // Received second frame after a first frame
                    // Decode the content
                    decodeSecondFrame(rxBuf);

                    printDataToSerial();

                    receivedFirstFrame = 0;
                }
                break;
            }
        }
    }
}

//===================================================================================
//                                  FUNCTIONS                                             
//===================================================================================

void initializeMCP2515()
{
    // Initialize MCP2515 running at 16MHz with a baudrate of 500kb/s and the masks and filters disabled.
    if(CAN0.begin(MCP_ANY, CAN_500KBPS, MCP_16MHZ) == CAN_OK)
        Serial.println("MCP2515 Initialized Successfully!");
    else
        Serial.println("Error Initializing MCP2515...");

    CAN0.setMode(MCP_NORMAL); // Change to normal mode to allow messages to be transmitted

    pinMode(CAN0_INT, INPUT); // Configuring pin for /INT input
}

void decodeFirstFrame(uint8_t *buffer)
{
    TimeMicro = byte2Double(0, buffer);
    AccX = byte2Int(4, buffer);
    AccY = byte2Int(6, buffer);
}

void decodeSecondFrame(uint8_t *buffer)
{
    AccZ = byte2Int(0, buffer);
    GyX = byte2Int(2, buffer);
    GyY = byte2Int(4, buffer);
    GyZ = byte2Int(6, buffer);
}

void printDataToSerial()
{
    Serial.print(TimeMicro);
    Serial.print("\t");
    Serial.print(AccX);
    Serial.print("\t");
    Serial.print(AccY);
    Serial.print("\t");
    Serial.print(AccZ);
    Serial.print("\t");
    Serial.print(GyX);
    Serial.print("\t");
    Serial.print(GyY);
    Serial.print("\t");
    Serial.println(GyZ);
}

uint32_t byte2Double(int pos, uint8_t *data)
{
    uint32_t output = 0;
    for (int i = 0; i < 4; i++)
    {
      output += ((uint32_t)data[i + pos]) << (i * 8);
    }
    return output;
}

int16_t byte2Int(int pos, uint8_t *data)
{
    int16_t output = 0;
    for (int i = 0; i < 2; i++)
    {
      output += ((int16_t)data[i + pos]) << (i * 8);
    }
    return output;
}
/*********************************************************************************************************
  END FILE
*********************************************************************************************************/

Please let me know if you have any comments or find some issues with this.

Answer 2

There are indeed a few coding issues in your sketch, especially in the receiver part. I will try to list them and suggest changes.

1. If you look closely to your received data, you will see that AccX==GyY and AccY==GyZ on every line of your output. This is because you use the global variable rxBuf1 in your byte2Int and byte2Double functions on the receiver side. This causes the output to consume rxBuf1 also for the second CAN frame. Instead you should use the pointer to data that you have prepared in the inputs for this purpose.
2. In the receiver sketch, for both times you read in the CAN message with CAN0.readMsgBuf you write the result to rxBuf1, although for the second read you most likely intended to write to rxBuf2. Now you simply overwrite rxBuf1 and therefore the content of the first received CAN message in the loop is never evaluated.
3. The switched outputs every few lines, in my opinion, are most likely due to a mixed order of both messages on the receiver side, when you receive CAN message 2 before CAN message 1. In order to avoid this, please follow the nice suggestions by @frarugi87 to send both messages with separate CAN IDs and to check for receiption of both before printing them.
4. I also agree that you can get rid of the acknowledge reply from the receiver. The can protocol already contains an acknowledge mechanism on the CAN transceiver that will notify the sender that at least one listening node has correctly received the message. If the sender does not get this acknowledge indication, it will repeat to send the message for some time.