If DigitalRead Function Error - expression cannot be used as a function

Hi, beginner programmer here...

I get the following error while trying to upload the following script:

Multiple_Lights_1PB__4LED_v3:93: error: expression cannot be used as a function

(digitalRead(ledPin_Option_3) == LOW )

Any clues?

Source:

/*Using a Single Button, create mutliple options based on how long the button is pressed

/* Modified to: read state of light to light up a serie of LED one by one */

The circuit:

  • LED attached from pin 13 to ground through a 220 ohm resistor

  • LED attached from pin 12 to ground through a 220 ohm resistor

  • one side of momentary pushbutton attached to pin 2

  • other side of momentary pushbutton attached to Ground

  • Note 1: on most Arduinos there is already an LED on the board
    attached to pin 13.

  • Note 2: In this circuit, when the button is pressed, Ground Voltage is what will be applied.

Created DEC 2014 by Scuba Steve
Modified JAN 2015 by Michael James
Both members of https://programmingelectronics.com

This code is in the public domain
*/

/////////Declare and Initialize Variables////////////////////////////

//We need to track how long the momentary pushbutton is held in order to execute different commands
//This value will be recorded in seconds
float pressLength_milliSeconds = 0;

// Define the minimum length of time, in milli-seconds, that the button must be pressed for a particular option to occur
int optionOne_milliSeconds = 100;
// int optionTwo_milliSeconds = 2000;
int optionThree_milliSeconds = 3000;
int optionFour_milliSeconds = 4000;
int optionFive_milliSeconds = 2000;

//The Pin your button is attached to
int buttonPin = 2;

//Pin your LEDs are attached to
int ledPin_Option_1 = 13;
int ledPin_Option_2 = 12;
int ledPin_Option_3 = 11;
int ledPin_Option_4 = 10;

void setup(){

// Initialize the pushbutton pin as an input pullup
// Keep in mind, when pin 2 has ground voltage applied, we know the button is being pressed
pinMode(buttonPin, INPUT_PULLUP);

//set the LEDs pins as outputs
pinMode(ledPin_Option_1, OUTPUT);
pinMode(ledPin_Option_2, OUTPUT);
pinMode(ledPin_Option_3, OUTPUT);
pinMode(ledPin_Option_4, OUTPUT);

//Start serial communication - for debugging purposes only
Serial.begin(9600);

} // close setup

