CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to support@ccsinfo.com

BUG ? delay_ms() function in ISR
Goto page Previous  1, 2, 3  Next
 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
kender



Joined: 09 Aug 2004
Posts: 768
Location: Silicon Valley

View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger

PostPosted: Wed May 17, 2006 4:07 pm     Reply with quote

Christophe wrote:
doesn't the

g_iDebounce = FALSE; // debouncing is over. clear the flag

has to be inside the 'if'


I have thought about this question some more and realized that the code that I have written for you has a substantial bug: it doesn't count the timer interrupts for debouncing. I have edited it in that previous message, please take a look.
Christophe



Joined: 10 May 2005
Posts: 323
Location: Belgium

View user's profile Send private message

PostPosted: Thu May 18, 2006 1:14 am     Reply with quote

I understand. By counting down from 6 tot 0 , you are sure to wait 30 ms that is needed for letting the switch debounce.

Here is my implementation:

Code:
//************************** EXTERNAL interrupt ******************************//
//Interrupt als de AAN/UIT schakelaar wordt bewogen
#INT_EXT
void ext_isr()
{
   On_Off_State = input(Aan_uit);   // status aan / uit schakelaar
   Debounce_counter = 2;                 // begin debouncing
}



Code:
//************************** Timer0 interrupt ********************************//
//Timer0 overloopt elke 64 * 256 us = 16,4 ms en bewaakt de batterijspanning
//Batterijspanning wordt doorgestuurd elke minuut.
#int_RTCC
RTCC_isr()
{
int8 k;

   teller++;
   if (teller == 3658)  // 3658 * 16ms = 01m00s
   {
      Meet_Vbat(&Vbat);       // Vbat meten
      if (!E3_SLEEP)          // Stuur de batterijspanning door naar E3 als deze
      {                       // niet in sleep staat
         Send_Vbat(Vbat);
      }
      teller = 0;
   }
      if (Debounce_counter == 0)
      {   
         if (On_off_State == input(AAN_UIT))
         {
            if (input(AAN_UIT))  // swich naar boven
            {

     // if (E3_OFF)
     // {
     //    BOOT_E3_NEEDED = TRUE;
     // }
               for (k = 0; k < 20 ;k++)
               {
                 printf("%c",0);
               }

            printf("%c%S%c", COMMANDO_BYTE, WAKE_COMMANDO,0);
            ext_int_edge(H_TO_L);
            }

            else     // switch naar beneden
            {
               if (SPACE_PRESSED)
               {
                  SPACE_PRESSED_WHILE_SHDN = TRUE;
               }

               printf("%c%c%S%c",0 ,COMMANDO_BYTE, SLEEP_COMMANDO,0);
               ext_int_edge(L_TO_H);
            }
         
         }       
      Debounce_Counter = 255;
      }           
   
   else if (Debounce_Counter < 255)   // 255 is a special value that means that no debouncing is happening
   {
      --Debounce_Counter;
   }

}// TIMER0


What can I do about the waiting due to the serial output inside the ISR? What is the exact problem there? Missing other interrupts; how is that possible?
Ttelmah
Guest







PostPosted: Thu May 18, 2006 7:12 am     Reply with quote

Use a serial transmit buffer (just as for serial receive). So:
Code:

//Buffer tests and handling
#define isempty(buff,free,in,out,size) (free>=size)
#define isfull(buff,free,in,out,size) (free<2)
#define tobuff(buff,free,in,out,size,chr) {buff[in]=chr;\
  --free;\
  in=((in+1) & (size-1));}
unsigned int RSTfrom(){
  /* Get character from the RS232 TX buffer */
  static unsigned int temp;
  temp=RSTbuff[RSTout];
  RSTout=(RSTout+1) & (STBUFF-1);
  RSTcount++;
  return(temp);
}


//Bits to allow direct register access
#byte   TXREG  =   0xFAD
#byte   PIR1 = 0xF9E
#define   TXIF   (4)

/* sizes of the individual buffers */
#define STBUFF   32

int RSTcount=STBUFF,RSTin=RSTout=0;
int RSTbuff[STBUFF];

#INT_TBE  /* Transmit buffer empty interrupt */
void TXINT(void)
   {
   /* If characters are waiting in the buffer, the next one is
   transferred to the UART otherwise the interrupt is disabled,
   waiting for the next transmission. */
   if (!isempty(RSTbuff,RSTcount,RSTin,RSTout,STBUFF)) {
      TXREG=RSTfrom();
   }
   else
   DISABLE_INTERRUPTS(INT_TBE);   /* RS232 TX */
}

