Is it possible to use a button as both an I/O input and external interrupt?

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

I would like my system to awaken from a sleep mode by pressing any one of two available buttons on the front. These buttons also serve as inputs when the system is awake.

 

Would it be possible to configure these buttons to serve this dual purpose?

 

My initial reaction is to run the INTx pin and the I/O pin both through the button to ground (i.e. the button is configured for pull-up pins).

 

Which brings me to my next question: Are interrupt pins configured as pull-up or pull-down?

 

Thanks in advance.

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

This is very common in fact. You would often use INTn or PCINT from a button simply as the wakeup source from SLEEP but once the AVR awakes that interrupt source may be disabled (until next time, just before you sleep) and instead some kind of timer interrupt polling for debounce mechanism takes over to scan the button while awake.

 

As for pull-up/down that is up to you but note that for many AVRs, when using INTn (not PCINT) it can only be used to wake up for a "low level" so its  inactive stated would want to be high. If this is a "modern" AVR (like the last decade) it should also offer PCINTs. These are a "better" wake up source and will activate on any transition (so it could be high-low or low-high).

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

Connect your button from the port pin to gnd, use either the internal pullup or use an external pull up resistor(10k) to vcc. 

Then,  your logic will be if PIN is LOW, button is active, if HIGH then button is in-active.

 

Jim

 

 

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

I am using an ATMEGA8 MCU. I connected my circuit as shown in the attachment. [cliff: to save time, here it is...]

 

http://www.avrfreaks.net/sites/default/files/forum_attachments/avr1_0.JPG

 

See the code below for the input configuration. Note that all inputs have pull-up resistors enabled:

DDRB &= ~(1<<PB1) | ~(1<<PB2);
PORTB |= (1<<PB1) | (1<<PB2);

DDRD &= ~(1<<PD2)|~(1<<PD3); //int0 and int1 to input
PORTD |= (1<<PD2)|(1<<PD3); //int0 and int1 with pull-up resistor

These are my INT0/INT1 ISR routines. The idea is to disable external interrupts and allow normal interaction between the buttons and the PB inputs when the MCU has awoken:

ISR(INT0_vect)
{
	GICR &= ~(1<<INT0)|~(1<<INT1);
}

ISR(INT1_vect)
{
	GICR &= ~(1<<INT0)|~(1<<INT1);
}

This is my sleep routine which enables the external interrupts:

	if(sleep_timer == 500)
	{
		cli(); //clear interrupt
		GICR |= (1<<INT0)|(1<<INT1);//enable external interrupts int0 and int1
		set_sleep_mode(SLEEP_MODE_PWR_DOWN); //set sleep mode to 'power down'
		lcd_command(_BV(LCD_DISPLAYMODE)); //turn off LCD display
		sleep_enable(); //enable the sleep bit
		sei(); //set interrupts to on
		sleep_cpu(); //sleep microcontroller

		//wake up and do all of these things
		sleep_disable(); //turn microcontroller back on
		lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON)); //turn screen back on
		sleep_timer = 0; //reset sleep counter 

My expectation was to press either button 1 or button 2 to take the MCU out of sleep mode and continue its increment/decrement routine.

 

What actually happened was that either button would take the MCU out of sleep mode, but neither would increment/decrement until the int0/int1 connection was disconnected.

 

I suspect that there may be some unwanted play between the external interrupt inputs and the button inputs, but I'm not 100% sure.

 

Please let me know if any additional information in needed to help.

 

Thanks

 

 

Attachment(s): 

Last Edited: Thu. Oct 5, 2017 - 08:38 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

justin2021 wrote:
but neither would increment/decrement
But you haven't shown this increment/decrement code?

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

I don't think you need to connect the button to two pins. With a pin change interrupt, the one pin can perform both functions, interrupt and input. That's the whole point of the pin change int, surely.

 

Quebracho seems to be the hardest wood.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
DDRB &= ~(1<<PB1) | ~(1<<PB2);
(and other similar lines)

That is basically a NOP.

 

Stefan Ernst

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

clawson wrote:

justin2021 wrote:
but neither would increment/decrement
But you haven't shown this increment/decrement code?

 

Here is the whole program. The increment/decrement code is found in the for(;;) loop:

 

#define F_CPU 8000000UL


#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/atomic.h>
#include <util/delay.h>
#include <stdio.h>
#include <avr/power.h>
#include <avr/sleep.h>
#include <hd44780.h>

uint8_t key_state; //debounced key state
uint8_t key_press; //press detect

int LCDValueInt = 0;
char LCDValueString[9];
int sleep_timer = 0;

ISR(TIMER1_COMPA_vect) ////// debounce interrupt code
{
	static uint8_t ct0 = 0xFF, ct1 = 0xFF;	// 8 * 2bit counters
	uint8_t i;
	i = ~PINB;				// read keys (low active)
	i ^= key_state;			// key changed ?
	ct0 = ~( ct0 & i );			// reset or count ct0
	ct1 = ct0 ^ (ct1 & i);		// reset or count ct1
	i &= ct0 & ct1;			// count until roll over ?
	key_state ^= i;			// then toggle debounced state
	key_press |= key_state & i;
	
	
	sleep_timer++;
	if(sleep_timer == 500)
	{
		cli(); //clear interrupt
		GICR |= (1<<INT0)|(1<<INT1);//enable external interrupts int0 and int1
		set_sleep_mode(SLEEP_MODE_PWR_DOWN); //set sleep mode to 'power down'
		lcd_command(_BV(LCD_DISPLAYMODE)); //turn off LCD display
		sleep_enable(); //enable the sleep bit
		sei(); //set interrupts to on
		sleep_cpu(); //sleep microcontroller

		//wake up and do all of these things
		sleep_disable(); //turn microcontroller back on
		lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON)); //turn screen back on
		sleep_timer = 0; //reset sleep counter 
	}
	
}

ISR(INT0_vect)
{
	GICR &= ~(1<<INT0)|~(1<<INT1);
}

ISR(INT1_vect)
{
	GICR &= ~(1<<INT0)|~(1<<INT1);
}

uint8_t get_key_press( uint8_t key_mask )
{
	ATOMIC_BLOCK(ATOMIC_FORCEON) // read and clear atomic !
	{		
		key_mask &= key_press;		// read key(s)
		key_press ^= key_mask;		// clear key(s)
	}
	return key_mask;
}

int main(void)
{

DDRC |= (1<<PC1)|(1<<PC2)|(1<<PC3)|(1<<PC4)|(1<<PC5);
DDRD |= (1<<PD0)|(1<<PD1)|(1<<PD4)|(1<<PD5)|(1<<PD6)|(1<<PD7);

DDRD &= ~(1<<PD2)|~(1<<PD3); //int0 and int1 to input
PORTD |= (1<<PD2)|(1<<PD3); //int0 and int1 with pull-up resistor


MCUCR &= ~(1<<ISC00)|~(1<<ISC01)|~(1<<ISC10)|~(1<<ISC11);//low level of int0 and int1 generates an interrupt request


///Initiate LCD
lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON));
lcd_init();
lcd_putc('0');

///Initiate Debouncer
DDRB &= ~(1<<PB1) | ~(1<<PB2);
PORTB |= (1<<PB1) | (1<<PB2);
key_state = ~PINB;

//initiate timer interrupt
TCNT1 = 0;
TCCR1B = (1<<CS10) | (1<<CS12) | (1<<WGM12); //sets freq to be divided by 1024, clears timer on compare match
OCR1A = 156; // 20ms
TIMSK |= (1 << OCIE1A);//enable output compare
sei(); //enable interrupts

for(;;)
{
	uint8_t button_state = get_key_press((1<<PB1)|(1<<PB2));
	
	if(button_state == (PINB & ((1<<PB2|1<<PB1))))
	{
		LCDValueInt = 0;
		lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON));
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		sleep_timer=0;
	}
	
	else if (button_state & (1<<PB1))
	{
		++LCDValueInt;
		lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON));
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		sleep_timer=0;
	}
	
	else if (button_state & (1<<PB2))
	{
		--LCDValueInt;
		lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON));
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		sleep_timer=0;
	}
}

}

 

John_A_Brown wrote:
I don't think you need to connect the button to two pins. With a pin change interrupt, the one pin can perform both functions, interrupt and input. That's the whole point of the pin change int, surely.

 

the PCINT interrupt vector isn't available in the ATMega8, unfortunately.

 

sternst wrote:
That is basically a NOP.

 

I'm not sure what you mean.

 

Thanks for the replies.

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

justin2021 wrote:

sternst wrote:
That is basically a NOP.

 

I'm not sure what you mean.

The right side is 0xFFFF and &= 0xFFFF does nothing.

Stefan Ernst

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

"NOP" is "No OPeration" - ie, does nothing.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
    uint8_t button_state = get_key_press((1<<PB1)|(1<<PB2));
    
    if(button_state == (PINB & ((1<<PB2|1<<PB1))))
    {
        LCDValueInt = 0;
        ...
    }
    
    else if (button_state & (1<<PB1))
    {
        ++LCDValueInt;
        ...
    }
    
    else if (button_state & (1<<PB2))
    {
        --LCDValueInt;
        ...
    }

You won't see any increment/decrement, because as soon as no button is pressed the counter is set to 0.

No matter what the actual intention of the first 'if' is (both buttons pressed?), the PINB in there looks very wrong.

Stefan Ernst

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

Just a passing comment but when you see the same code duplicated multiple times:

	if(button_state == (PINB & ((1<<PB2|1<<PB1))))
	{
		LCDValueInt = 0;
		lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON));
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		sleep_timer=0;
	}
	
	else if (button_state & (1<<PB1))
	{
		++LCDValueInt;
		lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON));
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		sleep_timer=0;
	}
	
	else if (button_state & (1<<PB2))
	{
		--LCDValueInt;
		lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON));
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		sleep_timer=0;
	}

It's sort of crying out for:

void displayLCD(int LCDValueInt) {
	lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON));
	lcd_clrscr();
	sprintf(LCDValueString, "%d", LCDValueInt);
	lcd_puts(LCDValueString);
	sleep_timer=0;

}
...

	if(button_state == (PINB & ((1<<PB2|1<<PB1))))
	{
		LCDValueInt = 0;
		displayLCD(LCDValueInt);
	}
	
	else if (button_state & (1<<PB1))
	{
		++LCDValueInt;
		displayLCD(LCDValueInt);
	}
	
	else if (button_state & (1<<PB2))
	{
		--LCDValueInt;
		displayLCD(LCDValueInt);
	}

In fact even that is unnecessary:

    if(button_state == (PINB & ((1<<PB2|1<<PB1))))
    {
        LCDValueInt = 0;
    }
    
    else if (button_state & (1<<PB1))
    {
        ++LCDValueInt;
    }
    
    else if (button_state & (1<<PB2))
    {
        --LCDValueInt;
    }
    displayLCD(LCDValueInt);   

Doing this the logic of what you are doing inside each conditional clause is far easier to see. And the repeated code is not repeated so there's just one copy to fix/amend not three.

 

But as Stefan says the first condition here is almost certainly wrong. In fact why would you ever set it back to 0 anyway? Surely you start it at 0 outside the update loop, then if the user presses "increment" you ++ or decrement you -- but otherwise you just leave it the same.

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

"John_A_Brown wrote:

I don't think you need to connect the button to two pins. With a pin change interrupt, the one pin can perform both functions, interrupt and input. That's the whole point of the pin change int, surely.

 

 

the PCINT interrupt vector isn't available in the ATMega8, unfortunately."

 

OK, I should have checked that.

However, if your original description is correct, can you not configure the pin for an interrupt before sleeping, and then configure as an input on waking?

 

Quebracho seems to be the hardest wood.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
DDRB &= ~(1<<PB1) | ~(1<<PB2);

is equivalent to:

DDRB &= (0b11111101) | (0b11111011);

which is:

DDRB &= 0b11111111;

 

I think you want:

DDRB &= ~(1<<PB1) & ~(1<<PB2);

 

David (aka frog_jr)

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

Or most people would write:

DDRB &= ~((1<<PB1) | (1<<PB2));

OR the bits together then NOT to invert as a mask for the AND.

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

clawson wrote:
OR the bits together then NOT to invert as a mask for the AND.
Obviously, I need to have another cup of coffee!

I originally was going to write that, and then in showing what was wrong with the original code, that thought must have gotten lost...

(even though both do work, clawson's version is much (IMHO) cleaner.)

David (aka frog_jr)

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

The first 'if' statement allows the counter to return back to 0 when both buttons are pressed. The increment decrement functions worked fine previously, but once I introduced the external interrupts, that's when I started having issues.

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

justin2021 wrote:
once I introduced the external interrupts, that's when I started having issues.

because 'volatile' then became necessary ... ?

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

More specifically:
volatile uint8_t key_press;

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

Kartman wrote:
More specifically: volatile uint8_t key_press;
Well you say that and I posted a comment to that effect earlier today but then I later came back and deleted my post. The reason? I went and took a look at the original "Danni code":

 

https://community.atmel.com/proj...

 

Loads of folks have used that code a lot of times. The code has (in getkey.c):

u8 key_state;				// debounced and inverted key state:
					// bit = 1: key pressed
u8 key_press;				// key press detect

ISR( TIMER0_COMPA_vect )		// every 10ms
{
    .....
  key_press |= key_state & i;		// 0->1: key press detect
}


u8 get_key_press( u8 key_mask )
{
  ATOMIC_BLOCK(ATOMIC_FORCEON){		// read and clear atomic !
    key_mask &= key_press;		// read key(s)
    key_press ^= key_mask;		// clear key(s)
  }
  return key_mask;
}

That apparently works and yet key_press was never volatile.

 

I think this is simply because the optimiser is not seeing a way to cache it and optimize away accesses.

 

Technically it is "safer"to include the volatile but in this case I don't think it will make any difference (otherwise tons of other "Danni users" would have been bitten by this.

 

So, no I don't think lack of volatile in this case is the reason for non functioning.

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

Thank you all for the input.

 

So I cleaned up the code (see below) and now either of my push buttons can wake the MCU as well as increment/decrement using the aforementioned schematic.

 

#define F_CPU 8000000UL


#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/atomic.h>
#include <util/delay.h>
#include <stdio.h>
#include <avr/power.h>
#include <avr/sleep.h>
#include <hd44780.h>

uint8_t key_state; //debounced key state
uint8_t key_press; //press detect

volatile int LCDValueInt = 0;
char LCDValueString[9];
volatile int sleep_timer = 0;

ISR(TIMER1_COMPA_vect) ////// debounce interrupt code
{
    static uint8_t ct0 = 0xFF, ct1 = 0xFF;	// 8 * 2bit counters
    uint8_t i;
    i = ~PINB;				// read keys (low active)
    i ^= key_state;			// key changed ?
    ct0 = ~( ct0 & i );			// reset or count ct0
    ct1 = ct0 ^ (ct1 & i);		// reset or count ct1
    i &= ct0 & ct1;			// count until roll over ?
    key_state ^= i;			// then toggle debounced state
    key_press |= key_state & i;
    
    
    sleep_timer++;
    if(sleep_timer == 500)
    {
        cli();
        GICR |= (1<<INT0)|(1<<INT1);//enable external interrupts int0 and int1
        set_sleep_mode(SLEEP_MODE_PWR_DOWN); //set sleep mode to 'power down'
        lcd_command(_BV(LCD_DISPLAYMODE)); //turn off LCD display
        sleep_enable(); //enable the sleep bit
        sei(); //set interrupts to on
        sleep_cpu(); //sleep microcontroller

        //wake up and do all of these things
        sleep_disable(); //turn microcontroller back on
        lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON)); //turn screen back on
        sleep_timer = 0; //reset sleep counter 
    }
    
}