void loop() {

//Record roughly the tenths of seconds the button in being held down
while (digitalRead(buttonPin) == LOW ){

delay(100); //if you want more resolution, lower this number
pressLength_milliSeconds = pressLength_milliSeconds + 100;

//display how long button is has been held
Serial.print("ms = ");
Serial.println(pressLength_milliSeconds);

}//close while

//Different if-else conditions are triggered based on the current state of DigitalRead
//Start with the longest time option first

//Option OFF - Execute the 5th option (LOW) if the button is held for the correct amount of time
if (pressLength_milliSeconds >= optionFive_milliSeconds){

digitalWrite(ledPin_Option_1, LOW);
digitalWrite(ledPin_Option_2, LOW);
digitalWrite(ledPin_Option_3, LOW);
digitalWrite(ledPin_Option_4, LOW);

}

// If no lights are HIGH make the 1st LED only = HIGH
if (digitalRead(ledPin_Option_1) == LOW )
(digitalRead(ledPin_Option_2) == LOW )
(digitalRead(ledPin_Option_3) == LOW )
(digitalRead(ledPin_Option_4) == LOW ) {

digitalWrite(ledPin_Option_1, HIGH);
digitalWrite(ledPin_Option_2, LOW);
digitalWrite(ledPin_Option_3, LOW);
digitalWrite(ledPin_Option_4, LOW);
}

//Option OFF - Execute the 5th option (LOW) if the button is held for the correct amount of time
if (pressLength_milliSeconds >= optionFive_milliSeconds){

digitalWrite(ledPin_Option_1, LOW);
digitalWrite(ledPin_Option_2, LOW);
digitalWrite(ledPin_Option_3, LOW);
digitalWrite(ledPin_Option_4, LOW);

}

// If LED 1 is HIGH make the 2nd LED = HIGH
if (digitalRead(ledPin_Option_1) == HIGH )
(digitalRead(ledPin_Option_2) == LOW )
(digitalRead(ledPin_Option_3) == LOW )
(digitalRead(ledPin_Option_4) == LOW ) {

digitalWrite(ledPin_Option_1, HIGH);
digitalWrite(ledPin_Option_2, HIGH);
digitalWrite(ledPin_Option_3, LOW);
digitalWrite(ledPin_Option_4, LOW);
}

//Option OFF - Execute the 5th option (LOW) if the button is held for the correct amount of time
if (pressLength_milliSeconds >= optionFive_milliSeconds){

digitalWrite(ledPin_Option_1, LOW);
digitalWrite(ledPin_Option_2, LOW);
digitalWrite(ledPin_Option_3, LOW);
digitalWrite(ledPin_Option_4, LOW);

}

// If LED 1,2 are HIGH make the 3rd LED = HIGH
if (digitalRead(ledPin_Option_1) == HIGH )
(digitalRead(ledPin_Option_2) == HIGH )
(digitalRead(ledPin_Option_3) == LOW )
(digitalRead(ledPin_Option_4) == LOW ) {

digitalWrite(ledPin_Option_1, HIGH);
digitalWrite(ledPin_Option_2, HIGH);
digitalWrite(ledPin_Option_3, HIGH);
digitalWrite(ledPin_Option_4, LOW);
}

//Option OFF - Execute the 5th option (LOW) if the button is held for the correct amount of time
if (pressLength_milliSeconds >= optionFive_milliSeconds){

digitalWrite(ledPin_Option_1, LOW);
digitalWrite(ledPin_Option_2, LOW);
digitalWrite(ledPin_Option_3, LOW);
digitalWrite(ledPin_Option_4, LOW);

}

// If LED 1,2,3 are HIGH make the 4th LED = HIGH
if (digitalRead(ledPin_Option_1) == HIGH )
(digitalRead(ledPin_Option_2) == HIGH )
(digitalRead(ledPin_Option_3) == HIGH )
(digitalRead(ledPin_Option_4) == LOW ) {

digitalWrite(ledPin_Option_1, HIGH);
digitalWrite(ledPin_Option_2, HIGH);
digitalWrite(ledPin_Option_3, HIGH);
digitalWrite(ledPin_Option_4, HIGH);
}

//Option OFF - Execute the 5th option (LOW) if the button is held for the correct amount of time
if (pressLength_milliSeconds >= optionFive_milliSeconds){

digitalWrite(ledPin_Option_1, LOW);
digitalWrite(ledPin_Option_2, LOW);
digitalWrite(ledPin_Option_3, LOW);
digitalWrite(ledPin_Option_4, LOW);

}

//close if options

//every time through the loop, we need to reset the pressLength_Seconds counter
pressLength_milliSeconds = 0;

} // close void loop

Thank you, Regards

Leaving aside the fact that the code posted will not compile due to nested block comments, what is this block of code intended to do ?

  if (digitalRead(ledPin_Option_1) == LOW )
     (digitalRead(ledPin_Option_2) == LOW ) 
     (digitalRead(ledPin_Option_3) == LOW )
     (digitalRead(ledPin_Option_4) == LOW )

It looks like you are trying to test the state of several pins and do something if they are all LOW or maybe if any of them are LOW. Which is it, if either ?

The multiple conditions for an if must be enclosed in the same brackets but you have not done this.

UKHeliBob:
It looks like you are trying to test the state of several pins and do something if they are all LOW ?

Correct. That is what I intend to do.

The multiple conditions for an if must be enclosed in the same brackets but you have not done this.

