0

I am trying to create a program that detects the state of three different buttons, connected to GPIO pins on a Raspberry Pi, and once all three have been HIGH once, an action is taken. Right now I have all the buttons working individually through callback functions, but the if statement inside the 'main' function does not seem to be running.

This is my first time using Python so please let me know if you see any other logical errors in the structure of my code. Still trying to get a hang of it, especially the GPIO library functions. Thanks in advance, I've posted my code below.

import RPi.GPIO as GPIO

GPIO.setmode(GPIO.BCM)

butOne = False
butTwo = False
butThree = False

# Setup button inputs
GPIO.setup(19, GPIO.IN, pull_up_down=GPIO.PUD_DOWN)
GPIO.setup(20, GPIO.IN, pull_up_down=GPIO.PUD_DOWN)
GPIO.setup(21, GPIO.IN, pull_up_down=GPIO.PUD_DOWN)

GPIO.add_event_detect(19, GPIO.RISING)
GPIO.add_event_detect(20, GPIO.RISING)
GPIO.add_event_detect(21, GPIO.RISING)

def butOne_callback(channel1):
    print("Button 1 /n")
    butOne = True

def butTwo_callback(channel2):
    print("Button 2 /n")
    butTwo = True

def butThree_callback(channel3):
    print("Button 3 /n")
    butThree = True

def main():
    GPIO.add_event_callback(19, butOne_callback)
    GPIO.add_event_callback(20, butTwo_callback)
    GPIO.add_event_callback(21, butThree_callback)

    if (butOne == True) and (butTwo == True) and (butThree == True):
        print("All Depressed")
main()

UPDATED CODE, according to Aditya Shankar's suggestions:

import RPi.GPIO as GPIO

GPIO.setmode(GPIO.BCM)

GPIO.setup(19, GPIO.IN, pull_up_down=GPIO.PUD_DOWN)
GPIO.setup(20, GPIO.IN, pull_up_down=GPIO.PUD_DOWN)
GPIO.setup(21, GPIO.IN, pull_up_down=GPIO.PUD_DOWN)

GPIO.add_event_detect(19, GPIO.RISING)
GPIO.add_event_detect(20, GPIO.RISING)
GPIO.add_event_detect(21, GPIO.RISING)

def butOne_callback(channel1):
    print("Button 1 /n")
    butOne = True
    check_all_depressed()

def butTwo_callback(channel2):
    print("Button 2 /n")
    butTwo = True
    check_all_depressed()

def butThree_callback(channel3):
    print("Button 3 /n")
    butThree = True
    check_all_depressed()

def check_all_depressed():
    if butOne and butTwo and butThree:
        print("All Depressed")

GPIO.add_event_callback(19, butOne_callback)
GPIO.add_event_callback(20, butTwo_callback)
GPIO.add_event_callback(21, butThree_callback)

Error received when code is run and button is pressed:

Traceback (most recent call last): File "/home/pi/Downloads/GPIO_test_06.py", line 21, in butTwo_callback check_all_depressed() File "/home/pi/Downloads/GPIO_test_06.py", line 29, in check_all_depressed if butOne and butTwo and butThree: NameError: name 'butOne' is not defined

Brayton Larson
  • 67
  • 1
  • 1
  • 8
  • 1
    Try wrapping the if statement in a `while True` loop and see what that does (place `while True` above the if statement then indent the 2 lines below it). – lyxαl Feb 05 '19 at 02:44
  • The problem lies also in the other functions, not only in `main()`. See https://stackoverflow.com/questions/14421733/global-and-local-variables-in-python – Michael Butscher Feb 05 '19 at 02:45
  • 1
    Each function that writes to a global need a global statement listing the names of the globals it writes to. – Dan D. Feb 05 '19 at 02:50

3 Answers3

2

Your if statement runs, but only once - immediately when the script is first started. By that time, the buttons haven't been pushed and thus it seems like it's not working.

One way of solving that would be to put the statement in a loop with a small delay and test for the condition in that loop. Something like:

import time

while not condition:
    time.sleep(1)

Another issue is one of style. You can write your condition:

(butOne == True) and (butTwo == True) and (butThree == True)

simply as:

butOne and butTwo and butThree

since they all are boolean values to begin with. In Python, you can even write:

all([butOne, butTwo, butThree])

That's not shorter, but if you had even more conditions, it would avoid repeating and again and again.

