Using Digital Read for Button Presses | Errors

Hi, I'm reasonably new to programming Arduino and I'm having some issues with using digital reads for button presses.

I have a project that was working using the onebutton module. But I required to change some of the button presses to digital reads to get a command out on a press, rather than release of the button.

The below are excerpts of code related to the 5 buttons. In each case, when I verify I'm getting the same error for each:

EDIT: NO LONGER GETTING THE BELOW ERROR AFTER DECLARING NEW FUNCTION WITH VOID.
BUT BUTTON EVENT STILL ISNT WORKING. SERIAL PRINT IS NOT WORKING WHEN BUTTON GOES HIGH.

error: expected unqualified-id before 'if'
if(switchstate1 == HIGH) {
^~

const int footswitchPin1 = 7;                      // setup a new OneButton on pin D7
int switchstate1 = 0; 
const int footswitchPin2 = 8;                      // setup a new OneButton on pin D8
int switchstate2 = 0; 
const int footswitchPin3 = 9;                      // setup a new OneButton on pin D9
int switchstate3 = 0; 
const int footswitchPin4 = 10;                      // setup a new OneButton on pin D10
int switchstate4 = 0;        
const int footswitchPin5 = 11;                      // setup a new OneButton on pin D11
int switchstate5 = 0; 

void setup() {
  pinMode(footswitchPin1, INPUT);
  pinMode(footswitchPin2, INPUT);
  pinMode(footswitchPin3, INPUT);
  pinMode(footswitchPin4, INPUT);
  pinMode(footswitchPin5, INPUT);
}

void loop () {
switchstate1 = digitalRead(footswitchPin1); // Reading status of footswitch 1
switchstate2 = digitalRead(footswitchPin2); // Reading status of footswitch 2
switchstate3 = digitalRead(footswitchPin3); // Reading status of footswitch 3
switchstate4 = digitalRead(footswitchPin4); // Reading status of footswitch 4
switchstate5 = digitalRead(footswitchPin5); // Reading status of footswitch 5

void FS1() {
  if (switchstate1 == HIGH)  {
    Serial.println("HIGH");
    if ( editSettings == 0) {         // recall program 1
      pgmNumber = 1;
      outputPinsState = EEPROM.read(pgmNumber);       // read output switch setting from eeprom for this program number
      for ( int thisOutput = 0; thisOutput < outputCount; thisOutput++ ) {        // set outputs
        if ( bitRead(outputPinsState, thisOutput ) == 1 ) {       // if outputState for this pin is active, set HIGH
          digitalWrite( outputPins[thisOutput], HIGH );
        } else {
          digitalWrite( outputPins[thisOutput], LOW );
        }
      }
      refreshDisplay(21);
    }
  } else {
    editSettings = 1;
    editSwitches = 1;
    currentSwitchIndex = 0 ;
    refreshDisplay(41); //refresh display switch change only
    if ( switchChange == 0 ) {
      switchChange = 1;
    }
  }
}

I think it maybe a formatting issue with the loop code, but I can't work it out.

Here's a copy of the the current sketch if I missed anything in the examples:

https://1drv.ms/u/s!AiHdMCbWd8VnheQKeGOLxOyi2Zc8dA?e=xLJedR

Thank you!

I think you are missing a close brace in the
void loop()

Check your capitalization.

I believe the closing brace is in the sketch, I just didn't copy it in with the excerpts. Where you just referring to the code I copied in or did you download the sketch?

I don't see any problems. Did you see something wrong?

Does it compile and run correctly? No means something is wrong. Welcome fellow coder!

My #1 suggestion is that you learn about arrays and shrink your code to have fewer lines to debug. There's more but if you do this first, everything involved will change including you.

Here is the Arduino Language Reference Page.
Bookmark this in a bookmark folder named Arduino.

Reference Page > arrays

Please for your own sake, in your IDE go through Examples -> 05 Control and play with the examples before you write a pile of code. These are examples using arrays already typed up as short lessons, easy peasy to mess with.

The first error a compiler finds can cascade errors as the compiler gets back on track. Fix the first and wherever else you did the same thing -then- recompile and see what's wrong next. Been there, been wrong 30 times before lunch and just keep debugging since it's worked before and it's GREAT when it does!

The C/C++ standard libraries are at AVR-LibC.
Reference to AVR C/C++ Standard Libraries, bookmark!.

1 Like

I see Many many things wrong:

several extra brackets

  if (switchstate4 == HIGH) {
  {

several partially commented out code blocks:

// button 6 short press: toggle switch output 1
// void Click6() {

  //   if ( editSettings == 0) {          // recall program 4

  if (switchstate4 == HIGH)
  {

click Tools> Auto Format it will show you many of those problems.
Hint: if the closing bracket on a function is not ALL the way to the left there are either too many or too few brackets somewhere above it.

There are a few issues... but the one causing the immediate issue is you have accidentally commented the function declarations.

void Click3,4,5,6

so the code that was in them is now not within a routine...

That commenting is on purpose, I'm replacing the click functions with digital reads instead so I can get a command out on a press, rather then a release of the button.

Well you can't just leave code lying around that was inside the functions previously. Either comment out the entire function, or move the remaining code somewhere else.

For some reason the auto format doesn't work for me in the IDE. I haven't chased it down yet.

Thanks for pointing out for partial comments! I've fixed those up, and wrapped everything in a void/function. It's compiling now at least, but still no working. One step!

void FS1() {

if(switchstate1 == HIGH)  {

   if ( editSettings == 0) {         // recall program 1

            pgmNumber = 1;

            outputPinsState = EEPROM.read(pgmNumber);       // read output switch setting from eeprom for this program number

            for ( int thisOutput = 0; thisOutput < outputCount; thisOutput++ ) {        // set outputs

              if ( bitRead(outputPinsState, thisOutput ) == 1 ) {       // if outputState for this pin is active, set HIGH

                digitalWrite( outputPins[thisOutput], HIGH );

              } else {

                digitalWrite( outputPins[thisOutput], LOW );

              }

            }

            refreshDisplay(21);
          }
          } else {
            
      editSettings = 1; 
      editSwitches = 1; 
      currentSwitchIndex = 0 ;
      refreshDisplay(41); //refresh display switch change only
   
      if ( switchChange == 0 ) {

        switchChange = 1;

      }
    
    }

  }

Hi, it wasn't compiling, but I've now declared the button as a function using void and it's now compiling. Example in another comment.

I'll take a look through some of the examples on arrays, I appreciate the time you took to respond. Thanks.

1 Like

OK, I was using the guts of the old function though. Sorry for the confusion, this sketch is a work in progress so I didn't want to move too much around or delete anything.

About 1/3 down the sketch, a function ends (longPressStart2) and without a new function, the "if()" statements start (this is where you are seeing errors). You copy/pasted this from an HTML document when a page-break occurred, losing code.

YOUR auto format does not work because the code does not have correct function termination.

The following is a code snip. The first close-brace you see is the end of the function longPressStart2.... then the "if" statements start... without being inside another function.

}

// button 3 short press: toggle switch output 1
//void Click3() {

if(switchstate1 == HIGH)  {
1 Like

Hello grelbatron
Take some time and describe the desired functionality of the sketch in simple words and this very simple and short form, including all dependencies between inputs and outputs.

Have a nice day and enjoy coding in C++.
Дайте миру шанс!

Thank you. I don't think any of the sketch was copied and pasted form a HTML source. I think maybe that's my poor attempt at trying to implement the digital reads. After the longPressStart2 function is where I'm starting to redefine some of the old click functions.

I've declared the new button presses as functions using void, but I'm still looking into a few issues. It's at least compiling now., but im unsure if it's a bum steer to do this. Does this seem like an appropriate step forward?

void FS2() {

  if(switchstate2 == HIGH)  {
          Serial.println("HIGH");
          if ( editSettings == 0) {       // recall program 2
            
            pgmNumber = 2;

            outputPinsState = EEPROM.read(pgmNumber);       // read output switch setting from eeprom for this program number

            for ( int thisOutput = 0; thisOutput < outputCount; thisOutput++ ) {        // set outputs

              if ( bitRead(outputPinsState, thisOutput ) == 1 ) {       // if outputState for this pin is active, set HIGH

                digitalWrite( outputPins[thisOutput], HIGH );

              } else {

                digitalWrite( outputPins[thisOutput], LOW );

              }

            }

            refreshDisplay(21);
          }
          } else {
            
      editSettings = 1; 
      editSwitches = 1;
      currentSwitchIndex = 1 ;
      refreshDisplay(41); //refresh display switch change only
   
      if ( switchChange == 0 ) {

        switchChange = 1;

      }
    
    }

  }

Hey, great! Post your new, full, code. A billion bits are only eaten one byte at a time. : )

1 Like

Hi Paul, thanks for takin the time to respond. I think the desired functionality should be pretty well scoped in the notes at the top of the program. But in short, the code is to enable 5 relays to be switched independently via or 5 momentary buttons. Essentially the button input will either trigger a a single relay state directly, or recall a preset eeprom value for all 5 relays.

I would, FIRST, clean up your code to make it easier to read. Of the 1,600 lines, there are about 400 commented-out lines doing nothing. Delete them (you can always reference old code to include them if they become necessary). Auto-Format the braces (functions). Remove the empty lines (probably 800 lines of blanks). Pretty code is happy code, and happy code makes happy results.

2 Likes

Hello grelbatron
The last state of the relays should be read during the setup() call.

1 Like