Arduino crashes randomly / Arduino CAN Bus

Hi all,

I am working on a Sensor Node that is supposed to read some values from a couple of analog input ports, do a little bit of calculation with it and put the result on the attached CAN-Bus (I designed a custom board using the MCP2515 and the MCP2551). The board seems to work fine and the program is really not that complex, but I am experiencing random crashes. I used an attached LED to show some kind of heartbeat and as the “crash” occures it just stops (no reboot, but just a halt; the LED stays on or off depending on the last state, but it doesn’t change anymore until I toggle the power).

Please find the program below. I also attached an image of the schematic I used. As mentioned before, the hardware appears to be working fine, it really seems to be related to the CAN bus as it seemed to be working fine with no other CAN devices attached to the Sensor Node.

Is it possible that I need to set a filter / mask on the MCP2515 and that the SPI input buffer is in an overflow (I don’t know enough about the internals of SPI to make a better description of that) because of all the arriving CAN frames?
The node is not supposed to receive any messages, so how could I just block them all?

I would really appreciate any help that might lead to a solution to this problem.
Thank you all in advance

Max

/*Begining of Auto generated code by Atmel studio */
#include <Arduino.h>

/*End of auto generated code by Atmel studio */

//Beginning of Auto generated function prototypes by Atmel Studio
void setup();
void loop();
//End of Auto generated function prototypes by Atmel Studio


#include <mcp_can.h>
#include <mcp_can_dfs.h>

#define DEBUG_MODE 0

#include <SPI.h>

const int SPI_CS_PIN = 10;

MCP_CAN CAN(SPI_CS_PIN);

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

	if(CAN_OK == CAN.begin(CAN_500KBPS))                   // init can bus : baudrate = 500k
	{
		#if DEBUG_MODE
			Serial.println("CAN BUS Shield init ok!");
		#endif
	}
	else
	{
		#if DEBUG_MODE
			Serial.println("CAN BUS Shield init fail");
			Serial.println("Init CAN BUS Shield again");
		#endif
		delay(100);
		goto START_INIT;
	}
	
	pinMode(3, OUTPUT);
	pinMode(4, OUTPUT);
}

int g_throttle = 0;

bool pin4 = false;

int counter = 0;

void loop()
{
	
	int s1 = analogRead(A0);
	int s2 = analogRead(A1);
	int s3 = analogRead(A2);
	int s4 = analogRead(A3);

	float v1 = ((s1 / 1023.0f * 5) - 0.5f) / 4;
	float v2 = (((s2 / 1023.0f * 5) - 0.5f) / 4);
	float v3 = ((s3 / 1023.0f * 5) - 0.5f) / 4;
	float v4 = (((s4 / 1023.0f * 5) - 0.5f) / 4);


	float throttle = ((v1 + (1 - v2)) / 2.0f) * 1000;
	throttle -= 85;
	throttle /= 1.2f;
	
	if ((v1 + v2) > 1.05 || (v1 + v2) < 0.95 || throttle < 0)
	{
		throttle = 0.0f;
	}
	if(throttle > 100)
	{
		throttle = 100;
	}
	
	int brake = ((v3 + (1 - v4)) / 2.0f) * 1000;
	if ((v3 + v4) > 1.05 || (v3 + v4) < 0.95)
	{
		brake = 0;
	}
	
	
	unsigned char data[2];
	bool notStop = false;
	
	/*if(throttle != g_throttle)
	{
		g_throttle = throttle;
	}*/
	
	if(throttle != 0)
	{
		notStop = true;
		digitalWrite(3, 1);
	}
	else
	{
		digitalWrite(3,0);
	}
	
	if(notStop)
	data[0] = 2;
	else
	data[0] = 0;
	
	uint8_t pwm = throttle;
	
	data[1] = pwm;
	
	//              ID  RTR  l  data
	CAN.sendMsgBuf(0x240, 0, 2, data);
	
	/*Serial.print("data[0]:");
	Serial.print(data[0]);
	Serial.print(" data[1]:");
	Serial.println(throttle);
	*/
	pin4 = !pin4;
	digitalWrite(4, pin4);
	
	counter++;
	if(counter % 10 == 0)
	{
		counter = 0;
		#if DEBUG_MODE
			Serial.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n");
			Serial.println("Throttle:\tV1\tV2\t\tBrake:\tV1\tV2");
			Serial.print(pwm);
			Serial.print("\t\t");
			Serial.print(v1);
			Serial.print("\t");
			Serial.print(v2);
			
			Serial.print("\t\t");
			Serial.print(brake);
			Serial.print("\t");
			Serial.print(v3);
			Serial.print("\t");
			Serial.println(v4);
		#endif
	}
	if(counter > 10)
	{
		counter = 0;
	}
	
	delay(50);
}

