Debounce code for push-buttons in project

Go To Last Post
103 posts / 0 new

Pages

Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello.

 

After performing a couple of searches for debouncing code here in AVR Freaks community  I found this code someone already wrote:

 

/************************************************************************
;*                                                                      *
;*              Testing Read and debounce up to 8 keys                  *
;*              Bulletproof: 4 equal samples needed                     *
;*                                                                      *
;*              Author: P. Dannegger                                    *
;*                                                                      *
;***********************************************************************/
#include 
#include 
#include 

#define KEY_INPUT       PINC
#define LED_OUTPUT      PORTB
#define LED_DIR         DDRB

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


SIGNAL (SIG_OVERFLOW0)
{
  static char ct0, ct1;
  char i;

  i = key_state ^ ~KEY_INPUT;           // 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
                                        // now debouncing finished
  key_press |= key_state & i;           // 0->1: key press detect
}


char get_key_press( char key_mask )
{
  cli();
  key_mask &= key_press;                // read key(s)
  key_press ^= key_mask;                // clear key(s)
  sei();
  return key_mask;
}


int main( void )
{
  key_state = 0;
  key_press = 0;

  TCCR0 = 1<<CS02;                      //divide by 256 * 256
  TIMSK = 1<<TOIE0;                     //enable timer interrupt

  LED_DIR = 0xFF;
  LED_OUTPUT = 0xFF;
  sei();
  for(;;)                                       // main loop
    LED_OUTPUT ^= get_key_press( 0xFF );        // toggle LEDs on key press
}

 

I'm now trying to integrate this code in my project code but I'm kind of struggling to understand the chronology of events, I mean what happens and when it happens.

 

But first things first.

I'm using an AtMega328P and 2 push-buttons attached at pins PD2 (INT0) and PD3 (INT1) and I'm using their interrupts to perform some actions and I need debouncing routine to ensure only one action per button push.

I'm using avrdude 6.3  under Debian 9 to upload the code into the uC and an USB to TTL UART converter to do the upload.

I'm also aware, from this link, that the name vector for TIMER interrupts have changed and I changed 

SIGNAL (SIG_OVERFLOW0)

to

ISR (TIMER0_OVF_vect)

 

I have also changed register names to match my AtMega328P as follows:

 

TCCR0 = 1<<CS02;

to

TCCR0B = 1<<CS02;

 

and 

TIMSK = 1<<TOIE0;

 

to

TIMSK0 = 1<<TOIE0;

 

I also got rid of the LEDs thing because I don't need them, so summarising, this is what my code looks like:

 

#define KEY_INPUT    PIND

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

void get_key_press(uint8_t key_mask){
  cli();
  key_mask &= key_press;                // read key(s)
  key_press ^= key_mask;                // clear key(s)
  sei();
}

void debounce_button_timer_setup(void){
  TCCR0B = 1 << CS02;               //divide by 256 * 256
  TIMSK0 = 1 << TOIE0;              //enable timer interrupt
  sei();
}


debounce_button_timer_setup();
get_key_press(0x0c);

 

This might be a little bit out of context because my project has several files and the code above is in different places, not all together just like I pasted it here.

 

My struggle is that the push buttons are attached at PD2 and PD3 and they are triggering the INT0 and INT1 interrupts and as the debounce code uses TIMERs to raise their flags to run the debounce code, I'm not sure when and where to use the get_key_press() function.

 

I might need to show how my INT0 and INT1 vectors code is:

 

ISR (INT0_vect){
   sweep_sta = SWEEP_STA_UP;
}

ISR (INT1_vect){
   sweep_sta = SWEEP_STA_DOWN;
}

So this is my code for the interrupts 0 and 1 and this sweep_sta variable is being checked inside the main() function infinite loop, so I'm not quite sure where to use the debounce code functions, I mean the get_key_press() function!

Any help is appreciated! I'm sorry if I'm missing any critical information!

 

Thanks

PsySc0rpi0n

This topic has a solution.
Last Edited: Sat. Jul 21, 2018 - 07:00 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Don't use the interrupts.

 

Debounce your buttons in the timer routine and check for a debounced button push in your main() routine. If a button is pushed then run your code.

#1 This forum helps those that help themselves

#2 All grounds are not created equal

#3 How have you proved that your chip is running at xxMHz?

#4 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

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

Why you suggest not to use interrupts for buttons, I presume?

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

PsySc0rpi0n wrote:
Why you suggest not to use interrupts for buttons, I presume?

Because buttons bounce -- which is why you would use debouncing.

 