void tchar(unsigned int chr) {
  /* routine to send one character on the RS232.
  This puts the specified character into the software transmit buffer
  (if data is allready being transmitted), or else sends the single
  character to the RS232 UART. */
  //Ensure the operation is not interrupted
  disable_interrupts(INT_TBE);

  //Handle 'buffer full' overflow - hold till character is sent.
  while (isfull(RSTbuff,RSTcount,RSTin,RSTout,STBUFF)) {
    if (BIT_TEST(PIR1,TXIF)==1) {
      /* Here the transmit hardware buffer is empty */
      TXREG=RSTfrom();
      BIT_CLEAR(PIR1,TXIF);
    }
  }
  /* put character into the output buffer */
  tobuff(RSTbuff,RSTcount,RSTin,RSTout,STBUFF,chr);
  /* Enable interrupts */
  enable_interrupts(INT_TBE);
}

You can then use 'printf(tchar,"What you want to send")', and the data will be buffered out. Leave the TXE interrupt _disabled_, till you send data (it is automatically enabled when characters are sent).
Notice also, that assuming you are using a serial 'receive' buffer, in a RX interrupt, the same buffer handling defines can be used, just with different buffer names, and a different 'from' routine.
This version of the buffer handling, uses one more variable, than the version I normally post, but is slightly quicker on some tests as a result.

Best Wishes
Christophe



Joined: 10 May 2005
Posts: 323
Location: Belgium

View user's profile Send private message

PostPosted: Thu May 18, 2006 7:57 am     Reply with quote

what does this do?

printf("%c%c%S%c",0 ,COMMANDO_BYTE, SLEEP_COMMANDO,0);

sends: 0dec, 170dec, 'S', 'L', 'E', 'E', 0

but when do you return from this function? when all bytes are sent?
Or does it just copy the bytes to a software buffer and the bytes are sent on the background. What is the difference with your functions?
Ttelmah
Guest







PostPosted: Thu May 18, 2006 8:49 am     Reply with quote