ISR(INT0_vect)
{
    GICR &= ~((1<<INT0)|(1<<INT1));
}

ISR(INT1_vect)
{
    GICR &= ~((1<<INT0)|(1<<INT1));
}

uint8_t get_key_press( uint8_t key_mask )
{
    ATOMIC_BLOCK(ATOMIC_FORCEON) // read and clear atomic !
    {		
        key_mask &= key_press;		// read key(s)
        key_press ^= key_mask;		// clear key(s)
    }
    return key_mask;
}

void displayLCD(int LCDValueInt)
{
    lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON));
    lcd_clrscr();
    sprintf(LCDValueString, "%d", LCDValueInt);
    lcd_puts(LCDValueString);
    sleep_timer=0;
}

int main(void)
{

DDRC |= (1<<PC1)|(1<<PC2)|(1<<PC3)|(1<<PC4)|(1<<PC5);
DDRD |= (1<<PD0)|(1<<PD1)|(1<<PD4)|(1<<PD5)|(1<<PD6)|(1<<PD7);

DDRD &= ~((1<<PD2)|(1<<PD3)); //int0 and int1 to input
PORTD |= (1<<PD2)|(1<<PD3); //int0 and int1 with pull-up resistor


MCUCR &= ~((1<<ISC00)|(1<<ISC01)|(1<<ISC10)|(1<<ISC11));//low level of int0 and int1 generates an interrupt request


///Initiate LCD
lcd_init();
displayLCD(LCDValueInt);

///Initiate Debouncer
DDRB &= ~((1<<PB1)|(1<<PB2));
PORTB |= (1<<PB1) | (1<<PB2);
key_state = ~PINB;

///initiate timer interrupt
TCNT1 = 0;
TCCR1B = (1<<CS10) | (1<<CS12) | (1<<WGM12); //sets freq to be divided by 1024, clears timer on compare match
OCR1A = 156; // 20ms (156)
TIMSK |= (1 << OCIE1A);//enable output compare
sei(); //enable interrupts

for(;;)
{
    uint8_t button_state = get_key_press((1<<PB1)|(1<<PB2));
    
    if(button_state == (PINB & ((1<<PB2|1<<PB1))))
    {
        LCDValueInt = 0;
        displayLCD(LCDValueInt);
    }
    
    else if (button_state & (1<<PB1))
    {
        ++LCDValueInt;
        displayLCD(LCDValueInt);
    }
    
    else if (button_state & (1<<PB2))
    {
        --LCDValueInt;
        displayLCD(LCDValueInt);
    }
}

}

 

But now a new issue has come up - When the external interrupt pin and input pin are both connected to a button, the increment/decrement tends to become inconsistent. Either button will randomly restart the counter to 0 at seemingly random intervals. When the external interrupts are disconnected from their respective buttons, the counter acts completely normal without any hangups. 

 

In the code, there are only two places that set the counter to 0 - when the MCU first starts...

//initialize LCDValueInt
volatile int LCDValueInt = 0;

...

///function that displays the current value of LCDValueInt
void displayLCD(int LCDValueInt)
{
    lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON));
    lcd_clrscr();
    sprintf(LCDValueString, "%d", LCDValueInt);
    lcd_puts(LCDValueString);
    sleep_timer=0;
}

...

///Initiate the LCD, which is 0 the first time the MCU is started
lcd_init();
displayLCD(LCDValueInt);

and when both button 1 and button 2 are pressed:

	if(button_state == (PINB & ((1<<PB2|1<<PB1))))
	{
		LCDValueInt = 0;
		displayLCD(LCDValueInt);
	}

To test which instance is occurring, I set both zeroes to unique numbers (i.e. 123 and 321). Both instances seem to be occurring at equal frequencies. And what makes matters stranger, when both of these instances are set to numbers other than zero, I will still occasionally see the LCD display zero!

 

