CAN message reception when 2 in a row

Hello Forum - this is my first post (after using Arduino for many years) - so please be patient with me :slight_smile:

Can anyone explain how the CAN driver treat the "second" message that is received before the first one is decoded? My device is sending "burst" of 2 CAN messages but the CAN driver only receives the first one.

The image shows the 2 CAN messages, and the red trace goes high when the ISR starts decoding a message and goes low when finished. Since it is way after the second CAN message, seems that it is just lost.
How can I prevent this? I need to get both CAN messages.

Thank you for any hints you may have!

Welcome to the forum.

Can you please provide some more specifics about your system? There are many different systems supporting CAN.

  • What Arduino are you using?
  • What CAN module/peripheral?
  • If the CAN module is extern how is it connected? e.g., SPI bus speed
  • Can you post your code? (Please use code tags. Click < / > )
  • Which CAN library are you using?
  • What do you mean by CAN driver?

In general there should not be an issue receiving multiple CAN messages. CAN is fairly slow (max 1Mbits/s. External CAN modules use SPI which can run multiple times that fast. For slow MCUs CAN controller like the MCP2515 have filters and buffers to help.

Hello Klaus,
Thanks for asking for more detail. I am using Arduino Mega, Generic CAN board with MCP2515, the "standard" Arduino CAN library - comes in from IDE, serial connection, SPI bus I have reduced to 500k from normal 1M to be more secure with data. I also have a touchscreen LCD that I have a GUI built on to show the CAN data.

The basic program works, I read CAN just fine, but because the CAN sending device sends 2 messages right after each other, it seems that the second in the burst is never getting processed. Does the MPC2515 have a message queue? If so, how do you access it? There are no functions in API that I see to do this. Because after the ISR returns, it is not called again until another "burst" of 2 messages comes in. So the second message in each burst is never available to the Arduino.

I tried to call CAN.parsePacket() after reading the data from the first message while still in the ISR but it never shows that bytes are available. The second message just seems to get lost.

Here are the appropriate code snippets.

in setup function, I have:

  CAN.setPins(CS,IRQ);                  //Set Chip select and interrupt pins for SPI to CAN chip
  CAN.setClockFrequency(16e6);            //Set CAN clock frequency
  CAN.onReceive(receiveCAN);            //Set callback function for reception of CAN message

Then in loop function, when user wants to start receiving the CAN messages, I have:

            CAN.setSPIFrequency(5E6);
            if (!CAN.begin(500E3)) {
              Serial.println("Starting CAN failed!");
              while (1);
            } else Serial.println("Can started");

My isr is:

void receiveCAN(int packetSize){ //This function is the callback for received CAN data
  int i=0;
  int j=0;
  int om=0;                      //old message flag
  int val;
  float fval;
  int bytes[8];
  
  long id= CAN.packetId();        //Get message ID
  int DLC = CAN.packetDlc();      //Get packet data length
//  int packetSize = CAN.parsePacket();  //Get packetsize
  digitalWrite(37, HIGH);
  if (CAN.packetRtr()) return;
  for (i=0; i< nummess; i++) {                  //Loop over messages in database so far
    if (Mess[i].id == id) {                     //If there is a match
/*      sprintf(string1,"Rec i=%d, id=%lx dl=%d\n",i,id, Mess[i].fl);
      Serial.print(string1); */
      om = 1;                                   //Set flag that this message is already in database (old message)
      j=0;                                      //Zero the byte counter        
      while (CAN.available()) {                 //While there are available bytes to read
        Mess[i].by[j++]= CAN.read();            //Grab the data
      }  
/*      sprintf(string1,"Exist %d: id=%lX, ps=%d, Data = %02X %02X %02X %02X %02X %02X %02X %02X\n",i, id,Mess[i].fl,Mess[i].by[0],Mess[i].by[1] ,Mess[i].by[2],Mess[i].by[3],Mess[i].by[4],Mess[i].by[5],Mess[i].by[6],Mess[i].by[7]);
      Serial.print(string1);*/
    }
  }
  if (om == 0) {                              //Else this a new message
/*    sprintf(string1,"New %x\n",id);
    Serial.print(string1); */
    Mess[i].id = id;                          //record message ID
    Mess[i].fl = packetSize;                  //Record # data bytes
    j=0;                                      //Zero the byte counter        
    while (CAN.available()) {                 //While there are available bytes to read
      Mess[i].by[j++]= CAN.read();            //Grab the data
    }  
/*    sprintf(string1,"New %d: id=%lX, ps=%d, Data = %02X %02X %02X %02X %02X %02X %02X %02X\n",i,id,Mess[i].fl,Mess[i].by[0],Mess[i].by[1] ,Mess[i].by[2],Mess[i].by[3],Mess[i].by[4],Mess[i].by[5],Mess[i].by[6],Mess[i].by[7]);
    Serial.print(string1); */
    nummess++;                                //Increment number of messages in database
  }
  
  if (i < NUMMESS) {
    for (j=0;j<Mess[i].nd;j++) {    //Loop over the number of data elements in the received message
      val = element(Mess[i].by,Data[Mess[i].data[j]].sb,Data[Mess[i].data[j]].len); //Retrieve the integer value
      fval = (float)val*Data[Mess[i].data[j]].sc + Data[Mess[i].data[j]].of;        //Use scaling and offset to get floating point value
    }
  }
  packetSize = CAN.parsePacket();
  if (packetSize != 0) {
    Serial.print("Got second message\n");
  }
  digitalWrite(37, LOW);
}

Here's a zoomed out scope trace- each burst of 2 messages generates only one interrupt - and only the first message gets received.

Thanks for any advice!
Best Regards,
Richard

SPI should work at much higher frequencies. Try 5MHz. You have some overhead in the library. Reading some bits, making decisions and then reading some more. You want all data read by the time the second message has arrived.

I would reduce the ISR to the minimum. Retrieve the data store it in a buffer and do the rest in the main loop. You want your trace pin to go HIGH and LOW while the second message arrives. That way you should not miss any interrupts.

You should not access to any other interface but SPI and your one digital pin inside the ISR.´e.g., no printing ...

The MCP2515 has two receive buffers.

For your trace, set the pin 37 first thing in your code. This way you will see when the interrupt will start processing. It should start while the second message is received.

Can you confirm the library is from Sandeep Mistry?

Can you post your complete sketch?

Hi Klaus,
Sorry typo on SPI, I use 5M instead of default 10M.
Yes library is the one from Sandeep Mistry.
In the ISR I set pin 37 first thing, and clear it last thing. All the printing is commented out. The receiving ISR only takes 100us. From the scope trace, you can see that the ISR is not even entered until after the second CAN message is received. This is my issue. The MPC2515 has 2 receive buffers but I am not sure how the library gets data from the buffers - does it automatically get data from both or only one? When received - does the library clear the buffers?
I am an old plain C guy, I am very weak at C++ so I have not really dived into the library code itself.

Here is the whole sketch - a lot is the LCD and UI.

type or paste code here

/*
 * CAN Program - reads CAN data
 * Assumes no data element more that 16 bits
 */
#include <CAN.h>
#include <EEPROM.h>
#include <LCDWIKI_GUI.h> //Core graphics library
#include <LCDWIKI_KBV.h> //Hardware-specific library
#include <TouchScreen.h> //touch library

LCDWIKI_KBV mylcd(ILI9486,A3,A2,A1,A0,A4); //Instantiate the LCD with parameters model,cs,cd,wr,rd,reset

//Define some items for touchscreen
#define YP A3  // must be an analog pin, use "An" notation!
#define XM A2  // must be an analog pin, use "An" notation!
#define YM 9   // can be a digital pin
#define XP 8   // can be a digital pin
//param calibration from kbv
#define TS_MINX 906
#define TS_MAXX 116
#define TS_MINY 92 
#define TS_MAXY 952
#define MINPRESSURE 10
#define MAXPRESSURE 1000
#define TOUCHTHRESH 100
#define PRESSDELAY  20           //ms delay for each press
// For better pressure precision, we need to know the resistance
// between X+ and X- Use any multimeter to read it
// For the one we're using, its 300 ohms across the X plate
TouchScreen ts = TouchScreen(XP, YP, XM, YM, 300);        //Instantiate the touchscreen

//define some colour values
#define BLACK   0x0000
#define BLUE    0x001F
#define RED     0xF800
#define GREEN   0x07E0
#define CYAN    0x07FF
#define MAGENTA 0xF81F
#define YELLOW  0xFFE0
#define WHITE   0xFFFF

#define GREY    (BLUE/16 + RED/16 + GREEN/16)
#define TFT_NAVY        0x000F      /*   0,   0, 128 */
#define TFT_DARKGREEN   0x03E0      /*   0, 128,   0 */
#define TFT_DARKCYAN    0x03EF      /*   0, 128, 128 */
#define TFT_MAROON      0x7800      /* 128,   0,   0 */
#define TFT_PURPLE      0x780F      /* 128,   0, 128 */
#define TFT_OLIVE       0x7BE0      /* 128, 128,   0 */
#define TFT_LIGHTGREY   0xC618      /* 192, 192, 192 */
#define TFT_DARKGREY    0x7BEF      /* 128, 128, 128 */
#define TFT_BLUE        0x001F      /*   0,   0, 255 */
#define TFT_GREEN       0x07E0      /*   0, 255,   0 */
#define TFT_CYAN        0x07FF      /*   0, 255, 255 */
#define TFT_RED         0xF800      /* 255,   0,   0 */
#define TFT_MAGENTA     0xF81F      /* 255,   0, 255 */
#define TFT_YELLOW      0xFFE0      /* 255, 255,   0 */
#define TFT_WHITE       0xFFFF      /* 255, 255, 255 */
#define TFT_ORANGE      0xFDA0      /* 255, 180,   0 */
#define TFT_GREENYELLOW 0xB7E0      /* 180, 255,   0 */
#define TFT_PINK        0xFC9F      /* 255, 128, 255 */
/*#define TFT_ORANGE      0xFC00      /* 255, 128,   0 */
#define BACKCOLOR BLACK

#define UPARROW 24            //ASCII code for up arrow 
#define DOWNARROW 25          //ASCII code for down arrow
#define ARROWWID 39           //Arrow button width in pixels
#define ARROWHEI 40           //Arrow button height in pixels
#define ARROWTEXTSIZE 5       //Arrow text size. 5*7 = 35 pixels high, fits in 40 high box
#define ARROWRAD 3            //Arrow button radius - make sharper corners than regular button
#define NORMAL  0             //Normal or inverted for buttons
#define INVERT 1
#define ARROWTEXTCOLOR BLACK  //Arrow text color
#define ARROWBUTTONCOLOR WHITE//Arrow button color
#define BUTTONWID 140         //Menu button width in pixels
#define BUTTONHEI 45          //Menu button height in pixels
#define BUTTONTEXTSIZE 4      //Menu button text size
#define BUTTONRAD 5           //Menu button corner radius - soft
#define SEQTEXTSIZE 8         //Sequence numbers size 6
#define TITLETEXTSIZE 6       //Title text size 6
#define TEXTSIZE 4            //Size of regular text
#define DATATEXTSIZE 2        //Size of data text
#define NUMDATALINES (270/(DATATEXTSIZE*9)-1) //Number of lines 
#define NUMDATACHARS 480/(DATATEXTSIZE*6) //Number of characters per line
#define DATAy 50              //x coord of where data starts
#define DATAx (wid-BUTTONWID) //Y of where data ends

//Generally have 4 lines. 50 pixels high. 40 pixels between lines
#define LINE1 25                      //y coordinate for center of items on "line 1" of the display
#define LINE2 115                     //y coordinate for center of items on "line 2" of the display
#define LINE3 225                     //y coordinate for center of items on "line 3" of the display - moved down 20 pixels from even spacing
#define LINE4 295                     //y coordinate for center of items on "line 4" of the display

//LCD Variables
int wid;              //Width of LCD (480)
int hei;              //Height of LCD (320)
#define match(x, y, x1, y1, x2, y2) (((x > x1 && x < x2) && (y > y1 && y < y2))? 1 : 0)

char uparrow[2] = {24,0}; //Define string for up arrow character
char dnarrow[2] = {25,0}; //Define string for down arrow character
char rtarrow[2] = {26,0}; //Define string for right arrow character
char lfarrow[2] = {27,0}; //Define string for left arrow character

//Define the locations of all the buttons
#define STARTx (wid-BUTTONWID/2)        //x coordinate for start/stop menu button -  right
#define STARTy LINE1                    //y coordinate for Start/stop menu button - top 
//#define STOPx (wid-BUTTONWID/2)         //x coordinate for stop menu button - right
//#define STOPy LINE1                     //y coordinate for Sart menu button - top 
#define UPx (wid-ARROWWID/2)            //x coordinate for up button
#define UPy (LINE2-3-ARROWHEI/2)        //y coordinate for up button
#define DNx (wid-ARROWWID/2)            //x coordinate for down button
#define DNy (LINE2+3+ARROWHEI/2)        //y coordinate for down button
#define MESSx (wid-BUTTONWID/2)  //x coordinate for message/data button
#define MESSy (LINE4)  //y coordinate for message/data button

// General global variables
char string1[80];     //Used to build strings to print
int ms;               //Menu State - define names for each state to make it easier to undersand
int mi;               //Menu item
int start;            //0 if stopped, 1 if running
int view;             //0 if data elements, 1 if show raw message data
int nummess;          //Number of messages seen so far. Starts at NUMMESS which are defined, others are added as come in on CAN

//Menu definitions
#define TOP 0

#define NUMDATA 21   //Number of data elements
struct Data {
  int mn;         //message number
  char dn[16];    //Data element name
  char sb;        //start bit
  char len;       //length
  char en;        //0 = MSByte first, 1 = LSByte first
  char sn;        //Signed = 1, unsigned = 0
  float sc;       //Scaling
  float of;       //Offset => Real = raw*scaling + offset
//  float mnn;    //Minumim value
//  float mxn;    //Maximum value
  char unit[6];   //Unit string
  float value;    //floating point value
} Data[]= {
  {1538,"Speed",38,16,0,1,1,-25000,"RPM",0},{1538,"DCV",31,9,0,0,1,0,"VDC",10.0},
  {1538,"IGBT Temp",54,12,0,0,0.1,-50,"Deg C",0},{1792,"TqCmd",14,12,0,0,0.5,-600,"Nm",0},
  {1792,"PSCmd",7,2,0,0,0,1,"",0},{1540,"Vbatt1",7,8,0,0,0.1,0,"V",0},
  {1540,"IGN",51,8,0,0,0.1,0,"V",0},
  {1538,"Speed",38,16,0,1,1,-25000,"RPM",0},{1538,"DCV",31,9,0,0,1,0,"VDC",10.0},
  {1538,"IGBT Temp",54,12,0,0,0.1,-50,"Deg C",0},{1792,"TqCmd",14,12,0,0,0.5,-600,"Nm",0},
  {1792,"PSCmd",7,2,0,0,0,1,"",0},{1540,"Vbatt1",7,8,0,0,0.1,0,"V",0},
  {1540,"IGN",51,8,0,0,0.1,0,"V",0},  
  {1538,"Speed",38,16,0,1,1,-25000,"RPM",0},{1538,"DCV",31,9,0,0,1,0,"VDC",10.0},
  {1538,"IGBT Temp",54,12,0,0,0.1,-50,"Deg C",0},{1792,"TqCmd",14,12,0,0,0.5,-600,"Nm",0},
  {1792,"PSCmd",7,2,0,0,0,1,"",0},{1540,"Vbatt1",7,8,0,0,0.1,0,"V",0},
  {1540,"IGN",51,8,0,0,0.1,0,"V",0}
} ;

#define NUMMESS 3  //Number of messages
static struct Message {
  long id;       //id = frame ID
  int fl;        //fl = frame length bytes
  int by[8];     //Raw data
  int rw;        //rw = read/write, 0=read, 1 = write
  int nd;        //Number of data elements
  int data[8];   //Array of pointers to data elements (index number
} Mess[40] = {
  {1538,8,{10,20,30,40,50,60,70,80},0,3,{0,1,2,-1,-1,-1,-1,-1}},  //Hex ID 602
  {1792,8,{10,20,30,40,50,60,70,80},1,2,{3,4,-1,-1,-1,-1,-1,-1}}, //Hex ID 700
  {1540,8,{10,20,30,40,50,60,70,80},0,2,{5,6,-1,-1,-1,-1,-1,-1}}  //Hex ID 604
};

#define CS 23   //Chip select pin
#define IRQ 21   //IRQ pin - must be interrupt capable 
//Note that for Mega - pin 50 is MISO, 51 is MOSI, 52 is SCK. 
//#define RX  4   //Receive data pin
//#define TX  5   //Transmit data pin
#define CF  16E6 //Clock frequency 8M or 16M

//Function to draw a rectangular button with text centered in the rectangle. x and y are center of button
void draw_button(char* label, int x, int y, int wid, int hei, int size, int textcolor, int buttoncolor, int radius) {
  int labelwid;                                           //Temp variable for width of text in pixels
  mylcd.Set_Draw_color(buttoncolor);                      //Set button color
  mylcd.Fill_Round_Rectangle(x-wid/2, y-hei/2, x+wid/2, y+hei/2, BUTTONRAD); //Draw the rounded rectangle
  mylcd.Set_Text_colour(textcolor);                       //Set text color
  mylcd.Set_Text_Size(size);                              //Set text size. Each char is 5x7 pixels * size. One pixel*size space
  mylcd.Set_Text_Mode(1);                                 //1 = don't use text background color
  labelwid = strlen(label) * size * 6 - size;           //Calculate length of string - in pixels. Account for 1/2 of last space
  mylcd.Print_String(label, x-(labelwid/2), y-(size*7/2));//Draw the text in center of box
}

//Function to draw an arrow, many parameters are #defined. x and y are center of button
void draw_arrow(char* arrow, int x, int y, int invert) {  
  if (invert <= 0) {
    draw_button(arrow, x, y, ARROWWID, ARROWHEI, ARROWTEXTSIZE, ARROWTEXTCOLOR, ARROWBUTTONCOLOR, ARROWRAD);
  } else {
    draw_button(arrow, x, y, ARROWWID, ARROWHEI, ARROWTEXTSIZE, ARROWBUTTONCOLOR, ARROWTEXTCOLOR, ARROWRAD);
  }
}

//Function to draw a menu button, some parameters are #defined. x and y are center of button
void draw_menu_button(char* label, int x, int y, int textcolor, int buttoncolor) { 
  draw_button(label, x, y, BUTTONWID, BUTTONHEI, BUTTONTEXTSIZE, textcolor, buttoncolor, BUTTONRAD);
}

//Draw the main screen - only once when switching to it
void draw_main_screen() {
  int textcolor = BLUE;
  int buttoncolor = YELLOW;
  draw_menu_button((char*)"Start", STARTx, STARTy, textcolor, buttoncolor); //Draw the Start/Stop button
  draw_menu_button((char*)"Mess.", MESSx, MESSy, textcolor, buttoncolor);   //Draw the Mess/Data button
  draw_arrow(uparrow, UPx, UPy, NORMAL);                                    //Draw an up arrow
  draw_arrow(dnarrow, DNx, DNy, NORMAL);                                    //Draw a down arrow
}

TSPoint read_TS(){
  int x,y;                                  //Temp variables for calculating x&y
  static TSPoint p;                         //Statically define touchscreen point object
  digitalWrite(13, HIGH);                   //Set chip select high
  p = ts.getPoint();                        //Get the point
  digitalWrite(13, LOW);                    //Clear chip select
  pinMode(XM, OUTPUT);                      //Reset the XM to output so LCD can use it
  pinMode(YP, OUTPUT);                      //Reset the YP to output so LCD can use it
  y = map(p.x, TS_MINX, TS_MAXX, hei, 0);   //Map the raw touchscreen data to LCD coordinates - rotated!
  x = map(p.y, TS_MINY, TS_MAXY, 0 ,wid);   //Map the raw touchscreen data to LCD coordinates - rotated!
  p.x = x;                                  //Set the p.x value
  p.y = y;                                  //Set the p.y value
  return p;                                 //Return the point
}

void receiveCAN(int packetSize){ //This function is the callback for received CAN data
  int i=0;
  int j=0;
  int om=0;                      //old message flag
  int val;
  float fval;
  int bytes[8];
  
  long id= CAN.packetId();        //Get message ID
  int DLC = CAN.packetDlc();      //Get packet data length
//  int packetSize = CAN.parsePacket();  //Get packetsize
  digitalWrite(37, HIGH);
  if (CAN.packetRtr()) return;
  for (i=0; i< nummess; i++) {                  //Loop over messages in database so far
    if (Mess[i].id == id) {                     //If there is a match
/*      sprintf(string1,"Rec i=%d, id=%lx dl=%d\n",i,id, Mess[i].fl);
      Serial.print(string1); */
      om = 1;                                   //Set flag that this message is already in database (old message)
      j=0;                                      //Zero the byte counter        
      while (CAN.available()) {                 //While there are available bytes to read
        Mess[i].by[j++]= CAN.read();            //Grab the data
      }  
/*      sprintf(string1,"Exist %d: id=%lX, ps=%d, Data = %02X %02X %02X %02X %02X %02X %02X %02X\n",i, id,Mess[i].fl,Mess[i].by[0],Mess[i].by[1] ,Mess[i].by[2],Mess[i].by[3],Mess[i].by[4],Mess[i].by[5],Mess[i].by[6],Mess[i].by[7]);
      Serial.print(string1);*/
    }
  }
  if (om == 0) {                              //Else this a new message
/*    sprintf(string1,"New %x\n",id);
    Serial.print(string1); */
    Mess[i].id = id;                          //record message ID
    Mess[i].fl = packetSize;                  //Record # data bytes
    j=0;                                      //Zero the byte counter        
    while (CAN.available()) {                 //While there are available bytes to read
      Mess[i].by[j++]= CAN.read();            //Grab the data
    }  
/*    sprintf(string1,"New %d: id=%lX, ps=%d, Data = %02X %02X %02X %02X %02X %02X %02X %02X\n",i,id,Mess[i].fl,Mess[i].by[0],Mess[i].by[1] ,Mess[i].by[2],Mess[i].by[3],Mess[i].by[4],Mess[i].by[5],Mess[i].by[6],Mess[i].by[7]);
    Serial.print(string1); */
    nummess++;                                //Increment number of messages in database
  }
  
  if (i < NUMMESS) {
    for (j=0;j<Mess[i].nd;j++) {    //Loop over the number of data elements in the received message
      val = element(Mess[i].by,Data[Mess[i].data[j]].sb,Data[Mess[i].data[j]].len); //Retrieve the integer value
      fval = (float)val*Data[Mess[i].data[j]].sc + Data[Mess[i].data[j]].of;        //Use scaling and offset to get floating point value
    }
  }
  packetSize = CAN.parsePacket();
  if (packetSize != 0) {
    Serial.print("Got second message\n");
  }
  digitalWrite(37, LOW);
}

int element(int* bytes, int st, int len) { // This function returns value from 8 byte array big endian format
  unsigned long data;
  unsigned int val;
  int startbit;

  data=(unsigned long)bytes;        //Cast the data into an unsigned long
  startbit=8*(st>>3)+(7-st<<3);     //Calculate start bit in order of bytes
  val=data>>(64-(startbit+len));    //Shift data so that what we want is in lowest bits
  val &= ((1<<len)-1);              //Mask off unused high bits
  return val;                       //Return the value
}

void splash() {
  mylcd.Set_Text_Size(TITLETEXTSIZE+2);                 //Set text size. Each char is 5x7 pixels * size. One pixel*size space
  mylcd.Set_Text_colour(BLUE);                          //Set text color
  mylcd.Print_String((char *)"CAN Viewer",CENTER,50);   //First part of splash screen
  delay(500);                                           //Wait 1/2 sec
  mylcd.Set_Text_Size(TITLETEXTSIZE-1);                 //Set text size. Each char is 5x7 pixels * size. One pixel*size space
  mylcd.Print_String((char *)"by", CENTER, hei/2);      //Next part of splash screen
  delay(500);                                           //Wait 1/2 sec
  mylcd.Set_Text_Size(TITLETEXTSIZE+2);                 //Set text size. Each char is 5x7 pixels * size. One pixel*size space
  mylcd.Print_String((char *)"RJH",CENTER,hei*3/4);     //Last part of splash screen
  delay(1000);                                          //Wait 1 second
}

void setup() {
  int i;
  
  CAN.setPins(CS,IRQ);                  //Set Chip select and interrupt pins for SPI to CAN chip
  CAN.setClockFrequency(16e6);            //Set CAN clock frequency
  CAN.onReceive(receiveCAN);            //Set callback function for reception of CAN message
  mylcd.Init_LCD();                     //Instantiate LCD
  Serial.begin(9600);
//  Serial.println(mylcd.Read_ID(), HEX); //Print LCD ID to serial port
  mylcd.Set_Rotation(3);                //Set lndscape mode, USB on right side
  mylcd.Set_Text_Mode(1);               //Set text mode to no background color
  mylcd.Fill_Screen(BLACK);             //Set screen to black
  wid = mylcd.Get_Width();              //Get width (480 pixels)
  hei = mylcd.Get_Height();             //Get height (320 pixels)
  splash();                             //Show splash screen
  ms = TOP;                             //Set menu state to TOP
  mi = 0;                               //Set menu item to 0 - this is used to scroll through data
  view=0;                               //0 means show data elements, not raw messages
  nummess=NUMMESS;                      //Start with number of messages defined
  mylcd.Fill_Screen(BLACK);             //Clear screen
  draw_main_screen();                   //Draw initial screen
  show_data(0);       
  pinMode(37, OUTPUT);      
}

void show_data(int mi) {
  int i,j;

  j= min(NUMDATALINES,NUMDATA); 
  mylcd.Set_Text_Size(DATATEXTSIZE);                      //Set text size. Each char is 5x7 pixels * size. One pixel*size space
  mylcd.Set_Text_colour(GREEN);                           //Set text color for title line
  mylcd.Print_String("Item Name      Value Unit",0,DATAy-5); //Write the title line
  mylcd.Set_Text_colour(WHITE);                           //Set text color
  for (i=0; i< j; i++) {                                                            //Loop over the data elements to show
    mylcd.Print_String(Data[i+mi].dn,0,50+(i+1)*9*DATATEXTSIZE);                    //Write the name
    mylcd.Print_String(Data[i+mi].unit, 21*6*DATATEXTSIZE, 50+(i+1)*9*DATATEXTSIZE);//Write the unit
  }
  update_data(0);
}

void update_data(int mi) {
  int i,j ;
 
  j= min(NUMDATALINES,NUMDATA); 
  mylcd.Set_Text_colour(WHITE);                           //Set text color
  mylcd.Set_Text_Size(DATATEXTSIZE);                      //Set text size. Each char is 5x7 pixels * size. One pixel*size space
  for (i=0; i< j; i++) {
    dtostrf(Data[i+mi].value,7, 1, string1);
    mylcd.Print_String(string1, 13*6*DATATEXTSIZE, 50+(i+1)*9*DATATEXTSIZE);
  }
}

void show_mess(int mi) {                                    //This function shows the message screen - all the static items. 
  int i,j;                                                  //Local variables

  j= min(NUMDATALINES,nummess); 
  mylcd.Set_Text_Size(DATATEXTSIZE);                        //Set text size. Each char is 5x7 pixels * size. One pixel*size space
  mylcd.Set_Text_colour(GREEN);                             //Set text color for title line
  mylcd.Print_String("ID  Data Bytes",0,DATAy-5);           //Write the title line
  mylcd.Set_Text_colour(WHITE);                             //Set text color
  for (i=0; i< j; i++) {                                    //Loop over the messages to show
    sprintf(string1,"%03X",Mess[i+mi].id);                  //Construct the string - message ID is 11 bits so 3 characters
    mylcd.Print_String(string1,0,50+(i+1)*9*DATATEXTSIZE);  //Write the name
  }
  update_mess(0);                                           //Show all the actual data
}

void update_mess(int mi) {                                  //This fucntion shows teh message data - no need to redraw the static items
  int i,j,k;                                                //Local variables
 
  j= min(NUMDATALINES,nummess);                             //Calculate the max number of items to show
  mylcd.Set_Text_colour(WHITE);                             //Set text color
  mylcd.Set_Text_Size(DATATEXTSIZE);                        //Set text size. Each char is 5x7 pixels * size. One pixel*size space
  for (i=0; i< j; i++) {                                    //Loop over the message IDs
      for (k=0;k<Mess[i+mi].fl;k++){                        //Loop over the number of bytes to print
        sprintf(string1,"%02X",Mess[i+mi].by[k]);           //Construct the string
        mylcd.Print_String(string1, (4+k*3)*6*DATATEXTSIZE, 50+(i+1)*9*DATATEXTSIZE); //Print the string to screen
      }
  }
}

#define LOOPDELAY 100

void clear_data_area(){
  mylcd.Set_Draw_color(BLACK);
  mylcd.Fill_Rectangle(0,DATAy+9*DATATEXTSIZE,DATAx,hei);
}

void loop() {
  int i,j,textcolor,buttoncolor;    //Local variables
  int packetSize=0;   
  TSPoint p;                        //Declare the touchscreen variable

/*  if (start) {
    packetSize = CAN.parsePacket();
//    sprintf(string1,"RPS %d\n",packetSize);
//    Serial.print(string1);
    if (packetSize) {
      receiveCAN(packetSize);
    }
  } */
//    sprintf(string1,"St=%d, ps = %d\n",start,packetSize);
//    Serial.print(string1);  
    switch (ms) {                   //Switch on menu state
    case TOP: 
      p=read_TS();                //Call the touchscreen input function
      if (p.z > TOUCHTHRESH){     //This is true if there is a touchscreen press
        if(match(p.x,p.y,STARTx-BUTTONWID/2,STARTy-BUTTONHEI/2,STARTx+BUTTONWID/2,STARTy+BUTTONHEI/2)) {//Check if press is on this rectangle
          textcolor = BLUE;
          buttoncolor = YELLOW;
          if (start) {                                                                          //If running
            draw_menu_button((char*)"Start", STARTx, STARTy, textcolor, buttoncolor);            //Draw the Stop button
            start = 0;                                                                          //Set next state to stopped
            CAN.end();
          } else {                                                                              //It is stopped
            draw_menu_button((char*)"Stop", STARTx, STARTy, textcolor, buttoncolor);           //Draw the Start button
            start = 1;                                                                          //Set to running
            CAN.setSPIFrequency(5E6);
            if (!CAN.begin(500E3)) {
              Serial.println("Starting CAN failed!");
              while (1);
            } else Serial.println("Can started");
          }
        }
        if(match(p.x,p.y,MESSx-BUTTONWID/2,MESSy-BUTTONHEI/2,MESSx+BUTTONWID/2,MESSy+BUTTONHEI/2)) {//Check if press is on this rectangle
          textcolor = BLUE;
          buttoncolor = YELLOW;
          if (view) {                                                                           //If view =1, that means in message mode
            draw_menu_button((char*)"Data", MESSx, MESSy, textcolor, buttoncolor);              //Draw the Data button
            view = 0;                                                                           //Set next state to data mode
            mi = 0;                                                                             //Reset the data counter to zero
            mylcd.Fill_Screen(BLACK);                                                           //Clear screen
            draw_main_screen();                                                                 //Draw initial screen
            show_data(mi);                                                                      //Sow the data
          } else {                                                                              //It is in data mode
            draw_menu_button((char*)"Mess.", MESSx, MESSy, textcolor, buttoncolor);             //Draw the Message button
            view = 1;                                                                           //Set to message mode
            mi = 0;                                                                             //Reset the data counter to zero
            mylcd.Fill_Screen(BLACK);                                                           //Clear screen
            draw_main_screen();                                                                 //Draw initial screen
            show_mess(mi);                                                                      //Show the messages
          }
        }
        if(match(p.x, p.y, UPx-ARROWWID/2,UPy-ARROWHEI/2,UPx+ARROWWID/2,UPy+ARROWHEI/2)) {      //Check if press is on this rectangle
          draw_arrow(uparrow, UPx, UPy, INVERT);                                                //Draw an up arrow inverted
          delay(PRESSDELAY);                                                                    //Delay before redrawing button
          draw_arrow(uparrow, UPx, UPy, NORMAL);                                                //Draw an up arrow
//          sprintf(string1,"mi=%d, NUMDATA=%d, NDL=%d\n",mi,NUMDATA,NUMDATALINES);
//          Serial.print(string1);
          if (view) {                                                                           //It is in message mode
             if ((mi < NUMMESS) && (NUMMESS-mi > NUMDATALINES)) {                                //Check if OK to scroll
              mi++;                                                                             //increment mi only if not all data fits
              clear_data_area();                                                                //Need to clear screen and redraw the data. Depends on view also.
              show_mess(mi);                                                                    //Redraw data - after incremened mi
            }
          } else {                                                                              //It is in data mode
            if ((mi < NUMDATA) && (NUMDATA-mi > NUMDATALINES)) {                                //Check if OK to scroll
              mi++;                                                                             //increment mi only if not all data fits
              clear_data_area();                                                                //Need to clear screen and redraw the data. Depends on view also.
              show_data(mi);                                                                    //Redraw data - after incremened mi
            }
          }
        }
        if(match(p.x, p.y, DNx-ARROWWID/2,DNy-ARROWHEI/2,DNx+ARROWWID/2,DNy+ARROWHEI/2)) {      //Check if press is on this rectangle
          draw_arrow(dnarrow, DNx, DNy, INVERT);                                                //Draw a down arrow inverted
          delay(PRESSDELAY);                                                                    //Delay before redrawing button
          draw_arrow(dnarrow, DNx, DNy, NORMAL);                                                //Draw a down arrow
          if (mi > 0) {
            mi--;                                                                               //decrement mi
            clear_data_area();                                                                  //Need to clear screen and redraw the data. Depends on view also.
          }
        }
      }
   }
   delay(LOOPDELAY);                                                                            //Delay per loop execution
}

The library will check the interrupt flags and read the buffer where the interrupt flag is set. I am not sure what happens when both Rx buffers are full at the same time e.g., because the interrupt was delayed for some reason.

Yes, the library clears the interrupt flag. Otherwise, the buffer would not be available for another message. This is a feature of the MCP2515 to avoid overwrite of an unprocessed message. (see MCP2515 datasheet section 4.1.3)

Have a look, most of the library code is plain C. I am sure you will be able to understand what is going on.

I do not believe that to be true.

Try this. Open the file MCP2515.cpp Change the following function to write your trace pin here.

void MCP2515Class::onInterrupt()
{
  digitalWrite(37, HIGH);  
  CAN.handleInterrupt();
  digitalWrite(37, LOW);
}

If I understand this correctly, by the time your callback is called, the message has already been parsed (and therefore copied from the MCP2515 to the Arduino). You should just get the data using read(). Here is why:

  • MCP2515.cpp -> MCP2515Class::onReceive stores the callback and attaches MCP2515Class::onInterrupt to the pin interrupt
  • MCP2515.cpp -> MCP2515Class::onInterrupt (see above) calls CAN.handleInterrupt();
  • MCP2515.cpp -> MCP2515Class::handleInterrupt calls parsePacket() and then your callback function

Hi Klaus,
I think I see the problem. In the parse packet function, it looks for intf & FLAG_RXnIF(0) first, then if that is false looks for intf & FLAG_RXnIF(1). So message in buffer 0 takes priority. At the end of the function, it clears intf - so even when I call parse packet again, it will not parse the message in buffer 1 even if the FLAG_RNxIF(1) is still set.

I think I can change the end of the parse packet function to only clear the intf register if both message buffers have no new data. I will have to clear the FLAG_RXnIF of the buffer that was processed before checking the opposite one. Then, in my ISR I can call parse packet again before returning and it will then process buffer 1.

I am away from my test setup today but will try this tomorrow.

Thoughts? Do you think this is a good path?

I really appreciate your interest and encouragement.

int MCP2515Class::parsePacket()
{
  int n;

  uint8_t intf = readRegister(REG_CANINTF);

  if (intf & FLAG_RXnIF(0)) {
    n = 0;
  } else if (intf & FLAG_RXnIF(1)) {
    n = 1;
  } else {
    _rxId = -1;
    _rxExtended = false;
    _rxRtr = false;
    _rxLength = 0;
    return 0;
  }

  _rxExtended = (readRegister(REG_RXBnSIDL(n)) & FLAG_IDE) ? true : false;

  uint32_t idA = ((readRegister(REG_RXBnSIDH(n)) << 3) & 0x07f8) | ((readRegister(REG_RXBnSIDL(n)) >> 5) & 0x07);
  if (_rxExtended) {
    uint32_t idB = (((uint32_t)(readRegister(REG_RXBnSIDL(n)) & 0x03) << 16) & 0x30000) | ((readRegister(REG_RXBnEID8(n)) << 8) & 0xff00) | readRegister(REG_RXBnEID0(n));

    _rxId = (idA << 18) | idB;
    _rxRtr = (readRegister(REG_RXBnDLC(n)) & FLAG_RTR) ? true : false;
  } else {
    _rxId = idA;
    _rxRtr = (readRegister(REG_RXBnSIDL(n)) & FLAG_SRR) ? true : false;
  }
  _rxDlc = readRegister(REG_RXBnDLC(n)) & 0x0f;
  _rxIndex = 0;

  if (_rxRtr) {
    _rxLength = 0;
  } else {
    _rxLength = _rxDlc;

    for (int i = 0; i < _rxLength; i++) {
      _rxData[i] = readRegister(REG_RXBnD0(n) + i);
    }
  }

  modifyRegister(REG_CANINTF, FLAG_RXnIF(n), 0x00);

  return _rxDlc;
}

Actually, now that I look deeper, seems that my last thought was not correct. Parse packet does only clear the IF bit for the buffer that was read. If the other buffer has received, its IF should remain set and when I call parse packet a second time it should see it....

There are several other libraries that work with the MCP2515.
For example GitHub - coryjfowler/MCP_CAN_lib: MCP_CAN Library
I would just try the receive demo to see if the behavior is any different MCP_CAN_lib/CAN_receive.ino at master · coryjfowler/MCP_CAN_lib · GitHub

Thanks for the suggestion - that is indeed a good thing to try and see if the different library is able to receive the messages that I am currently missing.

Thanks to those who helped - I tried the MCP_CAN library and it functions correctly- it receives the second message properly in a 2 message burst. Now I just have to work on my message decoding....

Hi,
What is sending the Canbus data?

Thanks.. Tom... :smiley: :+1: :coffee: :australia:

Hi Tom. I have a prototype control module that is sending the CAN messages. I am building a "Cheap" tool to monitor CAN for debugging.
Best Regards,
Richard