So I've been tackling a CAN bus based project. I am able to establish communication with my car and ask for 2 CAN Id's and the bytes of my choosing. I can light 1 LED when an Id and byte is met. The other LED will blink when the other ID and byte is met. In my code there is nothing to for it to flash I only want it to light up no blinking. I've been using delay for for the first LED and since its a blocking function, I believe that's where the blinking is coming from with the second LED. I'll have my code attached. I want to use millis instead since its non blocking /multitasking function. In the past I've been able to implement millis with simpler programs. I am lost on how to add it to this new project. If there is another better way to multitask I am open to it
#include <mcp_can.h>
#include <SPI.h>
#define LED 7 //LED PIN
#define LED2 6
#define Brake_Pedal 0x70 // value for Brake Pedal
#define Ebrake 0x00 // value for Ebrake
#define CAN_INT 2
long unsigned int rxId = 0x488; // brake pedal
long unsigned int rxId2 = 0x500; // Ebrake
unsigned char len = 0;
unsigned char rxBuf[8];
MCP_CAN CAN0(9); // Set CS to pin 9
void setup()
{
Serial.begin(115200);
if (CAN0.begin(MCP_STDEXT, CAN_500KBPS, MCP_16MHZ) == CAN_OK) Serial.print("MCP2515 Init Okay!!\r\n");
else Serial.print("MCP2515 Init Failed!!\r\n");
pinMode(2, INPUT); // Setting pin 2 for /INT input
pinMode(LED, OUTPUT);
pinMode(LED2, OUTPUT);
//setup filters to receive only CAN ID 0x512
CAN0.init_Mask(0, 0, 0x07FF0000); // Init first mask...
CAN0.init_Filt(0, 0, 0x05120000); // Init first filter...
CAN0.init_Mask(1, 0, 0x07FF0000); // Init second mask...
CAN0.init_Filt(2, 0, 0x05140000); // Init third filter...
CAN0.setMode(MCP_NORMAL); // Change to normal mode to allow messages to be transmitted
}
void loop()
{
if (!digitalRead(2)) // If pin 2 is low, read receive buffer
{
CAN0.readMsgBuf(&rxId, &len, rxBuf); // Read data: len = data length, buf = data byte(s)
Serial.print("ID: ");
Serial.print(rxId, HEX);
Serial.print(" Data: ");
for (int i = 0; i < len; i++) // Print each byte of the data
{
if (rxBuf[i] < 0x10) // If data byte is less than 0x10, add a leading zero
{
Serial.print("0");
}
Serial.print(rxBuf[i], HEX);
Serial.print(" ");
}
Serial.println();
if (rxId)
{
if (rxBuf[4] == Brake_Pedal)
{
digitalWrite(LED, HIGH); //LED ON
delay (500);
}
else {
digitalWrite(LED, LOW); //LED OFF
delay (500);
}
if (rxId2)
{
if (rxBuf[7] == Ebrake)
{
digitalWrite(LED2, HIGH); //LED ON
delay (500);
}
else {
digitalWrite(LED2, LOW); //LED OFF
delay (500);
}
}
}
}
}
So, you already figured out where your issues come from. Now you need to rethink how your code is written. Currently you are only doing one thing at a time. Your Arduino can do many thinks at the same time. To achieve this, you must follow a few rules until you know when you may break them.
do not use delay
make sure your loop runs as often as possible
for complex task do little steps and move on, do not wait for things to happen
start separating your code in loop() into functions and tasks
For this to work, you will need to keep track of the state your task is in. You can use global variables or static local variable for this. I like to use switch-case statements for simple state machines.
This will make your code look more complicated but it will allow you to extend your code with new functions without breaking what you have already done.
Here is the basic structure of a switch-case state machine.
void task()
{
static int state = 0; // This is only set to 0 the very first time when the function is called
switch (state )
{
case 0:
// do something only once
state++;
break;
case 1:
// do something until if condition is true
if (...)
{
state++;
}
break;
case 2:
// do something until if condition is true, the start from beginning
if (...)
{
state = 0;
}
break;
default:
state = 0;
break;
}
}
You can have as many of these as you want. Use the millis() technique inside a state/case to control timing before moving on to the next state.
You can use enums to give your states names and make the code easier to read.
Try turning your two LEDs into two tasks. If you have any question post your new code.
Note that you do not "control" timing as such - that suggests you are somehow "in control" of the time as if using delay().
What you do is to use millis() to see whether on any pass through each function, there is something to be done; whether it is yet the time for that thing to be done. If so, do it - instantly - and move on. If it is not yet time, move on immediately. In either case, you do not wait in any manner. If what you did in that pass through the function then implies the need to wait for another action, you set the "state" so that future passes check on the new criterion whether that is a particular time or some other event.
Work got busy so I wasn't able to get to trying this till yesterday. I have tried a few different approaches with no luck. My question is how much of my code needs to be changed to accommodate with switch case method?
I've noticed I cant use a 'void loop' and a 'void task' in the same code will not compile. Would I need to get rid of everything after 'void loop' and use 'void task' with switch case? I'm confused because of the sensors I'm using I have to call for the ID and byte
if (Serial.available() > 0) { // This is only set to 0 the very first time when the function is called
static int state = 0;
}
}
}
switch (0x600 )
{
case a:
if (0x20)
// do something only once
digitalWrite(7, HIGH);
break;
case b:
// do something until if condition is true
if (0x600 != 0x20)
{
digitalWrite(7, LOW);
}
break;
default:
state = 0;
break;
}
switch (0x500)
{
case 'A':
if (0x10)
// do something only once
digitalWrite(6, HIGH);
break;
case B:
// do something until if condition is true
if (0x500 != 0x00)
{
digitalWrite(6, LOW);
}
break;
default:
state = 0;
break;
}
}
Do not declare the variable inside the if statement? Variables are only valid within something called scope. Scope is a region inside your code, this can be a function, global or even just between some brackets. Google "C What is scope" for more detailed information.
The switch statement needs a variable not a value. See my example. case 0: means if the state variable has the value 0.
Check how logic in if() statements work. If (0x600 != 0x20) is always true. You want to compare a variable to a value or two variables to each other.
if ( a == 10 ) or if ( a == b ) but not if ( 1 == 2 ) even though the compiler will not complain because this is technically a valid C statement.