P.S.: Feel free to use the schematic if you like to. Basically it is just a merge of an Arduino Uno with a Sparkfun CAN Bus shield. You might want to add a common mode choke on the CAN lines, split the 120R resistor into 2x 60R and put a small capacitor to GND in between.

        goto START_INIT;

Never heard of while loops, huh?

    float v1 = ((s1 / 1023.0f * 5) - 0.5f) / 4;
    float v2 = (((s2 / 1023.0f * 5) - 0.5f) / 4);
    float v3 = ((s3 / 1023.0f * 5) - 0.5f) / 4;
    float v4 = (((s4 / 1023.0f * 5) - 0.5f) / 4);

Why are there not the same number of parentheses in each statement?

    Serial.print(" data[1]:");
    Serial.println(throttle);

The ID does not match what you are printing. That is silly.

    pin4 = !pin4;
    digitalWrite(4, pin4);

So is using pin in the name of a variable that holds state.

            Serial.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n");

WTF?

Judicious printing should show you where the crashing happens.

@PaulS Although you might be right, that the coding style is a bit obscure or unconventional (mostly due to the fact that it is snipped together from multiple soures and I haven't had the time to refactor it properly yet), all the code pieces you complained about are most surely not responsible for the crashes I experienced.

I asked for constructive help and introduced a thesis why these crashes may occur, but instead of discussing it, all you do is playing code style nazi. Not very helpful.

all you do is playing code style nazi. [1]

Since you are trying to find a bug, and reading and understanding the code is required to find a software bug, anything that gets in the way of comprehension will slow you down. Nothing is worse than confusing code. Ok, maybe confusing comments are worse. :stuck_out_tongue:

Sometimes, just starting a refactor/cleanup will reveal something to you, and you can patch the mess instead of completing the refactor.

Anyhoo…

I notice that there is no conditioning of the analog inputs. Are you sure they’re clean by the time they get to the analog pins? Nothing like a spike or noise to ruin your day.

Other notes:

  • Are you using Atmel Studio?
  • If you are building a CAN sensor, it must have an “address” of some kind in order to respond to a request. A “filter-by-address” will eventually be required, so you may as well let the MCP2515 handle it.
  • If you don’t have any filters, I wonder if the Arduino is busy handling every frame, or handling every CS_INT.
  • An abundance of interrupts could expose a latent bug in the CAN library. Filter!
  • Instead of using delay, you should consider using millis. That will also let you verify the loop timing.
  • Did you know prints will block when you have queued up 64 characters for printing?Cheers,
    /dev

[1] Teehee. Godwin’s Law!

For the sake of it, I would like to apologize for the Nazi comparison. I guess as a german guy you shouldn't do such jokes ;) Although I have to say that the critisizm of one spare pair of parenthesis is quite "kleinkariert". Look it up if you like to know.

Back to topic:

/dev: I notice that there is no conditioning of the analog inputs.

The sensors that are used are quite expensive and really close to the sensor node. I trust the signal quality. On the console it looks really good.

/dev: Are you using Atmel Studio?

Yes I am. No problems so far (besides a day of configuring and testing the workflow).

/dev: If you are building a CAN sensor, it must have an "address" of some kind in order to respond to a request. A "filter-by-address" will eventually be required, so you may as well let the MCP2515 handle it.

In a automotive CAN network, your devices don't have an address, but the CAN Message has an ID. In your CAN Matrix you define which message should be send / received by which device. The devices are listening to a range of message IDs they are interested in. In my case, the Sensor Node is only a sender and is not meant to receive any messages.

Therefore the question in the opening post if it wouldn't be a good idea to just block all incoming messages directly on the MCP2515. What would happen if I just don't read any incoming CAN (and therefore SPI via MCP2515) messages? Would it come to an input buffer overflow just like it would happen with the serial port?

/dev: If you don't have any filters, I wonder if the Arduino is busy handling every frame, or handling every CS_INT.

Thats the point. Up until now, I am not handling anything. I am just not reading any frames that might be incoming (and therefore I am not using the CS_INT neither).

/dev: Instead of using delay, you should consider using millis. That will also let you verify the loop timing.

Thanks for the hint. I will consider using it.

/dev: Did you know prints will block when you have queued up 64 characters for printing?

No I did not know that. Why is this so? Does it just slow down the programm or are there any latent dangers to consider?

Anyway I am currently not running the code in debug mode so meaning I don't get any console output. So this is definitely not the error.

Thanks again for you help.

Max

Although I have to say that the critisizm of one spare pair of parenthesis is quite "kleinkariert".

I'm sure you don't mean to trivialize the criticism of goto.

The sensors that are used are quite expensive and really close to the sensor node. I trust the signal quality. On the console it looks really good.

It is very unusual not to have conditioning, especially in an automotive environment. Not that the sensors aren't nice, but there are lots of things that can ruin your day. Power is messy, wires pick up induced spikes or noise... there's just no end to how bad it is. Unless you're inside a shielded box that has conditioned all of its inputs and power somewhere else, and this is an onboard connection, I would strongly suggest that you consider protecting the inputs with Zener or TVS diodes, at a minimum. Depending on the output impedence of the analog signals, you might also use series resistors.

There are lots of threads about conditioning the power, as well. Here's one. Caps, TVS/Zeners, and ferrite beads are frequently suggested.

I'm also a little skeptical because you say it looks good on the "console". Unless you mean "oscilloscope" or "power/voltage/current monitor", you don't really know if you have a power or signal problem. This is the first thing I'd check.

In a automotive CAN network, your devices don't have an address, but the CAN Message has an ID. In your CAN Matrix you define which message should be sent / received by which device. The devices are listening to a range of message IDs they are interested in. In my case, the Sensor Node is only a sender and is not meant to receive any messages.

Therefore the question in the opening post if it wouldn't be a good idea to just block all incoming messages directly on the MCP2515.

Yes, if your CAN node really doesn't listen to any messages, they should all be filtered. And...

What would happen if I just don't read any incoming CAN (and therefore SPI via MCP2515) messages? Would it come to an input buffer overflow just like it would happen with the serial port?

Well, I don't really know anything about the CAN library, but one oddity of the SPI interface is that while the master is sending information to the slave (i.e., clocking it out), the slave can send information back to the master at the same time (i.e., on the same clock).

This means that the Arduino could be receiving CAN messages, and the library may be spending time parsing those messages... and it may be queuing some kind of response... which might be using up memory. I just don't know, but I'd still suggest filtering as much as possible, even everything.

Why [does the print block]? Does it just slow down the program or are there any latent dangers to consider?

Because the Serial output buffer is only 64 characters. At 9600, it takes about 1ms to transmit each character. That's an eternity for a 16MHz processor, so it just queues them up until they trickle out. But once the queue is full, the print just spins. The TX interrupt clears out the queue in the background, and the print method eventually returns, milliseconds later. It's something you need to be aware of, in case you turn DEBUG on.

As far as what that can affect... it affects every foreground process (i.e., all non-interrupt code). The CAN library may depend on you calling something periodically, including sendMsgBuf. I don't know. And I don't know what its input process requires.

Using delay has the same effect. If there is anything else that needs to be performed in the foreground, delay keeps it from happening. Maybe it's not an issue for now, but if you later decide to listen to some kind of configuration message, then loop should be checking for it frequently. The delay will prevent reading that message and responding quickly. Just something else to be aware of...

Cheers, /dev

Although I have to say that the critisizm of one spare pair of parenthesis is quite "kleinkariert". Look it up if you like to know.

It was not a criticism of one pair of parentheses. It was the lack of consistency that I was questioning. Consistency is a good thing. Random styles are not. In my opinion, if the code looks good, it usually is. Or, the bugs are more insidious.

There is really nothing to be gained by sending a bunch of carriage returns to the serial port. They do not add value. One or two for separation is good. 3 dozen adds no value.

What they do is take time to send. That could block other parts of your code. And that is probably not a good thing.