Start with Ganssle ... it is interesting and entertaining if nothing else...

http://www.ganssle.com/debouncin...

http://www.ganssle.com/debouncin...

https://pubweb.eng.utah.edu/~cs5...

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

The only good use of buttons and interrupts is to wake a sleeping mpu.  Otherwise just pole your buttons using the timer overflow interrupt as suggested in the articles linked above.

 

Jim

 

Click Link: Get Free Stock: Retire early!

share.robinhood.com/jamesc3274

 

 

 

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

To be clear, it is fine to use timer interrupts to debounce, you generally don't want the switches themselves to do the interrupting.  

 

Switches generating interrupts can generate a fast lightning storm of them.  To avoid this flood of interrupts, the first one would typically disable further interrupts for some amount of time to prevent the runaway condition. 

When in the dark remember-the future looks brighter than ever.

Last Edited: Sat. Jul 7, 2018 - 02:59 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for the replies.

 

Ok, I think I understand now better the idea of debouncing. I mean, the debouncing routine presented in the code I posted in first post already takes care of the "surveillance" to the button searching for a key press, right?

I never seen this this way. I always thought that the procedure of using interrupts to trigger an action, no matter the way the interrupt was triggered, if with push-buttons or other way, was a complete separate thing the debounce procedure, but now I think that the debounce procedure takes care of the push-button pressing event and also the debounce routine. Am I correct? So, I don't need to trigger an interrupt with the push-button and then perform the debounce routine.

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

You’ve got the idea.

Interrupts are used when you want microsecond responses. Pushbuttons are in the tens of milliseconds. Also, using external interrupts is fraught with danger if you cannot limit the rate they are generated. If the interrupts are generated too frequently, all the processor will be doing is servicing interrupts and everything else grinds to a halt.

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

Ok. In my case, these 2 buttons are to increase or decrease the frequency of a signal that is being used in the circuit. So, that situation of the uC being busy only serving interrupts won't be of concern.

Anyway, that will require quite a change in my code. I need to remove the interrupt configuration code and redo it for the TIMER based interrupts! I'm reading the links suggested above!

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

Ok, I have finish reading the following links provided by theusch in post #4.

 

http://www.ganssle.com/debouncin...

http://www.ganssle.com/debouncin...

 

They provide a couple of debounce codes. Now, should I just quit using the code I posted in 1st post and use the ones from those 2 links, or should I try to go with the code I posted in 1st post?

In my case, I think I need 2 pins of the uC because one push-button have different actions from the other one and though I need to differentiate them. I can't have both push-buttons in the same uC pin because I need to know which push-button was pressed. What I mean is that I don't need a code that is able to handle more than one push-button per uC pin. And in this case, I already have push-button 1 on PD2 (hard wired, soldered) and push-button 2 (also hard wired, soldered) to PD3.

 

Does this makes any difference when choosing the code to use?

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

Don't quit #1, danni's debounce code is one of the best solutions out there.

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

clawson wrote:

Don't quit #1, danni's debounce code is one of the best solutions out there.

 

Ok, I also wanted to go with it.

 

The one I was reading from link 2 doesn't even uses TIMER interrupts. I thought it was using them but apparently not, at least as far as I could understand. In that piece of code, I can't see any configuration of uC TIMER interrupts!

 

Ok, now I need to integrate that code from 1st post into my project code. And I also need help to understand it so that I know where to place each part of the code.

 

So, to start I have a couple of questions. Knowing that I already have each push button attached to PD2 and PD3 with solder (project is already on a stripboard), I will have to send the mask to get_key_pressed() function separately, right? Each one for each push-button, correct? The other question is - is this function, the get_key_press(), that needs to be in a loop to constantly check for the key press or how is it done?

 

I'm still not very into this code.

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

Let me know if I understood it now:

 

get_key_press() function only tells the program in which uC pins to look, right?

 

Then the TIMER routine, checks those pins every time the overflow occurs, is this it?

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

I would suggest that you start by reading this...

https://www.avrfreaks.net/comment/726676#comment-726676

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

Ok, I have read the walkthrough but it only explains the code inside the ISR routine and the bitwise operations taking place! I can do it on paper when I understand the rest of the code.

 

I need to learn how to integrate this code in my project and for that I need to understand how the code works as a whole!

 

For instance, I need to understand if the get_key_pess() function role is the one I said and if so, how do I arrange the code to deal with a different action to each push-button I have!

 