The hardware, only has a couple of characters of 'buffering'. There is the shift register outputting the current byte, and a second 'holding' register with a byte waiting to send. Assuming the hadware buffers are empty when you start a printf (no guarantee of this...),If you send a string that is seven bytes long (as your's is), then the printf call will have to wait for 5 of these to send, before returning. At 9600bps, N81, this means a delay of 50 bit times. 5.2mSec. At lower baud rates, even longer.
The code I posted, uses a software buffer, and the transmit interrupt, to seperate the act of preparing the data, and actually sending it.
Seperately though, there is also the issue mentioned before, of using routines inside the interrupt. If you are making any use of printf outside the interrupt,then having an output like this, will result in interrupts being disabled inside every printf outside the interrupt routine, and lead to yet further delay problems...
Basically, you need to start thinking a different 'way' about things. Only perform actual time dependant tasks inside the interrupt, and keep these to the minimum/short. For your output routine, I'd just set a flag when the condition is seen, and in the main code outside the interrupt, when this flag goes true, send the string, and clear the flag. Thsi way the problem disappears.

Best Wishes

Best Wishes
Christophe



Joined: 10 May 2005
Posts: 323
Location: Belgium

View user's profile Send private message

PostPosted: Thu May 18, 2006 9:08 am     Reply with quote

1. What is exactly the problem with waiting inside an interrupt routine? Is that when inside,other interrupts are disabled? Meaning you can miss other interrupts?

2. I don't understand:

Quote:
Seperately though, there is also the issue mentioned before, of using routines inside the interrupt. If you are making any use of printf outside the interrupt,then having an output like this, will result in interrupts being disabled inside every printf outside the interrupt routine, and lead to yet further delay problems...


3. Give your opinion about my interrupt routines. What should you change:

Code:
//***************************** RDA interrupt ********************************//
//Als een karakter wordt ontvangen door de UART; inlezen en opslaan.
#int_RDA
RDA_isr()
{

        data[Read_pointer] = getc();

        Read_pointer = (Read_pointer+1)%BUFFER_SIZE;

        Byte_received = TRUE;

}//RDA ISR

//************************** Timer1 interrupt ********************************//
// Timer1 wordt gestart als de E3 het zegt
// Elke 500ms gebeurt er een interrupt. 1 sec na 2 interrupts.
#int_timer1
Timer1_isr()
{
      //putc(secs);
      Set_Timer1 ( 3035 );
     
      if(++int_count == 2)
      {
            ++secs;
            if(secs==60)
            {
                mins++;
                secs = 0;
            }

            if (mins==60)
            {
               hrs++;
               mins=0;
            }
         // Na x tijd de ES uitschakelen om energie te sparen

            if ((mins == off_mins) && (hrs == off_hours))
            {
               // putc(mins);
               //  BUZZER();
               //delay_ms(400);
               
               //delay_ms(400);
               //BUZZER();
              // TOGGLE_E3();
               SETUP_TIMER_1(T1_DISABLED);      // T1 uit
            }
            int_count = 0;
     }
}// TIMER1

//************************** Timer0 interrupt ********************************//
//Timer0 overloopt elke 64 * 256 us = 16,4 ms en bewaakt de batterijspanning
//Batterijspanning wordt doorgestuurd elke minuut.
#int_RTCC
RTCC_isr()
{
int8 k;

   teller++;
   if (teller == 3658)  // 3658 * 16ms = 01m00s
   {
      Meet_Vbat(&Vbat);       // Vbat meten
      if (!E3_SLEEP)          // Stuur de batterijspanning door naar E3 als deze
      {                       // niet in sleep staat
         Send_Vbat(Vbat);
      }
      teller = 0;
   }
      if (Debounce_counter == 0)
      {   
         if (On_off_State == input(AAN_UIT))
         {
            if (input(AAN_UIT))  // swich naar boven
            {

     // if (E3_OFF)
     // {
     //    BOOT_E3_NEEDED = TRUE;
     // }
               for (k = 0; k < 20 ;k++)
               {
                 printf("%c",0);
               }

            printf("%c%S%c", COMMANDO_BYTE, WAKE_COMMANDO,0);
            ext_int_edge(H_TO_L);
            }

            else     // switch naar beneden
            {
               if (SPACE_PRESSED)
               {
                  SPACE_PRESSED_WHILE_SHDN = TRUE;
               }

               printf("%c%c%S%c",0 ,COMMANDO_BYTE, SLEEP_COMMANDO,0);
               ext_int_edge(L_TO_H);
            }
         
         }       
      Debounce_Counter = 255;
      }           
   
   else if (Debounce_Counter < 255)   // 255 is a special value that means that no debouncing is happening
   {
      --Debounce_Counter;
   }
}// TIMER0

//************************** EXTERNAL interrupt ******************************//
//Interrupt als de AAN/UIT schakelaar wordt bewogen
#INT_EXT
void ext_isr()
{
   On_Off_State = input(Aan_uit);   // status aan / uit schakelaar
   Debounce_counter = 2;                 // begin debouncing
}
newguy



Joined: 24 Jun 2004
Posts: 1899

View user's profile Send private message

PostPosted: Thu May 18, 2006 9:35 am     Reply with quote

Christophe wrote:
1. What is exactly the problem with waiting inside an interrupt routine? Is that when inside,other interrupts are disabled? Meaning you can miss other interrupts?

2. I don't understand:

Quote:
Seperately though, there is also the issue mentioned before, of using routines inside the interrupt. If you are making any use of printf outside the interrupt,then having an output like this, will result in interrupts being disabled inside every printf outside the interrupt routine, and lead to yet further delay problems...



1. Other interrupts still occur while you're servicing an interrupt, but they can't be serviced/acted upon until the processor finishes dealing with the interrupt it is currently servicing.

Ever see the very old TV show called "I Love Lucy"? I'm reminded of the episode where she works in a candy factory. She has to package chocolates as they roll by on a conveyor belt. Although the belt starts off slowly, it quickly gains speed to the point where no person can handle the chocolates as they appear.

Your serial RS232 link can be thought of as a conveyor belt with the inbound characters being the chocolates. Whatever you do, you must be able to "catch" the next inbound character to assure no data is lost.

Say your routine is to "catch" chocolates and place them in a box. When you turn away from the conveyor to put the lid on the box you must either put the lid on the box quickly and turn back to the conveyor in time to catch the next chocolate, or you must be able to fill the next box while putting the lid on the first. If you turn away to put the lid on but take too long to do so, some chocolates will hit the floor.

Start to make sense now?

Interrupt routines must be short - extremely short. Time spent within an ISR is time that the processor can't spend servicing other interrupts. Standard practice is to not do any work (i.e. sorting, calculations, etc.) within an ISR. Instead, set a flag - i.e.
Code:
box_full_of_chocolates = TRUE;

Then in your main routine, you'd have something like this:

Code:
while (TRUE) {
   restart_wdt();
   if (box_full_of_chocolates) {
      box_full_of_chocolates = FALSE;
      put_lid_on_box();
   }
}


Your "chocolate_isr" would simply put chocolates in the box. The act of putting the lid on the box is handled outside of any ISR, because that action can be interrupted by a higher priority event - like making sure that no chocolates hit the floor.

2. If a function is called within an ISR and outside of an ISR, the compiler will disable interrupts whereever it is used outside of an interrupt. The reason for this is to prevent re-entrancy.
dyeatman



Joined: 06 Sep 2003
Posts: 1910
Location: Norman, OK

View user's profile Send private message

PostPosted: Thu May 18, 2006 9:42 am     Reply with quote

EXCELLENT analogy and explanation!!

dave
Ttelmah
Guest







PostPosted: Thu May 18, 2006 10:18 am     Reply with quote

Very clear!. :-)
One further point. When one interrupt event occurs, inside another, it's flag is set, and it'll be serviced when you exit the current handler. However if a _second_ event occurs on an interrupt, before the first is handled, the 'odds' are that this is now lost for ever.
Also many events having timings for how quickly they must be serviced. So (for instance), if you have an interrupt driven I2C slave handler, then you _must_ have handled the first byte, before the next one finishes clocking in, or data will be lost. The serial, has effectively just under two characters of buffering, from the moment the interrupt flag goes 'true', but if this overflows, the chip will set an error flag, and the UART will actually stop receiving.
KISS, with the meaning modified to 'keep it short & simple', is a good acronym for how to work with interrupts.:-)

