I have a problem in the performance of my program

Projects Programming
1

The idea is to make some LEDs light in one direction and press the button to change the other.
The problem is that when I press the button, the first thing he does is go one way and then the other infinitely until he stops pressing the button.
When you press the button, you should turn to the usual opposite side until you stop pressing it.
Tell me if the error is in the program and if I do not place images of the arduino and its wiring.

int boton=1;

void setup()
{
pinMode(13, OUTPUT);
pinMode(11, OUTPUT);
pinMode(9, OUTPUT);
pinMode(7, OUTPUT);
pinMode(5, OUTPUT);
pinMode(1,INPUT);
}
void loop()
{
if (digitalRead(boton)==LOW){
digitalWrite(13, HIGH );
digitalWrite(11, LOW );
digitalWrite(9, LOW );
digitalWrite(7, LOW );
digitalWrite(5, LOW );
delay(200);
digitalWrite(13, LOW );
digitalWrite(11, HIGH );
digitalWrite(9, LOW );
digitalWrite(7, LOW );
digitalWrite(5, LOW );
delay(200);
digitalWrite(13, LOW );
digitalWrite(11, LOW );
digitalWrite(9, HIGH );
digitalWrite(7, LOW );
digitalWrite(5, LOW );
delay(200);
digitalWrite(13, LOW );
digitalWrite(11, LOW );
digitalWrite(9, LOW );
digitalWrite(7, HIGH );
digitalWrite(5, LOW );
delay(200);
digitalWrite(13, LOW );
digitalWrite(11, LOW );
digitalWrite(9, LOW );
digitalWrite(7, LOW );
digitalWrite(5, HIGH );
delay(200);
digitalWrite(13, LOW );
digitalWrite(11, LOW );
digitalWrite(9, LOW );
digitalWrite(7, LOW );
digitalWrite(5, LOW );
delay(200);
}
else
digitalWrite(13, LOW );
digitalWrite(11, LOW );
digitalWrite(9, LOW );
digitalWrite(7, LOW );
digitalWrite(5, HIGH );
delay(200);
digitalWrite(13, LOW );
digitalWrite(11, LOW );
digitalWrite(9, LOW );
digitalWrite(7, HIGH );
digitalWrite(5, LOW );
delay(200);
digitalWrite(13, LOW );
digitalWrite(11, LOW );
digitalWrite(9, HIGH );
digitalWrite(7, LOW );
digitalWrite(5, LOW );
delay(200);
digitalWrite(13, LOW );
digitalWrite(11, HIGH );
digitalWrite(9, LOW );
digitalWrite(7, LOW );
digitalWrite(5, LOW );
delay(200);
digitalWrite(13, HIGH );
digitalWrite(11, LOW );
digitalWrite(9, LOW );
digitalWrite(7, LOW );
digitalWrite(5, LOW );
delay(200);
digitalWrite(13, LOW );
digitalWrite(11, LOW );
digitalWrite(9, LOW );
digitalWrite(7, LOW );
digitalWrite(5, LOW );
delay(200);
}

2

Press ctrl+T for auto formatting and you will probably spot the error as well.

You do know that pressing the button in the middle of the animation is pointless this way?

3

Hi,
Welcome to the forum.

Please read the first post in any forum entitled how to use this forum.
http://forum.arduino.cc/index.php/topic,148850.0.html then look down to item #7 about how to post your code.
It will be formatted in a scrolling window that makes it easier to read.

Can you please post a copy of your circuit, in CAD or a picture of a hand drawn circuit in jpg, png?

What model Arduino are you using?

Can you please tell us your electronics, programming, Arduino, hardware experience?

You are using pin1 for your button, pin1 and pin0 are used for programming the controller, so you will first have to use another pin for your button.

Thanks.. Tom.. :slight_smile:

4 
        digitalWrite(5, LOW );
        delay(200);
    }
    else 
        digitalWrite(13, LOW );
    digitalWrite(11, LOW );

Looks like you forget a set of brackets around your 'else' clause so only one line is part of the 'else'.

5

Your code followed a simple logic that was very repetitive and could be reduced quite a bit, here's how i would do it.

#define button 1
byte ledOn = 0;
byte led_Array[5] = {13, 11, 9, 7, 5};
byte direction = 13;
byte lock = 0;
byte prevLock = 0;

void setup()
{
  pinMode(button, INPUT);
  for (int i = 0; i < 4; i++) {
    pinMode(led_Array[i], OUTPUT);
    digitalWrite(led_Array[i], LOW);
  }
}

void loop()
{
  if (digitalRead(button) == LOW) {
    direction = 13;
    lock = 1;
  }
  else {
    direction = 5;
    lock = 2;
  }
  blink();
}

void blink() {
  // sort array order, only needs to run once each button press
  if (lock != prevLock) {
    byte direct = direction;
    for (int i = 0; i < 4; i++) {
      led_Array[i] = direct;      
      if (direction == 13) {
        direct -= 2;
      }
      else {
        direct += 2;
      }
    }
    prevLock = lock;
  }
  // blink sequence
  digitalWrite(led_Array[ledOn], HIGH);
  delay(200);
  digitalWrite(led_Array[ledOn], LOW);
  if (ledOn < 4) {
    ledOn++;
  }
  else {
    ledOn = 0;
  }
}
6

#define button 1

Assuming this is an Uno.
Then you shouldn't use pin1.
That pin is already used by the USB<>Serial chip.

Also avoid pin13 if you can. It could 'flash' during bootup.
Leo..

7

Wawa:
#define button 1

Assuming this is an Uno.
Then you shouldn't use pin1.
That pin is already used by the USB<>Serial chip.

Also avoid pin13 if you can. It could 'flash' during bootup.
Leo..

Agreed, i too would not use those, but i just wanted to follow his pin format, which seems to be all the odd pins except 3, im not sure why his is using those exact pins but whatever :confused:.

8

Just measured pull up current on pin1 with an Uno clone (CH340 USB<>Serial chip).

Fixed pull up current measured by shorting TX pin1 to ground with a DMM set to 200mA was 3.2mA.
RX pin0 was more than twice that current, at 7.9mA.

All measured with a blank sketch (no Serial.begin).
Leo..

9

Some comments on the program of KawasakiZx10r. He is indeed showing the extreme receptivity in the program. But:

#define button 1

The compiler supports C++, use const byte instead. Way saver and gives way more useful errors when you mess up :slight_smile:

While you're add it, give it a more useful name like 'ButtonPin' and 'LedPins' (the plural already indicates array).

I would switch to using the internal pulls ups. Saves the hassle of connecting a resistor to the button.

The sorting is just plain stupid ::slight_smile: Sorry, that way you make the pins hard coded again. Why not simply reverse the order of the looping?

Also, the program isn't a clone of OP's because you can now change direction while the sequence is going on, the code of OP finishes a sequence first.

And you're only looping 4 of the 5 leds and never turn them all off...

So my take, also made it non-blocking

const byte ButtonPin = 1;
const byte LedPins[] = {13, 11, 9, 7, 5};
const byte NrLeds = sizeof(LedPins);
//Yes, this is my exception to plurals if I prefix it with Nr (NumbeR, because I think No is confusing
const unsigned int BlinkInterval = 200;

bool direction = false;
bool currentLed = 0; //0 = no led on

unsigned long blinkMillis = -BlinkInterval; //so it will start right away

void setup(){
  for(byte i = 0; i < NrLeds; i++){
    pinMode(LedPins[i], OUTPUT);
  }
  
  pinMode(ButtonPin, INPUT_PULLUP);
}

void loop(){
  if(currentLed == 0){
    checkButton();
  }
  
  updateLeds();
}

void checkButton(){
  direction = digitalRead(ButtonPin);
}

void updateLeds(){
  if(millis() - blinkMillis >= BlinkInterval){
    blinkMillis = millis();
    
    //if a led is on, turn it off
    if(currentLed){
      digitalWrite(LedPins[currentLed - 1, LOW);
    }
    
    //turn on next led
    updateNextLed();
    if(currentLed){
      digitalWrite(LedPins[currentLed - 1], HIGH);
    }
  }
}

void updateNextLed(){
  //goto next led
  if(direction){
    if(currentLed == NrLeds){
      currentLed = 0;
    }
    else{
      currentLed++;
    }
  }
  else{
    if(currentLed == 0){
      currentLed = NrLeds;
    }
    else{
      currentLed--;
    }
  }
}