AVR C - system to prevent reading input state too often

[konradIC13]

Hello,

Im further improving my code for fan control. What im trying to implement is protection from reading input state of switches too often (something like debouncing but not really). Im controlling fans with 3 buttons, there are several combinations that turns on diffrent fans. For example setting switches 100 turns on FanA and FanB while 1 0 1 turns on FanB and FanC. If uC reads input from switches all the time when i want to set 1 0 1 to turn on FanB and FanC when i set first switch to 1 it turns FanA for a moment untill i set last switch to 1.

Thats why i want to make uC to read switch inputs every 1 second (around...).

Unfortunately my code is not working, uC is not reacting to any switch changes and i quite have no idea why, here is my code:

/*
 * PB1, PB2, PB3 outputs to LED segments
 * PD1, PD2, PD3 outputs to fan control
 * PC3, PC4, PC5 inputs from switches
 */ 

#define F_CPU 1000000L
#include <avr/io.h>
#include <avr/interrupt.h>

volatile uint8_t buttons;              //this will store button states after check every 1 second
volatile uint8_t cnt=0;                 //this will count amount of interrupts

ISR(TIMER0_0VF_vect)
{
	cnt++;                                   //variable that will count how many interrupts have happened
	if(cnt>3)                                //if 4 interrupts happened (around 1 s)
	{
		static uint8_t buttons_old = 0, buttons_new;
		
		buttons_new = (PINC & 0b00111000);        //reading buttons state from PC5,PC4,PC3 and writing to buttons_new (ignoring others)
		
		if(buttons_old == buttons_new)             //if buttons state is same as last time
		buttons = buttons_new;                    //update global buttons
		
		buttons_old = buttons_new;                 //and update buttons_old to remember new state of buttons
		cnt=0;
	}	
}

int main(void)
{
	char SW1,SW2,SW3;
	DDRB =0b00001110;		  //PB1, PB2, PB3 output (other inputs)
	PORTB=0b11110001;		  //PC1, PC2, PC3 initial condition low (other pulled up)
	DDRC =0b00000000;		  //PC3, PC4, PC5 input (other inputs)
	PORTC=0b11111111;		  //PC3, PC4, PC5 pulled up (other pulled up
	DDRD =0b00001110;		  //PD1, PD2, PD3 output (other input)
	PORTD=0b11110001;		  //PD1, PD2, PD3 initial condition low (other pulled up)
	
	TCCR0=0b00000101;         //prescaler 1024 (interrupt every ~250ms)
	TIMSK=0b00000001;         //enable interrupts
	sei();
	
    while(1)
    {
	SW1 = (buttons & 0b00001000);  //PC5 value after check every 1s from buttons to SW1
	SW2 = (buttons & 0b00010000);  //PC4 value after check every 1s from buttons to SW2
	SW3 = (buttons & 0b00100000);  //PC3 value after check every 1s from buttons to SW3

/* Red LED segment control
*/  
	if (((SW1&&SW3)||(SW1&&(!SW2)))!=0)
	{   
		PORTB |= (1<<PB1);    //set PB1 output as 1
	}
	else
	{
		PORTB &=~ (1<<PD1);   //set PB1 output as 0
	}
/* Green LED segment control
*/	
	if (((SW1&&(!SW3))||(SW1&&(!SW2)))!=0)
	{
		PORTB |= (1<<PB2);    //set PB2 output as 1
	}
	else
	{
		PORTB &=~ (1<<PB2);   //set PB2 output as 0
	}
/* Blue LED segment control
*/		
	if (((SW1&&SW2)||(SW1&&SW3))!=0)        //LED B
	{
		PORTB |= (1<<PB3);    //set PB3 output as 1
	}
	else
	{
		PORTB &=~ (1<<PB3);   //set PB3 output as 0
	}
/* FAN1 control
*/	
	if (((SW1&&SW2)||(SW1&&(!SW3)))!=0)
	{
		PORTD |= (1<<PD1);    //set PD1 output as 1
	}
	else
	{
		PORTD &=~ (1<<PD1);   //set PD1 output as 0
	}
/* FAN2 control
*/	
	if (((SW1&&SW2)||(SW1&&SW3))!=0)
	{
		PORTD |= (1<<PD2);    //set PD2 output as 1
	}
	else
	{
		PORTD &=~ (1<<PD2);   //set PD2 output as 0
	}
/* FAN3 control
*/	
	if ((SW1&&SW3)!=0)
	{
		PORTD |= (1<<PD3);    //set PD3 output as 1
	}
	else
	{
		PORTD &=~ (1<<PD3);   //set PD3 output as 0
	}

	}
}

