Monitor changes in an array of incomming messages.

[SBSP]

For fun I mess with CAN-BUS by reverse engineering the information.
When monitoring my Jeep's CAN-BUS it delivers messages like this.

ID,BIT1,BIT2 ect to .....BIT8

I.E 1023 is the ID Then the content of the message in values between 1 to 255.

Example Message Format
1023 255, 255 ,255 ,255, 255, 255, 255, 255

1023, 1,0,0,0,0,0,0,0 = Left Blinker on
1023, 1,1,0,0,0,0,0,0 = Left Blinker off
1023, 2,1,0,0,0,0,0,0 = Right Blinker on
1023, 2,0,0,0,0,0,0,0 = Right Blinker off

Data streams in line by line at 125Kb/s which is quite quick and its impossible to detect changes in messages visually.
So my attempt to catch changing messages when pressing a button with in the Jeep is to do the following.

I group lines by ID, which dramatically makes things a bit more readable , when grouping it will create a list of about 60 to 70 IDs
I then load the message into an array and if the ID comes in I compare the new message with the one in memory and then if it changed I indicate it with a different colour of text.

The tricky part is this is done directly on a micro controller and I'm using an Arduino Mega because it has a more memory compared to an UNO. An I'm taking advantage of Putty's VT100 features by skipping lines back and forth to update the line that changed.

But I feel my my code is very very in efficient.

Below is the code
Warning! : It's messy but if you look closely its actually very simple.
I'm sure I can improve it a little bit but I would like to make it as efficient as possible.

FYI this is just my way of learning , So its just a hobby. I want to do this on the Arduino and not use external software. Currently the below works relatively well. But I now want to clean it up.

The code really starts after

          BIT1=buf[0];

          BIT2=buf[1];

          BIT3=buf[2];

          BIT4=buf[3];

          BIT5=buf[4];

          BIT6=buf[5];

          BIT7=buf[6];

          BIT8=buf[7];

So the question is.
How would you have attempted this ? Or what can i do to optimize the code
I'm willing to start from scratch using a totally different method.

