Buttoning Up this button code: Increment Buttons with Interrupt Service Routine

Using Arduino Programming Questions
1

What should happen:

Each button press,(X 's are buttons), increments the buttonPushCounterOn value or buttonPushCounteroff value up by one, then restarts after 60. The incremented value of seconds On or Seconds Off prints to LCD accordingly. buttonPushCounterOn value is multiplied by 1,000 to give SecondsOn for use elsewhere. buttonPushCounteroff value is multiplied by 10,000 to give MinutesOff for use elsewhere.

It had been working inconsistently in another program so I took it out to improve it. Now I can't get it to compile. " expected unqualified id before string constant".

Here’s what the interface looks like:
_LCD
| |
[u]| **S.ON / **M.Off|[/u]
X X
On Off

Here is the code:

 //Button Incrementor
      volatile int  buttonPin = 0;  //Uno 2    // the pin that the ON pushbutton is attached to
      volatile int  buttonPin1 = 1; //Uno 3   // the pin that the OFF pushbutton is attached to

 // Variables will change:
 long int buttonPushCounterOn = 0;            // counter for the number of button presses
 long int buttonPushCounterOff = 0;           // counter for the number of button presses
 int buttonStateOn5 = 0;                      // current state of the button
 int buttonStateOff6 = 0;                     // current state of the button
 int lastButtonState = 0;                     // previous state of the button
 int lastButtonStateOff = 0;                  // previous state of the button
 
 
  // include the library code:  
 #include <LiquidCrystal.h>
 // initialize the library with the numbers of the interface pins
 LiquidCrystal lcd (7,8,9,10,11,12);          //11RS,12E,13D4,14D5,15D6,16D7

 void setup(){

    pinMode(buttonPin, INPUT);
    pinMode(buttonPin1, INPUT);
    
    lcd.begin(16,2);                          // set up the LCD's number of columns and rows:
    lcd.print("www.MISTFITZ.com");            // Print a message to the LCD.
      delay(4000);
    lcd.clear();
    lcd.setCursor(2,1);
    lcd.print("-S.ON");
    lcd.setCursor(10,1);
    lcd.print("-M.OFF");
    
    
    attachInterrupt(buttonPin, ISR ,RISING);
    attachInterrupt(buttonPin1, ISR ,RISING); }
    
void ISR()
          { 
 
  // read the pushbutton ON input pin:
      buttonStateOn5 = digitalRead(buttonPin);
          // compare the buttonState to its previous state
            if (buttonStateOn5 != lastButtonState) 
          // if the state has changed, increment the counter
            if (buttonStateOn5 == HIGH)
         buttonPushCounterOn+=1;
            if(buttonPushCounterOn > 60)buttonPushCounterOn =0; // rollover after 60. 
     lastButtonState = buttonStateOn5; // save the current state as the last state, for next time through the loop
           
           lcd.setCursor(0,1);
           lcd.print("  ");

           lcd.setCursor(0,1);
           lcd.print(buttonPushCounterOn,DEC);
          
  // read the pushbutton OFF input pin:
      buttonStateOff6 = digitalRead(buttonPin1);
          // compare the buttonState to its previous state
            if (buttonStateOff6 != lastButtonStateOff) 
          // if the state has changed, increment the counter
            if (buttonStateOff6 == HIGH)
         buttonPushCounterOff+=1;
            if(buttonPushCounterOff > 60)buttonPushCounterOff =0;   // rollover after 60. 
      lastButtonStateOff = buttonStateOff6; // save the current state as the last state, for next time through the loop
      
           lcd.setCursor(8,1);
           lcd.print("  ");
           lcd.setCursor(8,1);
           lcd.print(buttonPushCounterOff,DEC);
         }
           
           
void loop(){
    // Timing of the Solenoid   

        delay(buttonPushCounterOn * 1000);                    // buttonPushCounterOn * 1000 gives us seconds. 

        delay(buttonPushCounterOff * 10000);      }            // buttonPushCounterOff * 1000 gives us seconds.  
}
2

And the syntax error is at which line?

Mark

3

Try Auto Format on the code and you will get a clue to one problem, but the more subtle problem seems to be the name of the ISR

4

but the more subtle problem seems to be the name of the ISR

:smiley: ooch!!

Mark

5

Copy and paste the FULL error message - there is all ways a line number.

Clue to the IDE will place the cursor on the line of the error message.

I can't just build your code as I don't have the same lib installed, so you have to do some work.

Mark

6

A few hints

  1. keep the ISR short. while your in the ISR other interrupt driven things such as mills, serial print/write may not happen. Move the lcd writes/printsetc out of the isr as they take a long time to execute.

  2. if you change a variable in the ISR make sure that it is declared as volatile.

  3. ditch the delays take a look at blink without delay and then in the playground at finite state machines (FSM).

Mark

7

After you had "fixed the right bracket thing" did you not think to Auto Format the code again ?

As to your current problem. How are the buttons wired ? Do you have any pull up or pull down resistors in the buttons to keep them at a known voltage, or are the button pins just floating in the breeze when the button is not pressed, free to pick up any stray voltage that pasees by ?

As to the ISR. Remove all the code from it apart from the small portion that updates the counters, set a flag to indicate that the update has occurred and deal with the update in the loop() function. Better still, create separate ISRs for each button. Then you don't need to read the buttons in the ISR at all. Even better still, unless you are doing this to learn about ISRs, don't use them at all. Read the button pins, update the variables and display them, all in the loop() function.

8

The loop() function is doing practically nothing. Why are you not just polling the pins? You have to poll them anyway, when the ISR triggers to figure out which one caused the ISR to fire.

This looks like an exercise in how to use interrupts. If they were really necessary, you'd be using better names, and TWO interrupt service routines.

9

That’s why I couldn't use Blink Without Delay option. The on millis are the same as the off millis.

There is no reason why you cannot use the BWD technique with asymmetrical on/off periods.

const int ledPin = 13;
int ledState = LOW;
unsigned long previousMillis = 0;
unsigned long interval = 1000;

void setup() 
{
  pinMode(ledPin, OUTPUT);      
}

void loop()
{
  unsigned long currentMillis = millis();
  if(currentMillis - previousMillis > interval) 
  {
    previousMillis = currentMillis;   
    ledState = !ledState;
    digitalWrite(ledPin, ledState);

    if (interval == 1000)
    {
      interval = 100;
    }
    else
    {
      interval = 1000;
    }
  }
}

As to the error in your current code, you have inserted the debounce function in the wrong place

void setup()
boolean debounce (boolean last)

Where is the { which starts the setup() function code ?