Beginners problem with coding

Hi All, I am very much a newbie, teaching myself how to use and Arduino as best as I can. I have seen all the tutorials and youtube vids that I can. I am trying to flash 7 Leds in sequence when a button switch is pressed, then hold them on. I can do this with a sketch I did without the switch, but cannot get the switch part to work. I have tried everything I can think of to no avail. This is the sketch that compiles and uploads but just holds the leds on dimly. I am using the 5v and ground of the arduino Uno board and pin 11 to try and detect the switch operation.



int  switchpin = 11;  // pin 11 connected to switch
int timer = 500;           // The higher the number, the slower the timing.
int timer2 = 3000;
int thisPin;
void setup() {

  // set up digital pin read for switch
   pinMode (thisPin, OUTPUT);
  // use a for loop to initialize each pin as an output:
   pinMode (switchpin, INPUT);
  
    
   }


void loop() {
  int switchPin = 11;
  if (digitalRead (switchpin) == HIGH);

  for (int thisPin = 2; thisPin < 9; thisPin++) {
    


 

  // loop from the lowest pin to the highest:
  for (int thisPin = 2; thisPin < 9; thisPin++)
  
    // turn the pin on:
    
    digitalWrite(thisPin, HIGH);
    delay(timer);
    // turn the pin off:
    digitalWrite(thisPin, LOW);
  
   

  // loop from the highest pin to the lowest:
  for(int thisPin = 9; thisPin >= 2; thisPin--) 
    // turn the pin on:
    digitalWrite(thisPin, HIGH);
    delay(timer2);
    
}}

Any help would be greatly appreciated

For a start:

Without enclosing the lines that are a part of the for loop code block, only the firs line after the for will execute as part of the for loop.

Enclose all the lines that you want to repeat in curly brackets. See the for structure reference.

for (int thisPin = 2; thisPin < 9; thisPin++) 
{
    // turn the pin on:    
    digitalWrite(thisPin, HIGH);  this is the only line that is in the for loop
    delay(timer);
    // turn the pin off:
    digitalWrite(thisPin, LOW);
}

Does your switch have a pulldown resistor so that the unpressed switch input is not floating?

Can you show the working code without the switch? Your loops look weird, and I'm surprised they worked without the switch.

That sets, only, pin 0 to output. Is that what you intend?

Thanks groundFungus. The switch has a pull up resistor, but even when I disconnect lead pin 11 from the switch circuit the sketch should wait until the voltage is high but it doesn't

This works without the switch

int timer = 500;           // The higher the number, the slower the timing.
int timer2 = 3000;

void setup() {
  // use a for loop to initialize each pin as an output:
  for (int thisPin = 2; thisPin < 9; thisPin++) {
    pinMode(thisPin, OUTPUT);
  }
}

void loop() {
  // loop from the lowest pin to the highest:
  for (int thisPin = 2; thisPin < 9; thisPin++) {
   
    digitalWrite(thisPin, HIGH);
    delay(timer);
    // turn the pin off:
    digitalWrite(thisPin, LOW);
  }

  // loop from the highest pin to the lowest:
  for (int thisPin = 9
; thisPin >= 2; thisPin--) {
    // turn the pin on:
    digitalWrite(thisPin, HIGH);
    delay(timer2);
}}

type or paste code here

That sets, only, pin 0 to output. Is that what you intend?

But then the loop assigns pin 2 to pin 8 as OUTPUT

The ";" at the end means that nothing is executed if this is true.

@skindog386

I modify your code.

int  switchpin = 11;  // pin 11 connected to switch
int timer = 500;           // The higher the number, the slower the timing.
int timer2 = 3000;
int thisPin;
void setup() 
{

  // set up digital pin read for switch
// for loop need this ---> { }
for (int thisPin = 2; thisPin < 9; thisPin++)
{

   // this will set pin 2 to pin 8 as output
   pinMode (thisPin, OUTPUT);
}
  // use a for loop to initialize each pin as an output:
   pinMode (switchpin, INPUT);

 }


void loop()
 {
  // I prefer you hook up like +5 V --- Resistor -- pin 11 --- push button --- GND
// when the switch is press ... it will produce a LOW - 0 - 0 V signal
  if (digitalRead (switchpin) == LOW)
 {
    // you forgot the  {   }
   delay(100); // debounce time
   for (int thisPin = 2; thisPin < 9; thisPin++) 
  {
      // loop from the lowest pin to the highest:
      // turn the pin on
     digitalWrite(thisPin, HIGH);
     delay(timer);
     // turn the pin off:
     digitalWrite(thisPin, LOW);
  }
 // end of loop one
 // loop from the highest pin to the lowest:
  for(int thisPin = 8; thisPin > 1; thisPin--) 
 {
     // turn the pin on:
     digitalWrite(thisPin, HIGH);
     delay(timer2);
}
// end of loop 2
}
// closing the if () stament
    
}

The code should compile. But as for working as intended... that you have to check it out.

Start with this working code and try to add the code for the switch into it. You missed and changed some things when building the new code from it.

Thank you so much, lots of info in there I can use. Much appreciated

Thats correct, that is what I wanted as waiting for button push

It doesn't wait... it just moves to the next line of code.

if (condition)
  ; // do nothing

  // do next thing

If you want to wait then you could use while instead of if.

It doesn't technically wait, but since it loops, doesn't it effectively wait? Assuming power draw isn't an issue. I'm learning too and trying to help other new people while I am between projects.

No because in the original sketch the subsequent for loops are executed regardless of the value of the button. If they were enclosed within {} (as @sergetechone has done in post #9) then yes it would effectively wait.

1 Like

Hi guys, thanks so much for your help so far, but I have one more small issue to solve. I have got the 7 leds to come on individually one at a time then scroll backwards through the sequence. However I am trying to make all 7 leds come on at the same time for a couple of seconds then go out. I have written this code the long way coz if I use the same loop and a timer delay the leds come on sequentially. However, I cant seem to get past this error.

int  switchpin = 11;  // pin 11 connected to switch
int timer = 250;           // The higher the number, the slower the timing.
int timer2 = 250;
int timer3 = 1000;
int thisPin;
int allpin2 = 2;
int allpin3 = 3;
int allpin4 = 4;
int allpin5 = 5;
int allpin6 = 6;
int allpin7 = 7;
int allpin8 = 8;



void setup() 


{



  // set up digital pin read for switch
// for loop need this ---> { }
for (int thisPin = 2; thisPin < 9; thisPin++)
{

   // this will set pin 2 to pin 8 as output
   pinMode (thisPin, OUTPUT);
}
  // use a for loop to initialize each pin as an output:
   pinMode (switchpin, INPUT);

 }
 

pinMode (allpin2, OUTPUT);
PinMode (allpin3,OUTPUT);
PinMode (allpin4,OUTPUT);
PinMode (allpin5,OUTPUT);
PinMode (allpin6,OUTPUT);
PinMode (allpin7,OUTPUT);
PinMode (allpin8,OUTPUT);
}
void loop()
 {
  // I prefer you hook up like +5 V --- Resistor -- pin 11 --- push button --- GND
// when the switch is press ... it will produce a LOW - 0 - 0 V signal
  if (digitalRead (switchpin) == LOW)
 {
    // you forgot the  {   }
   delay(100); // debounce time
   for (int thisPin = 2; thisPin < 9; thisPin++) 
  {
      // loop from the lowest pin to the highest:
      // turn the pin on
     digitalWrite(thisPin, HIGH);
     delay(timer);
     // turn the pin off:
     digitalWrite(thisPin, LOW);
  }
 // end of loop one
 // loop from the highest pin to the lowest:
  
 
 {
     // turn the pin on:
     digitalWrite(allpin2, HIGH);
     digitalWrite(allpin3, HIGH);
     digitalWrite(allpin4, HIGH);
     digitalWrite(allpin5, HIGH);
     digitalWrite(allpin6, HIGH);
     digitalWrite(allpin7, HIGH);
     digitalWrite(allpin8, HIGH);
     delay(timer);
     
digitalWrite(allpin2, LOW); 
digitalWrite(allpin3, LOW);
digitalWrite(allpin4, LOW);
digitalWrite(allpin5, LOW);
digitalWrite(allpin6, LOW);
digitalWrite(allpin7, LOW);
digitalWrite(allpin8, LOW);


           
 }
// end of loop 2
}
// closing the if () stament
    
}

the errors are the following.
C:\Users\Skindog home\7 led switch 2\7 led switch 3\7 led switch 3.ino:38:9: error: expected constructor, destructor, or type conversion before '(' token
pinMode (allpin2, OUTPUT);
^
C:\Users\Skindog home\7 led switch 2\7 led switch 3\7 led switch 3.ino:39:9: error: expected constructor, destructor, or type conversion before '(' token
PinMode (allpin3,OUTPUT);
^
C:\Users\Skindog home\7 led switch 2\7 led switch 3\7 led switch 3.ino:40:9: error: expected constructor, destructor, or type conversion before '(' token
PinMode (allpin4,OUTPUT);
^
C:\Users\Skindog home\7 led switch 2\7 led switch 3\7 led switch 3.ino:41:9: error: expected constructor, destructor, or type conversion before '(' token
PinMode (allpin5,OUTPUT);
^
C:\Users\Skindog home\7 led switch 2\7 led switch 3\7 led switch 3.ino:42:9: error: expected constructor, destructor, or type conversion before '(' token
PinMode (allpin6,OUTPUT);
^
C:\Users\Skindog home\7 led switch 2\7 led switch 3\7 led switch 3.ino:43:9: error: expected constructor, destructor, or type conversion before '(' token
PinMode (allpin7,OUTPUT);
^
C:\Users\Skindog home\7 led switch 2\7 led switch 3\7 led switch 3.ino:44:9: error: expected constructor, destructor, or type conversion before '(' token
PinMode (allpin8,OUTPUT);
^
C:\Users\Skindog home\7 led switch 2\7 led switch 3\7 led switch 3.ino:45:1: error: expected declaration before '}' token
}
^

exit status 1

Compilation error: expected constructor, destructor, or type conversion before '(' token

Once again many thanks for taking the time to help

pinMode vs. PinMode

I am sorry, I dont know what you mean. The word pinMode is written the same in all instances.............. I think

Are you sure?????

PinMode is not the same as pinMode. The capital p is the difference. C++ is really anal about case.