I already tried that, but again I am not a programmer. Could you give me a syntaxe case example for this particular block?

Thank you,
Regards

if (digitalRead(ledPin_Option_1) == LOW  && digitalRead(ledPin_Option_2) == LOW && digitalRead(ledPin_Option_3) == LOW && digitalRead(ledPin_Option_4) == LOW )
{
//do some stuff
}

Please use code tags (</> button on the toolbar) when posting code or error/warning messages, not quote tags as you did above.

Hi ardy_guy, thank you so much for the reply.

Your suggestion doesn’t seems to work even though there are no compilation errors.

What’s funny is that my pushbutton return inverted values with INPUT_PULLUP. It returns 1 when idle and 0 when pressed. Could it be a wiring issue on my breadboard?

Coming back to the code, I can’t seems to achieve the purpose of the loop. No lights will go HIGH with the following code. The wiring doesn’t seems to be the problem.

Once again, a newbe here… thank you for your patience.

Any clues?
Is there a more concise way to code this?

/////////Declare and Initialize Variables////////////////////////////

//The Pin your button is attached to
int buttonPin = 2;

// variables will change:
int buttonState = 1;         // variable for reading the pushbutton status

//Pin your LEDs are attached to
int ledPin_Option_1 = 13;
int ledPin_Option_2 = 12;
int ledPin_Option_3 = 11;
int ledPin_Option_4 = 10;


//Variables will change
int ledState_Option_1 = LOW;
int ledState_Option_2 = LOW;
int ledState_Option_3 = LOW;
int ledState_Option_4 = LOW;


void setup() {

  // Initialize the pushbutton pin as an input pullup
  // Keep in mind, when pin 2 has ground voltage applied, we know the button is being pressed
  pinMode(buttonPin, INPUT_PULLUP);     

  //set the LEDs pins as outputs
  pinMode(ledPin_Option_1, OUTPUT); 
  pinMode(ledPin_Option_2, OUTPUT); 
  pinMode(ledPin_Option_3, OUTPUT); 
  pinMode(ledPin_Option_4, OUTPUT);

  //Start serial communication - for debugging purposes only
  Serial.begin(9600);  

}

void loop() {

////////////////PUSH BUTTON STATE
  // read the input pin of button:
  int buttonState = digitalRead(buttonPin);

/*  
  // print out the state of the button:
  Serial.println(buttonState);
  delay(1);        // delay in between reads for stability

 */ 

    // read the state of the pushbutton value:
  buttonState = digitalRead(buttonPin);

  
//////////////////LED OPTION 1 - STATE
    // read the input pin of LED:
  int ledState_Option_1 = digitalRead(ledPin_Option_1);

/* 
  // print out the state of the LED:
  Serial.println(ledState_Option_1);
  delay(1);        // delay in between reads for stability
*/

    // read the state of the LED value:
  ledState_Option_1 = digitalRead(ledPin_Option_1);

  


/////////////////LED OPTION 2 - STATE  
    // read the input pin of LED:
  int ledState_Option_2 = digitalRead(ledPin_Option_2);

/*
  // print out the state of the LED:
  Serial.println(ledState_Option_2);
  delay(1);        // delay in between reads for stability
*/

    // read the state of the LED value:
  ledState_Option_2 = digitalRead(ledPin_Option_2);

  
  

////////////////LED OPTION 3 - STATE  
    // read the input pin of LED:
  int ledState_Option_3 = digitalRead(ledPin_Option_3);

/* 
  // print out the state of the LED:
  Serial.println(ledState_Option_3);
  delay(1);        // delay in between reads for stability

*/
    // read the state of the LED value:
  ledState_Option_3 = digitalRead(ledPin_Option_3);

  


///////////////LED OPTION 4 - STATE  
    // read the input pin of LED:
  int ledState_Option_4 = digitalRead(ledPin_Option_4);

/* 
  // print out the state of the LED:
  Serial.println(ledState_Option_4);
  delay(1);        // delay in between reads for stability

*/
    // read the state of the LED value:
  ledState_Option_4 = digitalRead(ledPin_Option_4);

  

//////////////////////////////////////////////////////////////////////////////////////////////////

 

  // check if the pushbutton is pressed.
  // if it is, the buttonState is LOW
  // If no ledState_Option are HIGH make the 1st LED only = HIGH
  if (( buttonState == LOW) && (digitalRead(ledState_Option_1) == LOW ) &&  (digitalRead(ledState_Option_2) == LOW ) && (digitalRead(ledState_Option_3) == LOW ) && (digitalRead(ledState_Option_4) == LOW )) 
 
  {  
    digitalWrite(ledPin_Option_1, HIGH);
    digitalWrite(ledPin_Option_2, LOW);
    digitalWrite(ledPin_Option_3, LOW);
    digitalWrite(ledPin_Option_4, LOW);
  }


    // check if the pushbutton is pressed.
  // if it is, the buttonState is LOW 
  // If ledState_Option_1 HIGH make the 1st & 2nd LED only = HIGH
  if (( buttonState == LOW) && (digitalRead(ledState_Option_1) == HIGH ) &&  (digitalRead(ledState_Option_2) == LOW ) && (digitalRead(ledState_Option_3) == LOW ) && (digitalRead(ledState_Option_4) == LOW )) 
  
  {  
    digitalWrite(ledPin_Option_1, HIGH);
    digitalWrite(ledPin_Option_2, HIGH);
    digitalWrite(ledPin_Option_3, LOW);
    digitalWrite(ledPin_Option_4, LOW);
  }
}

