Button not working correctly. Static bool, Potentiometer. uno

const byte button = 15;  // potentiometer wire 1 connected to pin A1 , other wires connected to ground and +5v

void setup()
{
  pinMode(2, OUTPUT ); // PINS 2 - 13, 16- 19 are output leds
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(10, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(12, OUTPUT);
  pinMode(13, OUTPUT);
  pinMode(14, INPUT); // A0 , random seed pin 
  pinMode(15, INPUT); // A1 , my delay value pin 
  pinMode(16, OUTPUT); 
  pinMode(17, OUTPUT);
  pinMode(18, OUTPUT);
  pinMode(19, OUTPUT);

  digitalWrite(2, LOW) ; // all LED's are off to start
  digitalWrite(3, LOW) ;
  digitalWrite(4, LOW) ;
  digitalWrite(5, LOW) ;
  digitalWrite(6, LOW) ;
  digitalWrite(7, LOW) ;
  digitalWrite(8, LOW) ;
  digitalWrite(9, LOW) ;
  digitalWrite(10, LOW) ;
  digitalWrite(11, LOW);
  digitalWrite(12, LOW);
  digitalWrite(13, LOW);  
  digitalWrite(A1, LOW); // connects to button and sets my delay value
  digitalWrite(16, LOW);  // A2
  digitalWrite(17, LOW);  // A3
  digitalWrite(18, LOW);  // A4
  digitalWrite(19, LOW);  // A5 

  digitalWrite(2, HIGH) ; // All LEDS flash on for inital start up, used to make sure they are all functioning
  digitalWrite(3, HIGH) ;
  digitalWrite(4, HIGH) ;
  digitalWrite(5, HIGH) ;
  digitalWrite(6, HIGH) ;
  digitalWrite(7, HIGH) ;
  digitalWrite(8, HIGH) ;
  digitalWrite(9, HIGH) ;
  digitalWrite(10, HIGH) ;
  digitalWrite(11, HIGH);
  digitalWrite(12, HIGH);
  digitalWrite(13, HIGH);  
  digitalWrite(16, HIGH);  
  digitalWrite(17, HIGH);  
  digitalWrite(18, HIGH);  
  digitalWrite(19, HIGH); 

  delay (2000) ;
  
  digitalWrite(2, LOW) ; // all LED's are off to start
  digitalWrite(3, LOW) ;
  digitalWrite(4, LOW) ;
  digitalWrite(5, LOW) ;
  digitalWrite(6, LOW) ;
  digitalWrite(7, LOW) ;
  digitalWrite(8, LOW) ;
  digitalWrite(9, LOW) ;
  digitalWrite(10, LOW) ;
  digitalWrite(11, LOW);
  digitalWrite(12, LOW);
  digitalWrite(13, LOW);  
  digitalWrite(A1, LOW); // connects to button and sets my delay value
  digitalWrite(16, LOW);  // A2
  digitalWrite(17, LOW);  // A3
  digitalWrite(18, LOW);  // A4
  digitalWrite(19, LOW);  // A5 
  
  pinMode(button,INPUT);  //button at pin A1 
  randomSeed(analogRead(A0));   
  
}

void loop ()
{
 static bool ledflag = false;
  
  if (digitalRead(button) == HIGH)   // if button is on ( button connected to pin A1 and 5v pin )
    ledflag = !ledflag;  // Toggle
  delay(20);  // Crude form of debounce

  if (ledflag)         // if led = 1 , button is on, if button is on run loop for random led
 
  {
  
    Serial.begin(9600);
    int myDelay = analogRead(A1) ;     // Set myDelay based off of potentiometer value connected to pin A1 and 5v, Range .2 - 300 ohms  
//int myDelay =200;
    int Led = random (2 , 13) ;   // pick random number from 2-12, store value in variable called LED
    digitalWrite(Led, HIGH) ;     // turn on the random LED
    delay(myDelay);               // leave on for # of milliseconds set by potentiometer ( 1-10 dial )
    digitalWrite(Led, LOW);       // turn off that led
    Serial.println(myDelay);
    Serial.println(Led);
     delay(myDelay) ;              // delay off for amount of time it was on
  
  }
}

I have an old machine rigged up with a 30 led's

If I turn on the power without the switch plugged into pin A1 the code acts correctly. All the leds flash then nothing happens because it is waiting for a HIGH signal from pin A1 to continue to the next part of the code where the lights will flash randomly. If my switch ( that is connected to 5v pin and A1) is off and I check the voltage between the A1 hot wire and the LED Ground, it reads a few mV, then I plug it into PIN A1 expecting nothing to happen because 1mV is not above the 3V threshold for HIGH, But the lights start blinking as if the HIGH value was met. But even stranger is when I remove the PIN from A1, the lights continue to blink. They should stop blinking because the start of the loop looks for the initial HIGH or LOW condition to start, which seemed to work when the uno was first turned on without pin A1 connected. so I defiantly have a software issue and I might have a hardware issue. Also, I keep getting a value of around 900 for my potentiometer regardless of how I adjust the knob ( the resistance changes when checked with voltmeter, may not be grounded properly) , which is hooked up to the 5v pin and to the on off switch connected to A1 .Lastly, I don't have a pull down resister hooked up yet to pin A1 but i dont think that is the problem since the voltage was so low and because im only going through one cycle to test it.

image


A mess of wires

I think that you should learn about button debouncing and the difference between digital and analog inputs.

Do not mix pin 15 and A1, pins by name and number, digitalRead() and analogRead() of the same pin, switch or pot on A1, and use pullups wherever required.

Hello
Your task has the great opportunity to train the ussage of OOP structures like enum, struct, array, range-based for loop and so on.
Try it!
At first you have to clean up the applicaton of used I/O pins.
Have a nice day and happy coding.

if the button is connected between the pin and 5V, what would cause it to be pulled low?

button switches are commonly connected between the pin and ground and the pin is configured as INPUT_PULLUP so that the pin will be HIGH when the button is not pressed.

the other aspect of you code is that it's not checking for a change in state and each iteration thru loop while the pin is HIGH will cause ledFlag to toggle

shouldn't Serial.begin() be called once in setup()?

isn't A1 your "button" pin?

consider


#undef MyHW
#ifdef MyHW
byte pinLeds [] = { 10, 11, 12, 13 };
const byte button = A1;

#else
byte pinLeds [] = { 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 16, 17, 18, 19 };
const byte button = 15;
#endif

#define N_LED     sizeof(pinLeds)

enum { LedOff = HIGH, LedOn = LOW };
bool ledflag = false;

byte butLst;

// -----------------------------------------------------------------------------
void loop ()
{
    byte but     = digitalRead (button);

    Serial.print   (button);
    Serial.print   ("  ");
    Serial.print   (butLst);
    Serial.print   ("  ");
    Serial.println (but);

    if (butLst != but)  {
        butLst = but;
        delay (20);         // debounce

        if (LOW == but)
            ledflag = ! ledflag;
    }

    if (ledflag)  {
#if 0
        byte led = random (10, 13);
#else
        byte led = random (2, 13);
#endif
        digitalWrite (led, ! digitalRead (led));
        delay (100);
    }
}

// -----------------------------------------------------------------------------
void setup ()
{
    Serial.begin (9600);

    for (unsigned n = 0; n < N_LED; n++)  {
        digitalWrite (pinLeds [n], LedOn);
        pinMode      (pinLeds [n], OUTPUT);
    }

    delay (2000);
    for (unsigned n = 0; n < N_LED; n++)
        digitalWrite (pinLeds [n], LedOff);

    pinMode (button, INPUT_PULLUP);
    butLst = digitalRead (button);
}

Indeed. So let's see a proper schematic.

Interesting idea, but would I still be able to use the Pin A1 to read the pot value for my delay?
as shown in my first picture the potentiometer is connected to the switch in series, that's how the machine was originally wired. So A1is the pin for the switch and the delay value, which i would like to use to control the speed and duration of the blinking lights. But i guess i could disconnect the pot from the switch and plug it into two pins just like the suggested switch.

why is my code not checking for the HIGH condition at the beginning of each loop ? and are you saying that the HIGH condition with the toggle would cause the lights to turn on the first run through the loop then off the second time ?

what is the 0 value ? is this saying that if the button is off, led's 10-13 will shine ?

Here you seem to be consecutively turning pins on, assigning them as an output then turning them off.
As mentioned I would like a random Led to turn on for an adjustable amount of time. so I don't think this is what I'm looking for.

One thing I am also trying to add to my code is to select a random number of leds up to 10 at a time, then have them all light up at once, maybe u would have an idea how i could attempt that, maybe with an array ?

Thanks for your help and insight !

pot wires from left to right are, black, red , white. The white wire isn't grounded. I'm going to fix that by attaching it to Arduino ground

Ok, that don't look good to me. Typical way of connecting a pot is to tie one end to +V, the other to GND and the wiper goes to your signal input. So in your case let's say black to GND, white to 5V (or whatever your 'duino runs at) and red to A1 or whatever ADC pin you're using.

The way you've tied it now is as a variable resistor/rheostat. Yes, can work, but why not do it the proper way and be done with it.

You can figure out which pins to connect +v and GND to on your pot by measuring the resistance between any two pins; the ones you measure a resistance between that does not change as you turn the pot are the ones you connect GND and V+ to. The remaining is your wiper pin that goes to the microcontroller.

1 Like

this is confusing. the middle wire of a pot is typically the wiper that travels from the left to righ tterminals

if the middle is 5V and the right is ground, what happens if the wiper is turned all the way to the right connecting 5V to ground?

it's not really clear how you expect to use this circuit. it would make more sense to connect the wiper to A1, the right terminal to ground and wire the switch between the right terminal and 5V.

the input, A1 always be 0V if the button is not pressed. when the button is pressed, an analogRead (A1) would measure the setting of the wiper from 0-5V.

with the above approach, conditional could be executed while the button is pressed (no need for ledflag)

    if (digitalRead(button) == HIGH)  {
        int myDelay = analogRead(A1);
        int Led     = random (2 , 13);

        digitalWrite(Led, HIGH) ;     // turn on the random LED
        delay(myDelay);
        digitalWrite(Led, LOW);       // turn off that led
        delay(myDelay) ;              // delay off for amount of time it was on
    }

each iteration thru loop() where the button is HIGH toggles ledflag.

the following demonstrates recognizing a button state change and that it was pressed


const byte butPin = A1;
const byte ledPin = 13;

byte butLst;

void setup ()
{
    pinMode (ledPin, OUTPUT);
    digitalWrite (ledPin, HIGH);

    pinMode (butPin, INPUT_PULLUP);
    butLst = digitalRead (butPin);
}

void loop  ()
{
    byte but = digitalRead (butPin);
    if (butLst != but)  {
        butLst = but;
        delay (20);         // debounce

        if (LOW == but)
            digitalWrite (ledPin, ! digitalRead (ledPin));
    }
}
1 Like

Ok awesome, Thanks! I thought I was supposed to connect it to the wire with changing resistance !

I just realized a potential problem with using the switch and pot in series, if I adjust the pot so the voltage is below 3v the switch will be considered filliped and the lights will stop.

If I attached the pot to 5v pin and A2 , then the switch to 5V and A1 could I still use this if statement above for the button ?

Thanks !

but now there is no "pull-down". you could add a resistor or change the logic to use the internal pull-up, and then you might want to recognize a button press and toggle a state

Which I think was what was so confusing to me all along - how you are trying to manage to analog read a potentiometer AND use the same input for a switch.

Maybe it could be figured out. Easier: just use one analog input for the pot, and one digital input, however you wire and configure it, for the switch or pushbutton.

As may have been said aready.

a7

Ya I should have picked up on that sooner, I was focused on saving pins, but since I'm going to use two boards I have some pins to spare.

so from everyone's feed back this is my new wiring diagram for the pot and switch.


2 question:

  1. can I connect the switch to an analog or pin, say A3 ? (how could I code A3 to supply voltage would that be the led flag) ?

  2. If so, would I use the conditional approach or the Ledflag ?

conditional

Ledflag

Thanks !

regardless, i would suggest wiring the switch between a pin and ground and configure the pin as INPUT_PULLUP (this works for analog pins as well)

if you recognize a button "press", you could use a press to toggle "ledflag" avoiding the need to hold the button down

1 Like

wow cant believe I forgot to ground the button. sorry guys

Its a switch so in one position the circuit will be open and in the other it will be closed, would it be easier to use an if statement with the HIGH criteria than toggle in this case ?

You better start drawing circuit diagrams with the standard component shapes instead of funny items and wires.

ok, so I have rewired the switch and pot so that they are now separate and connected to pins A1 and A2 respectively. They are both grounded, the pot wires are 5v,A2,G from left to right.
The pot works great! And the switch is returning the correct values when open and closed!

my problem is that the switch function is still not working, I tried the if statement that gcjr recommended and added a print function so that I could confirm the value of the switch. When the switch is reading 0 the loop runs and when the switch is reading 1 the loop still runs. I would like the loop to stop if SwitchVal = 1 and to continue if Switchval = 0

void setup()
{
  Serial.begin(9600);
  
  pinMode(2, OUTPUT ); // PINS 2 - 13, 17- 19 are output leds
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(10, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(12, OUTPUT);
  pinMode(13, OUTPUT);
  pinMode(14, INPUT); // A0 , random seed pin 
  pinMode(15, INPUT_PULLUP); // A1 , switch pin
  pinMode(16, INPUT_PULLUP); //A2 , pot pin sets mydelay 
  pinMode(17, OUTPUT);
  pinMode(18, OUTPUT);
  pinMode(19, OUTPUT);

  digitalWrite(2, LOW) ; // all LED's are off to start
  digitalWrite(3, LOW) ;
  digitalWrite(4, LOW) ;
  digitalWrite(5, LOW) ;
  digitalWrite(6, LOW) ;
  digitalWrite(7, LOW) ;
  digitalWrite(8, LOW) ;
  digitalWrite(9, LOW) ;
  digitalWrite(10, LOW) ;
  digitalWrite(11, LOW);
  digitalWrite(12, LOW);
  digitalWrite(13, LOW);  
  //digitalWrite(15, LOW); // switch
 // digitalWrite(16, LOW);  // A2
  digitalWrite(17, LOW);  // A3
  digitalWrite(18, LOW);  // A4
  digitalWrite(19, LOW);  // A5 

  digitalWrite(2, HIGH) ; // All LEDS flash on for inital start up, used to make sure they are all functioning
  digitalWrite(3, HIGH) ;
  digitalWrite(4, HIGH) ;
  digitalWrite(5, HIGH) ;
  digitalWrite(6, HIGH) ;
  digitalWrite(7, HIGH) ;
  digitalWrite(8, HIGH) ;
  digitalWrite(9, HIGH) ;
  digitalWrite(10, HIGH) ;
  digitalWrite(11, HIGH);
  digitalWrite(12, HIGH);
  digitalWrite(13, HIGH);  
  //digitalWrite(16, HIGH);  
  digitalWrite(17, HIGH);  
  digitalWrite(18, HIGH);  
  digitalWrite(19, HIGH); 

  delay (2000) ;
  
  digitalWrite(2, LOW) ; // all LED's are off to start
  digitalWrite(3, LOW) ;
  digitalWrite(4, LOW) ;
  digitalWrite(5, LOW) ;
  digitalWrite(6, LOW) ;
  digitalWrite(7, LOW) ;
  digitalWrite(8, LOW) ;
  digitalWrite(9, LOW) ;
  digitalWrite(10, LOW) ;
  digitalWrite(11, LOW);
  digitalWrite(12, LOW);
  digitalWrite(13, LOW);  
  //digitalWrite(15, LOW); // A1 = switch
  //digitalWrite(16, LOW);  // A2
  digitalWrite(17, LOW);  // A3
  digitalWrite(18, LOW);  // A4
  digitalWrite(19, LOW);  // A5 
  
  
  randomSeed(analogRead(A0));   
  
}

void loop ()
{
 int SwitchVal = digitalRead(15);  // varable switchval is 0 if switch is closed , 1 if switch is open  
 Serial.println(SwitchVal, DEC);
  
  if (SwitchVal == 0); {   // if switch is on ( switch connected to pin A1 and ground ) then loop will blink leds  
  delay(20);  // Crude form of debounce   // when switch is fliped to on switchval = 0, when switch is filliped to any other position switchval = 1 
                                          //, when switchval =1, loop should not run because of the if statment. but it still runs regadless of varable change!?
    
    int myDelay = analogRead(A2) ;     // Set myDelay based off of potentiometer value connected to pin A2 and 5v,and ground, Range .2 - 300 ohms  
//int myDelay =200;
    int Led = random (2 , 13) ;   // pick random number from 2-12, store value in variable called LED
    digitalWrite(Led, HIGH) ;     // turn on the random LED
    delay(myDelay);               // leave on for # of milliseconds set by potentiometer ( 1-10 dial )
    digitalWrite(Led, LOW);       // turn off that led
   // Serial.println(myDelay);
   // Serial.println(Led);
     delay(myDelay) ;              // delay off for amount of time it was on
  
 }
}

Thanks so much for your help

This statement does exactly nothing. The remaining block executes always.

Drop the semicolon that terminates the statement.

1 Like

Thanks that was it !