I mean, if the funciton get_key_press() is only to tell the code which pins of the uC are to be taken into account, how do I then tell the code to perform different actions according to which push-button was pressed!

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

Direct from danni (Dannegger ):

 

Thats, what the function get_key_press was made for.

You must only give the bitmask of the key, which you want to test and it returns non zero, if the key was pressed.

 

see here:

 

https://www.avrfreaks.net/forum/...

 

 

When in the dark remember-the future looks brighter than ever.

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

avrcandies wrote:

Direct from danni (Dannegger ):

 

Thats, what the function get_key_press was made for.

You must only give the bitmask of the key, which you want to test and it returns non zero, if the key was pressed.

 

see here:

 

https://www.avrfreaks.net/forum/...

 

 

 

I understand that. My question was rather to confirm (or not) if that function was only to tell the code which pins were to be tested and if it was needed to be used only once or not!

But I think it's now clear it is used only once to tell the code which pins to check.

 

Anyway, in the example code the mask set to get_press_key() is 0xff, meaning that we have 8 pins to be checked. Is this correct? If I want to check only pins PD2 and PD3, the mask needs to be 0x0c, right?

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

PsySc0rpi0n wrote:

Anyway, in the example code the mask set to get_press_key() is 0xff, meaning that we have 8 pins to be checked. Is this correct? If I want to check only pins PD2 and PD3, the mask needs to be 0x0c, right?

 

Correct. This code, and many other examples, reads and debounces a whole port, ie all 8 bits, and you then look at individual bits to see what each pin is doing.

 

I often read and debounce all 4 ports (32 bits) in applications as the code, and time, to do it is so small.

#1 This forum helps those that help themselves

#2 All grounds are not created equal

#3 How have you proved that your chip is running at xxMHz?

#4 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

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

Brian Fairchild wrote:

PsySc0rpi0n wrote:

 

Anyway, in the example code the mask set to get_press_key() is 0xff, meaning that we have 8 pins to be checked. Is this correct? If I want to check only pins PD2 and PD3, the mask needs to be 0x0c, right?

 

Correct. This code, and many other examples, reads and debounces a whole port, ie all 8 bits, and you then look at individual bits to see what each pin is doing.

 

I often read and debounce all 4 ports (32 bits) in applications as the code, and time, to do it is so small.

 

Ok. But I'm trying this code and LED is not changing state.

 

LED is on pin PB1, i.e. pin 15 of my uC and it is always ON. The button is on pin 5, i.e. PD3. Code is as follows:

 

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

#define KEY_INPUT    PIND
#define LED_OUTPUT   PORTB
#define LED_DIR      DDRB

uint8_t key_state = 0;
uint8_t key_press = 0;

uint8_t get_key_press(uint8_t key_mask){
   cli();
   key_mask &= key_press;              //read key(s)
   key_press ^= key_mask;              //clear key(s)
   sei();

   return key_mask;
}

void timer_intr_setup(void){
   TCCR0B = 1 << CS02;                 //divide 256 * 256
   TIMSK0 = 1 << TOIE0;                //enable timer interrupt
}


int main(void){

   timer_intr_setup();

   LED_DIR = 0xff;
   LED_OUTPUT = 0xff;
   sei();
   for(;;){
      LED_OUTPUT ^= get_key_press(0x0c);
   }
}

ISR (TIMER0_OVF_vect){
   static uint8_t ct0, ct1;
   uint8_t i;

   i = key_state ^ ~KEY_INPUT;         //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;                     //thne toggle debounced state
   //now debounce is finished
   key_press |= key_state & i;         //0->1 key press detect
}

 

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

Describe the wiring. I don't see pull-ups being enabled so I assume you have pull-up/pull-down in the electronics?

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

clawson wrote:

Describe the wiring. I don't see pull-ups being enabled so I assume you have pull-up/pull-down in the electronics?

 

