Take the 2-minute tour ×
Electrical Engineering Stack Exchange is a question and answer site for electronics and electrical engineering professionals, students, and enthusiasts. It's 100% free, no registration required.

I am writing a code with interrupt routine. For debugging the code am using UART. So, here is the snippet:

volatile int overflow_count = 0;

int main(void) {
  while (1) {
    continue;
  }
}

ISR(TIMER1_OVF_vect) {
  char* time = "";
  overflow_count++;
  sprintf(time, "overflow-%d", overflow_count);
  uart_send(time);
  if (overflow_count == 2) {
    overflow_count=0;
    uart_array("Alarm code execution");
  }
}

Until first time the interrupt routine is called overflow_count value is 0 and after that, value is not incremented by one as per the code. Instead a random value (eg. 142851) is displayed while sending it from serial port. So, it never satisfies the condition if (overflow_count == 2). I am curious to know why it is not working.

share|improve this question
5  
It's probably a corrupted memory area because you don't allocate room for the string. Try char time[20]=""; and see how it goes. –  PeterJ May 2 at 6:29
    
Hi, but what about overflow_count. It is an integer right? So, the condition if(overflow_count==2) must be satisfied. –  gzix May 2 at 6:37
4  
When you try to put the string in time to display it depending on where the compiler has put things it could overwrite your counter. –  PeterJ May 2 at 6:39
4  
first of all, please avoid using sprintf and UART functions inside an ISR.. You also cant really declare a time "string" like this, you should have a proper array declaration such as char ISR_TXBuffer[32] = {0}; and then perhaps sprintf the variable "time" into that buffer, and transmit that. You should really just do the overflow count, and set a flag or something so that another loop can do the printing of the buffer rather than in an ISR –  KyranF May 2 at 6:47
2  
The point is some UART or print functions might be "unsafe" to call during an ISR. So even for debugging purposes you should not do it this way. –  KyranF May 2 at 7:50

1 Answer 1

up vote 11 down vote accepted

Others have already pointed out the original problem in the comments above, namely that you need to reserve space for the character array.

I have made a few other changes to your original code, including:

  • Allocate space for the character array
  • Move code out of the interrupt service routine and back into main()
  • Using a global flag to communicate between the ISR and main()
  • Initialising all array elements to zero
  • Initialising global variables in main()

You should avoid doing any processing in the ISR, moving code out to main() accomplishes this. Using a global flag is one way of communicating from the ISR but you should be careful of race conditions between the two. Depending on your application there are a number of ways to ensure this doesn't happen.

Initialising global variables in main() means that they should be reset if your processor resets (your architecture might have special reset vectors for this).

Note that the code is untested but something along these lines might help you out.

volatile int overflow_count;
volatile char overflow_flag;

int main(void)
{
  char time[20] = { 0 };

  // Initialise global variables in case of reset
  overflow_count = 0;
  overflow_flag = 0;
  // You could memset time to NULs here also

  while(1)
  {
    if (overflow_flag) {
      // Depending on the application you may need to
      // implement some kind of mutex here, or disable
      // interrupts until you have finished.  Resetting
      // the flag as the first instruction offers the 
      // least chance you'll miss interrupts.
      overflow_flag = 0;

      sprintf(time, "overflow-%u", overflow_count);
      uart_send(time);

      if(overflow_count==2)
      {
        overflow_count=0;
        uart_array("Alarm code execution");
      }
    }
  }
}

ISR(TIMER1_OVF_vect)
{
  overflow_count++;
  overflow_flag = 1;
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.