ISR's and a compliacted life...

So I've been futzing around with some CANBUS code, and have it working as expected. It's based on DaveAK's original MCP2515 code, and does about 95% of what I need it to do.

The next logical step would be to use an ISR to catch inbound messages and add them to a buffer. I've read loads about using volatile variables when an ISR is going to modify them, so have started down that road.

Complication comes in - I think! - that I'm using a typedef structure to hold each message. In the structure def's, I've modified the elements to be volatile - as follows :

typedef struct
{
volatile	unsigned long id;	// EID if ide set, SID otherwise
volatile	byte srr;			// Standard Frame Remote Transmit Request
volatile	byte rtr;			// Remote Transmission Request
volatile	byte ide;			// Extended ID flag
volatile	byte dlc;			// Number of data bytes
volatile	byte data[8];		// Data bytes
}  Frame;

Couple of questions -

1] Is the above concept even sound? I realize you want to limit the time spent in the ISR, but it makes sense to me to be able to async add messages to a buffer using an ISR. If this is going to put too much load (or time) in the ISR, it's pointless to even wander down this path.

2] Is the above code sane? I'm rapidly exhausting my C skills!

3] I'm open to suggestions or code examples of using an ISR to populate a ring buffer - which I think is the "right" path to follow if this is viable...

It may not be necessary to make everything volatile, or it may make more sense to make the instance of the structure as whole volatile.
Difficult to say for sure.

I'm open to suggestions or code examples of using an ISR to populate a ring buffer

Have a look at how it is implemented in HardwareSerial - you already have the source.

Yeah, trolling through hardwareserial now - may be over my head! :slight_smile:

On your other point - if I make the whole thing volatile, do I need to declare the internal components volatile as well?

On your other point - if I make the whole thing volatile, do I need to declare the internal components volatile as well?

Without seeing code it is difficult to say.
Variables used only in the ISR don't need to be qualified "volatile", but shared variables may need to be, and if they're bigger than a single byte, probably need to be further protected from accesses that could split by interrupts.

This covers a few issues to consider when using interrupts, it based on reading rc signals, but the techniques, protection etc are the same whatever the application -

Duane B

SystemsGuy:
On your other point - if I make the whole thing volatile, do I need to declare the internal components volatile as well?

I doubt it.

Nick, Duane - thanks for the link and the input.

The ISR would best cast read a byte and write 16 bytes, worse case is read a byte and write 32 bytes. Sounds like trying to do that in an ISR is not advisable - so I'm going to have to think some more about the logic I want.

I suspect I'll end up with the ISR reading the byte interrupt buffer state for me, and then leave the read/buffer routines in the main function. As I said, I need a beer or two to reconsider! :slight_smile: