Interlocked buttons with clear troubleshooting

Hi,
I'm building a foot controller to select presets on a DSP based audio processor. The interface is quite simple; pull a pin low to select that preset. The manufacturer even provides +5VDC and ground for powering external circuitry.

My first sketch just interlocked 3 buttons; press one button and it turns the other outputs off and the selected one on. It works fine, but you can't turn all of the selections off; one always remains on.
I then tried to add a function to clear a selection by looking at the state of the output; if an output is already on and it's button is pressed again, turn it off.
Unfortunately, it doesn't work and I'm mystified as to why it doesn't work. I can't get any of the outputs to turn on. What am I missing? Thanks in advance.

// interlocked_buttons_2
// machambers 9/12/2021
// Arduino Uno & SparkFun MIDI Shield
// HIGH = Off, LOW = On
// in version 1 interlocks work, but you can't turn all selections off

// Constants won't change:
const int button1 = 2;   // the pin that the pushbutton is attached to
const int led1 = 5;      // the pin that the LED is attached to
const int button2 = 3;
const int led2 = 6;
const int button3 = 4;
const int led3 = 7;

void setup() {
  // initialize the button pins as inputs:
  pinMode(button1, INPUT_PULLUP);
  pinMode(button2, INPUT_PULLUP);
  pinMode(button3, INPUT_PULLUP);
  // initialize the LED pins as outputs:
  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);
  pinMode(led3, OUTPUT);
  digitalWrite(led1, HIGH);
  digitalWrite(led2, HIGH);
  digitalWrite(led3, HIGH);
}

void loop() {

  if (digitalRead(button1) == LOW && led1 == HIGH) {
    // if the current state is LOW then the button went from off to on
    // if led1 is HIGH it is not enabled; turn off led2 & led3 then enable led1
    digitalWrite(led2, HIGH);
    digitalWrite(led3, HIGH);
    digitalWrite(led1, LOW);
    // below added to turn selection off if it's already on
  } else if (digitalRead(button1) == LOW && led1 == LOW) {
    // if led1 is low it is enabled; disable it
    digitalWrite(led1, HIGH);
  } while (digitalRead(button1) == LOW);
  delay(50);

  if (digitalRead(button2) == LOW && led2 == HIGH) {
    digitalWrite(led1, HIGH);
    digitalWrite(led3, HIGH);
    digitalWrite(led2, LOW);
  } else if (digitalRead(button2) == LOW && led2 == LOW) {
    digitalWrite(led2, HIGH);
  } while (digitalRead(button2) == LOW);
  delay(50);

  if (digitalRead(button3) == LOW && led3 == HIGH) {
    digitalWrite(led1, HIGH);
    digitalWrite(led2, HIGH);
    digitalWrite(led3, LOW);
  } else if (digitalRead(button3) == LOW && led3 == LOW) {
    digitalWrite(led3, HIGH);
  } while (digitalRead(button3) == LOW);
  delay(50);

}
if (digitalRead(button1) == LOW && led1 == HIGH)

You are testing the value of the led pin not the state of the led pin

Hello
In short words:

  • Button 1 pressed ---> make led pattern 1
  • Button 1 released ---> make nothing
  • Button 2 pressed ---> make led pattern 2
  • Button 2 released ---> make nothing
  • Button 3 pressed ---> make led pattern 3
  • Button 3 released ---> make nothing
  • and so on

Does these desribtion met your project?

Are you suggesting that I use true and false instead of high and low?
I'll give it a try.
Thanks

Not quite.
Button 1 pressed > turn off all other LEDs, turn on LED 1
Button 2 pressed > turn off all other LEDs, turn on LED 2
Button 3 pressed > turn off all other LEDs, turn on LED 3
Button 3 pressed while LED 3 is on > turn off LED 3
...
If the button associated with an LED is pressed while the LED is on, the LED should be turned off.
Pressing any button turns off all other LEDs and turns on the one associated with the button.

how about

#undef MyHW
#ifdef MyHW
const byte pinsBut [] = { A1, A2, A3 };
const byte pinsLed [] = { 11, 12, 13 };

#else
const byte pinsBut [] = { 2, 3, 4 };
const byte pinsLed [] = { 5, 6, 7 };
#endif

#define N   sizeof(pinsBut)

void setup() {
    for (unsigned n = 0; n < N; n++)  {
        pinMode (pinsBut [n], INPUT_PULLUP);
        pinMode (pinsLed [n], OUTPUT);
    }
}

void loop() {
    for (unsigned n = 0; n < N; n++)
        digitalWrite (pinsLed [n], digitalRead (pinsBut [n]));
}

No. I am suggesting that you use

if (digitalRead(button1) == LOW && digitalRead(led1) == HIGH)

The value of led1 is never going to change. It will always be 5 in your code but the state of pin 5 may change depending on what you write to it.

Before you ask, yes, you can read the state of a pin defined as an output

