Making a LED switch but it's a bit more complex

Projects Programming
1

Hi everyone !

I'm searching for help with my code. I have to make a LED switch with a push button that switches the LED on on the first press then nothing when I release it. And if I press the button a second time it does nothing when I presse but when I release it it switches the LED off.
The LED is on by default.

Here is the code I made that doesn't work properly, it works the first time then after the second on switching it becomee random :

int pbPin = 2;
int ledPin = 3;
int lstStt;
bool on;
int state;


void setup() {
  
  pinMode(pbPin, INPUT);
  pinMode(ledPin, OUTPUT);
  digitalWrite(ledPin, HIGH);
  lstStt = digitalRead(pbPin);
  on = true;

}

void loop() {
  
  state = digitalRead(pbPin);

  if (lstStt != state) {

    state = digitalRead(pbPin);

    while (state) {

      state = digitalRead(pbPin);

      if (!state && on) {

        digitalWrite(ledPin, LOW);
        on = false;

      } else if (state && !on) {

        digitalWrite(ledPin, HIGH);
        on = true;
        lstStt = true;
        break;

      }

    }

  }

}
3

In my experience the easiest way to tidy up the code and add the code tags is as follows
Start by tidying up your code by using Tools/Auto Format in the IDE to make it easier to read. Then use Edit/Copy for Forum and paste what was copied in a new reply. Code tags will have been added to the code to make it easy to read in the forum thus making it easier to provide help.

4

Using a button library (eg Toggle) and a state machine approach (here is a small introduction to the topic: Yet another Finite State Machine introduction) would make this pretty trivial to write esp. if your button is bouncing.

1 Like
5

I don't think my button is bouncing, I put a pull-down resistor with my button to prevent this.

6

resistors don't debounce. They just ensure a defined state when the button is not pressed.
Debounce:

1 Like
7

An even better resource to bookmark:

8

How did you wire the button?

  pinMode(pbPin, INPUT);

In addition to bouncing, switches that are not pulled up (or down) will tend to report random values HIGH and LOW when the switch is open.

You need at least to slow your loop down as a crude debouncing method, a 25 millisecond delay as the very last line of a free running loop will let you temporarily postpone learning about real debouncing. Temporarily. You will want to figure that out one day, and the articles @camsysca linked, Gammon and Ganssle are worth every minute you spend with them when that day arrives.

But first you should read about "state change detection", which is an example you can find in the IDE and study the competent article that goes with it:

https://docs.arduino.cc/built-in-examples/digital/StateChangeDetection/

Then it will be easy.

  • if the lamp is off, turn it on when you see the switch become pressed.
  • if the lamp is on, turn it off when you see the switch get released.

The state change detection a place where you can act on one edge, and a place to act on the other. Normally we don't mess with the user like you are doing (!) and the place where the other transition (released, maybe) has no code at all - it records but ignores the transition because it doesn't care but for the getting pressed part.

a7

1 Like
9

here is an example of what it could look like with a state machine and the Toggle button library

click to see the code 
 /* ============================================
  code is placed under the MIT license
  Copyright (c) 2024 J-M-L
  For the Arduino Forum : https://forum.arduino.cc/u/j-m-l

  Permission is hereby granted, free of charge, to any person obtaining a copy
  of this software and associated documentation files (the "Software"), to deal
  in the Software without restriction, including without limitation the rights
  to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
  copies of the Software, and to permit persons to whom the Software is
  furnished to do so, subject to the following conditions:

  The above copyright notice and this permission notice shall be included in
  all copies or substantial portions of the Software.

  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
  AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  THE SOFTWARE.
  ===============================================
*/


#include <Toggle.h>

const byte buttonPin = 2;
const byte ledPin = 9;
Toggle button;

enum {WAITING_FIRST_PRESS, WAITING_FIRST_RELEASE, WAITING_SECOND_RELEASE} state = WAITING_FIRST_PRESS;

