1
\$\begingroup\$

I have to count the number of times a button is pressed in 5 seconds and display it on an LED array.

If the number of times the button is clicked is 1 then the last LED should glow (0001 - binary value of 1). Similarly for 2 clicks, the third LED should glow (0010 - binary of 2).

But when I try this, I always get the output for the case 0, no matter how many times I click the button.

What is the problem?

#include<avr/io.h>
#include<avr/interrupt.h>
#include<util/delay.h>

int extratime=0;
int num_presses=0;


int main(void)
{
  DDRD = 0b11111111;
  PORTD = 0b00000000;
  DDRB = 0b11110000;
  PORTB = 0b00000000;
  char button_state;   


  TCCR1B = (1 << WGM12);
  OCR1A = 15625;
  TIMSK = (1 << OCIE1A);
  sei();
  TCCR1B |= (1 << CS12) | (1 << CS10);

  while(1)
  {
    if (bit_is_clear(PINB, 0)) //button is pressed
    {     
      if (button_state==0) //was previously not pressed)
      {
        num_presses++;
      }
      button_state=1;
    }
    else 
    {
      if (button_state==1) //was previously pressed
      {

      }
      button_state=0;
    } 


  }
}

ISR(TIMER1_COMPA_vect)
{
  extratime++;
  if(extratime == 5)
  {           
    switch(num_presses)
    {
             case 0: PORTD = 0b10010000;
                     _delay_ms(10000);
                     break;
             case 1: PORTD = 0b10000000;
                      _delay_ms(10000);
                      break;
              case 2: PORTD = 0b01000000;
                      _delay_ms(10000);
                      break;
              case 3: PORTD = 0b11000000;
                      _delay_ms(10000);
                      break;
              case 4: PORTD = 0b00100000;
                      _delay_ms(10000);
                      break;
              case 5: PORTD = 0b10100000;
                      _delay_ms(10000);
                      break;
             default: PORTD =0b11110000;
                     break;  
    } 
    extratime = 0;  
    num_presses = 0;
  }

}

this is my new code even this doesnt work.... #include #include #include

int extra_time=0;
int num_presses=0;

int main(void)
{
    DDRD = 0b11111111;
    PORTD = 0b10000000;
    DDRB = 0b00000111;
    PORTB = 0b00000000;
    char button_state;  

    TCCR1B = (1 << WGM12);
    OCR1A = 15625;
    TIMSK = (1 << OCIE1A);
    sei();
    TCCR1B |= (1 << CS12) | (1 << CS10);

    while(1)
    {
      if (bit_is_clear(PINB, 0)) //button is pressed
    {    
       if (button_state==0) //was previously not pressed)
     {
    num_presses++;
     }
     button_state=1;
    }
      else
     {
       if (button_state==1) //was previously pressed
     { 

     }
      button_state=0;
     }

     if (TCNT1 >= 15625)
    {
      if (extra_time == 5)
       {
         switch(num_presses)
       {
           case 0:
                 PORTD = 0b11100000; 
                 break;
           case 1:
                 PORTD = 0b00100000;
                 break;
           case 2:
                 PORTD = 0b01000000;
                 break;
           case 3:
                 PORTD = 0b01100000;
                 break;
           default:
                 PORTD = 0b10100000 ;
                 break;
          }
         extra_time = 0;
     }
     else
     {
         ++extra_time;
     }
     TCNT1 = 0;
     }
   }
  }

Pls help .. I have to submit this project tomorrow.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ There are so many problems in the code that I'm not sure where to start. I think the 1st thing that you would like to do (also to keep from being downvoted) is draw a flow chart of the process you want your machine to perform. Do this with no interrupts at all at first as your usage of the Timer interrupt here is not very good. After you sort out that part we can continue to work towards a solution. \$\endgroup\$ – user34920 Jun 14 '14 at 19:48
  • 1
    \$\begingroup\$ Some problems. The num_presses variable should be volatile. Delays shouldn't be inside a ISR. You should put them inside the main loop. While the ISR is running, the main loops is paused, even when you call delay inside the ISR. You cant write 15625 to OCR1A, as it's a 8 bit register, and can only go up to 255. \$\endgroup\$ – Gerben Jun 14 '14 at 21:06
1
\$\begingroup\$

If you always get the case for zero, then it seems like your microcontroller doesn't register when button is pressed. There can be many reasons and without actual circuit, hard to tell exactly what could be wrong. I suggest you double check your circuit and see if pressing a button actually changes voltage (replace MCU with single LED and when you press button, it should turn off)

And when it's working, there will be an issue with button bounce. If you're not debouncing it with hardware, then you can see in image below that there's a small period of time when values jump between logical 0 and 1, so you would need to either use internal timers to delay button registering or register a button press after certain amount of samples (there are a lot of software debounce libraries on the internet)enter image description here

I've also compiled a bit modified program for 1MHz mega32 with LCD, code can be seen here. Program working as intended, it registers a number of button clicks and prints them. Used a pull-up setup (just like your code suggests). Since there's no hardware debouncing, it sometimes registered a few more clicks than I've pressed. So for now, CHECK your circuit, then DOUBLE CHECK it.
enter image description here


UPDATED

Example code (compiled using atmel studio 6.2)
Used with below circuit. Keep in mind, that this setup has button bouncing and may register more presses than you actually pressed.enter image description here

If this setup does not work, then it's possible MCU is defective (but unlikely), so again - DOUBLE CHECK your circuit.
See if every connection is correct;
See if your wiring (diodes, switches etc) are correct;
Since you can't upload your circuit for some reason, if you're using external clock source, see if it's connected correctly and fuse bytes are set (then set F_CPU to your external clock frequency)
Avoid using large delay cycles, use for (;;) if you must.
And again, I've given example of circuit with which code works well enough (not counting lack of software debounce) and if there's something different in your setup, you MUST PROVIDE additional information if you want more help.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks I tried to check whether the switch was working as you suggested and it works.I also changes the button_presses variable as volatile int. But now my micro-controller behaves differently, the LEDs glow in random order. the output from the microcontroller changes constantly. \$\endgroup\$ – user3264385 Jun 15 '14 at 11:23
  • \$\begingroup\$ Please update your original post with new code and circuit. Also, you might want to move whole interrupt routine into while loop and instead of setting OCR1A, simply check TCNT1 value when it's >15625 with if. Example is here. If you still want to use interrupt, try volatile int extra_time = 0; as changing values in interrupt routines can cause various problems with compilers. \$\endgroup\$ – elent Jun 15 '14 at 12:17
  • \$\begingroup\$ The circuit should be very simple - MCU + switch as the pull-up resistors are internal. The output to the LEDs can and should be tested with a simple 3 lines of code. \$\endgroup\$ – user34920 Jun 15 '14 at 13:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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