Of course program compliles without any errors but still something must be not right here, dont now where. I think i explained everything in comments, what i expected from used functions to do.

 

[wkrug]

static uint8_t buttons_old = 0, buttons_new;
 
		buttons_new = (PINC & 0b00111000);        //reading buttons state from PC5,PC4,PC3 and writing to buttons_new (ignoring others)
 
		if(buttons_old == buttons_new)             //if buttons state is same as last time
		buttons = buttons_new;                    //update global buttons
 
		buttons_old = buttons_new;                 //and update buttons_old to remember new state of buttons

I think that couldn't work.
You define a local variable called buttons_old and advice the value of 0.
Then you geht the actual buttons and store them into buttons_new.
Now you give a if statement where checking the value of buttons_old ( =0! ) has the same value than buttons_new.
That will be only in one case, when all buttons are pressed at the same time.
The last buttons_old = buttons_new; will be deleted at the end of the interupt routine - It's only a local variable or when the static uint8_t buttons_old = 0 rises again.
I would suggest you to declare a volatile count variable into the interrupt routine for the button handling and put the button routine into the main loop.
The count variable counts down until it has the value 0.
In the main loop:
The count variable will be set to the wished value, when the OLDBUTTON in not the NEWBUTTON;
Then the Action will be done and the Button handling is blocked until the counter variable is 0 again.
That will give you a debouncing. 10 up to 20ms are enough.
Button reaction times that will be longer than 200ms are uncomfortable for the user.
When one Button is pressed a helper Variable will be set. In further programm this helper variable will be readed out and resetted when action is done.

A section of a Main programm part of one of my Applications.

uc_tast=read_tast();                        //Tasts will be read out
      if(uc_tast!=uc_oldtast&&uc_tasttimer==0)    //Any Tast changed? Debounce time ended?
        {
        uc_oldtast=uc_tast;
        if(uc_tast!=NONE){ub_change=1;}           //No tast is pressed
        uc_tasttimer=uc_tasttime;
        tasthandling(uc_tast);
        }
      if(ub_change>0)
        {
        make_settings();                          //Make settings appending from actual Parameters
        ub_change=0;
        refresh_display();
        ui_displaytimer=ui_displaytime;
        }

The according Timer Overflow Routine

// Timer 0 overflow interrupt service routine
interrupt [TIM0_OVF] void timer0_ovf_isr(void)
{
if(uc_tasttimer>0){uc_tasttimer--;};   //Countdown Tast Debouncing Timer
if(ui_displaytimer>0){ui_displaytimer--;}; //Countdown Display refresh Timer

//For Parameter blinking
if(ui_displaytimer==1)
    {
    if (ub_blink>0)
        {
        ub_blink=0;
        }
    else
        {
        ub_blink=1;
        }
    }
}

 

[konradIC13]

Hey, thank you for answer, i just looked at it but ill read it more closely later (its a bit late here).

Just a note, im not trying to debounce switches, i want to give user a time to set switches as he wants before the uC will read them and do anything.
So the program starts, user is switching switches, during that time uC is not changing anything on outputs since switches are being switched. Then user stops playing with swtches, uC is then checking after 1 second if switches are still in same state ( there were no switch changes during 1 second) and only after that time it reads their state and is taking appropriate action. So well.. it will be technically debouncing but with very long reaction time (1s) but i guess the time is just matter of playing with prescaler.