What's funny is that my pushbutton return inverted values with INPUT_PULLUP. It returns 1 when idle and 0 when pressed.

That's what I would expect. INPUT_PULLUP causes the pin to be held HIGH when the switch is open and, if wired correctly such that closing the switch takes the pin to GND, then it will go LOW

Is there a more concise way to code this?

When you have several variables with a numeric suffix you should look to the possibility of using arrays and iterating through them instead of repeating the code several times.

UKHeliBob:
That's what I would expect. INPUT_PULLUP causes the pin to be held HIGH when the switch is open and, if wired correctly such that closing the switch takes the pin to GND, then it will go LOW

Thanks! I did use the PULLUP because the signal on the Serial COM monitor was not stable. Is there any way to have a stable "0" instead of using an INPUT_PULLUP that inverts the signal?

UKHeliBob:
When you have several variables with a numeric suffix you should look to the possibility of using arrays and iterating through them instead of repeating the code several times.

Could you give an example to illustrate or a link?

Thank you
Regards

Is there any way to have a stable “0” instead of using an INPUT_PULLUP that inverts the signal?

Use an external pulldown resistor and wire the switch to take the pin HIGH when the switch is closed, but I have to ask, why bother ? Change the logic in the program to match the use of INPUT_PULLUP

Example of using arrays to read pins and write outputs

const byte ledPins[] = {12, 11, 10};
const byte buttonPins[] = {A1, A2, A3};
const byte NUMBER_OF_LEDS = sizeof(ledPins);

void setup()
{
  Serial.begin(115200);
  for (int p = 0; p < NUMBER_OF_LEDS; p++)
  {
    pinMode(ledPins[p], OUTPUT);
    pinMode(buttonPins[p], INPUT_PULLUP);
  }
}

void loop()
{
  for (int p = 0; p < NUMBER_OF_LEDS; p++)
  {
    digitalWrite(ledPins[p], digitalRead(buttonPins[p]));  //read the inputs and set the outputs to match
  }
}

Thanks UKHeliBob!