Best Wishes
rnielsen



Joined: 23 Sep 2003
Posts: 852
Location: Utah

View user's profile Send private message

PostPosted: Thu May 18, 2006 1:21 pm     Reply with quote

Man, now I've got the munchies.

*walks over to the vending machine and starts pulling out loose change* Wink
Christophe



Joined: 10 May 2005
Posts: 323
Location: Belgium

View user's profile Send private message

PostPosted: Mon May 22, 2006 2:52 am     Reply with quote

Newguy,

are you perhaps a school teacher? Many thanks for the clear explanation. Now I understand interrupts and handling them a lot better. Guess almost everything about interrupts and keeping them short was tackled there.

My implementations: feel free to comment them:

Code:
//***************************** RDA interrupt ********************************//
//Als een karakter wordt ontvangen door de UART; inlezen en opslaan.
#int_RDA
RDA_isr()
{
   data[Read_pointer] = getc();
   Read_pointer = (Read_pointer+1)%BUFFER_SIZE;
   Byte_received = TRUE;
}//RDA ISR

//************************** Timer1 interrupt ********************************//
// Timer1 wordt gestart als de E3 het zegt
// Elke 500ms gebeurt er een interrupt. Seconden, minuten en uren worden geteld.
#int_timer1
Timer1_isr()
{
   Set_Timer1 ( 3035 );
   if(++int_count == 2)
   {
      ++secs;
      if(secs == 60)
      {
         mins++;
         secs = 0;      }
      if (mins == 60)
      {
         hrs++;
         mins = 0;
      }
// Na x tijd de ES uitschakelen om energie te sparen
      if ((mins == off_mins) && (hrs == off_hours))
      {
         OFF_TIMER_AFGELOPEN = TRUE;
      }
      int_count = 0;
   } // 1 seconde
}// TIMER1

//************************** Timer2 interrupt ********************************//
//Timer0 overloopt elke 10 ms. Er wordt een vlag geset, deze wordt
//afgehandeld in main om de ISR zo kort mogelijk te houden.
#int_Timer2
Timer2_isr()
{
   if (teller)
      --teller;
   
   if (Debounce_counter == 0)
   {
      if (On_off_State == input(AAN_UIT))
      {
         BUTTON_DEBOUNCED = TRUE;
      }
      Debounce_Counter = 255; // 255 is a special value that means that no debouncing is happening
   } // If debounce nodig

   else if (Debounce_Counter < 255)
   {
      --Debounce_Counter;
   }
}// TIMER2

//************************** EXTERNAL interrupt ******************************//
//Interrupt als de AAN/UIT schakelaar wordt bewogen
#INT_EXT
void ext_isr()
{
   On_Off_State = input(Aan_uit);   // status aan / uit schakelaar
   Debounce_counter = 3;                 // begin debouncing
}


output routines in main() while(1):

