SAMC21 CAN / BOSCH MTTCAN message RX handling

[Simon]

I have written an RX handler but I am not sure if this is robust enough. It will work to receive the one message that I am at the moment but I should be prepared for heavy traffic.

The main players I believe are:

The message received interrupt that has to be reset
The get index of the FIFO
The put index of the FIFO
The fill level

At the moment I deal with the message in the current get index update the get index and then reset the interrupt. My first quandary is that if in the time the message is processed another could arrive, if I reset the interrupt at the end then I assume that having emerged from the interrupt it will not fire again for that message as I just reset the bit.

So I now reset the interrupt bit on entry to the interrupt and update the get index before leaving so that if I emerge from it with another message already arrived it will re-enter and process it immediately rather than essentially be a message behind.

But with this logic I just keep exiting and re entering as required.

But if I simply look at the fill level I will be able to process everything in the que. So presumably I would:

Code: [Select]


while (fill_level)
{
       process message
}

So that I process each message in turn until the buffer is empty and I do not rely purely on the interrupt to process the latest message.

Is my logic correct?

[PlainName]

in the time the message is processed another could arrive

What are you doing to process it? That quote suggests you are doing something significant, and typically you try not to do that in ISRs.

Providing you have sufficient space for the next message (and maybe even if you don't), one way around your quandary is to check for a new message manually, without relying on receiving an interrupt, before you end dealing with the stuff. That would be typical of handling edge-triggered interrupts, but level-triggered are not susceptible to the same problem.

[ataradov]

Your description is way to abstract to tell if the approach is good or not. Generally, if you are processing the messages slower than they arrive, you will overflow sooner or later.

So, it is just a question of how big of a burst you want to handle. You can leave unhanded messages in the CAN FIFO and red then one by one once you are done with the previous one. Or you can read the messages as fast as possible in the interrupt and place them into some software queue. Both approaches are valid.

If you are ok with polling the level, then you don't need interrupts at all, but then the only good solution is the bigger FIFO in the CAN and just process messages as they come in. Software buffer would not make sense in that case.

Or you can have CAN place things into the software buffer from the interrupts, and the firmware may process things from the software buffer by polling.

There are a lot of ways to implement things.

[Simon]

My interrupt routine checks for the current get index and transfers the data to the relevant variables and exits. As such it only ever processes one message. If for any reason there is more than one these unread messages will just stay there.

So if I check the buffer fill level instead and keep going until it is empty I should never run into this problem. What I do wonder is how long does it take for the buffer fill level to update when I update the F0GI value in RXF0S.

Code: [Select]


void CAN1_Handler_rx_by_id2()
{
if (CAN1->IR.bit.RF0N)
{
static can_rx_data_t sorting_buffer ;

uint8_t n = CAN1->RXF0S.bit.F0GI ; // make n the get index

CAN1->IR.reg = 0x1 << 0 ; // reset  RF0N Rx FIFO 0 New Message


while(CAN1->RXF0S.bit.F0FL ) // while the get index is less than the put index
{
sorting_buffer.time_stamp = ret_rtc_us() ;

// copy data from RX fifo into holding variable that has the same layout as can data types.

memcpy(&sorting_buffer , &can_1_rx_buffer[n].CAN_BUFFER_DATA , 8) ;

// check for extended frame if not shift the std id down
uint8_t exd_id_bit = can_1_rx_buffer[n].XTD ;

if (exd_id_bit)
{
sorting_buffer.id = can_1_rx_buffer[n].ID ;
}
else
{
sorting_buffer.id = can_1_rx_buffer[n].ID >> 18 ;
}

CAN1->RXF0A.bit.F0AI = CAN1->RXF0S.bit.F0GI ; // acknowledge data read so that get index increases

// memcpy from the sorting buffer variable to the destination variable.

memcpy(can_1_rx_variable_index[can_1_rx_buffer[n].FIDX + (exd_id_bit * CAN_1_STD_MESSAGE_FILTER_NUM - 1)] , &sorting_buffer , 32 ) ;
}
}
}

[/quote]

As you say yes I could just run that at regular intervals rather than in an interrupt.

[PlainName]

As you say yes I could just run that at regular intervals rather than in an interrupt

That's  not what I meant. You use the interrupt to kick off the process but on finishing you just take a look to see if you've missed a new one. If so, deal with it otherwise back to waiting for the interrupt. So it's not a regular interval thing, just making sure you're really done at the end.

I don't know how the interrupts works on this, but my guess (which may be wrong!) is that receiving the message triggers the interrupt and it then stays active until you ack it. When you ack it, it won't trigger again until a new message comes in. In other words, the receipt of the message is the trigger, not the message being in the buffer (subtle difference). If that is the case then you should ack the interrupt as soon as you have the message details (index position, etc) and before spending time processing. Then when you exit the ISR it will just go back in and do it all again if a new message has come in whilst you've been processing, otherwise you're done.

The trade-off in that is the overhead of doing a return from interrupt and going back in again, vs doing a check in a loop. The loop would potentially halt everything if you did get data coming in too fast, whereas exiting the ISR would potentially allow a different ISR to sneak in and keep things going. (I really dislike having potentially endless loops in ISRs!)

[Simon]

Ah right, so this is what I currently use which does as you suggest I think:

Code: [Select]


void CAN1_Handler_rx_by_id()
{
if (CAN1->IR.bit.RF0N)
{
static can_rx_data_t sorting_buffer ;

uint8_t n = CAN1->RXF0S.bit.F0GI ; // make n the get index

CAN1->IR.reg = 0x1 << 0 ; // reset  RF0N Rx FIFO 0 New Message

sorting_buffer.time_stamp = ret_rtc_us() ;

// copy data from RX fifo into holding variable that has the same layout as can data types.
memcpy(&sorting_buffer , &can_1_rx_buffer[n].CAN_BUFFER_DATA , 8) ;

// check for extended frame if not shift the std id down
uint8_t exd_id_bit = can_1_rx_buffer[n].XTD ;

if (exd_id_bit)
{
sorting_buffer.id = can_1_rx_buffer[n].ID ;
}
else
{
sorting_buffer.id = can_1_rx_buffer[n].ID >> 18 ;
}

// memcpy from the sorting buffer variable to the destination variable.

memcpy(can_1_rx_variable_index[can_1_rx_buffer[n].FIDX + (exd_id_bit * CAN_1_STD_MESSAGE_FILTER_NUM - 1)] , &sorting_buffer , 32 ) ;

CAN1->RXF0A.bit.F0AI = CAN1->RXF0S.bit.F0GI ; // acknowledge data read so that get index increases
}
}


I immediately record the current get index and acknowledge the interrupt so that if another message arrives while the interrupt is running it will re enter as you say.

The context here is that I know it will be possible as I have messages arriving as fast as every 80µs at 500kbps, I don't know how long that interrupt takes to process. if I were using 1Mbps that would be 40µs. I am operating a canopen network so each device sends 4 PDO messages as soon as it gets a sync message.

[PlainName]

You should be able to time the ISR using the CPU clock. Just record accumulated elapsed time for a number of interrupts, and how many there were, then dump it at quiet moments.

You probably need to simulate a flood to find out if it works as expected. Maybe you can do that with your network, but another way would be to extend the ISR with a loop that just consumes time at the end. Long enough to make sure it can't keep up with whatever message rate is coming in. Doing it like that it will become overwhelmed at some point, but up until then you can detect if you've missed a message, and that's all you care about for this.