Yes, I have a wire that I can change either to PD2 or to PD3 to a 10k ohm resistor that is connected to +5VDC. And from the same pin I have another wire that I use to perform the pull down to ground to simulate the button (I don't have a spare push-button here with me).

 

PD2_____

               /_____10k ohm______+5VDC (PD2 is connected to pull-up resistor)

PD3____ \     |

                     |

                     |

                  GND (wire that I manually drive to gnd to simulate the push-button pressing)

 

 

 

 

PD2_____

               /_____10k ohm______+5VDC (PD3 is connected to pull-up resistor)

PD3____ \    |

                     |

                     |

                     |

                  GND (wire that I manually drive to gnd to simulate the push-button pressing)

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

So the pins are "active low"? In which case you need to invert your sensing test. Also if they are active low with pull ups why bother with external resistors when the AVR ports already have them for "free"?

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

clawson wrote:
So the pins are "active low"? In which case you need to invert your sensing test.
It's the other way round. Peter's code is for active low, and you have to change it for active high.

 

But there are two volatiles missing. And there is a good chance that get_key_press() got inlined, and then the missing volatiles will definitely harm.

 

Stefan Ernst

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

Pins PD2 and PD3 are pulled up by external pull up resistors. The wire I have to ground is floating. It only connects to ground when I manually do it. I'm not sure I explained myself correctly! I hope I did.

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

Yes, I have a wire that

Your diagram is a mess (or just drawn wrong)..EACH pin needs a pull up resistor.  The pins are not to be connected together in any manner.   Why do you show them together?   Note that pull ups are built in, so they could be used.

 

Be sure to verify with your meter, the actual pin voltages are going up & down (approx. 5V & 0V).

When in the dark remember-the future looks brighter than ever.

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

If you are using an ATmega328P then you should ignore all the general advice that applies to all the other types of AVRs and concentrate on Arduino UNO_Arduino Nano library code.   The UNO/Nano Arduino code is specifically for the mega328P and allows the developer to not have to deal with all the low-level hardware aspects of the AVR, such as push-button interfacing.  If you develop for a mega328p using C, then you should do a parallel application project for the Arduino platform.  Because most customers will want the option of having the code in the Arduino code framework if the CPU is a AVR Mega328P.  Using the Arduino platform significantly reduces code maintenance and revision costs, because all the internal operations of the CPU are handled by standardized "plug'n play" libraries.  And because there are many thousands of people who know how to code for Arduino, which means that you don't have to hire specialized expensive embedded-systems engineers for maintenance and revisions.  It doesn't make sense to use a $80/hr engineer for an application that will go into 1 to 100 units of a device running an 80-cent microcontroller.

 

  There are 5 or 6 downloadable libraries for push-switch buttons available.  I use (and have documented) the Alex Brevik library and reviewed the Mat Hertel library.  They both use the 1 millisecond timer0 interrupt that Arduino uses as its internal application code heartbeat.   Both libraries create a class for the push-switch button that compares the status of the button each millisecond to the previous millisecond.

 

  There are four things that an Arduino developer (someone who doesn't have to be concerned about how the actual AVR works on the peripheral register level, and therefore doesn't waste development time on stupid things like switch debouncing) will do with a push button.  One is detect a simple quick ordinary press-and-release.  Two is detect a "double-click", where the button is pressed twice in rapid succession.  Three is a single long press (where the button is held for longer than 0.8 second, and then released).   Four is a long press where the button is held down, and an action happens after each interval of time passes.  An example of this mode is setting the time on a clock where the minutes value will increase every second as long as the "Minutes" button is pressed.

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

using an ATmega328P then you should ignore all the general advice that applies to all the other types of AVRs

...not have to deal with all the low-level hardware aspects of the AVR...

...someone who doesn't have to be concerned about how the actual AVR works on the peripheral register level, and therefore doesn't waste development time on stupid things

That might be a bit strongly worded. Not sure I'd want the described person designing an airbag control module.  Replacing thinking with canned solutions is not always the right answer.  Of course those are useful tools to thoughtfully reach a solution, while balancing the critical tasks against the repetitive & mundane.  

When in the dark remember-the future looks brighter than ever.

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

avrcandies wrote:

Yes, I have a wire that

Your diagram is a mess (or just drawn wrong)..EACH pin needs a pull up resistor.  The pins are not to be connected together in any manner.   Why do you show them together?   Note that pull ups are built in, so they could be used.

 

Be sure to verify with your meter, the actual pin voltages are going up & down (approx. 5V & 0V).

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

PsySc0rpi0n wrote:
avrcandies wrote:

Yes, I have a wire that

Your diagram is a mess (or just drawn wrong)..EACH pin needs a pull up resistor.  The pins are not to be connected together in any manner.   Why do you show them together?   Note that pull ups are built in, so they could be used.

 

Be sure to verify with your meter, the actual pin voltages are going up & down (approx. 5V & 0V).

Where did I said PD2 and PD3 were connected together? I never did. They are one close to the other in the AtMega328 uC. Thats why I draw them close to each other, and not to transmit the idea of being connected together.

I just tried to illustrate the wiring I have. Once more:

PD2 and PD3 are supposed to be connected to push-buttons but for this test I don't have spare push-buttons, so I'm simulating them by using a wire to pull them to ground. And each pin PD2 and PD3 have their own pull up resistors (it's not needed I know, but I have them there) and that wire that I'm using to pull PD2 or PD3 to ground to simulate the event of pressing the push-button, is a wire I can remove from PD2 and use it on PD3 or vice versa.

I have already verified with my scope that PD2 and PD3 are being correctly actuated, I mean, when I do nothing, they have 5V, when I use the wire and pull PD2 or PD3 to ground, they go to around 0V.

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

Simonetta, thanks for the insights. However, I'm doing this on the bare AtMega328 only for the fun. I'm not going to produce anything to release on the market. This is for a school project, so the goal is to learn different stuff.
Thanks

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

PsySc0rpi0n wrote:
Where did I said PD2 and PD3 were connected together?
Here...

PD2_____

               /_____10k ohm______+5VDC (PD2 is connected to pull-up resistor)

PD3____ \     |

                     |

                     |

                  GND (wire that I manually drive to gnd to simulate the push-button pressing)

As avrcandies says that sure looks (as drawn) as if PD2 and PD3 are connected to ONE 10K resistor that pulls up to Vcc (+5VDC). He did say:

avrcandies wrote:
(or just drawn wrong).
so I guess it must actually be that. The hope is that you really meant something like....

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

You have a hardware fault.

 

How do I know? Because I took your code and dropped it onto a dev board with correctly wired switches and LEDs and it works. As the only difference is that I am using known good hardware and you are using untested hardware it must be your hardware.

 

 

You can see LED bit 3 on, I can toggle it with switch bit 3.

 

 

 

 

#1 This forum helps those that help themselves

#2 All grounds are not created equal

#3 How have you proved that your chip is running at xxMHz?

#4 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

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

Brian Fairchild wrote:
As the only difference is
Is it? So you are using the same version of the compiler with the same settings?

 

As I already said: there are two volatiles missing in the code. And whether that harms or not, depends on whether the compiler decides to inline get_key_press or not.

 

Stefan Ernst

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

sternst wrote:

So you are using the same version of the compiler with the same settings?

 

Ah, of course, he's probably using the brain-dead AVR-GCC whilst I'm using something sensible. wink

 

On a side note....I've never understood why the default action on some compilers is to optimise away variables which aren't volatile. After all, I've made a global variable called 'splat'. I've used a variable called splat in this routine and I've used a variable called splat in that routine. Clearly my intention is that things I write here appear over there. Why optimise it away?

#1 This forum helps those that help themselves

#2 All grounds are not created equal

#3 How have you proved that your chip is running at xxMHz?

#4 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

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

Brian Fairchild wrote:
Why optimise it away?
The variables are not optimized away, neither in your example nor in the given code here. What is optimized away are memory reads/writes.

Stefan Ernst

Last Edited: Mon. Jul 9, 2018 - 10:56 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

sternst wrote:

What is optimized away are memory reads/writes.

 

Does a tree falling in a forest, that no-one hears, make a sound?

 

Does a memory location that is not read or written stop being a variable?

 

devil

#1 This forum helps those that help themselves

#2 All grounds are not created equal

#3 How have you proved that your chip is running at xxMHz?

#4 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

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

Brian Fairchild wrote:
Does a memory location that is not read or written stop being a variable?
Does "variable" always have to be equal to "memory location"?

 

Edit: And no one said that ALL read/writes are optimized away. In the give code it will only be the ones in the loop.

Stefan Ernst

Last Edited: Mon. Jul 9, 2018 - 11:19 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I've noticed the lack of volatile in Danni's code before now and yet the evidence seems to suggest that it always "works" anyway so there has to be something in the sequence that defeats the compiler's ability to optimize read/writes away. However I do agree that "volatile" would add a certain amount of bullet proofing to that.

Brian Fairchild wrote:
the brain-dead AVR-GCC
How intriguing - so a compiler aggressively optimising (perhaps more so than other compilers) is evidence of "brain death"? I'd have thought it showed superior intelligence - but what do I know?

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

clawson wrote:
I've noticed the lack of volatile in Danni's code before now and yet the evidence seems to suggest that it always "works" anyway so there has to be something in the sequence that defeats the compiler's ability to optimize read/writes away.
It "works" in real projects, because very likely get_key_press gets called more than once then. And in a real project there is a good chance that the function itself is in a different compilation unit.

 

In the code here it is a single call in a tight loop with the implementation of the function in the same file, so inlining comes into play and with it the problems because of the missing volatiles.

 

BTW:

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


SIGNAL (SIG_OVERFLOW0)

"char" and "SIGNAL"? It looks like a very early version of the code.

And in newer versions key_state and key_press are volatile.

Stefan Ernst

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

clawson wrote:

I've noticed the lack of volatile in Danni's code before now and yet the evidence seems to suggest that it always "works" anyway so there has to be something in the sequence that defeats the compiler's ability to optimize read/writes away.

 

I guess what we need is for someone to drop the code in post #19 into AVRGCC and see what gets produced. 

 

clawson wrote:

...so a compiler aggressively optimising (perhaps more so than other compilers) is evidence of "brain death"? I'd have thought it showed superior intelligence - but what do I know?

 

Surely a smart compiler would know that a variable used in an ISR is going to be used elsewhere and not optimise it, or the access to it, away - but, like you, what do I know? wink

#1 This forum helps those that help themselves

#2 All grounds are not created equal

#3 How have you proved that your chip is running at xxMHz?

#4 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

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

Brian Fairchild wrote:
Surely a smart compiler would know that a variable used in an ISR is going to be used elsewhere and not optimise it
How can it know? It could be (and very likely is) in a different compilation unit.

 

@Stefan, the "master copy" of Danni's debounce code is surely?....

 

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

 

no sign of "volatile" in that.

Last Edited: Mon. Jul 9, 2018 - 12:41 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

How can it know? It could be (and very likely is) in a different compilation unit.

 

 

I have never delved into GCC's internals but I'd have thought...

 

1) It knows what an ISR is as it generates a different prologue/epilogue.

2) It must flag variable 'types' across different compilation units anyways to identify size mismatches.

 

