ATTINY817 Xmini - how to check if a button is released

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

Hello,

I am having some difficulties with my code.

My code should operate like so:

1. Start LED if button is clicked once

2. Flash LED if button is clicked twice

3. Turn LED off if pressed for X seconds

 

My code works for 1 and 2 but cannot find any documentation on how to find out if the button is released.

Any ideas anyone?

my code is as follows:

/*
* LED_TOGGLE.c
*
* Created: 31/12/2019 19:37:02
* Author : Tamir
*/

#define F_CPU 16000000UL
#include <avr/io.h>            // Call IO Library
#include <util/delay.h>        // call delay library (_delay_ms)
#include <avr/interrupt.h>

#define BLINK_DELAY    200    // Number of milliseconds to delay when blinking

// 0 - NONE
// 1 - ON
// 2 - BLINK
volatile int clickCounter = 0;

ISR(PORTC_PORT_vect) {
    while(PORTC.IN & (PIN5_bm));    // wait until button is released
    clickCounter = (clickCounter + 1)%3;
    if(clickCounter == 0) clickCounter = 1;        // Counter shouldnt go back to zero
    PORTC.INTFLAGS |= (1 << 5);
}

int main (void)
{
    //atmel_start_init();      /* Initializes MCU, drivers and middleware */
    
    PORTC.DIRCLR = PIN5_bm;    /* Configure SW0 User Button as input */
    PORTC.PIN5CTRL |= (1 << 1) | (1 << 0);    //Sense falling edge
    
    /* Configure Yellow LED0 pin as output */
    PORTC.DIRSET = PIN0_bm;
    
    sei();        // Enable Interrupt
    
    while (1){
        
        // on first button press set led on.
        if(clickCounter == 1){
            PORTC.OUTSET = PIN0_bm;
        }
        
        // on second button press, start blinking
        if(clickCounter ==2){
            PORTC.OUTCLR = PIN0_bm;
            _delay_ms(BLINK_DELAY);
            PORTC.OUTSET = PIN0_bm;
            _delay_ms(BLINK_DELAY);

        }

    }
}

Last Edited: Thu. Jan 16, 2020 - 09:14 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Buttons and interrupts are not a great idea in the first place. Google "ganssle bounce" if you don't already know why.

 

Oh and don't blithely use "int" unless you know what atomicity is. (as it happens it can't count beyond 2 so it should not be an issue but in this case a uint8_t is more than wide enough. 

 

Also I'm a bit confused as to why the counter cannot be 0 - that is the default ?? If it's really storing "state" rather than count wouldn't an enum be a better approach anyway?

 

Finally a while() in an ISR() is a recipe for disaster. Instead operate the ISR as a state machine and watch for edge transitions.

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

Hi Clawson,

Thanks for the quick reply!

Unfortunately i'm very new with micro controllers so am unaware of many things.

 

1. I'll read about "ganssle bounce", thanks. Can you point towards the more reliable approach?

2. will change to uint8_t

3. The behavior is that once the device is turned off - one click starts the led, second click starts flashing and then every click just toggles between the two modes.

 

What I'm still missing is how to know when the button is released - I want to turn the LED off after the button is pressed for, lets say, 3 seconds.

 

Best,

Tamir.

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

Tamirell wrote:
1. I'll read about "ganssle bounce", thanks. Can you point towards the more reliable approach?
The issue with bounce is that you may think you press the button once but as the metal contacts close they actually make multiple connect-release-connect-release pulses. So the general approach is to "read the state a few times over a shortish (say 50ms) period of time and only consider the button to be fully open or fully closed when it is seen in the same state for a number of periods". This then implies the use of a timer and a "history" to monitor the last N states. When the state has been seen the same a number of times you are in a fixed state. So rather than a pin interrupt you tend to use a timer interrupt. 

 

Rather than reinvent the wheel there's example code been shown here many times ("danni", "theusch") that does the job if state tracking in a interrupt to hold the state of one or even many buttons

Tamirell wrote:
I want to turn the LED off after the button is pressed for, lets say, 3 seconds.
Already that implies the use of a timer anyway. You need to keep sampling the button multiple times over the period of 3s and only if it has remained in the same state for all the time is that "long press" detection considered to be complete.

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

Just write a Timer interrupt e.g.  every 20ms

Check your button in the timer ISR().   Remember its old_state.   Update counter.

 

If the button state is steady for 100ms (5 interrupts) you update the global state e.g. "is_pressed" or "is_released"

 

This effectively debounces your switch.

If the state is steady pressed for 2 seconds (100 interrupts) you register a "long press"

 

Draw your flowchart on paper.   You can see how the program will work.   e.g. when to reset counter,

 

Never use interrupts on switches.    Just read switch at regular time interval.

 

Note that if your global_state is accessed by the ISR() and the main program it should be volatile.

 