Additionally, there is the rare occasion that the LCD will freeze and won't respond to button presses. This is pretty rare and has only happened about twice within the last hour of straight testing.

 

Although this might very well be a software issue, I'm beginning to wonder if there is a hardware issue at hand as well. I've attached a simplified schematic of what two pull-up inputs might look like when tied to one grounding source (i.e. the button). Is this assumption correct? And if so, would this affect the MCU behavior?

 

Any advice is appreciated.

 

Thanks.

Attachment(s): 

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

A brief update -

 

I've made some slight modifications to my source code and hardware

  • I added bypass capacitors to the VCC and AVCC pins
  • I grounded any floating i/o pins
  • I changed LCDValueInt from a global variable into a local variable in Main()

 

Connecting the external interrupt inputs and inc/dec inputs to the same button still reinitializes the LCDValueInt back to zero fairly frequently, but removing the external interrupt inputs reduces the reinitialization to almost nothing (maybe 1 reinitialization every 150+ presses of either the inc or dec button). Additionally, I can connect the external interrupts directly to ground and also get a very low reinitialization rate.

 

I've attempted to isolate the external interrupt and input signals from each other using diodes (see the image attached), but it didn't seem to change anything, so I'm almost certain that it's a software issue.

Below is the revised version of my code.

 

I'm aware that some of the code isn't optimized for readability and reduced redundancy, but I don't think that it would affect the seemingly random behavior that the MCU is exhibiting (though I could be wrong).

 

#define F_CPU 8000000UL
#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/atomic.h>
#include <util/delay.h>
#include <stdio.h>
#include <avr/power.h>
#include <avr/sleep.h>
#include <hd44780.h>

volatile uint8_t key_state; //debounced key state
volatile uint8_t key_press; //press detect
volatile int sleep_timer = 0;

ISR(TIMER1_COMPA_vect) ////// debounce interrupt code
{
	static uint8_t ct0 = 0xFF, ct1 = 0xFF;	// 8 * 2bit counters
	volatile uint8_t i;
	i = key_state ^ ~PINB;				// read keys (low active) and check if key changed
	ct0 = ~( ct0 & i );			// reset or count ct0
	ct1 = ct0 ^ (ct1 & i);		// reset or count ct1
	i &= ct0 & ct1;			// count until roll over ?
	key_state ^= i;			// then toggle debounced state
	key_press |= key_state & i;

	sleep_timer++;
	if(sleep_timer == 500)
	{
		cli();
		GICR |= (1<<INT0)|(1<<INT1);//enable external interrupts int0 and int1
		set_sleep_mode(SLEEP_MODE_PWR_DOWN); //set sleep mode to 'power down'
		lcd_command(_BV(LCD_DISPLAYMODE)); //turn off LCD display
		sleep_enable(); //enable the sleep bit
		sei(); //set interrupts to on
		sleep_cpu(); //sleep microcontroller

		//wake up and do all of these things
		sleep_disable(); //turn microcontroller back on
		lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON)); //turn screen back on
		sleep_timer = 0; //reset sleep counter
	}

}

ISR(INT0_vect)
{
	GICR &= ~((1<<INT0)|(1<<INT1));
	sleep_timer = 0;
}

ISR(INT1_vect)
{
	GICR &= ~((1<<INT0)|(1<<INT1));
	sleep_timer = 0;
}

uint8_t get_key_press( uint8_t key_mask )
{
	ATOMIC_BLOCK(ATOMIC_FORCEON) // read and clear atomic !
	{
		key_mask &= key_press;		// read key(s)
		key_press ^= key_mask;		// clear key(s)
	}
	return key_mask;
}

int main(void)
{

int LCDValueInt = 0;
char LCDValueString[9];

DDRC |= (1<<PC1)|(1<<PC2)|(1<<PC3)|(1<<PC4)|(1<<PC5);
DDRD |= (1<<PD0)|(1<<PD1)|(1<<PD4)|(1<<PD5)|(1<<PD6)|(1<<PD7);

//Initiate Debouncer
DDRB &= ~((1<<PB1)|(1<<PB2));
PORTB |= (1<<PB1) | (1<<PB2);
key_state = ~PINB;

DDRD &= ~((1<<PD2)|(1<<PD3)); //int0 and int1 to input
PORTD |= (1<<PD2)|(1<<PD3); //int0 and int1 with pull-up resistor

MCUCR &= ~((1<<ISC00)|(1<<ISC01)|(1<<ISC10)|(1<<ISC11));//low level of int0 and int1 generates an interrupt request

//Initiate LCD
lcd_init();
lcd_clrscr();
lcd_putc('0');

//initiate timer interrupt
TCNT1 = 0;
TCCR1B = (1<<CS10) | (1<<CS12) | (1<<WGM12); //sets freq to be divided by 1024, clears timer on compare match
OCR1A = 156; // 20ms (156)
TIMSK |= (1 << OCIE1A);//enable output compare
sei(); //enable interrupts

for(;;)
{
	volatile uint8_t button_state = get_key_press((1<<PB1)|(1<<PB2));

	if(button_state == (PINB & ((1<<PB2|1<<PB1))))
	{
		LCDValueInt = 999;
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		sleep_timer = 0;
	}

	else if (button_state & (1<<PB1))
	{
		++LCDValueInt;
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		sleep_timer = 0;
	}

	else if (button_state & (1<<PB2))
	{
		--LCDValueInt;
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		sleep_timer = 0;
	}
}

}

 