Given that, I don't see a reason why it couldn't, when encountering a variable being used in an ISR, flag it as such for later interpretation in other units.

 

It might be that it's just not possible to do. I know that GCC is very processor agnostic at the front end and it might be that the necessary information to make smarter microcontroller-friendly decisions doesn't get carried across to the lower level processor specific parts.

#1 This forum helps those that help themselves

#2 All grounds are not created equal

#3 How have you proved that your chip is running at xxMHz?

#4 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

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

clawson wrote:
@Stefan, the "master copy" of Danni's debounce code is surely?....

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

no sign of "volatile" in that.

I didn't know that he didn't update his code here on AVRFreaks.

 

His latest version (at least I think it is) can be found here:

https://www.mikrocontroller.net/articles/Entprellung#Komfortroutine_.28C_f.C3.BCr_AVR.29

(it includes detection of short, long, and repeated key presses)

 

Edit: "repeated key presses" in conjunction with "detection" is misleading.

It provides an auto-repeat for long presses.

Stefan Ernst

Last Edited: Mon. Jul 9, 2018 - 01:27 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Anyways....

 

 

although I don't use it I do have a copy of Studio 6.2 installed. So I took the code from post #19, changed the ports used, dropped it into Studio and compiled it.

 

Guess what? It works without any mention of 'volatile'.

 

 

For reference, my build window...

 

 

Quote:

------ Build started: Project: GccApplication2, Configuration: Debug AVR ------

Build started.

Project "GccApplication2.cproj" (default targets):

Target "PreBuildEvent" skipped, due to false condition; ('$(PreBuildEvent)'!='') was evaluated as (''!='').

Target "CoreBuild" in file "C:\Program Files\Atmel\Atmel Studio 6.2\Vs\Compiler.targets" from project "D:\Documents\Atmel Studio\6.2\GccApplication2\GccApplication2\GccApplication2.cproj" (target "Build" depends on it):

Using "RunCompilerTask" task from assembly "C:\Program Files\Atmel\Atmel Studio 6.2\Extensions\Application\AvrGCC.dll".

Task "RunCompilerTask"

Shell Utils Path C:\Program Files\Atmel\Atmel Studio 6.2\shellUtils

C:\Program Files\Atmel\Atmel Studio 6.2\shellUtils\make.exe all 

Building file: .././GccApplication2.c

Invoking: AVR/GNU C Compiler : 4.8.1

"C:\Program Files\Atmel\Atmel Toolchain\AVR8 GCC\Native\3.4.1061\avr8-gnu-toolchain\bin\avr-gcc.exe"  -x c -funsigned-char -funsigned-bitfields -DDEBUG  -O1 -ffunction-sections -fdata-sections -fpack-struct -fshort-enums -mrelax -g2 -Wall -mmcu=atmega1284p -c -std=gnu99 -MD -MP -MF "GccApplication2.d" -MT"GccApplication2.d" -MT"GccApplication2.o"   -o "GccApplication2.o" ".././GccApplication2.c" 