void handleState() {
  button.poll();

  switch (state) {

    case WAITING_FIRST_PRESS:
      if (button.onPress()) {
        Serial.println(F("First press"));
        digitalWrite(ledPin, HIGH);
        state = WAITING_FIRST_RELEASE;
      }
      break;

    case WAITING_FIRST_RELEASE:
      if (button.onRelease()) {
        Serial.println(F("First release"));
        state = WAITING_SECOND_RELEASE;
      }
      break;

    case WAITING_SECOND_RELEASE:
      if (button.onRelease()) {
        Serial.println(F("Second release"));
        digitalWrite(ledPin, LOW);
        state = WAITING_FIRST_PRESS;
      }
      break;
  }
}

void setup() {
  pinMode(ledPin, OUTPUT);
  button.begin(buttonPin);
  Serial.begin(115200);
  // make sure button is released
  while (true) {
    button.poll();
    if (button.isReleased()) break;
  }
  Serial.println(F("Ready"));

}

void loop() {
  handleState();
}```
1 Like
10

That logic is not correct. Ask me how I know. :frowning:

There is more to the state than it takes into account, @J-M-L's code shows that.

a7

11

yeah, you'll turn it off with the first release :slight_smile:

1 Like
12

The state change detection example could stretch to cover it pretty easily--One RISING(FALLING) edge to turn on, and two FALLING(RISING) edges to turn off. (But then you need to keep track of two state variables button_state and how many transitions_to_go. An explicit state machine with one state variable would be more general. But if it was "turn-off-after-seventh-release", I'd use the two state variables.)

13

Yes.

I thought a solution that relied on doing the sate change detection would be valuable.

I used an additional state variable that toggles with button going down.

Here's a timing digram of how my fix works.

IMG_2439
IMG_24391548×868 286 KB

Here's the code, which runs on @J-M-L's "hardware":

const byte buttonPin = 2;
const byte ledPin = 9;

# define PRESST LOW

void setup() {
  Serial.begin(115200);
  Serial.println(F("Ready"));

  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);
}

bool lastButton;
bool control;  

void loop() {

  bool thisButton = digitalRead(buttonPin) == PRESST;  // true if switch is closed
  delay(50);    // poor man's debounce. we loop so slowly we never see it.

// detect state change?
  if (thisButton == lastButton)
    return;  // nope, nothing to do!

// button just went closed
  if (thisButton) {
    control = !control;
    if (control)
      digitalWrite(ledPin, HIGH);
  }

// button just went open
  if (!thisButton)
    if (!control)  digitalWrite(ledPin, LOW);

// record for state change detection
  lastButton = thisButton;
}

I can't get accustomed to the press On release Off feel of that button. I had to keep reminding myself that it was the OP's goal, not broken.

a7

14

you could use an else instead of testing again thisButton for the opposite value and condensate a bit

15

Here's a quick modification of the StateChangeDetection example to fit @J-M-L 's hardware:

/* Modified from 
 https://github.com/arduino/arduino-examples/blob/main/examples/02.Digital/StateChangeDetection/StateChangeDetection.ino 
for https://forum.arduino.cc/t/making-a-led-switch-but-its-a-bit-more-complex/1300283/9
as per:
https://forum.arduino.cc/t/making-a-led-switch-but-its-a-bit-more-complex/1300283/15?u=davex

 to: 
 * switch LED pin to 9 to match @J-M-L's https://wokwi.com/projects/408715571761622017 hardware
 * switch button to Active-LOW with internal pullup
 * Add a lightState variable to track how many off-edges are needed to turn off.

*/

/*
  State change detection (edge detection)

  Often, you don't need to know the state of a digital input all the time, but
  you just need to know when the input changes from one state to another.
  For example, you want to know when a button goes from OFF to ON. This is called
  state change detection, or edge detection.

  This example shows how to detect when a button or button changes from off to on
  and on to off.

  The circuit:
  - pushbutton attached to pin 2 from +5V
  - 10 kilohm resistor attached to pin 2 from ground
  - LED attached from pin 13 to ground through 220 ohm resistor (or use the
    built-in LED on most Arduino boards)

  created  27 Sep 2005
  modified 30 Aug 2011
  by Tom Igoe

  This example code is in the public domain.

  https://docs.arduino.cc/built-in-examples/digital/StateChangeDetection/
*/

// this constant won't change:
const int buttonPin = 2;  // the pin that the pushbutton is attached to
const int ledPin = 9;    // the pin that the LED is attached to

// Variables will change:
int buttonPushCounter = 0;  // counter for the number of button presses
int buttonState = 0;        // current state of the button
int lastButtonState = 0;    // previous state of the button
int lightState = 0;  // Add a state variable for the LED
const int NumOffs = 2; // Add a constant for how many off-edges to see before turning off

void setup() {
  // initialize the button pin as a input:
  pinMode(buttonPin, INPUT_PULLUP);
  // initialize the LED as an output:
  pinMode(ledPin, OUTPUT);
  // initialize serial communication:
  Serial.begin(9600);
}


void loop() {
  // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin);

  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) {
    // if the state has changed, increment the counter
    if (buttonState == LOW) {
      // if the current state is LOW then the button went from off to on:
      buttonPushCounter++;
      Serial.println("on");
      Serial.print("number of button pushes: ");
      Serial.println(buttonPushCounter);
      if(lightState == 0 ){
        lightState = NumOffs; 
      }
    } else {
      // if the button went from on to off:
      Serial.println("off");
      if(lightState > 0){
        --lightState;
      }
    }
    // Delay a little bit to avoid bouncing
    delay(50);
  }
  // save the current state as the last state, for next time through the loop
  lastButtonState = buttonState;


// Handle the lightState
  if (lightState > 0 ) {
    digitalWrite(ledPin, HIGH);
  } else {
    digitalWrite(ledPin, LOW);
  }
}
16

Nice. I had thought of using the counter that is in the example code and just checking the LSB, then leapt (!) to using another bool.

Yes. I think here the uncondensated literal code makes it easier to see actions explicitly performed on the two edges. Sometimes if I do use else I feel like I wanna comment it if the if isn't right there, or even when it is, especially if other if/else statements find themselves in those blocks.

And I may have been trying to shoehorn it into structural similarity to my failed logic.

I did not worry about the initial conditions. If you are orsssing the button when power is cycles or the board otherwise resets, what happens in any version that works if not so challenged.

a7

17

I think the state change detection as typically presented is harder to understand, or at least it is easier once a few things get learnt.

After the check for a change in the input, if there is one we can deduce that a pressed button must just have become pressed, and similarly an unpressed button must just have been released.

But we use the state as proxy for those events. Properly, from a logic point of view, but confusing.

Borrowing from some code I saw that handled multiple inputs and recasting it to exemplify the code for this using just one button, I wrote the following code. It examines the switch and calculates a boolean value for each edge, leading or trailing.

The code can then test and use those more meaningful variables directly.

I did not include code for the OP's requirment.

Try it here:

Wokwi_badge UA State Change Flags


// https://wokwi.com/projects/408924267717447681
// https://forum.arduino.cc/t/making-a-led-switch-but-its-a-bit-more-complex/1300283

const byte buttonPin = 2;

const byte toggleLED0 = 12;
const byte toggleLED1 = 11;

# define PRESST LOW

void setup() {
  Serial.begin(115200);
  Serial.println("Ready.\n");

  pinMode(buttonPin, INPUT_PULLUP);

  pinMode(toggleLED0, OUTPUT);
  pinMode(toggleLED1, OUTPUT);
}

bool prevousButton;  // true means button is pressed. down. shorted.

void loop() {

  bool thisButton = digitalRead(buttonPin) == PRESST;    // true if switch is closed

// detect state change?
  if (thisButton != prevousButton) {

// yes the button is different to last time, set flags
    bool leadingEdge = thisButton && !prevousButton;     // button is down and wasn't, became pressed
    bool trailingEdge = !thisButton && prevousButton;    // button is up and wasn't, became released

// record for state change detection
    prevousButton = thisButton;

// just toggle. one on the leading edge
    if (leadingEdge)
      digitalWrite(toggleLED0, digitalRead(toggleLED0) == HIGH ? LOW : HIGH);

// and the other one on the trolling edge
    if (trailingEdge)
      digitalWrite(toggleLED1, digitalRead(toggleLED1) == HIGH ? LOW : HIGH);

    delay(50);    // poor man's debounce. only when the state change is done
  }
}

Careful readers will point out that both leadingEdge and trailingEdge cannot be true at once. At the cost of whatever it does, I leave it untagnled, so each event is the same, and any use of the event always is expressly referring to the flag, not the opposite of something tested previously with the code in an else block.

Multiple uses of either can be in one block or not; in theOP's requirement this lead me to have the thing that marks every other press separate to the logic that does the odd on with a leading edge and off with the second next trailing edge. It would be the same in my flag version, just referring to the edges and not the state of the button. Which, again, is correct but you have to think about it.

a7

1 Like
18

Since you checked for a change in the previous line, doesn't this simplify to:

// yes the button is different to last time, set flags
    bool leadingEdge = thisButton;     // button is down and wasn't, became pressed
    bool trailingEdge = !thisButton;    // button is up and wasn't, became released
19

Yes, but either you miss or make my point.

With the simplification, we are right back to wondering why the state of the button is now the edge… even if what explains it is the previous line.

The explicit "that was then, this is now" of both initial value expressions means the logic is right there.

I'm still working on this, maybe

  bool thisButton = digitalRead(buttonPin) == PRESST;    // true if switch is closed

// look for edges
  bool leadingEdge = thisButton && !prevousButton;     // button is down and wasn't, became pressed
  bool trailingEdge = !thisButton && prevousButton;    // button is up and wasn't, became released

  prevousButton = thisButton;

  if (leadingEdge) ... 

  if ( trailingEdge) ... 

  if (leadingEdge || trailingEdge) delay(25);  // ride out any bouncing

And fitting in the crude debounce, or using a global poor man's loop killing always delay(25) or getting around to placing the button stuff in its own millis() based mechanism. I usually do the "ignore buttons for awhile after an event" pattern.

In the bitwise version, the reading is N bits worth of switches and the logical operators are replaced with bitwise logical operators and we detect all edges leading or trailing with the reading and the two exprsssions. When I am at a larger information device I will check confirm that your optimisation can be done in that case.

a7

1 Like
20

I like that better. Maybe even add the changed case:

const byte buttonPin = 2;

const byte toggleLED0 = 12;
const byte toggleLED1 = 11;

# define PRESST LOW

void setup() {
  Serial.begin(115200);
  Serial.println("Ready.\n");

  pinMode(buttonPin, INPUT_PULLUP);

  pinMode(toggleLED0, OUTPUT);
  pinMode(toggleLED1, OUTPUT);
}

bool previousButton;  // true means button is pressed. down. shorted.

void loop() {

  bool thisButton = digitalRead(buttonPin) == PRESST;    // true if switch is closed

  // detect state change?

  bool buttonChanged = thisButton != previousButton;     // button is different than last time
  bool leadingEdge = thisButton && !previousButton;     // button is down and wasn't, became pressed
  bool trailingEdge = !thisButton && previousButton;    // button is up and wasn't, became released

  // record for state change detection
  previousButton = thisButton;

  // just toggle. one on the leading edge
  if (leadingEdge)
    digitalWrite(toggleLED0, digitalRead(toggleLED0) == HIGH ? LOW : HIGH);

  // and ther other one on the trailing edge
  if (buttonChanged) {
    digitalWrite(toggleLED1, digitalRead(toggleLED1) == HIGH ? LOW : HIGH);
    delay(50);    // poor man's debounce. only when the state change is done
  }
}

which shows nice, boolean ways of testing the three different flavors of state changes, so one could use what they need. But if a user decides to discard some of the tests, the non-loop-kneecapping conditional debounce scheme still needs to see both edges.

21

It took me longer to agree instantly than I will admit.

FWIW, a button library that shall go unnamed because I forget all about it after flunking it was flunked, in fact, because it seemed to agree with the idea that switches do not bounce on break.

They can. They do.

a7

1 Like