So I feel that I'm getting closer to solving the issue, but reinitialization of the counter makes the MCU function *almost* unusable.

 

Any ideas on what might cause a reset of a local variable? Specifically how two interacting inputs might cause a local variable to reset?

 

Thanks

 

 

Attachment(s): 

Last Edited: Sun. Oct 8, 2017 - 10:18 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Normally one wouldn’t initiate a sleep in an interrupt context. You’re doing some funky stuff to make it work that might have some unintended consequences. I’d suggest moving the sleep logic out of the isr and into your main loop.

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

Kartman wrote:
Normally one wouldn’t initiate a sleep in an interrupt context. You’re doing some funky stuff to make it work that might have some unintended consequences. I’d suggest moving the sleep logic out of the isr and into your main loop.

 

I moved the sleep routine, but I'm still getting a reinitialized LCDValueInt when external interrupt and the inc/dec inputs are connected to the same button.

 

One theoretical question I'm asking myself at this point - what in a program would cause a value to reinitialize and restart main()? It's almost like by connection the two inputs together, pressing the button eventually causes some memory overflow that possibly restarts all MCU SRAM.

Any thoughts?

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

What happens if the cpu restarts? Values return to 0? Most likely.
Look for a cpu restart - flash a led or something. Rule out the obvious first.

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

I appreciate the feedback.

 

So I utilized the MCUSR register to see if there might be a clue on what's resetting the CPU. Based on what caused a restart, the LCD will print a message to the screen:

 

#define F_CPU 8000000UL
#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/atomic.h>
#include <util/delay.h>
#include <stdio.h>
#include <avr/power.h>
#include <avr/sleep.h>
#include <hd44780.h>

volatile uint8_t key_state; //debounced key state
volatile uint8_t key_press; //press detect
volatile int sleep_timer = 0;

ISR(TIMER1_COMPA_vect) ////// debounce interrupt code
{
	static uint8_t ct0 = 0xFF, ct1 = 0xFF; // 8 * 2bit counters
	volatile uint8_t i = key_state ^ ~PINB;	// read keys (low active) and check if key changed
	ct0 = ~( ct0 & i );	// reset or count ct0
	ct1 = ct0 ^ (ct1 & i); // reset or count ct1
	i &= ct0 & ct1;	// count until roll over ?
	key_state ^= i;	// then toggle debounced state
	key_press |= key_state & i;
	
	sleep_timer++;
}

ISR(INT0_vect)
{
	GICR &= ~((1<<INT0)|(1<<INT1));
	DDRD |= (1<<PD2)|(1<<PD3); //int0 and int1 to output
	PORTD |= (1<<PD2)|(1<<PD3); //int0 and int1 goes high
	sleep_timer = 0;
}

ISR(INT1_vect)
{
	GICR &= ~((1<<INT0)|(1<<INT1));
	DDRD |= (1<<PD2)|(1<<PD3); //int0 and int1 to output
	PORTD |= (1<<PD2)|(1<<PD3); //int0 and int1 goes high
	sleep_timer = 0;
}

uint8_t get_key_press( uint8_t key_mask )
{
	ATOMIC_BLOCK(ATOMIC_FORCEON) // read and clear atomic !
	{		
		key_mask &= key_press;		// read key(s)
		key_press ^= key_mask;		// clear key(s)
	}
	return key_mask;
}

