CAN Interrupts on ATMEGA16M1

Go To Last Post
18 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi ,

I am having some troubles with CAN interrupts on the ATMEGA16M1.Initially I received CAN messages using the interrupt routine below:

ISR (CAN_INT_vect)
{              	
   uint8_t length, save_can_page, byte;
   save_can_page = CANPAGE;         // Save current MOB
   CANPAGE = CANHPMOB & 0xF0;      // Selects MOB with highest priority interrupt

////////////////////////// RX Interrupt //////////////////////////////

   if (CANSTMOB & (1<<RXOK)) //pg 190
   {     // Interrupt caused by receive finished                     

        length = (CANCDMOB & 0x0F);   // DLC, number of bytes to be received

        for (byte=0;byte

I sent messages on the CAN bus using the following function:

void can_tx(uint8_t CAN_MSG[8],uint16_t ID,uint8_t DLC) 
{

   CANPAGE = 0x00;      // Select MOb0 for transmission
   
   
   while ( CANEN2 & ( 1 << ENMOB0 ) ); // Wait for MOb 0 to be free
   
   CANSTMOB = 0x00;       // Clear mob status register
   
   //CANIDT4 = 0x00;      // Not used for 11 bit identifier   
   //CANIDT3 = 0x00;      // Not used for 11 bit identifier  
   uint8_t low = ID & 0x07; 	// Keep 3 LSBs mask other bits
   uint16_t high = ID & 0x7F8; 	// Keep 8 MSBs mask 3 LSBs
   CANIDT2 = (low<<5);      	// Shift left 5 bits as 3 ID bits are CANIDT2[5:7]
   CANIDT1 = (high>>3);     	// Shift right 3 bits to remove 3 LSBs used in CANIDT2
   
   for ( uint8_t i = 0; i < DLC; i++ )
   {

		CANMSG = CAN_MSG[i]; 		

   } // for
   
   CANCDMOB = ( 1 << CONMOB0 ) | ( DLC << DLC0 );    // Enable transmission, data length= DLC of specific message (CAN Standard rev 2.0A(11 bit identifiers))         

   while ( ! ( CANSTMOB & ( 1 << TXOK ) ) );   // wait for TXOK flag set

   CANCDMOB = 0x00;   // Disable Transmission

   CANSTMOB = 0x00;   // Clear TXOK flag

} 

This method worked quite well for me in initial testing; however I am very concerned with the following line:

while ( ! ( CANSTMOB & ( 1 << TXOK ) ) );   // wait for TXOK flag set 

My concern is that if my product is not connected to a CAN bus then this loop will run indefinitely which is something i do not desire. I plan to get around this by using interrupts for TX messages also, therefore eliminating the need for the "while loop". In my current code i have set up MOb1 which i use for my RX which has my ID filters working as desired. Now i want to use Mob0 for my TX but i am confused with the operation of CANPAGE and setting up different MOb for different tasks.

For example when setting MOb1 and MOb0 do i have to select each MOb in CANPAGE as below:

	for ( int8_t mob=0; mob<5; mob++ ) 
	{ 

      CANPAGE = ( mob << 4 );        // Selects Message Object 0-5

      CANCDMOB = 0x00;             // Disable mob

      CANSTMOB = 0x00;           // Clear mob status register;
    }

    CANPAGE = (1<<MOBNB0);      // Select MOB1
	CANIE2 = (0 << IEMOB0);     // Enable interrupt on MOb1
	CANGIE = ( 1 << ENIT ) | ( 1 << ENTX );   // Enable interrupts on  transmit

	CANPAGE = (1<<MOBNB1);      // Select MOB2
	CANIE2 = (1 << IEMOB1);     // Enable interrupt on MOb2
	CANGIE = ( 1 << ENIT ) | ( 1 << ENRX );   // Enable interrupts on receive and transmit

Any assistance would be gratefully helpful as I cannot seem to understand this datasheet, maybe i have been just staring at it too long.

Thanks in advance.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A simplistic way to timeout a potentially infinite block is simply:

long timeout = 12345678;

while ( (! ( CANSTMOB & ( 1 << TXOK ) ) ) && timeout-- );   // wait for TXOK flag set 

You want to follow this with some code to check if timeout==0 and take appropriate action. Note also that the optimiser might eat the timeout-- stuff so it could take a little more work than this. Another approach is:

start_timer();
TCNT = 0;
while ( (! ( CANSTMOB & ( 1 << TXOK ) ) ) && (TCNT <= 123));   // wait for TXOK flag set 

Again you need to check whether it timed out after this (possibly just another check of the TXOK bit?)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi Clawson,

Thanks for the reply.

I had been thinking of using a timeout but I am a little concerned that if the CAN busload is quite high that it might take sometime to get my messages onto the bus. From this I think using a TX interrupt would be better.

In the ATMEGA16M1 and from all the posts I have read on older ATMEGA CAN chips, the Message Objects are configured using pages, it is this part that really confuses me. That’s what I would like some more information on.

Thanks again.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It is true that the 11 bit identifiers do not use CANIDT3 and CANIDT4. However, none of the Mob registers have any initial reset value at all. So, your code still needs to initialize these two registers.

void can_tx(uint8_t CAN_MSG[8],uint16_t ID,uint8_t DLC) 
{
….
   uint8_t low = ID & 0x07; 	// Keep 3 LSBs mask other bits
   uint16_t high = ID & 0x7F8; 	// Keep 8 MSBs mask 3 LSBs
   CANIDT2 = (low<<5);      	// Shift left 5 bits as 3 ID bits are CANIDT2[5:7]
   CANIDT1 = (high>>3);     	// Shift right 3 bits to remove 3 LSBs used in CANIDT2
….

Try something simpler/clearer like this to set the ID in the CANIDT registers:

     CANIDT1 = (unsigned char) (ID >> 3); // Set 11 bit format ID data frame.
     CANIDT2 = (unsigned char) (ID << 5);
     CANIDT3 = 0x00;
     CANIDT4 = (0 << RTRTAG);  // Data Frame.
….

Actually, I would not use all capital ID and all capital DLC names for variables. I would suggest using lower case for the variable names. It is easy to confuse all capital variable names with how #define names are usually done. According to the Bosch specification, the maximum ID value is 0x7EF.

When doing a Tx MOb just set all the CANIDM registers to zero. For a Rx MOb this will work:

     CANIDM1 = (unsigned char) (mask >> 3); // Set 11 bit ID mask.
     CANIDM2 = (unsigned char) (mask << 5);
     CANIDM3 = 0x00;
     CANIDM4 = (1 << RTRMSK) | (1 << IDEMSK);
….

Any mask bit CANIDM.IDMSKn bit set to zero will force a Rx compare match (it will not matter if the Rx CANIDT.IDTn bit value is set or not set) and any mask bit set to 1 will allow a normal Rx compare match (it will matter if the Rx CANIDT.IDTn bit value is set or not set). The maximum 11 bit format mask value is 0x7FF (anything larger will get truncated in the >> 3 and << 5 shifts).

Why not just do this:
CANIE2 = 0x3F;

As long as each Mob is setup as disabled in the CAN initialization code, the only MObs that will ever cause any CAN IT interrupt or set the CANIT.CANIT polling bit are MObs that you set as Rx or as Tx. So, setting all the CANIE2 MOb bits does not matter when all the MObs are correctly initialized. You must also set all the MOb CANCDMOB command bits to disabled in the initialization anyway, because the CANCDMOB registers do not have any reset default values, so they need to be initialized to get rid of uninitialized garbage.

This could be a big problem:

ISR (CAN_INT_vect)
{              	
…..
        length = (CANCDMOB & 0x0F);   // DLC, number of bytes to be received

        for (byte=0;byte

Try something like this:

#define MOB_MAX_DLC     ((1 << DLC3) | (0 << DLC2) | (0 << DLC1) | (0 << DLC0))

#define MOB_DLC_LIMIT(dlc) (((dlc) > MOB_MAX_DLC) ? MOB_MAX_DLC : (dlc))

        length = MOB_DLC_LIMIT(CANCDMOB & 0x0F);

It is possible for the CANCDMOB.DLC field to return a value larger then 8 (8 being the maximum number of CAN data bytes). The CAN hardware treats any value larger then 8 as a value of 8. If your software does not also do this maximum value limiting, you could easily overwrite the can_data[byte] array size and crash your program :cry:.

Think of each MOb as a separate task for the CAN hardware. All MObs have the exact same number of registers in each MOb. Yes, you must set CANPAGE before you access any MOb registers. Do not forget setting CANPAGE also sets its CANPAGE.INDXn bits and the CANPAGE.AINC bit. The INDXn bit value points at one of the 8 CANMSG bytes when you read/write CANMSG. You only need to set CANGIE one single time in the CAN initialization code.

        CANGIE = ( 1 << ENIT ) | ( 1 << ENRX ) | ( 1 << ENTX );  // Enable Rx and Tx CAN IT interrupts.

Of course, do not forget the interrupt vector code for CAN IT or your program will not work. You will only get CANSTMOB.RXOK interrupts for MObs you set a Rx command in. You will only get CANSTMOB.TXOK interrupts for MObs you set a Tx command in.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Oops, forgot something:

Rather than put timeouts on CANSTMOB flag bit activity, I like to use interrupts and setup software retries. Whenever I get a CANSTMOB Rx error while in CAN Rx error passive state (CANREC > 127) I disable the MOb, set a software flag and my program uses the software flag to periodically re-enable the MOb and try it again. If the re-enabled/retried MOb keeps getting new Rx errors in error passive state, it will keep disabling/delay/retrying. Then I use a programmable retry limit counter, which includes an optional retry forever setting. I do the same thing only for CANSTMOB.AERR Tx errors where CANGSTA.ERRP is set (CAN Rx or Tx error passive state). If there is no GUI CAN node in your CAN network to report errors to a user, consider turning on an error LED as a diagnostic indicator for the node.

The reason for the Rx error passive retry is because the Rx has no bus off error confinement (only the Tx has this), so it keeps trying to Rx forever unless your program stops it. As long as it keep getting Rx passive errors it will keep sending error passive flags on the CAN bus. It could be something like a CAN node with a Rx problem specific to that one CAN node that will always keep messing up forever, so the delay between software retry attempts gives the CAN bus a rest from bad node Rx passive error flags and gives the CAN bus a chance to return to Rx error active state where it should normally operate at.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi Mike,

Thanks for all the valuable information it has been really helpful, I appreciate you taking the time to explain everything so well.

I have made your recomended changes but now I have encountered a problem.

First I'll explain my changes. I set MOb0 to be the TX MOB and MOb1 to be the RX MOb. I also am trying to only allow CAN IDs 0 - 10 be received. To do this i used the following code:

	for ( int8_t mob=0; mob<5; mob++ ) 
	{ 

      CANPAGE = ( mob << 4 );        // Selects Message Object 0-5

      CANCDMOB = 0x00;             // Disable mob

      CANSTMOB = 0x00;           // Clear mob status register;
    }


	CANPAGE = 0x00;      // Select MOB0 & use for TX
	
	CANIDM1 = 0x00;
	CANIDM2 = 0x00;
	CANIDM3 = 0x00;
	CANIDM4 = 0x00;

	CANPAGE = 0x01;      // Select MOB1 & use for RX

    CANIE2 = 0x3F;      // Enable interrupts on all MObs
	//CANIE2 = ( 1 << IEMOB1 );      // Enable interrupts on mob1 for reception

    CANGIE = ( 1 << ENIT ) | ( 1 << ENRX ) /*| ( 1 << ENTX )*/;   // Enable interrupts on receive
   

    uint16_t mask = 0x07F0; // will allow 0-15, only use 0 - 10  
	
    CANIDM1 = (unsigned char) (mask >> 3); // Set 11 bit ID mask.
    CANIDM2 = unsigned char) (mask << 5);
    CANIDM3 = 0x00;
    CANIDM4 = (1 << RTRMSK) | (1 << IDEMSK); 


    CANCDMOB = ( 1 << CONMOB1) | ( 8 << DLC0);  // Enable Reception 11 bit IDE DLC8
   
    CANGCON |= ( 1 << ENASTB );         // Enable mode. CAN channel enters in enable mode once 11 recessive bits have been read

In my TX function i made the changes as recomended and my unit is transmtiing as desrired. Below is my TX function

void can_tx(uint8_t can_msg[8],uint16_t id,uint8_t dlc) 
{

	

   CANPAGE = 0x00;      // Select MOb0 for transmission
   
   CANSTMOB = 0x00;       // Clear mob status register


   CANIDT1 = (unsigned char) (id >> 3); // Set 11 bit format ID data frame.
   CANIDT2 = (unsigned char) (id << 5);
   CANIDT3 = 0x00;
   CANIDT4 = (0 << RTRTAG);  // Data Frame. 

   for ( uint8_t i = 0; i < dlc; i++ )
   {

		CANMSG = can_msg[i]; 		

   } // for
   
   CANCDMOB = ( 1 << CONMOB0 ) | ( dlc << DLC0 );    // Enable transmission, data length = dlc (CAN Standard rev 2.0A(11 bit identifiers))         

   do
   {
   		if(ms_flag == 1)
		{
			tx_timeout_counter++;
			ms_flag = 0;
		}

   }while ( (!(CANSTMOB & ( 1 << TXOK ) ) ) && (tx_timeout_counter >= 10));   // wait for TXOK flag set

   CANCDMOB = 0x00;   // Disable Transmission

   CANSTMOB = 0x00;   // Clear TXOK flag

   if(tx_timeout_counter >= 10)
   {
   		tx_timeout_counter = 0;
		tx_timeout = 1;
   }
   else
   {
   		tx_timeout_counter = 0;
		tx_timeout = 0;
   }

} 

My RX interrupt is now changed as follows:

ISR (CAN_INT_vect)
{              	
   uint8_t length, save_can_page, byte;
   save_can_page = CANPAGE;         // Save current MOB
   CANPAGE = CANHPMOB & 0xF0;      // Selects MOB with highest priority interrupt

   if (CANSTMOB & (1<<RXOK)) //pg 190
   {     // Interrupt caused by receive finished      
                  

        length = MOB_DLC_LIMIT(CANCDMOB & 0x0F);   // DLC, number of bytes to be received

        for (byte=0;byte

My issue is now when i run my code I can not get the RX interrupt to fire. I have tested this by toggling an LED everytime i enter the ISR but the LED's state never changes. I believe that i have set the RX ISR up correctly but evidentally i have not.

Any input would be gratefully appreciated.

Thanks.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Complete your CAN initial setup first, before issuing the first MOb command. See this post for can_init() example AT90CAN (you will need some ATmega changes) setup code using SWRES (recommended):
https://www.avrfreaks.net/index.p...

Do not write CANCDMOB until after the CANGCON.ENASTB bit is set. You should rearrange your CAN initialization code to fix this.

I'm not sure exactly what you meant:

pmcdonnell wrote:
I also am trying to only allow CAN IDs 0 - 10 be received.
Is this “0x000 - 0x010” or is it “0x000 – 0x00A” (0 - 10 decimal)?

Any CANIDT.IDTn ID of 0x000 to 0x00F and a CANIDM.IDMSKn mask of 0x7F0, will Rx all IDs 0x000 to 0x00F.
Any CANIDT.IDTn ID of 0x000 to 0x01F and a CANIDM.IDMSKn mask of 0x7E0, will Rx all IDs 0x000 to 0x01F.
Any CANIDT.IDTn ID of 0x200 to 0x21F and a CANIDM.IDMSKn mask of 0x7E0, will Rx all IDs 0x200 to 0x21F (the mask works with the ID value).

A CANIDT.IDTn ID of 0x000, 0x002, 0x008 or 0x00A with a CANIDM.IDMSKn mask of 0x07F5, will Rx IDs 0x000, 0x002, 0x008 and 0x00A.
A CANIDT.IDTn ID of 0x001, 0x003, 0x009 or 0x00B with a CANIDM.IDMSKn mask of 0x07F5, will Rx IDs 0x001, 0x003, 0x009 and 0x00B.

If you want to only allow certain IDs you will need to select from a binary range of IDs. After a CANSTMOB.RXOK the CANIDT register will be changed to the actual ID value of the CAN frame that caused the RXOK. So, after the RXOK you read CANIDT:

    unsigned int read_id_11f;        // Rx updated ID value when any CANIDM bits = 0
    CANPAGE = (mob << MOBNB0);       // Select MOb, AINC = 0, INDEX2:0 = 0
    // After this CANSTMOB.RXOK bit is set....
// First check that the CANCDMOB.IDE bit was cleared by the RXOK (11 bit format). You may
// skip this if you only ever use 11 bit format and never clear the Rx CANIDM4.IDEMSK bit.
    read_id_11f = ((unsigned int) CANIDT1 << 3) | ((unsigned int) CANIDT2 >> 5);
    if ((CANIDT4 & (1 << RTRMSK)) == 0) {  // Is this a data frame Rx?
        length = MOB_DLC_LIMIT(CANCDMOB & 0x0F);   // DLC, number of bytes to be received
        for (byte=0;byte

Now you have recovered the actual Rx updated 11 bit format ID (read_id_11f), the number of CANSMG bytes received (length) and the actual CANMSG byte values (can_data[byte]). This code does not handle remote CAN frames, it just treats them as zero data byte length data frames. This will need to be changed if you decide to use remote frames for real.

BTW, if the above code is part of the ISR (CAN_INT_vect) code, then read_id_11f, length and can_data[8] will need to be declared as volatile global variables. Then how do you tell your main() code a new CAN message has been received? Something outside the ISR will have to be made aware the read_id_11f, length and can_data[8] variables have new Rx information in them, especially before the next CAN message comes along and completes another RXOK, which will overwrite and change these variable values yet again. For a simple single MOb for all Rx activity, I would probably create an unsigned char volatile global variable called rx_status=0. The ISR code would set rx_status=CANSTMOB upon a RXOK. Then the main() while(1) polling loop would check for rx_status & (1<<RXOK) to know when new CAN data arrived and clear rx_status.

The purpose of the CANIDM mask register and the acceptance filter is simply to allow the CAN hardware to ignore certain ranges of ID values. All this does is relive the AVR software from processing CAN IDs your software does not care about. For example a MOb with CANIDT.IDTn = 0x000 and CANIDM.IDMSKn = 0x000, will Rx all CAN traffic IDs in that MOb and your software will have to process every single one that is received (actually CANIDT.IDTn could be any value from 0x000 to 0x7EF when the mask is 0x000). On the other hand, a MOb with CANIDT.IDTn = 0x000 and CANIDM.IDMSKn = 0x7FF, will only Rx ID 0x000 and this MOb will ignore all other CAN ID values on the CAN bus (in this case, your software for this MOb will not even be aware any other IDs are being sent on the CAN bus).

One thing is you will only ever get any CANSTMOB.RXOK after a matching (see the acceptance filter in the data sheet) Rx MOb actually receives CAN traffic. In other words you may wait forever for RXOK to set if no matching CAN traffic is ever sent on the CAN bus. The first question is: if any other CAN node is sending an error free Tx that matches your Mob number 1 Rx setup? If no matching error free Tx exists, then not getting a RXOK is expected normal behavior. Only if a matching error free Tx exists, then not getting a RXOK is abnormal broken behavior.

I do not think your mask=0x07F5 was doing what you thought it was doing. Maybe this bad mask value was blocking the Rx Mob ID you expected to trigger your RXOK?

Last Edited: Thu. Feb 23, 2012 - 06:32 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi Mike,

Again thanks for all your time.

I have rearranged my initialisation of the CAN as follows:

	CANGCON = ( 1 << SWRES );   // Software reset

	for ( int8_t mob=0; mob<5; mob++ ) 
	{ 

      CANPAGE = ( mob << 4 );        // Selects Message Object 0-5

      CANCDMOB = 0x00;             // Disable mob

      CANSTMOB = 0x00;           // Clear mob status register;
    }

	
	CANBT1 = 0x0E; 
	CANBT2 = 0x0C; 
	CANBT3 = 0x37; 

	CANGIE = ( 1 << ENIT ) | ( 1 << ENRX ) /*| ( 1 << ENTX )*/;   // Enable interrupts on receive

	CANPAGE = 0x00;      // Select MOB0 & use for TX
	
	CANIDM1 = 0x00;
	CANIDM2 = 0x00;
	CANIDM3 = 0x00;
	CANIDM4 = 0x00;


	CANPAGE = 0x01;      // Select MOB1

    CANIE2 = 0x3F;      // Enable interrupts on all MObs, does not affect disabled MObs   

	uint16_t mask = 0x07F0;  // mask lower 4 bits, 0 - 0x0F, only use 0 - 0x0A
	
	CANIDM1 = (unsigned char) (mask >> 3); // Set 11 bit ID mask.
    CANIDM2 = (unsigned char) (mask << 5);
    CANIDM3 = 0x00;
    CANIDM4 = (1 << RTRMSK) | (1 << IDEMSK); 

	CANGCON |= ( 1 << ENASTB );         // Enable mode. CAN channel enters in enable mode once 11 recessive bits have been read


    CANCDMOB = ( 1 << CONMOB1) | ( 8 << DLC0);  // Enable Reception 11 bit IDE DLC8

This is what i got from the link you gave me is this correct?

In relation to the IDMASK i want to accept IDs from 0x00 to 0x0A, i understand that this is binary so the best mask i can set is 0x00 to 0x0F and i am pretty sure this is set correct in my code.

I however still do not get an RX interrupt. i do enable my RX interrupt to use 8 data bytes using the following line of code:

 CANCDMOB = ( 1 << CONMOB1) | ( 8 << DLC0);  // Enable Reception 11 bit IDE DLC8

However most of the messages i receive are only using data byte 0 as a flag. Would this cause my ISR not to fire? I have edited my ISR like you recommended as shown below:

ISR (CAN_INT_vect)
{              	
   uint8_t length, save_can_page, byte;
   save_can_page = CANPAGE;         // Save current MOB
   CANPAGE = CANHPMOB & 0xF0;      // Selects MOB with highest priority interrupt

   if (CANSTMOB & (1<<RXOK)) //pg 190
   {     // Interrupt caused by receive finished      
   
   		toggle_bit(DEBUG_PORT,DEBUG_PIN);  // debug LED

		can_rx_id = (((unsigned char)((CANIDT2>>5) | (CANIDT1 << 3)));
		
		if ((CANIDT4 & (1 << RTRMSK)) == 0) // Is this a data frame Rx? 
		{  
        	length = MOB_DLC_LIMIT(CANCDMOB & 0x0F);   // DLC, number of bytes to be received

  	        for (byte=0;byte

Thanks again.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

First, do you know for sure another different CAN node is actually sending any Tx CAN frame with an ID of 0x000 to 0x00A? Even a perfectly setup Rx MOb will not trigger the RXOK interrupt until it receives a matching Tx ID.

BTW, did you remember to do a:
sei();
somewhere in your main() initialization code. If you never enabled AVR global interrupts, no AVR interrupt will ever fire.

Actually, this comment needs updating since this is not an error:

        // Note - the DLC field of the CANCDMO register is updated by the received MOb. If the value differs from expected DLC, a warning CANSTMOB.DLCW is set.

If you always read the Rx updated CANCDMOB.DLC (limited to 8 maximum) the CANSTMOB.DLCW bit may be ignored. The way CAN works is you tell the MOb how many CANMSG bytes to expect when you enable the Rx MOb. After the Rx completes with a CANSTMOB.RXOK without the CANSTMOB.DLCW bit being set, tells your software it got only the expected number of CANMSG bytes. If you always read the Rx updated CANCDMOB.DLC anyway, then this warning bit is not really useful. If your software absolutely requires a specific number of Rx CAN data bytes, it is easy enough to check for without using the warning bit.

I did not see:

// Volatile so the ISR value changes are not missed in non-interrupt code.
volatile uint16_t   can_rx_id;       // Global so non-interrupt code may also use this.
volatile uint8_t    length;          // Global so non-interrupt code may also use this.
volatile uint8_t    can_data[8];     // Global so non-interrupt code may also use this.
volatile uint8_t    a_new_mob1_rx_happened_flag = 0;

ISR (CAN_INT_vect) 
{
    uint8_t save_can_page, byte; 
…...
        // After the RXOK is processed (only one example of one way to do this):
        a_new_mob1_rx_happened_flag = CANSTMOB;
…...
    CANSTMOB = 0;
…...
}

main ()
{
    …. all the normal main() initialization code, etc.
    sei();
    while (1) {    // You may never exit main in embedded code.
        ….. other main() code
        if ( a_new_mob1_rx_happened_flag & (1 << RXOK)) {
            // * Do something with the global can_rx_id actual Rx ID value.
            // You could just ignore results for ID values from 0x00B to 0x00F. Ignored
            //   results still need to clear a_new_mob1_rx_happened_flag.
            // * Do something with the global length value.
            // * Do something with the global can_data[n] Rx data values.
            // You are committed to finish this main() polling code before a new CAN
            //   frame completes with another RXOK. There are methods to extend the
            //   amount of time you have before the next CAN frame completes.
            a_new_mob1_rx_happened_flag = 0;
        }
        ….. other main() code
    }  // end of while(1) loop - no executable code past this point.
}

There are lots of other ways to do this. Some ways are more efficient than others. At this point we should keep the focus on getting that first RXOK to fire.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I found one problem and made other comments.

You have the wrong number of MObs in your initialization:

for ( int8_t mob=0; mob<5; mob++ )    // this is Mobs 0 to 4.

for ( int8_t mob=0; mob<6; mob++ )    // this is Mobs 0 to 5.

You left an uninitialized MOb 5 that will do who knows what with no initialized CANCDMOB command bits. This is a serious problem.

An alternative for the mask is:

    CANIDM1 = (unsigned char) (0x07F0 >> 3); // Set 11 bit ID mask.
    CANIDM2 = (unsigned char) (0x07F0 << 5);
    CANIDM3 = 0x00;
    CANIDM4 = (1 << RTRMSK) | (1 << IDEMSK);

You may use a constant instead of a uint16_t mask variable if you want to.

The reason your rewritten can_rx_id= still works is because of compiler automatic integer promotion. You should make sure it works just to be sure.

Have you tried putting your toggle_bit() test code before the “if (RXOK)” in the ISR? Just to see if the interrupt is ever triggering at all.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi Mike,

Thanks for the reply.

I’ll start off by answering your previous questions.

I am using and IXXAT USB-to-CAN and a National Instruments USB-8473s CAN bus monitor to confirm that i am sending messages to my AVR and receiving messages from my AVR.

I have called sei(); in my main code to initialise interrupts and anything i pass from an interrupt is initialised to be a volatile.

I have made the change to the MOb loop as suggested, the code is as follows:

	for ( int8_t mob=0; mob<6; mob++ ) 
	{ 

      CANPAGE = ( mob << 4 );        // Selects Message Object 0-5

      CANCDMOB = 0x00;             // Disable mob

      CANSTMOB = 0x00;           // Clear mob status register;
    }

I have spent a lot of time debugging my code today and i believe i have narrowed the problem area down. I commented out all my calls to my TX function and tested if my pin would toggle each time i entered my RX ISR. This worked every time so you were 100% correct about the ISR being fine. This leads me to believe that my TX function is the issue here.
Below is my full TX function:

void can_tx(uint8_t can_msg[8],uint16_t id,uint8_t dlc) 
{



   CANPAGE = 0x00;      // Select MOb0 for transmission
    

   while ( CANEN2 & ( 1 << ENMOB0 ) ); // Wait for MOb 0 to be free

   CANSTMOB = 0x00;       // Clear mob status register
 
   CANIDT1 = (unsigned char) (id >> 3); // Set 11 bit format ID data frame.
   CANIDT2 = (unsigned char) (id << 5);
   CANIDT3 = 0x00;
   CANIDT4 = (0 << RTRTAG);  // Data Frame. 

   for ( uint8_t i = 0; i < dlc; i++ )
   {

		CANMSG = can_msg[i]; 		

   } // for
   
   CANCDMOB = ( 1 << CONMOB0 ) | ( dlc << DLC0 );    // Enable transmission, data length=8 (CAN Standard rev 2.0A(11 bit identifiers))         

   //while ( ! ( CANSTMOB & ( 1 << TXOK ) ) );   // wait for TXOK flag set 

   do
   {
   		if(ms_flag == 1)  // ms_flag comes from a timer which through interrupts sets the flag every ms
		{
			tx_timeout_counter++;
			ms_flag = 0;
		}

   }while ( (!(CANSTMOB & ( 1 << TXOK ) ) ) && (tx_timeout_counter >= CAN_TX_TIMEOUT));   // wait for TXOK flag set

   CANCDMOB = 0x00;   // Disable Transmission

   CANSTMOB = 0x00;   // Clear TXOK flag

   if(tx_timeout_counter >= CAN_TX_TIMEOUT)
   {
   		tx_timeout_counter = 0;
		tx_timeout = 1;
   }
   else
   {
   		tx_timeout_counter = 0;
		tx_timeout = 0;
   }


} 

My AVR handles a lot of control signals in my unit as well as the CAN so the idea is that if the CAN Bus is not connected then the unit should function as usual, hence the timeout is there to get out of the while loop if the TXOK flag does not is not set within 20ms (CAN_TX_TIMEOUT is defined to be 20).

When i re-enabled my TX function, i have the code set to send a message every 5 seconds as a test. When turn on my unit CAN message are enabled from the start. However at the start of testing the unit does not send any CAN messages. However after i send the unit some messages, number varies but usually between 4 and 10, the unit then begins to send messages. To test this further i tested this with my toggle pin inside my RX ISR, when the unit is not transmitting (at the start) my pin toggles with each message sent to the AVR, however when the unit begins to send messages my pin will not toggle again but the unit will continue sending messages every 5 seconds.

I really believe that the error must be contained in my TX function.

Any thoughts?

Thanks again for all the help.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

More information on my debugging.

In my code I have a routine which runs every 1ms. I set a pin to toggle in this routine every time the routine is run (every ms). I have now set a second pin to toggle inside my Rx ISR.

In my first test I disabled my TX function and ran the code. My 1ms pin toggled as expected. Also anytime I sent a message from my CAN bus monitor to the unit the ISR pin toggled hence proving that my ISR was functioning correctly.

Next I ran the same test but this time I enabled my TX function. As mentioned before my unit sends CAN message every 5 seconds, so my TX function should be called 5 seconds after the unit is powered on and every 5 seconds from then on. During the initial 5 seconds my 1ms toggle pin toggled as expected, also during the initial 5 seconds my RX ISR pin toggled each time I sent a message.

After the first 5 seconds things got really interesting. Once the initial 5 seconds elapsed my 1ms toggle pin stayed in the one state continuously. During this time my RX ISR toggle pin still toggled when I sent the unit a message. However after a certain time (in seconds to tens of seconds), which does not appear to be consistent, my unit would begin to transmit CAN messages, once the unit begins to transmit messages my 1ms toggle pin began to toggle again, however my Rx ISR pin would not toggle anymore.

Could this be due to the TX MOb (which is CANPAGE = 0x00) stopping the Rx ISR seeing as I use the following lines inside my CAN_INT_vect?

CANPAGE = CANHPMOB & 0xF0;      // Selects MOB with highest priority interrupt

Also why would my unit start TXing messages after a certain time?

I think I am close to unearthing the error.

Any thoughts would be gratefully appreciated.

Thanks.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I may be wrong, but it sounds like when your AVR CAN node starts to Tx on the CAN bus, the AVR completely stops receiving CAN traffic from the other IXXAT USB-to-CAN node. This could make sense under these specific conditions:

We know you are not using AVR TTC mode (TTC modifies the AVR automatic error response). If every AVR Tx has CAN bus errors, this will cause the AVR to keep automatically retrying this failed Tx on the CAN bus. This means every time the IXXAT tries to Tx it will get clobbered with an error flag, not complete its Tx and the constantly failing/retrying AVR Tx will keep preventing any IXXAT Tx from working.

Except for CANSTMOB.AERR, constant Tx errors will trigger CAN Tx error confinement and eventually shut down the AVR CAN Tx. The shutdown will automatically end and re-enable the AVR CAN Tx after the AVR receives 128 occurrences of 11 consecutive recessive CAN bus bits.

A modified scenario would be if there are no AVR Tx CAN bus errors, but your software keeps feeding new highest ID priority AVR Tx tasks faster than the last AVR Tx takes to complete, you will completely block all AVR Rx CAN activity because the AVR is always in nonstop Tx that wins every Tx ID priority arbitration. This would mean your software would be flooding the CAN bus with higher priority ID CAN messages (not a CAN bus caused problem). However, since you tested with your Tx code commented out, you have proved this is not the problem.

Keep in mind the AVR will never receive its own CAN bus Tx and never set CANSTMOB.RXOK from its own Tx.

The point of mentioning these scenarios is that I have an almost impossible time trying to see if your CAN activity due to CAN bus conditions is normal or abnormal.

You are at the point where you need to start tracking CANREC and CANTEC. For every AVR CAN bus error one of these will increment, usually by a value as high as 8. Every CAN message without any error will decrement one of these usually by 1 (never below 0). As CAN bus traffic continues, CANREC and CANTEC may dynamically change values depending on any error activity. In a perfect CAN bus world both of these are always at zero.

Since you know you have CAN bus activity, if your code is not working and CANREC/CANTEC are always a zero value, then no CAN bus errors are being detected. If these are both zero and your CAN is not working, this would indicate a software bug and/or inappropriate use of the CAN bus (like flooding the CAN bus for example).

If CANREC/CANTEC are not zero, then you will want to set CANGIE.ENERR and modify your CAN ISR to process the CANSTMOB error bits. This will allow you to see whatever problems the AVR is experiencing. I have found simply concatenating aggregate CANSTMOB errors in a volatile global variable is a good diagnostic.

Here is a conceptual example that uses a_new_mob1_rx_happened_flag to track CANSTMOB errors and display them in main():

#define MOB_ENERR_FLGS     ((1 << BERR) | (1 << SERR) | (1 << CERR) | (1 << FERR) | (1 << AERR))

ISR (CAN_INT_vect)
{
    uint8_t save_can_page, mob, byte;
    save_can_page = CANPAGE;                // Save current MOB
    CANPAGE = CANHPMOB & 0xF0;    // Selects MOB with highest priority interrupt

    mob = (CANPAGE >> MOBNB0);
    if (mob == 1) {                                       // Is this the Rx MOb?
        a_new_mob1_rx_happened_flag |= CANSTMOB;  // Concatenate the flags.
        CANSTMOB = 0;
        if ( a_new_mob1_rx_happened_flag & (1 << RXOK)) {
…...
}

main ()
{
    …..
    while (1) {    // You may never exit main in embedded code.
        ….. other main() code
        if ( a_new_mob1_rx_happened_flag & MOB_ENERR_FLGS) {
            ….. Now display the CANSTMOB error flags and CANREC/CANTEC.
        }
        if ( a_new_mob1_rx_happened_flag & (1 << RXOK)) {  // Just like the last example.
        …..

This example is not perfect. The polling MOB_ENERR_FLGS will keep finding the same error over and over again on every polling loop. Your debug code would have to take this into account and compensate. This does not count every CAN bus error or show the newest error, but it does show what errors actually occurred since you enabled the Rx MOb. It has the advantage of allowing slow debug output (like USART Tx code) in main(), keeping it away from the ISR where slow debug code would destroy the AVR interrupt availability. This type of debug is usually enough information to diagnose your CAN bus problem.

In your Tx code when you disable a MOb by writing CANCDMOB=0, the CAN state machine hardware takes this as a disable request (or call it a MOb disable attempt). If the CAN hardware is using the CAN bus Tx MOb when you issue the disable command, it may not take effect immediately. What you need to do is after writing CANCDMOB=0 (disable) to a previously active Tx MOb, is wait until the MOb corresponding CANEN2 busy bit clears. The MOb is only actually disabled after this CANEN2 busy bit clears. This is called on-going communication when you tell the CAN hardware to disable a MOb, it delays the actual disable and keeps using the CAN bus. In fact your disable attempt may fail and the Tx MOb may complete like normal, the Tx MOb may fail with a last error or if there is no on-going communication the MOb will immediately disable like you asked it to.

*edit: Oops, i missed the CANEN2 check at the Tx beginning the first time. So, you do have that covered.

If you are timing out a Tx, issuing a disable command, then on-going communication keeps the MOb alive and you start another Tx in the still busy MOb - well obviously you have just created a huge bug that will confuse the CAN hardware with totally unpredictable results. This means nothing on your testing where all Tx activity was commented out, but it could mean everything in your testing with Tx enabled.

Last Edited: Sun. Feb 26, 2012 - 10:02 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

pmcdonnell wrote:
Could this be due to the TX MOb (which is CANPAGE = 0x00) stopping the Rx ISR seeing as I use the following lines inside my CAN_INT_vect?
No, this should not be a problem because CANGIE.ENTX is cleared. Since CANGIE.ENTX is cleared, the CANSIT.SIT0 bit should never set for MOb 0 and CANHPMOB uses CANSIT to tell it which MOb to select.

The ISR code CANPAGE & 0xF0 is a little strange. Actually the CANHPMOB.CGPn bits are designed to map to the CANPAGE.AINC and CANPAGE.INDXn bits. Since the SWRES does reset default the general registers (not MOb registers) CANHPMOB=0 is the reset default. Unless you change the CANHPMOB.CGPn bits yourself, they are already the correct value for a simpler CANPAGE=CANHPMOB assignment where AINC=0 and INDXn bits are a zero value. The 0xF0 was never intended to be used as a bit mask value.

This is the common usage for CANHPMOB:

    while ((CANPAGE = CANHPMOB) < 0xF0) {

The 0xF0 is because when CANHPMOB sets CANPAGE to a value equal or higher than 0xF0 it means there are no more MObs with CANGIE enabled CANSTMOB interrupt flags set. The idea behind CANHPMOB is when the CAN hardware sets any CANSTMOB flag bit or bits and a corresponding CANGIE/ENTX/ENRX/ENERR bit is set, it sets the corresponding MOb CANSIT2 bit. The while(CANHPMOB) gets the lowest numbered MOb from CANSIT2 first, then you processes that MOb clearing its CANSTMOB byte which also automatically clears the MOb corresponding CANSIT bit. Take note that CANSIT is read only, so you may not write to it yourself. Now the next while(CANHPMOB) loop no longer sees the just processed first MOb (since its CANSIT bit is now cleared) and proceeds to the next higher MOb with its CANSIT bit set. CANHPMOB uses CANSIT to pick its way through all the MObs from lowest to highest until you run out of MObs to process, then CANHPMOB returns a value of 0xF0 or higher.

Using CANHPMOB when you only used two total MObs does not help your ISR efficiency. However, when you use all the MObs it is a big efficiency improvement over having to poll each and every CANSTMOB value on every ISR. CANHPMOB allows you to quickly locate only the MObs that need servicing for each ISR.

For testing with only a single dedicated Rx MOb that uses the CAN ISR, you could do this:

    CANPAGE = (1 << MOBNB0);  // Set MOb 1

and not use CANHPMOB at least for now (maybe later after you enable CANGIE.ENTX or if you use a third MOb).

One good reason for using multiple Rx MObs is you may setup the exact same Rx MOb in multiple MObs. If your ISR treats all the duplicate Rx MObs correctly this is what will happen. The first Rx acceptance filter trigger will pick a matching lowest numbered active MOb to get the CAN Rx data and set this MOb CANSTMOB.RXOK flag (RXOK disables the MOb and it no longer receives anything from the CAN bus). The second Rx acceptance filter trigger will pick a matching lowest numbered active MOb to get the CAN Rx data and set this MOb CANSTMOB.RXOK flag. Depending on how many duplicate Rx MObs you use, each one will Rx the next CAN traffic for that MOb setup. When the ISR finally gets called each MOb is an independent buffer for all the previous CAN Rx traffic and you may re-enable each RXOK MOb for a new Rx. Now your code is not constrained to always servicing a single MOb before the next CAN Tx is received. Two duplicate MObs will double the time limit on this constraint, three will triple it, etc. Only if really needed, the CAN time stamp may be used to sort out what order all the MObs received CAN traffic in which MOb.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi Mike,

Thanks again for your great explanations and time.

In relation to your first post, i did the following debugging steps.

First of all neither my IXXAT nor NI can bus monitors were seeing any bus errors. Despite this i did monitor CANTEC and CANREC as recommended. Both of these stayed at 0 when my bus was connected. If i disconnected the bus from my AVR and reconnected a minute later I would see that CANTEC had incremented but once the bus was connected again this decremented with each CAN message sent from the AVR.

From all of my debugging today it just seemed like my RX ISR was being disabled once my TX started, so just as a reset i added the following lines to my TX function:

   CANPAGE = 0x01;      // Select MOB1
   CANCDMOB = 0x00;  // disable Reception 11 bit IDE DLC8

Before i did anything in my CAN TX function i disabled my RX ISR, then at the then end of my TX function i re-enabled my RX ISR with the following lines:

   CANPAGE = 0x01;      // Select MOB1
   CANCDMOB = ( 1 << CONMOB1) | ( 8 << DLC0);  // Enable Reception 11 bit IDE DLC8

From initial testing this seems to have fixed my issues. Do you think this method is the right approach?

Also i tried using your line in my ISR:

CANPAGE = (1 << MOBNB0);  // Set MOb 1 

But this caused my ISR to fire roughly every 800us. Instead of this i used the following line as i am only using one MOb with interrupts

CANPAGE = 0x01;  // Set MOb 1 

As said previously my code appears to be working (initial testing) but i would be very interested in your opinions.

Thanks again for all the help.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Duh, we both missed the problem. Sometimes the mind reads something and accepts it even though it is wrong. Every place in your program code where you set:

    CANPAGE = 0x01;  // Set MOb 1

to set the Rx MOb, needs to be changed to this:

    CANPAGE = (1 << MOBNB0);  // Set MOb 1 pointing at CANMSG byte 0

The CANPAGE=0x01 is MOb 0 pointing at CANMSG byte number 1, not MOb 1. This MOb 1 selection needs to be consistent through your entire program. If anyplace in your code uses 0x01 for MOb 1 it will actually be messing up your Tx MOb 0.

When you only changed this in one place in the ISR it did not help since it was still wrong everywhere else in your program. Actually you could use 0x10 for MOb 1 instead.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well here is a perfect example why everyone should use the bit names when writing to registers instead of writing hex or binary values.

I have now set all my selections of MOb1 to the following:

CANPAGE = (1 << MOBNB0);  // Set MOb 1 pointing at CANMSG byte 0 

I then removed the disabling and re-enabling of the RX in my TX function and it seems to work correctly during initial testing.

I still think this was a blessing in disguise as i now have a much better understanding of the CAN on the AVR.

Mike thanks for all the time and insight. It has really helped me and improved my knowledge.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If you Private Message (PM) me an e-mail address, I will send you a work in progress 8 bit AVR CAN document. It is a 423kb PDF file. It has more in depth information in it.