David.

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

You can also just wait for a switch release for a certain amount of time. Its not the best solution, but its a good start that works as a quick solution. You can figure out where the major flaw is, which should probably lead you to whatever the next step may be.

 

FYI- you also do this in your isr code-

PORTC.INTFLAGS |= (1 << 5);

whenever you do an |= when dealing with flags, a red flag should go up

you want to preserve the other flags, so a simple = will do as you only want to write a 1 to the flag bit position of interest (1 clears bit, 0 does nothing)

 

#define F_CPU 16000000UL
#include <avr/io.h>
#include <util/delay.h>
#include <stdint.h>
#include <stdbool.h>

 

//asuming sw0- high is on

void sw0_init(){ PORTC.DIRCLR = PIN5_bm; }
bool sw0_ison(){ return PORTC.IN & PIN5_bm; }
int sw0_check(){
    static uint8_t count;
    if( sw0_ison() == false ) return count;
    count++;
    //wait for release (sw0 not on for 50ms)
    for( uint8_t i = 0; i < 50; _delay_ms(1) ){       
        if( sw0_ison() == false ) i++;
        else i = 0; //start counting from 0 if on
    }
    return count;
}

//assuming led0 - high is on
void led0_init(){ PORTC.DIRSET = PIN0_bm; }
void led0_on(){ PORTC.OUTSET = PIN5_bm; }
void led0_off(){ PORTC.OUTCLR = PIN5_bm; }

 

void delay_ms(uint16_t ms){ for( ; ms--; _delay_ms(1) ); }
void led0_blink1(uint16_t ms){
    led0_on();  delay_ms(ms);
    led0_off(); delay_ms(ms);
}

 

int main (void)
{
    sw0_init();
    led0_init();

 

    while (1){
        switch( sw0_check() & 3 ){ //limit to 0-3
            case 0: led0_off();         break;
            case 1: led0_on();          break;
            case 2: led0_blink1(200);   break;
            case 3: led0_blink1(50);    break;
        }
    }
}

Last Edited: Thu. Jan 16, 2020 - 01:24 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you all,

You have all been very helpful, and I have made some progress.

@clawson , @david.prentice , @ curtvm I read all your notes and hopefully implemented everything properly.

I added a timer using TCA and now sampling the button state every ~0.5hz.

So if the user clicks the button, the LED starts.

If he holds it for more than 3 seconds the LED turns off.

My code follows, your comments are welcomed:

 

*
* LED_TOGGLE.c
*
* Created: 31/12/2019 19:37:02
* Author : Tamir
*/

#define F_CPU 16000000UL
#include <avr/io.h>			// Call IO Library
#include <util/delay.h>		// call delay library (_delay_ms)
#include <avr/interrupt.h>
#include <stdint.h>
#include <stdbool.h>

bool sw0_ison(){ return !(PORTC.IN & PIN5_bm); }
void led0_on(){ PORTC.OUTSET = PIN0_bm; }
void led0_off(){ PORTC.OUTCLR = PIN0_bm; }
volatile uint8_t Statecounter = 0;
volatile uint8_t counter = 0;
bool IsButtonClicked = false;

//------------------------------------------------
//TCA - 16 bit Timer/Counter Type A initialization
void timerInit(void)
{
	// Write a TOP value to the Period register (TCA.PER)
	TCA0.SINGLE.PER = 1627;

	// Enable the peripheral by writing a '1' to the ENABLE bit in the Control A register (TCA.CTRLA).
	// The counter will start counting clock ticks according to the pre scaler setting in the Clock Select bit
	// field (CLKSEL) in TCA.CTRLA.

	TCA0.SINGLE.CTRLA |= (TCA_SINGLE_CLKSEL_DIV1024_gc) | (TCA_SINGLE_ENABLE_bm);// TCA0.SINGLE.CTRLA addresses the control A register for timer A, system clock / 1024, timer enabled

	TCA0.SINGLE.INTCTRL |= (TCA_SINGLE_OVF_bm); // Enable timer interrupts on overflow on timer A
}

//------------------------------------------------
ISR(TCA0_OVF_vect)
{
	// LED Toggling
	//PORTC.OUTTGL = PIN0_bm; // toggle PIN 0, Port C
    if(sw0_ison())
	{
		counter++;
		if(counter % 2 == 1 && counter < 10 && IsButtonClicked == false )
		{
			led0_on();
			IsButtonClicked = true;
		}
			
		//if(counter % 2 == 0 && counter < 10 )
		//{
			//PORTC.OUTTGL = PIN0_bm;
		//}
		if(counter >= 6)
		{
			led0_off();
			counter = 0;
		}
	}
	else
	{
		counter = 0;
		IsButtonClicked = false;
	}
	
	// Clear the interrupt flag, to prevent instantly jumping back into the ISR again.
	TCA0.SINGLE.INTFLAGS = TCA_SINGLE_OVF_bm;
	
}

//------------------------------------------------
int main(void)
{
    PORTC.DIRCLR = PIN5_bm;	/* Configure SW0 User Button as input */
	PORTC.DIRSET = PIN0_bm; //* Configure Yellow LED0 pin as output */
	
	timerInit();
	sei(); // Enable global interrupts

	while(1){}
}

 

I could still use your guidance with my next step:

If the user clicks the button for the second time, I want to use the LED in PWM mode(instead of blinking).

I read some posts about it & most implementations were using the same timer (TCA).

Any other ideas/leads ?

 

Thanks!

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
bool sw0_ison(){ return !(PORTC.IN & PIN5_bm); }
void led0_on(){ PORTC.OUTSET = PIN0_bm; }
void led0_off(){ PORTC.OUTCLR = PIN0_bm; }

If you are going to use "mini" functions like this I would suggest:

static inline __attribute__((always_inline)) bool sw0_ison(){ return !(PORTC.IN & PIN5_bm); }
static inline __attribute__((always_inline)) void led0_on(){ PORTC.OUTSET = PIN0_bm; }
static inline __attribute__((always_inline)) void led0_off(){ PORTC.OUTCLR = PIN0_bm; }

"static" tells the compiler that nothing outside this file will try to use these so no need to create a globally callable copy. "inline" is a hint to say "at the point of invocation just include the following short code - don't call to here with associated overhead" (that's especially important when the code is "called" from an ISR). The always_inline attribute is a far stronger command to the compiler to follow the "inline" hint!

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

Thanks! changed it.

Do you have any ideas regarding the PWM implementation?

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

>If you are going to use "mini" functions like this I would suggest:

 

I probably wouldn't worry about it. You could save 20 bytes (just by using static), you could also save another 20 by using VPORT registers. Not a big deal when 99% of your flash is unused. Or 50%.

 

There is usually little reason to go beyond static-

static bool sw0_ison(){ return !(PORTC.IN & PIN5_bm); }

as the compiler will pick the 'best' option depending on what is in the function, and the 'static' is really all the info it needs to do so.

 

The inline keywords/attributes have various meanings to the compiler, so unless you have a firm grasp of all the variations (no one really does) I would just leave them alone. For example, one purpose is to simply get the compiler/linker to basically not complain when a function is used in a header (either about seeing it multiple times, or that it may be unused). 

 

Since I may be to blame for the 'mini' functions, I would hate to see someone think that they also require tacking on another half-line of attributes and keywords. If the function is used only in the current file, you can add 'static' to the front of it since it will only be used in the current file (and the compiler will most likely inline it if that makes sense to do). If you want some of these 'mini' functions in their own header file, then inline will be used but that is advanced and not suited to someone who is' 'very new with micro controllers'. Even if you leave the static off, you are still good. You have 16 million clocks per second, if it takes you an extra 20 to do the job, no one will know the difference- your led will not care, your user who is pressing the switch will not care. The main focus should be producing code that is correct. The size of it is less important.

 

You will see many who will use macros (defines) to do the same thing-

static void led0_off(){ PORTC.OUTSET = PIN5_bm; }

vs

#define LED0_OFF  PORTC.OUTSET = PIN5_bm

 

led0_off();

LED0_OFF;

 

but I would rather talk directly to the compiler as there will be no question about what you are looking at and what the compiler may complaining about. The function can grow in size without having to start doing multi-line defines. It all produces the same code in the end (or it should), so pick whatever you like, and the purpose of either is to be able to think about your problem at hand and not have to deal with figuring out what it takes to turn on an led each time you want to do that.

 

 

>I read some posts about it & most implementations were using the same timer (TCA).

>Do you have any ideas regarding the PWM implementation?

 

Pick the timer that has the feature you need to do a specific job. If TCA has your pwm feature, then maybe your simple timer interrupt can be moved to TCB so you have TCA free to do something more useful. You also have TCD and RTC, so don't use the feature rich timer to do a mundane task others could do. TCA could also do more than one thing, and keeping time could also be done as its hobby.

 

 

 

 

 

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

curtvm wrote:
Not a big deal when 99% of your flash is unused. Or 50%.
Yeah but this site is about programming 0.5K/1K/2K "micro"controllers. 20 bytes in 512 is significant !

 

Anyway it was more about not CALLing functions from ISRs that prompted my post.

curtvm wrote:
you have a firm grasp of all the variations (no one really does) I would just leave them alone.
I do - hence the "attribute"
curtvm wrote:
no one will know the difference-
Again these are low spec, low resource "micro" contollers, cycles, like bytes, DO count.
curtvm wrote:
but I would rather talk directly to the compiler
On thta we agree. The function versions (esp in C++) are type safe and MISRA forbid the use of macro functions anyway.