Code:
//*********************** On / Off switch event Handler **********************//
   if (BUTTON_DEBOUNCED)
   {
      BUTTON_DEBOUNCED = FALSE;
 // Na 3 interrupts = 32.8ms wordt de debounce routine gestart.
         if (input(AAN_UIT))  // switch naar boven: RB0 = 3V
         {
           // if (E3_OFF)
           // {
           //    BOOT_E3();
           // }
            for (k = 0; k < 20 ;k++)
            {
               putc(0);
            }
            printf("%c%S%c", COMMANDO_BYTE, WAKE_COMMANDO,0);
            ext_int_edge(H_TO_L);   // Volgende EXT_ISR() is bij overgang L 2 H
         }
         else                 // switch naar beneden
         {
            if (SPACE_PRESSED)
            {
               SPACE_PRESSED_WHILE_SHDN = TRUE;
            }
            printf("%c%c%S%c",0 ,COMMANDO_BYTE, SLEEP_COMMANDO,0);
            ext_int_edge(L_TO_H);
         }
   }

//***************************** ONE MINUTE Teller ****************************//
// De batterijspanning wordt gemeten elke minuut en zowieso doorgestuurd.
   if (teller == 0)
   {
      teller = 6000;
      Meet_Vbat();
      if (!E3_SLEEP)
         Send_Vbat();
   }
 
//*************** TIMER1 OFFTIMER afgelopen vlag afhandelen ******************//
   If (OFF_TIMER_AFGELOPEN)
   {
      SETUP_TIMER_1(T1_DISABLED);
      OFF_TIMER_AFGELOPEN = FALSE;

      putc(mins);
      BUZZER();
      delay_ms(400);
      BUZZER();
      // TOGGLE_E3();
   }
Ttelmah
Guest







PostPosted: Mon May 22, 2006 3:53 am     Reply with quote

The only problem you may have is with accuracy on Timer1. I haven't worked out what actual clock rate you are running it off, but the point in time, when you set the timer to a count, will vary, depending on what else is going on (if for instance, the chip was in another interrupt when this occurs). Provided you don't mind a slight jitter in this, there is not a problem, but if you are wanting accurate timings, this will need to be sorted. Now you seem to be using this as a 'clock', and in which case, this is almost certain to drift. I'd consider getting rid of timer1 completely, and just implementing a counter in timer2. So:
Code:

    //Inside timer2
   static int timer_tick;

   if ((--timer_tick)==0) {
        timer_tick=100;
        //Do your seconds update here

   }

The 'cost' is slightly longer spent in timer2, but this is tiny, while the saving is all the overhead of processing the extra interrupt, and the potential accuracy problem mentioned.
You are actually approaching a common system for handling events, which is to have a master 'tick' interrupt at some frequency like 100Hz, and use this to provide all the 'times' in one location.
Generally setting a timer 'to' a value, is an unreliable technique in many circumstances.
It is looking a lot better. :-)

Best Wishes
Christophe



Joined: 10 May 2005
Posts: 323
Location: Belgium

View user's profile Send private message

PostPosted: Mon May 22, 2006 5:17 am     Reply with quote

1. what means the 'static' word?

2. Why is the variable created inside the isr? Can it be created as a global variable?

3. I'll think about that implementation. I have to count especially minutes and hours. That is most important.
The timer application is an off-timer; eg. after 8 hours put some device off to save power and disable hours timer.
kender



Joined: 09 Aug 2004
Posts: 768
Location: Silicon Valley

View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger

PostPosted: Mon May 22, 2006 5:45 am     Reply with quote

Christophe wrote:
1. what means the 'static' word?

http://msdn2.microsoft.com/en-us/library/s1sb61xd.aspx
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccelng/htm/decla_7.asp
This MSDN article was written for C++, but it applies to CCS C as well.

Christophe wrote:
2. ... Can it be created as a global variable?

Yes, it might as well be created as a global variable. But, if the variable is static, chances that some other piece of code will unadvertantly modify up are smaller. For example, this should produce a compile-time error:

Code:

#int_
void MyISR()
{
  static int8 iCount = 0;
  ++iCount;
}

void main()
{
  iCount = 2; // compile err
  printf("%d", iCount); // another compile err
}


Plus the code is more readable.


Last edited by kender on Tue May 23, 2006 5:50 am; edited 1 time in total
Christophe



Joined: 10 May 2005
Posts: 323
Location: Belgium

View user's profile Send private message

PostPosted: Mon May 22, 2006 5:53 am     Reply with quote

It has to be initialised to 100. Where is that done? the article says it's initialised 0.
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Goto page Previous  1, 2, 3  Next
Page 2 of 3

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group