void candump()
{
  int intNoDataCount=0;
  clearAndHome();

  //clean memory
  memset(arryCANID, 0, sizeof(arryCANID));
  memset(arryCANMSG, 0, sizeof(arryCANMSG));
  String BuildCANMSG;
  int arryCount=0;
  int ArryIDIsOld=0;
  int ArryMessageIsOld=0;
  int ArryRCPPOS = 1;
  int ArryLastListPos=0;
  if(IsInitiated==0){Serial.println("CAN-BUS shield not initiated. Type \033[1;31mhelp\033[0m to get a list of initiation commands.");return;}
  unsigned char len = 0;
  unsigned char buf[8];

  int CANID,BIT1,BIT2,BIT3,BIT4,BIT5,BIT6,BIT7,BIT8;

  while(IsDumping==1)
  {
  delay(SlowDown);
  readincommingserial();

      if(CAN_MSGAVAIL == CAN.checkReceive())            // check if data coming
      {
          intNoDataCount=0;
          CAN.readMsgBuf(&len, buf);    // read data,  len: data length, buf: data buf
          unsigned long canId = CAN.getCanId();   
          CANID=canId;
          BIT1=buf[0];
          BIT2=buf[1];
          BIT3=buf[2];
          BIT4=buf[3];
          BIT5=buf[4];
          BIT6=buf[5];
          BIT7=buf[6];
          BIT8=buf[7];
  
      //Check Duplicates and print when necesarry
      if(HexMode=="off")
      {
        BuildCANMSG=String(BIT1) + " " +  String(BIT2) + " " +  String(BIT3) + " " +  String(BIT4)  + " " +  String(BIT5)  + " " +  String(BIT6)  + " " +  String(BIT7)  + " " +  String(BIT8);
      }else
      {
        BuildCANMSG=String(BIT1,HEX) + " " +  String(BIT2,HEX) + " " +  String(BIT3,HEX) + " " +  String(BIT4,HEX)  + " " +  String(BIT5,HEX)  + " " +  String(BIT6,HEX)  + " " +  String(BIT7,HEX)  + " " +  String(BIT8,HEX);
      }

    //BOF Check if Received ID has already been received
    ArryIDIsOld=0;
    ArryMessageIsOld=0;
    for (int arryCount=1; arryCount <= 99; arryCount++)
    {
      if(arryCANID[arryCount] == CANID)
        {     
          ArryIDIsOld=1;     
          //Check If Message Changed
          if(arryCANMSG[arryCount]==BuildCANMSG){MsgColor=0;}else{ MsgColor=41;}
      
            //Clear line to prevent text from flowing over old test
            Serial.print("\033[" + String(arryCount) + ";1H");
            Serial.print("\033[0m;                                     ");
            //Print fresh line
            Serial.print("\033[" + String(arryCount) + ";1H");
            if(HexMode=="off"){Serial.print("\033[1;" + String(MsgColor)  + "m" + String(CANID) + " " +  BuildCANMSG);}else{Serial.print("\033[1;" + String(MsgColor)  + "m" + String(CANID,HEX) + " " +  BuildCANMSG);}
            Serial.print(" ");
            Serial.print(char(BIT1));
            Serial.print(char(BIT2));
            Serial.print(char(BIT3));
            Serial.print(char(BIT4));
            Serial.print(char(BIT5));
            Serial.print(char(BIT6));
            Serial.print(char(BIT7));
            Serial.print(char(BIT8));       
          break;
        }
    }

    if(ArryIDIsOld==0)
    {
      ArryLastListPos++;
      //Print Message because it has not yet been received
     Serial.print("\033[" + String(ArryLastListPos) + ";1H");
      //Serial.print(CANID);
     if(HexMode=="off"){Serial.print("\033[1;0m" + String(CANID) + " " +  BuildCANMSG);}else{Serial.print("\033[1;0m" + String(CANID,HEX) + " " +  BuildCANMSG);}
            Serial.print(" ");
            Serial.print(char(BIT1));
            Serial.print(char(BIT2));
            Serial.print(char(BIT3));
            Serial.print(char(BIT4));
            Serial.print(char(BIT5));
            Serial.print(char(BIT6));
            Serial.print(char(BIT7));
            Serial.print(char(BIT8)); 

      //Update Array with CAN ID so that it will not repeat a new line 
      arryCANID[ArryLastListPos] = CANID;
      arryCANMSG[ArryLastListPos]=BuildCANMSG;

    }
   //EOF Check Duplicates and print when necesarry

      }
      else
      {
        intNoDataCount++;
        if(intNoDataCount>=1000)
        {
        intNoDataCount=0;
        Serial.println("No data found. Check physical connection and connection speed.");
        break;
        }
      }


   }

}

 

[cguy]

Your code would be a lot more readable if you just left the data in the char array instead of copying it out.You could also then use a for loop to display the bytes.

Personally, I wouldn’t bother with the bytes and would use a 64-bit integer instead (displayed as hex).

I also don’t know why your need an array, if you are just waiting for change, just keep a variable with the last (64-bit) value.

 

[SBSP]

Your code would be a lot more readable if you just left the data in the char array instead of copying it out.You could also then use a for loop to display the bytes.

Personally, I wouldn’t bother with the bytes and would use a 64-bit integer instead (displayed as hex).

I also don’t know why your need an array, if you are just waiting for change, just keep a variable with the last (64-bit) value.

The library used to fetch the data brings them through in an array called 'buf' so I assigned them to my own variables. I could have just used it as is without copying them. The monitoring code actually comes from another function I created to emulate the data so that I dont have to connect it to my Jeep every time I want to test. Will fix it.

The reason why i didn't consolidate them into a single variable is because I wanted to be able to identify the actual part that changed by looping though it again. But its not necessary anymore . So thanks I never thought of that after looking at the end result. At the moment there's 2 arrays running in parallel and its consuming allot of memory

      arryCANID[ArryLastListPos] = CANID;
      arryCANMSG[ArryLastListPos]=BuildCANMSG;

Using your suggestion i will free up some memory. I can consolidate them and just split them up to get the ID and to know which line to update.