In file included from .././GccApplication2.c:3:0:

c:\program files\atmel\atmel toolchain\avr8 gcc\native\3.4.1061\avr8-gnu-toolchain\avr\include\util\delay.h(90,3): warning: #warning "F_CPU not defined for <util/delay.h>" [-Wcpp]

# warning "F_CPU not defined for <util/delay.h>"

   ^

Finished building: .././GccApplication2.c

Building target: GccApplication2.elf

Invoking: AVR/GNU Linker : 4.8.1

"C:\Program Files\Atmel\Atmel Toolchain\AVR8 GCC\Native\3.4.1061\avr8-gnu-toolchain\bin\avr-gcc.exe" -o GccApplication2.elf  GccApplication2.o   -Wl,-Map="GccApplication2.map" -Wl,--start-group -Wl,-lm  -Wl,--end-group -Wl,--gc-sections -mrelax -mmcu=atmega1284p  

Finished building target: GccApplication2.elf

"C:\Program Files\Atmel\Atmel Toolchain\AVR8 GCC\Native\3.4.1061\avr8-gnu-toolchain\bin\avr-objcopy.exe" -O ihex -R .eeprom -R .fuse -R .lock -R .signature -R .user_signatures  "GccApplication2.elf" "GccApplication2.hex"

"C:\Program Files\Atmel\Atmel Toolchain\AVR8 GCC\Native\3.4.1061\avr8-gnu-toolchain\bin\avr-objcopy.exe" -j .eeprom  --set-section-flags=.eeprom=alloc,load --change-section-lma .eeprom=0  --no-change-warnings -O ihex "GccApplication2.elf" "GccApplication2.eep" || exit 0

"C:\Program Files\Atmel\Atmel Toolchain\AVR8 GCC\Native\3.4.1061\avr8-gnu-toolchain\bin\avr-objdump.exe" -h -S "GccApplication2.elf" > "GccApplication2.lss"

"C:\Program Files\Atmel\Atmel Toolchain\AVR8 GCC\Native\3.4.1061\avr8-gnu-toolchain\bin\avr-objcopy.exe" -O srec -R .eeprom -R .fuse -R .lock -R .signature -R .user_signatures "GccApplication2.elf" "GccApplication2.srec"

"C:\Program Files\Atmel\Atmel Toolchain\AVR8 GCC\Native\3.4.1061\avr8-gnu-toolchain\bin\avr-size.exe" "GccApplication2.elf"

   text    data     bss     dec     hex filename

    324       0       4     328     148 GccApplication2.elf

Done executing task "RunCompilerTask".

Using "RunOutputFileVerifyTask" task from assembly "C:\Program Files\Atmel\Atmel Studio 6.2\Extensions\Application\AvrGCC.dll".

Task "RunOutputFileVerifyTask"

Program Memory Usage : 324 bytes   0.2 % Full

Data Memory Usage : 4 bytes   0.0 % Full

Done executing task "RunOutputFileVerifyTask".

Done building target "CoreBuild" in project "GccApplication2.cproj".

Target "PostBuildEvent" skipped, due to false condition; ('$(PostBuildEvent)' != '') was evaluated as ('' != '').

Target "Build" in file "C:\Program Files\Atmel\Atmel Studio 6.2\Vs\Avr.common.targets" from project "D:\Documents\Atmel Studio\6.2\GccApplication2\GccApplication2\GccApplication2.cproj" (entry point):

Done building target "Build" in project "GccApplication2.cproj".

Done building project "GccApplication2.cproj".

 

Build succeeded.

========== Build: 1 succeeded or up-to-date, 0 failed, 0 skipped ==========

#1 This forum helps those that help themselves

#2 All grounds are not created equal

#3 How have you proved that your chip is running at xxMHz?

#4 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

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

Brian Fairchild wrote:
Guess what? It works without any mention of 'volatile'.
I don't get it.

Do you really think that a test with a random (old) compiler version (4.8.1) and a random optimizer setting (-O1) does proof anything?

Stefan Ernst

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

sternst wrote:

Do you really think that a test with a random (old) compiler version (4.8.1) and a random optimizer setting (-O1) does proof anything?

 

 

Instead of sitting there at your keyboard picking holes in people's attempt to help why don't you offer some proof.

 

 

Oh, and I've tried every optimisation setting from -O0 to -O3 and all of them work.