Finally, you've chosen to create a main function, that runs the main program. It's probably a good idea to include all the code above your function definitions in there as well. After all, it's all part of your main program and it is all meant to run only once. This way, you also avoid accidentally using global variables inside of functions, which can result in unexpected (but technically correct) behaviour.

Grismar
  • 27,561
  • 4
  • 31
  • 54
2

Answer:

remove the if condition

add a function check_all_depressed()

add the function to the end of all three button callbacks, like this

def butOne_callback(channel1):
    global butOne
    print("Button 1 /n")
    butOne = True
    check_all_depressed()

check_all_depressed looks like this -

def check_all_depressed():
    if butOne and butTwo and butThree:
        print("All Depressed")

Explanation: So, there are callbacks and there is general program flow. basically python programs follow an order of events of occurrence, that is top to bottom, callbacks occur outside this flow.

Aditya Shankar
  • 702
  • 6
  • 12
  • Thank you for the response. I implemented your code and am getting an undefined variable error. The problem I believe is that the variables are not global and therefore are defined in the individual button callbacks but not in the "check_all" function. I thought about passing in the variable, but I'm not sure how that would work being that each callback has an individual variable it would pass in. How can I keep track of the butOne, butTwo, and butThree variables outside of the callbacks? Thank you for any advice, I'll add my updated code to the OP. – Brayton Larson Feb 10 '19 at 18:59
  • @BraytonLarson Fixed the issue, the `global` thing is required to change global variables, sorry ! – Aditya Shankar Feb 11 '19 at 04:40
1

Fundamentally, the way the GPIO package supports events is by waiting for rising and falling edges on the channels you select. This is done in a background thread, regardless of what happens in the main thread. Your if statement runs once, immediately after the buttons are configured, and then the main thread ends.

There are two types of solutions you can implement. One is to force the main thread to wait for a change in state. The other is to respond to changes in state in the callbacks.

To force main to wait:

import RPi.GPIO as GPIO

channels = [19, 20, 21]

def main():
     GPIO.setmode(GPIO.BCM)
     for chan in channels:
         GPIO.setup(chan, GPIO.IN, pull_up_down=GPIO.PUD_DOWN)
     for chan in channels:
         GPIO.wait_for_edge(chan, GPIO.RISING)

     # Now all your buttons have been pressed

This is probably the more efficient way to do it. Minimal setup and no explicit multithreading.

The alternative is to listen for the input in a separate thread. You can configure your callbacks to respond to a rising edge as you have done with add_event_callback, but keep in mind that that function is mostly for setting up multiple callbacks. A more succinct way would be to move the calls to add_event_detect into main and combine them with add_event_callback:

GPIO.add_event_detect(chan, GPIO.RISING, callback=...)

From a programmatic point of view, I'd use the fact that all the channels are treated almost the same and only define one callback. In fact all that matters in your setup is the channel number and the channel name:

import RPi.GPIO as GPIO

channels = {19: 1, 20: 2, 21: 3}

class callback:
    def __init__(self):
         self.pressed = dict.fromkeys(channels, False)
    def __call__(self, channel):
         print(f'Button {channels[channel]} pressed')
         self.pressed[channel] = True
         if sum(self.pressed.values()) == len(self.pressed):
             # All buttons have been pressed

def main():
     GPIO.setmode(GPIO.BCM)
     cb = callback()
     for chan in channels:
         GPIO.setup(chan, GPIO.IN, pull_up_down=GPIO.PUD_DOWN)
         GPIO.add_event_detect(chan, GPIO.RISING, callback=cb)

Notice that in both examples, I have avoided global state aside form the channel configurations. The second way sets the callback to an instance of a callable class to do so. The alternative is to define the callback as a nested function in main.

Mad Physicist
  • 107,652
  • 25
  • 181
  • 264
  • Thank you for taking the time to respond and writing a detailed explanation. I have a few questions about your second solution though: 1. First off obviously it isn't working for me when I run the code, no output is produced. 2. Does the code you have written run independently or do I need to do extra setup outside of the functions you've written. 3. What is the purpose of "chan" and "cb" if they are just set equal to "channel" and "callback". There is obviously plenty I need to learn about the syntax in Python, thank you for any explanation you can take the time to provide. – Brayton Larson Feb 10 '19 at 19:35
  • `cb` is an instance of the callback class. `chan` is a loop variable. It takes the value of each element of `channels` in succession. – Mad Physicist Feb 10 '19 at 20:22
  • You may need to get a reference to the gpio background thread and join on it from main since it might be a demon thread. – Mad Physicist Feb 10 '19 at 20:26