@machambers

To be clear, you have momentary contacts like a pushbutton, and you are trying to do an old school “radio button” type thing.

Press a button, get that station. Press another, switch stations.

And I forget how, but there was a way to get all the buttons to pop out.

Old multi-line telephones had the same thing.

I didn’t look further, but

 arduino radio buttons code

had a crap-ton of plausible links. But you close now, so.

This

while (digitalRead(button2) == LOW);
  delay(50);

Is wrong in several ways, use

    do {
        delay(50);
    } while (digitalRead(button2) == LOW);
  

For your simple denouncing need. You made the while look like it was part of the if/else, the IDE autoformat tool might show you that syntactically it was a separate statement entirely.

And bounces are rapid, best to wait first and then do the checking otherwise you can catch a glitch. In this case it might not make a difference, but.

EDIT: Curiously, IDE auto format curtly informs me "No changes necessary for Auto Format". When I "fix" the code layout by Manual Format, so to speak, it set "finished",having done nothing, and then reports no changes necessary for subsequent Auto Formats.

So Auto Format isn't all it could be or my caffeine hasn't kicked in yet.

a7

Thank you for your help.
Now I understand why what I did didn't work.
The sketch now works as I intended.

Hi,

If you have entered/edited code and it is still formatted correctly, you get that response.

When you manually "fixed" the code, did you look and see where in the layout auto it fixed it ?
Even an extra unnecessary space removed is a fix.

Tom... :smiley: :+1: :coffee: :australia:

      if (buttonState2 == HIGH) {
        ledState3 = !ledState3;
      } while (1);

is fine with auto format. No changes necessary.

The only blanks (spaces) I could get it to care about were spaces after a semicolon at the end of a line… kinda hard to see. :expressionless:

TBH I never noticed the report of the auto format process until this day.

So auto format does some good but is less than perfect, shall we say. Since I tend to just write perfectly formatted code (!), my use of auto format is to fix code that has been improperly posted, or if properly posted code that is way off - usually just to find out where some dumb mistake like a missing brace (or pair) has caused errors, be they syntax or logic errors.

a7

In contrast to you I use Auto Format frequently when writing code and most certainly on code copied from the forum and I have it configured to suit me

For instance, when presented with

if (buttonState2 == HIGH) {
        ledState3 = !ledState3;
      } while (1);

my Auto Format turns it into

  if (buttonState2 == HIGH)
  {
    ledState3 = !ledState3;
  }
  while (1);

but that is just a matter of taste

Hi,
There is a bit of script in the IDE AutoFormat that makes the { drop to the next line like yours.

I remember long long time ago, there was a thread about how to make the { drop to the indented next line, rather than stay on the end of the statement.

My IDE doesn't and I'm not sure if the 2.whateveritis Beta does.

Tom... :smiley: :+1: :coffee: :australia:

I know why it is formatted as it us because I set it up to do it that way, and it is the way I prefer it

There is apparently a facility to change the Auto Format layout in Beta 2.0 of the IDE but I have not got to the bottom of it, which is one reason why I have not seriously tried the new IDE

Hi,

How??
Tom... :smiley: :+1: :australia:

In the same folder as preferences.txt you may or may not have a file named formatter.conf. It is this file that controls the actions of Auto Format

Here is mine

# This configuration file contains a selection of the available options provided by the formatting tool "Artistic Style"
# http://astyle.sourceforge.net/astyle.html
#
# If you wish to change them, don't edit this file.
# Instead, copy it in the same folder of file "preferences.txt" and modify the copy. This way, you won't lose your custom formatter settings when upgrading the IDE
# If you don't know where file preferences.txt is stored, open the IDE, File -> Preferences and you'll find a link

# 2 spaces indentation
indent=spaces=2

# also indent macros
indent-preprocessor

# indent classes, switches (and cases), comments starting at column 1
indent-classes
indent-switches
indent-cases
indent-col1-comments

# put a space after if/for/while
 pad-header

# Move opening brackets onto new line
 --style=allman --style=bsd --style=break -A1

# delete empty lines in functions
 --delete-empty-lines 

# Insert space padding around operators. 
 --pad-oper

Hi,
Thanks, I don't appear to have one, but I'll use yours and make a copy.
See what happens, thanks mate.

UPDATE.. Just copied and saved it, reboot of IDE and magic, detent { works.

Tom... :smiley: :+1: :coffee: :australia:

I do know that with some minor trouble the behavior of auto format can be adjusted.

But it seems odd that out of the box I would have to adjust it to reformat the example I gave, which makes it look like there’s such a thing as a if-while loop… and you know for sure that it wouldn’t take a noob to think there should be such a thing and just how it would function.

And noobs are the least likely to go to to the trouble of fixing auto format.

I don’t use it for myself, as I said. I think by now the formatter I use lives somewhere between my elbow and my fingers. :expressionless:

a7