Any ideas why these two function in the loop one after the other won't work?

  // check if the pushbutton is pressed.
  // if it is, the buttonState is LOW
  // If no ledState_Option are HIGH make the 1st LED only = HIGH
  if (( buttonState == LOW) && (digitalRead(ledState_Option_1) == LOW ) &&  (digitalRead(ledState_Option_2) == LOW ) && (digitalRead(ledState_Option_3) == LOW ) && (digitalRead(ledState_Option_4) == LOW )) 
 
  {  
    digitalWrite(ledPin_Option_1, HIGH);
    digitalWrite(ledPin_Option_2, LOW);
    digitalWrite(ledPin_Option_3, LOW);
    digitalWrite(ledPin_Option_4, LOW);
  }


    // check if the pushbutton is pressed.
  // if it is, the buttonState is LOW 
  // If ledState_Option_1 HIGH make the 1st & 2nd LED only = HIGH
  if (( buttonState == LOW) && (digitalRead(ledState_Option_1) == HIGH ) &&  (digitalRead(ledState_Option_2) == LOW ) && (digitalRead(ledState_Option_3) == LOW ) && (digitalRead(ledState_Option_4) == LOW )) 
  
  {  
    digitalWrite(ledPin_Option_1, HIGH);
    digitalWrite(ledPin_Option_2, HIGH);
    digitalWrite(ledPin_Option_3, LOW);
    digitalWrite(ledPin_Option_4, LOW);
  }
}

Any ideas why these two function in the loop one after the other won't work

What do you mean by "won't work" ?

If you have modified the program since you last posted it then please post it in a new reply.

Can you please describe in words what the program is supposed to do.

You are absolutely right!

Here is the summary of the project:

The actual code aim to go from 2 light off, to 1 light on, to 2 lights ON.

These are preliminary LED tests for a 4 channel relay for 4 suspended light bulbs. These are at the moment before the project, activated ALL-4 on/off with a single switch.

The goal is to use the Arduino to design a state machine that will be activated with the light switch.

At the moment the program has only 2 output.

What do you mean by “won’t work” ?

No LED goes ON with the actual code.

arduihome21:
The actual code aim to go from 2 light off, to 1 light on, to 2 lights ON.

So, this?

465925 state diagram.GIF

ardy_guy:
So, this?

465925 state diagram.GIF

Yes that’s it!

The complete program would be 4 lights incremented one by one then ALL OFF.

And to clarify, it's always the same button? That adds a slight complication since a short human press is centuries to the Arduino and you'll still be pressing it when it gets to the next state and it will take that as a signal to move on immediately. So you need to do a state change detect: when you get to the next state with the button still pressed, it says aha, not a new press and stays where it is.

Then you have a switch....case where case 0 is idle, case 1 is 1 led etc. There's a variable called say currentState, so you are only ever in one discrete state. In each state you do what you need to do with the leds at that point.

Because you can only ever be in one state, the button detection can only take you to the next allowable state. In each part of the switch...case, ie in each state, you "listen" for the button, and if it says move on, you simply change the currentState to 2 or 3 or whatever, and in THAT case next time through loop it does more stuff to more leds then listens and moves on.

When you leave the last state you turn all the leds off and change currentState back to 0.

ardy_guy:
When you leave the last state you turn all the leds off and change currentState back to 0.

Although you might not expect a state to know what the next state does, so leave the turning off for the idle state to do, since it "owns" that state and can do what it likes there.

The complete program would be 4 lights incremented one by one then ALL OFF.

If I understand the requirement correctly, each time the button becomes pressed an extra LED will be turned on so, all off, 1 on, 2 on, 3 on, 4 on, all off

Look at the StateChangeDetection example in the IDE to see how to detect when a button becomes pressed and increment a variable when it happens. Based on the value of the variable you can turn the appropriate LEDs on and off.

I am open to any other interesting sys architecture with double clicks for different state changes in both directions.

Any fancy suggestions?
Cheers Bob

I am open to any other interesting sys architecture with double clicks for different state changes in both directions.

Can I suggest that you learn to walk before trying to run. Get the basic version working first.

UKHeliBob:
Can I suggest that you learn to walk before trying to run. Get the basic version working first.

I'm pretty quick. Don't be shy.