Basic two button project working 90% of the time...GRRRR

Projects General Guidance
1

So i have a very basic little program that is for a drive through.
The idea being that a member presses a button to get service if someone is not immediately there to assist them. When there is movement on a tray the magnetic contractor opens indicating that the person is being helped and that we need to stop the buzzing.

What hardware i use:
I have a LCD screen that just flashes a message when the member pushes a button.
One button (that closes) that gets pressed by the member/person wanting service.
A buzzer that only goes off at the initial button press and then every 30s for 20 buzzes regardless on how many times they press the button (to keep people sane on the inside).
I also have a magnetic contact switch (that opens) when a drawer gets pushed out to a member for service so i use this to tell the controller that the person is being served.
On both switches i have a pull down resistor. - can provide a schematic if needed

Here is my entire debug code...
WARNING
I am not a programmer, i am sure i do things very badly. :slight_smile:

#include <LiquidCrystal.h>

//start of config variables
String line1 = "Someone will be";
String line2 = "with you shortly";
long ms_interval = 30000;
//end of config variables

bool showmessage = false;
bool timerrunning = false;
unsigned long currentmills;
bool addressed = false;
int buzzcounter =1;
bool messageblink = false;
LiquidCrystal lcd(8, 9, 7, 6, 5, 4);

void setup() {

    
    timerrunning = false;
    // put your setup code here, to run once:
    analogWrite(11, 20); //LCD Contrast
    lcd.begin(16, 2);
    pinMode(2, INPUT); //Member Push Button
    pinMode(10, OUTPUT); //Backlight Button
    pinMode(12, OUTPUT); //Bell or buzzer
    attachInterrupt(0, MemberPressISR, RISING); //Digital input 2
    attachInterrupt(1, TrayMovementISR, FALLING); //Digital input 3
  
    Serial.begin(9600);
    while (!Serial) {
      ; // wait for serial port to connect. Needed for native USB port only
    }
    Serial.println("Serial Started");
}

void loop() 
          {
            
              // put your main code here, to run repeatedly:
              digitalWrite(10, LOW); //turns on the LCD Backlight
              
              
              if (showmessage == true && timerrunning != true )// they pressed the button and they have not pressed it before
                {
                  Serial.println("showmessage == true && timerrunning != true");
                  
                  //start the countdown timer
                  currentmills = millis();
                  timerrunning = true;
                  //show they message on the LCD screen to acknowledge the button press
                  messageblink = true;
                  // initial BEEP               
                  digitalWrite(12, HIGH);
                  delay(250);
                  digitalWrite(12, LOW);
                  
                }
            
              if (timerrunning)//if the timer is running check that we are not over the delay interval
                {
                  Serial.println("Timer Running");
                  
                    if (millis() - currentmills > ms_interval && addressed != true)//if we are over the first interval period then buzz
                      {
                        Serial.println("Over the first 30s interval");
                        
                        digitalWrite(12, HIGH);
                        delay(250);
                        digitalWrite(12, LOW);
                        currentmills = millis();
                        buzzcounter = buzzcounter + 1; //increment buzz counter
                      }
                    else if (currentmills > millis()) //if the counter has looped around during a press just buzz
                      {
                        Serial.println("Just Buzz - counter has looped around");
                        
                        digitalWrite(12, HIGH);
                        delay(250);
                        digitalWrite(12, LOW);
                        currentmills = millis();
                        buzzcounter = buzzcounter + 1; //increment buzz counter
                      }
                    
                    if (buzzcounter > 20) //the buzzer has been going for 20 = 10 minutes then turn off buzzer
                    {
                      Serial.println("Buzzed more than 20 times");
                      
                      timerrunning = false;
                      buzzcounter = 0;
                    } 
                    
                }
                
           if (addressed == true)
                      {
                        Serial.println("Addressed = true");
                        delay(2000); //  for debouncing on the drawer/Tray switch ?
                        
                        timerrunning = false;
                        buzzcounter = 0;
                        messageblink = false; 
                        addressed = false;
                        showmessage = false;
                        
                      }
                
            if (messageblink)
            {     
                  Serial.println("Blinking Message - On - Off 500ms - On");
                  
                  //On period
                  lcd.display();
                  digitalWrite(10, HIGH); //turns on the LCD Backlight
                  lcd.setCursor(0, 0);
                  lcd.print(line1);
                  lcd.setCursor(0, 1);
                  lcd.print(line2);
                  delay(500);
                  //Off Period
                  lcd.noDisplay(); // turns off the backlight
                  delay(500);
                  lcd.display();                
                  digitalWrite(10, HIGH); //turns on the LCD Backlight               
                  lcd.setCursor(0, 0);
                  lcd.print(line1);
                  lcd.setCursor(0, 1);
                  lcd.print(line2);
                  lcd.noDisplay();
            
            }

         }

void MemberPressISR() {
  showmessage = true;

}
void TrayMovementISR() {
  addressed = true;
  
}

From serial monitor debugging i have received the following responses which to me does not make sense at all...
I was actually there when this happend, no one pressed the button for at least 10 min and then just randomly the buzzer started there was tray movement, but then without someone being there to push the button the buzzer started... how is showmessage being set to true without a button press?

Serial Started
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
showmessage == true && timerrunning != true
Timer Running
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
showmessage == true && timerrunning != true
Timer Running
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
showmessage == true && timerrunning != true
Timer Running
Addressed = true
Addressed = true
Addressed = true
showmessage == true && timerrunning != true
Timer Running
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
showmessage == true && timerrunning != true
Timer Running
Blinking Message - On - Off 500ms - On
Timer Running
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
showmessage == true && timerrunning != true
Timer Running
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
showmessage == true && timerrunning != true
Timer Running
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
showmessage == true && timerrunning != true
Timer Running
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true
Addressed = true

Any help or advice would be greatly appreciated.
Thanks

2

You are using interrupts to detect button presses:

    attachInterrupt(0, MemberPressISR, RISING); //Digital input 2
    attachInterrupt(1, TrayMovementISR, FALLING); //Digital input 3

How have you wired the connections to pins 2 & 3 ? These should not float and should have pullup or pulldown resistors otherwise you may get spurious 'button presses'.

3

What value of resistor? Could it be EMI triggering false signals?

4

Hi,

You are using your button to pull your pushbutton input up to 5V.

You need a 10K resistor between gnd and the push-button input pin2 to pull the input LOW when the button is open circuit.

Also

 pinMode(2, INPUT); //Member Push Button
    pinMode(10, OUTPUT); //Backlight Button
    pinMode(12, OUTPUT); //Bell or buzzer

make your pin numbers descriptive

byte PushbuttonPin = 2;
byte BacklightPin = 10;
byte BuzzerPin = 12;

In setup make

pinMode(PushbuttonPin, INPUT); //Member Push Button
pinMode(BacklightPin, OUTPUT); //Backlight Button
pinMode(BuzzerPin, OUTPUT); //Bell or buzzer

That way this

 digitalWrite(10, LOW); //turns on the LCD Backlight

Becomes this

 digitalWrite(BacklightPin, LOW); //turns on the LCD Backlight

And you don't need the comment, your code becomes self explanatory.

Before you repost your code, in the IDE press CTRL-T, it will format your code into logical indents.

Tom.. :slight_smile:

5

DrAzzy:
What value of resistor?

10k is common; anything 1-100k will work.

Alternative: wire your button between GND and the pin, then use INPUT_PULLUP, enabling the internal pull-up resistors. No need for external components but your button becomes active LOW so you have to reverse your logic.

Could it be EMI triggering false signals?

On a floating pin, that's possible.

6

6v6gt:
You are using interrupts to detect button presses:

    attachInterrupt(0, MemberPressISR, RISING); //Digital input 2

attachInterrupt(1, TrayMovementISR, FALLING); //Digital input 3