#1 This forum helps those that help themselves

#2 All grounds are not created equal

#3 How have you proved that your chip is running at xxMHz?

#4 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

Last Edited: Mon. Jul 9, 2018 - 02:59 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Brian,

 

Stefan is right. GCC is a fiercely optimising compiler. If not today then tomorrow with a different issue of the compiler or just some small change in one of its 100's of command line switches this code will prove dangerous. It is the programmer's responsibility to make known to the compiler anything that is shared between paths of execution and that is done using "volatile". By some complete fluke it may appear to work in one build with one issue of the compiler and one set of build switches but it's going to bite one day unless it's done properly and for GCC that means making shared items volatile. It's so important that it's been FAQ entry #1 in the GCC manual almost right from the start:

 

http://nongnu.org/avr-libc/user-...

 

and by the same token it's been the #1 issue in my signature for a decade or more for the same reason. It's fundamental to the use of avr-gcc and is one of the most common reasons for "it's not working" threads here on Freaks that involve GCC.

Last Edited: Mon. Jul 9, 2018 - 03:43 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Brian:

Because I took your code and dropped it onto a dev board

That is a nice board...Is it available somewhere??...reminds me of my favorite, defunct, STK500. 

 

https://youtu.be/89g1P_J40JA?t=179

When in the dark remember-the future looks brighter than ever.

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

clawson wrote:

Stefan is right. GCC is a fiercely optimising compiler. ...

 

It'll be interesting if (when?) the OP posts either the .LSS so we can see what's being generated or at least tells us the toolchain they are using. They were using...

 

Quote:

I'm using Atmel AVR 8-bit Toolchain 3.5.4 - Linux 64-bit which includes avr-gcc-4.8.1.

 

...I guess until then it's all speculation.

 

 

#1 This forum helps those that help themselves

#2 All grounds are not created equal

#3 How have you proved that your chip is running at xxMHz?

#4 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

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

avrcandies wrote:

That is a nice board...Is it available somewhere??...reminds me of my favorite, defunct, STK500. 

 

Kanda STK200X.

 

It's my go-to board for testing out code on real hardware. 8 push buttons which can be jumped onto any port. The same with 8 LEDs. Takes just about any DIP AVR and has an onboard FTDI USB USART which comes in handy as a debug channel. And an LCD connector.

 

I made one mod, which can be seen in the picture, in that I remove the soldered in crystal and put in a chopped-up IC socket to allow my to change the oscillator frequency.

 

It is more expensive than an Arudino but it's way more useful as a bench top unit. I was thinking earlier today that I might get a couple more. They are normally sold as a kit with programmer etc but I have bought just the board from Kanda before now.

 

 

Here is the user manual.

Attachment(s): 

#1 This forum helps those that help themselves

#2 All grounds are not created equal

#3 How have you proved that your chip is running at xxMHz?

#4 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

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

Hello... I've been reading the latest posts, and to be honest, it's completely out of my (nearly none) comfort zone! So, can I provide anything from my compilation log or anything you need to help you guys in the discussion that is happening? Let me know and I'll paste here or somewhere else.

 

Anyway, about the original post.

 

I might have mislead my idea when I tried to explain my wiring. I'll try again.

 

So, I have just picked up a spare AtMega328p, placed it on a breadboard along with the crystal and caps and all the basic stuff needed to make it work.

Then, as I don't have spare push-buttons for the reset and for the testing I wanted to do, what I did was to place one of the leads of a 10k ohm pull-up resistor connected to +5VDC line. The other lead of this resistor is somewhere on the bredboard not connected to anything. Then I used a wire to connect this lead of the 10k ohm resistor to one of the 2 pins, PD2 or PD3.

 

PD2\

       \

         \_____10k ohm_____+5VDC

          .

         .

       .

PD3.

 

So, what I wanted to say here is that the thing in red is a wire I can disconnect from PD2 and connect to PD3 just to be able to make the test to the 2 pins with only 1x 10k ohm resistor, instead of having 2x 10k ohm resistor, 1 for each pin.

 

And adding to that drawing I have another wire going from the left lead of the 10k ohm resistor (the one in red) to ground when I want to manually simulate the event of pressing the push-button.

I'm really sorry if my drawing was (is still) poor, but it's the best I can think of to represent what I have on the breadboard.

 

About the code compilation, yes, I'm using Debian 9 repository avr-gcc (GCC) 4.9.2 from within a makefile I downloaded somewhere from the internet. I can also upload the makefile if needed.

Pages