"SOLVED "Were am i going wrong with my code?

Using Arduino Programming Questions
#1

Hi guys looking for some help with my code, I am using a canbus module mcp2515 to sniff certain messages on the bus and have been successful with this so far. I have codes for my headlights and locking/unlocking that all work, I generated the code below in order to turn the headlights on when unlocking my vehicle which works Great!, Although in my else if statement If i lock the vehicle i want the lights to turn off but they stay on and continue too forever… Hopefully someone can notice the fault within my code :slight_smile:

#include <mcp_can.h>
#include <SPI.h>
#ifdef ARDUINO_SAMD_VARIANT_COMPLIANCE
#define SERIAL SerialUSB
#else
#define SERIAL Serial
#endif
#define CAN_INT 2

const int SPI_CS_PIN = 10;
unsigned char len = 0;
unsigned char Size = 0;
unsigned char size2 = 0;
unsigned char buf[8];
unsigned char flagRecv = 0;
unsigned long lastBusActivity = millis();
unsigned long cancheck = 300;

unsigned char UNLOCK[3] = {0x7F, 0x32, 0x7F};
unsigned char LIGHTS[6] = {0x00, 0x00, 0xC0, 0x00, 0x10, 0x00}; 
unsigned char LIGHTSOFF[7] = {0x00, 0x00, 0x40, 0x00, 0x00, 0x00, 0x00}; 
unsigned char LOCK[3] = {0x00, 0x00, 0x00};   

MCP_CAN CAN(SPI_CS_PIN);                                    // Set CS pin

void setup() {
  SERIAL.begin(115200);
  attachInterrupt(digitalPinToInterrupt(CAN_INT), MCP2515_ISR, FALLING);
  CAN.begin(CAN_33KBPS);           // init baudrate at 33kBPS (( single K LINE bus NON PRIORITY))

}
void MCP2515_ISR() {
  flagRecv = 1;
}

void loop() {
  if (flagRecv) {   

    flagRecv = 0;

      lastBusActivity = millis();

       while (CAN_MSGAVAIL == CAN.checkReceive()) {    
         CAN.readMsgBuf(&len, buf);
       }
        if (CAN.readMsgBuf(&Size, UNLOCK)) {             /*  IF THE MCP2551 RECIEVES THE UNLOCK ID   */
         CAN.sendMsgBuf(0x305, 0, 6, LIGHTS);           /*  SEND THE HEADLIGHTS ON MESSAGE TO THE BUS  */
        } 
         else if (CAN.readMsgBuf(&size2, LOCK)) {       /*  IF THE BUS RECIEVED THE LOCK DOORS ID */
          CAN.sendMsgBuf(0x230, 0, 7, LIGHTSOFF);        /*  SEND LIGHTS OFF MESSAGE TO THE BUS   */
      }
    }
  }
#2 
      CAN.sendMsgBuf(0x230, 0, 7, LIGHTSOFF);        /*  SEND LIGHTS OFF MESSAGE TO THE BUS   */

What are the 4 parameters to this function ?

#3

sorry i do not know what you mean, 0x230 is message ID, 0 , 7 is the above unsigned char data LIGHTSOFF. Apologies if i have read you wrong

#4

RyanAES:
Hi guys looking for some help with my code, I am using a canbus module mcp2515 to sniff certain messages on the bus and have been successful with this so far. I have codes for my headlights and locking/unlocking that all work, I generated the code below in order to turn the headlights on when unlocking my vehicle which works Great!, Although in my else if statement If i lock the vehicle i want the lights to turn off but they stay on and continue too forever… Hopefully someone can notice the fault within my code :slight_smile:

#include <mcp_can.h>

#include <SPI.h>
#ifdef ARDUINO_SAMD_VARIANT_COMPLIANCE
#define SERIAL SerialUSB
#else
#define SERIAL Serial
#endif
#define CAN_INT 2

const int SPI_CS_PIN = 10;
unsigned char len = 0;
unsigned char Size = 0;
unsigned char size2 = 0;
unsigned char buf[8];
unsigned char flagRecv = 0;
unsigned long lastBusActivity = millis();
unsigned long cancheck = 300;

unsigned char UNLOCK[3] = {0x7F, 0x32, 0x7F};
unsigned char LIGHTS[6] = {0x00, 0x00, 0xC0, 0x00, 0x10, 0x00};
unsigned char LIGHTSOFF[7] = {0x00, 0x00, 0x40, 0x00, 0x00, 0x00, 0x00};
unsigned char LOCK[3] = {0x00, 0x00, 0x00};

MCP_CAN CAN(SPI_CS_PIN);                                    // Set CS pin

void setup() {
  SERIAL.begin(115200);
  attachInterrupt(digitalPinToInterrupt(CAN_INT), MCP2515_ISR, FALLING);
  CAN.begin(CAN_33KBPS);          // init baudrate at 33kBPS (( single K LINE bus NON PRIORITY))

}
void MCP2515_ISR() {
  flagRecv = 1;
}

void loop() {
  if (flagRecv) {

flagRecv = 0;

lastBusActivity = millis();

while (CAN_MSGAVAIL == CAN.checkReceive()) {   
        CAN.readMsgBuf(&len, buf);
      }
        if (CAN.readMsgBuf(&Size, UNLOCK)) {            /*  IF THE MCP2551 RECIEVES THE UNLOCK ID  /
        CAN.sendMsgBuf(0x305, 0, 6, LIGHTS);          /  SEND THE HEADLIGHTS ON MESSAGE TO THE BUS  /
        }
        else if (CAN.readMsgBuf(&size2, LOCK)) {      /  IF THE BUS RECIEVED THE LOCK DOORS ID /
          CAN.sendMsgBuf(0x230, 0, 7, LIGHTSOFF);        /  SEND LIGHTS OFF MESSAGE TO THE BUS  */
      }
    }
  }

looking at your code, I feel you have been just damn lucky!!!

To start with, you are using the ISR to detect if a msg is received by the MCP2515

yet in ‘void loop’ you do CAN_MSGAVAIL == CAN.checkReceive() to do what… to check if msg is receive AGAIN!!!

you also do

while (CAN_MSGAVAIL == CAN.checkReceive()) {
CAN.readMsgBuf(&len, buf);
}

which basically means you are OVERWRITING msg over msg as long as CAN_MSGAVAIL == CAN.checkReceive() remain TRUE ie you will lose information!!!

you then do

CAN.readMsgBuf(&Size, UNLOCK)

but wait a sec… did you not just read the some msg in the while loop!!! should you not be comparing ‘buf’ with the ‘UNLOCK’ array? ? ? ?

this function just return a ‘OK message received’ if it sucessfully read one
ie AS LONG AS this line has a CAN msg to read, IT WILL EXECUTE CAN.sendMsgBuf(0x305, 0, 6, LIGHTS);

and that is probably why you else statement never gets executed.

in any case IMHO this code of yours MAKES NO SENSE!

#5

Appreciate your feed back as you can see i am not exactly the best at coding etc but it makes sense from what you are saying now, Appreciate the help :slight_smile:

#6