How have you wired the connections to pins 2 & 3 ? These should not float and should have pullup or pulldown resistors otherwise you may get spurious 'button presses'.

Yes I have pulldown resistors on both pins (2,3)

7

Thanks TomGeorge for the tips. Will do that in the future :slight_smile:

Thanks everyone for the repsonses, wow i am quite amazed, i was expecting two views and an add post :slight_smile:

Not sure how i should proceed since most people are asking about pulldown resistors and i dont want to write 4 responses saying that same thing, how do quote multiple people in the same post?

But basically i do have my pulldown resistors 10k (i always add them in for inputs).
What i was thinking is should i add a capacitor in parallel with the resistor so that it will absorb any switch bouncing? Or should i reduce my pulldown resistor such that it is more immune to noise?

I could have missed something but from my perspective that was the one possible cause suggested above, pull down resistors (silly me i should have mentioned it).

I guess i could try wvmarle 's suggestion and use the internal pull up resistors but i already have the resistors soldered in... worth a try?

Any other ideas/suggestions?

Thanks again for all the responses :slight_smile:

8

vermin_sapper:
4 responses saying that same thing, how do quote multiple people in the same post?

First of all: you don't have to as we all will read all the responses.
Secondly: there's a "Quote" button under each post, click it and you'll get it added to the editor at the bottom.

What i was thinking is should i add a capacitor in parallel with the resistor so that it will absorb any switch bouncing?

That will help with bounce, but the symptoms you describe are not consistent with button bounce. That's when you get multiple on/off readings on a single button press. Your symptoms are much more consistent with floating pins, which is why you got all those questions on pull-up/pull-down resistors.

Or should i reduce my pulldown resistor such that it is more immune to noise?

Unless you have a particularly noisy environment (e.g. a pinball machine with all its motors and other electromagnets) and/or really long wires to your buttons, a 10k (or the ~20k internal pull-up) is normally good enough.

I guess i could try wvmarle 's suggestion and use the internal pull up resistors but i already have the resistors soldered in... worth a try?

I'd leave them in and try it for your next project, as to use the internal pull-up resistors you have to resolder your buttons (to connect to GND) and remove the existing pull-down resistors. It's really not worth changing your existing project for it. Both methods should work just fine. The main advantage of using the internal resistors is that you save on external components (cheaper, less work, less board space).

9

You have another problem which could cause random behaviour. It is related to your use of interrupts to detect button presses. It is unusual to do this and normally a button is simply polled. However, then, the code is much more sensitive to delay statements because the button presses may occur during a delay period and be missed.

Anyway, any variable which is updated in an Interrupt service routine ( e.g. showmessage ) must be declared as volatile to warn the compiler not to use a copy of it, which may get out of date.

10

wvmarle:
First of all: you don't have to as we all will read all the responses.
Secondly: there's a "Quote" button under each post, click it and you'll get it added to the editor at the bottom.
That will help with bounce, but the symptoms you describe are not consistent with button bounce. That's when you get multiple on/off readings on a single button press. Your symptoms are much more consistent with floating pins, which is why you got all those questions on pull-up/pull-down resistors.
Unless you have a particularly noisy environment (e.g. a pinball machine with all its motors and other electromagnets) and/or really long wires to your buttons, a 10k (or the ~20k internal pull-up) is normally good enough.

I'd leave them in and try it for your next project, as to use the internal pull-up resistors you have to resolder your buttons (to connect to GND) and remove the existing pull-down resistors. It's really not worth changing your existing project for it. Both methods should work just fine. The main advantage of using the internal resistors is that you save on external components (cheaper, less work, less board space).

Thanks good info to have. Will also keep it in mind for future projects.

6v6gt:
You have another problem which could cause random behaviour. It is related to your use of interrupts to detect button presses. It is unusual to do this and normally a button is simply polled. However, then, the code is much more sensitive to delay statements because the button presses may occur during a delay period and be missed.