int main(void)
{

uint8_t mcusr_copy = MCUSR;
MCUSR = 0;

lcd_init();
lcd_clrscr();
lcd_putc('a');

if (mcusr_copy & (1<<WDRF))
{
	lcd_clrscr();
	lcd_puts("WDRF");
	sleep_timer = 0;
}

else if (mcusr_copy & (1<<BORF))
{
	lcd_clrscr();
	lcd_puts("BORF");
	sleep_timer = 0;
}

else if (mcusr_copy & (1<<EXTRF))
{
	lcd_clrscr();
	lcd_puts("EXTRF");
	sleep_timer = 0;
}

else if (mcusr_copy & (1<<PORF))
{
	lcd_clrscr();
	lcd_puts("PORF");
	sleep_timer = 0;
}

else if (mcusr_copy == 0)
{
	lcd_clrscr();
	lcd_puts("NO RESET");
	sleep_timer = 0;
}

int LCDValueInt = 0;
char LCDValueString[9];

DDRC |= (1<<PC1)|(1<<PC2)|(1<<PC3)|(1<<PC4)|(1<<PC5);
DDRD |= (1<<PD0)|(1<<PD1)|(1<<PD4)|(1<<PD5)|(1<<PD6)|(1<<PD7);

//Initiate Debouncer
DDRB &= ~((1<<PB1)|(1<<PB2));
PORTB |= (1<<PB1) | (1<<PB2);
key_state = ~PINB;

DDRD |= (1<<PD2)|(1<<PD3); //int0 and int1 to output
PORTD |= (1<<PD2)|(1<<PD3); //int0 and int1 goes high

MCUCR &= ~((1<<ISC00)|(1<<ISC01)|(1<<ISC10)|(1<<ISC11));//low level of int0 and int1 generates an interrupt request

//initiate timer interrupt
TCNT1 = 0;
TCCR1B = (1<<CS10) | (1<<CS12) | (1<<WGM12); //sets freq to be divided by 1024, clears timer on compare match
OCR1A = 156; // 20ms (156)
TIMSK |= (1 << OCIE1A);//enable output compare
sei(); //enable interrupts

for(;;)
{
	volatile uint8_t button_state = get_key_press((1<<PB1)|(1<<PB2));
	
	if(button_state == (PINB & ((1<<PB2|1<<PB1))))
	{
		LCDValueInt = 999;
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		sleep_timer = 0;
	}
	
	else if (button_state & (1<<PB1))
	{
		++LCDValueInt;
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		sleep_timer = 0;
	}
	
	else if (button_state & (1<<PB2))
	{
		--LCDValueInt;
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		sleep_timer = 0;	
	}
	
	if(sleep_timer == 500)
	{
		cli();
		GICR |= (1<<INT0)|(1<<INT1);//enable external interrupts int0 and int1
		DDRD &= ~((1<<PD2)|(1<<PD3)); //int0 and int1 to input
		PORTD |= (1<<PD2)|(1<<PD3); //int0 and int1 with pull-up resistor
		set_sleep_mode(SLEEP_MODE_PWR_DOWN); //set sleep mode to 'power down'
		lcd_command(_BV(LCD_DISPLAYMODE)); //turn off LCD display
		sleep_enable(); //enable the sleep bit
		sei(); //set interrupts to on
		sleep_cpu(); //sleep microcontroller

		//wake up and do all of these things
		sleep_disable(); //turn microcontroller back on
		lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON)); //turn screen back on
		DDRD |= (1<<PD2)|(1<<PD3); //int0 and int1 to output
		PORTD |= (1<<PD2)|(1<<PD3); //int0 and int1 goes high
		sleep_timer = 0; //reset sleep counter
	}
}

}

 

So far, all of my messages have been "NO RESET" when the system goes back to zero (or in my case for testing purposes, 'a'), which sounds to me like it wasn't reset by any of the recognized means.

 

You might also notice that I attempted a new method of configuring my external interrupt pins - they are set to high outputs until right before the systems goes to sleep, where they are then turned into inputs with a pull-up resistor. Once the corresponding button is pressed and the systems wakes up, they revert to high outputs.

 

Hardware-wise, I added a diode and resistor in front of the normally high outputs (i.e. the external interrupt pins) right before the inc/dec button. My understanding is that without the resistor, the 5V output would short directly into the inc/dec input and down to ground. Please let me know if this assumption is incorrect.

 

Either way, these fixes do not correct the issue. With the external interrupts and inc/dec inputs connected to the same button, I am still somehow returning back to zero on my counter after random intervals. The only way this doesn't occur is if I disconnect the external interrupts from the inc/dec buttons.

 

Any thoughts?

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

A thought...

 

You are using level triggered external interrupts. When an interrupts occurs the uC vectors to the ISR and executes it. In doing so it clears INTFx in the GIFR. In there you disable the interrupts by writing to GICR. However, I wonder if the INTFx bits are being set as soon as they are cleared because your low level will almost certainly last longer than the ISR.

 

After disabling the interrupts, try clearing INTFx by writing to GIFR.

'This forum helps those who help themselves.'

 

pragmatic  adjective dealing with things sensibly and realistically in a way that is based on practical rather than theoretical consideration.

Last Edited: Thu. Oct 12, 2017 - 06:09 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A final update (hopefully) - 

 

I think I found a fix that eliminates all of my problems. During the initialization state, I set the external interrupt pins to low outputs on the right side of the button, and I set the inc/dec pins as inputs with a pull-up resistor on the left side of the button. When the system is about to go into sleep mode, I set the external interrupt pins to inputs with a pull-up resistor, and the inc/dec pins as low outputs. Then when it wakes up, it reverts the pin configurations back to the initialization state.

 

The code for archival purposes:

 

#define F_CPU 8000000UL
#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/atomic.h>
#include <util/delay.h>
#include <stdio.h>
#include <avr/power.h>
#include <avr/sleep.h>
#include <hd44780.h>

volatile uint8_t key_state; //debounced key state
volatile uint8_t key_press; //press detect
volatile int sleep_timer = 0;

ISR(TIMER1_COMPA_vect) ////// debounce interrupt code
{
	static uint8_t ct0 = 0xFF, ct1 = 0xFF; // 8 * 2bit counters
	volatile uint8_t i = key_state ^ ~PINB;	// read keys (low active) and check if key changed
	ct0 = ~( ct0 & i );	// reset or count ct0
	ct1 = ct0 ^ (ct1 & i); // reset or count ct1
	i &= ct0 & ct1;	// count until roll over ?
	key_state ^= i;	// then toggle debounced state
	key_press |= key_state & i;
	
	sleep_timer++;
}

ISR(INT0_vect)
{
	GICR &= ~((1<<INT0)|(1<<INT1));
	GIFR |= (1<<INTF0);
	sleep_timer = 0;
}

ISR(INT1_vect)
{
	GICR &= ~((1<<INT0)|(1<<INT1));
	GIFR |= (1<<INTF1);
	sleep_timer = 0;
}

uint8_t get_key_press( uint8_t key_mask )
{
	ATOMIC_BLOCK(ATOMIC_FORCEON) // read and clear atomic !
	{		
		key_mask &= key_press;		// read key(s)
		key_press ^= key_mask;		// clear key(s)
	}
	return key_mask;
}

int main(void)
{

lcd_init();
lcd_clrscr();
lcd_putc('0');

int LCDValueInt = 0;
char LCDValueString[9];

DDRC |= (1<<PC1)|(1<<PC2)|(1<<PC3)|(1<<PC4)|(1<<PC5);
DDRD |= (1<<PD0)|(1<<PD1)|(1<<PD4)|(1<<PD5)|(1<<PD6)|(1<<PD7);

//Initiate Debouncer
DDRB &= ~((1<<PB1)|(1<<PB2)); //pin b1 and b2 as input
PORTB |= (1<<PB1) | (1<<PB2); //pin b1 and b2 pull-up resistor enabled
key_state = ~PINB;

DDRD |= (1<<PD2)|(1<<PD3); //int0 and int1 to output
PORTD &= ~((1<<PD2)|(1<<PD3)); //int0 and int1 goes low

MCUCR &= ~((1<<ISC00)|(1<<ISC01)|(1<<ISC10)|(1<<ISC11));//low level of int0 and int1 generates an interrupt request

//initiate timer interrupt
TCNT1 = 0;
TCCR1B = (1<<CS10) | (1<<CS12) | (1<<WGM12); //sets freq to be divided by 1024, clears timer on compare match
OCR1A = 156; // 20ms (156)
TIMSK |= (1 << OCIE1A);//enable output compare
sei(); //enable interrupts

for(;;)
{
	volatile uint8_t button_state = get_key_press((1<<PB1)|(1<<PB2));
	
	if(button_state == (PINB & ((1<<PB2|1<<PB1))))
	{
		LCDValueInt = 0;
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		sleep_timer = 0;
	}
	
	else if (button_state & (1<<PB1))
	{
		++LCDValueInt;
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		//key_state = 0; //holding down increases count rapidly
		sleep_timer = 0;
	}
	
	else if (button_state & (1<<PB2))
	{
		--LCDValueInt;
		lcd_clrscr();
		sprintf(LCDValueString, "%d", LCDValueInt);
		lcd_puts(LCDValueString);
		//key_state = 0; //holding down decreases count rapidly
		sleep_timer = 0;	
	}
	
	if(sleep_timer == 500)
	{
		cli();
		GICR |= (1<<INT0)|(1<<INT1);//enable external interrupts int0 and int1
		DDRD &= ~((1<<PD2)|(1<<PD3)); //int0 and int1 to input
		PORTD |= (1<<PD2)|(1<<PD3); //int0 and int1 with pull-up resistor
		DDRB |= (1<<PB1)|(1<<PB2); //pin B1 and B2 to output
		PORTB &= ~((1<<PB1) | (1<<PB2)); //pin B1 and B2 output low
		set_sleep_mode(SLEEP_MODE_PWR_DOWN); //set sleep mode to 'power down'
		lcd_command(_BV(LCD_DISPLAYMODE)); //turn off LCD display
		sleep_enable(); //enable the sleep bit
		sei(); //set interrupts to on
		sleep_cpu(); //sleep microcontroller

		//wake up and do all of these things
		sleep_disable(); //turn microcontroller back on
		lcd_command(_BV(LCD_DISPLAYMODE) | _BV(LCD_DISPLAYMODE_ON)); //turn screen back on
		DDRD |= (1<<PD2)|(1<<PD3); //int0 and int1 to output
		PORTD &= ~((1<<PD2)|(1<<PD3)); //int0 and int1 goes low
		DDRB &= ~((1<<PB1)|(1<<PB2)); //pin b1 and b2 to input
		PORTB |= (1<<PB1) | (1<<PB2); //enable b1 and b2 pull-up resistor
		sleep_timer = 0; //reset sleep counter
	}
}

}

 

Thanks for all of the input.