Anyway, any variable which is updated in an Interrupt service routine ( e.g. showmessage ) must be declared as volatile to warn the compiler not to use a copy of it, which may get out of date.

Hmm interesting point so you are suggesting declaring my global variables as follows
from

//end of config variables

bool showmessage = false;
bool timerrunning = false;
unsigned long currentmills;
bool addressed = false;

to

//end of config variables

volatile bool showmessage = false;
bool timerrunning = false;
unsigned long currentmills;
volatile  bool addressed = false;

that way where ever it is used in the code the compiler will use the actual variable and not a copy of the variable... hmm that never would have occurred to me. Will give that a try. Thanks

11

Hi,
As @6V6GT has said, the use of interrupts to detect button presses is unusual.

To make you code easier to implement, forget using interrupts.

All you need to do is read the inputs at the start of void loop(), then in the rest of your code act on the result of that read.

Its called polling, the speed of your controller will mean you will not miss even a 10mS press of the button.

You are worried that the code will miss someone pressing the button, believe me for your relatively short bit of code and the controller speed, the time it takes for the button, after pressing, to rebound under the button spring pressure is not quick enough to be missed.

This is a good simple exercise in utilizing interrupts, but it is really over kill.

Tom... :slight_smile:

12

TomGeorge:
Hi,
As @6V6GT has said, the use of interrupts to detect button presses is unusual.

To make you code easier to implement, forget using interrupts.

All you need to do is read the inputs at the start of void loop(), then in the rest of your code act on the result of that read.

Its called polling, the speed of your controller will mean you will not miss even a 10mS press of the button.

You are worried that the code will miss someone pressing the button, believe me for your relatively short bit of code and the controller speed, the time it takes for the button, after pressing, to rebound under the button spring pressure is not quick enough to be missed.

This is a good simple exercise in utilizing interrupts, but it is really over kill.

Tom... :slight_smile:

Thanks Tom will try that first. :slight_smile:

13

In your first post you said:-

can provide a schematic if needed

In the absence of one, just to check, when you say you have a pull down resistor. This is connected directly between the input pin and ground isn't it? And your button connected also to the input pin and the other end to 5V. Only we have seen this configuration interpreted incorrectly before.

14

Something i noticed while changing the code is that i never declared the pinmode as output for pin #3 - the tray movement switch... going to change the code to not use interrupts but just thought i would share my findings before the code change

Grumpy_Mike:
In your first post you said:-In the absence of one, just to check, when you say you have a pull down resistor. This is connected directly between the input pin and ground isn't it? And your button connected also to the input pin and the other end to 5V. Only we have seen this configuration interpreted incorrectly before.

Yes Mike the resistors are connected directly to pins 2,3 and ground (each having their own resistor).

15

Well i don't want to jinx it i think the polling instead of using interrupts solved the issue. Still not sure i understand why it solved.
I get the difference between polling and interrupts and that using interrupts is unusual in this instance but say if i had a bigger chunk of code such that polling was not an option then wouldn't i resort to interrupts?
Maybe it was because of what @6v6gt said that the variable needs to be declared as volatile bool...
also could it have been that i did not declare the pinmode for the one input, member button (see original code at the top of the code, drawer/tray switch was never declared as input) ?

Neways thanks for helping me out.
Much appreciated, hopefully it stays resolved :slight_smile:

16

The power-on default mode of the output pins is INPUT, with the internal pull-up resistors turned off. If you have it set it to output, and you try to read the button, you will read the state of the pin - HIGH or LOW as set by you. So not explicitly setting a pin to INPUT should not make a difference (but it's better to do so, if only for clarity).

The bool not being volatile is more likely to be the cause of your problems.

17

@wvmarle Thanks - good to know.

Project update - so i changed the interrupts and also the volatile bool and it is now working 100%.
And its been running about a week or more, so i would like to say thanks to everyone for helping me out.

Marked as Solved :slight_smile:

Thanks :slight_smile: