Problem using Buttons to swich LED's on and off

Question

Im making a prototype of a model lift shaft. The LEDs in this example will eventually be replaced with relays. Side Note For those who do not know, a limit switch is a microswitch plces at the top,bottom of the lift shaft that the lift car will activate when it reaces the set height. so here is my code:

import RPi.GPIO as GPIO
GPIO.setmode(GPIO.BOARD)
GPIO.setup(7,False)# Button Limit switch
GPIO.setup(11,False)#button - Call (ground)
GPIO.setup(12,False)#button- Up (Ground floor)
GPIO.setup(13,GPIO.OUT)#LED
GPIO.setup(15,GPIO.OUT)#LED
GPIO.setup(16,False)#button - Down (1st floor)
GPIO.setup(18,False)#button - Call (1st floor)
GPIO.setup(19,False)#button - Limit switch
GPIO.output(13, False)
GPIO.output(15, False)

def down():
    if GPIO.input(7) == False:
        GPIO.output(13,True)
        print ("Going Down")
        while GPIO.output(13, True):
            if GPIO.input(7) == True:
                GPIO.output(13, False)
                Print ("You have Reached The Ground Floor")
                button_select()
            else:
                pass
    else:
        pass

def button_select():
    if GPIO.input(11) == True:
        Print ("Called to Ground Floor")
        down()
    elif GPIO.input(16) == True:
        Print ("Down To Ground Floor selected")
        down()
    else:
        pass

the problem I have is that when I press the limit switch on the ground floor, the LED stays lit, rather than turning off. If this were a real lift system, I'm sure you could see why this is a problem! I'm using sudo idle on raspian wheezy. Any help to sole this would be greatly appreciated. I'll keep you informed if I find a solution.

Answer 1

I reworked your code to be a little tidier, but then realised that the RPIO module actually provides an event-handler like interface which is much preferable to waiting in loops. In fact, this may be the source of your original bug. So I've reworked your example to use interrupts to handle the events (button presses, limit switches) that are present in your system.

Here it is in parts. First, further to the comment about 'magic numbers', here we make them even less magic by defining some constants that we can use throughout the code. This allows us to refer to buttons by what they do:

import RPIO
RPIO.setmode(RPIO.BOARD)

# Define constants for clarity
LIMIT_SWITCH_DOWN = 7
BUTTON_CALL_GROUND = 11
BUTTON_UP_GROUND = 12

LED_MOVING_DOWN = 13
LED_MOVING_UP = 15

BUTTON_DOWN_1ST = 16
BUTTON_CALL_1ST = 18
LIMIT_SWITCH_UP = 19

Next follows the equivalent to your own setup, but now using these and the RPIO constants. Hopefully you can see how this is clearer:

RPIO.setup(LIMIT_SWITCH_DOWN, RPIO.IN)
RPIO.setup(BUTTON_CALL_GROUND, RPIO.IN)
RPIO.setup(BUTTON_UP_GROUND, RPIO.IN)

RPIO.setup(LED_MOVING_DOWN, RPIO.OUT)
RPIO.setup(LED_MOVING_UP, RPIO.OUT)

RPIO.setup(BUTTON_DOWN_1ST, RPIO.IN)
RPIO.setup(BUTTON_CALL_1ST, RPIO.IN)
RPIO.setup(LIMIT_SWITCH_UP, RPIO.IN)

# Initialisation
RPIO.output(LED_MOVING_DOWN, False)
RPIO.output(LED_MOVING_UP, False)

Next, we need to define our event handler functions. These are the functions that will be called when an event occurs. See the documentation on RPIO interrupts:

def limit_switch_down(gpio_id, val):
    if val == True: # We've hit the limit switch
        # Turn off the light (?! and stop the motor!)
        RPIO.output(LED_MOVING_DOWN, False)
        print("You have reached the first floor")

def limit_switch_up(gpio_id, val):
    if val == True: # We've hit the limit switch
        # Turn off the light (?! and stop the motor!)
        RPIO.output(LED_MOVING_UP, False)
        print("You have reached the first floor")

Taking these first two as an example, when the button LIMIT_SWITCH_DOWN is pressed, the top function is hit. Note that because the gpio_id is passed, you could combine this into a single function:

def limit_switch(gpio_id, val):
    if val == True:
        if gpio_id == LIMIT_SWITCH_DOWN:
            # Turn off the light (?! and stop the motor!)
            RPIO.output(LED_MOVING_DOWN, False)
            print("You have reached the first floor")

        elif gpio_id == LIMIT_SWITCH_UP:
            # Turn off the light (?! and stop the motor!)
            RPIO.output(LED_MOVING_UP, False)
            print("You have reached the first floor")

The rest of the functions follow the same pattern:

def button_call_ground(gpio_id, val):
    # Turn on the down light
    RPIO.output(LED_MOVING_DOWN, True)
    print("Going down!")

def button_call_1st(gpio_id, val):
    # Turn on the down light
    RPIO.output(LED_MOVING_UP, True)
    print("Going up!")

def button_up_ground(gpio_id, val):
    # Turn on the down light
    RPIO.output(LED_MOVING_UP, True)
    print("Going up!")

def button_down_1st(gpio_id, val):
    # Turn on the down light
    RPIO.output(LED_MOVING_DOWN, True)
    print("Going down!")

Now the callback functions are defined, we can attach these functions to the interrupts as follows. Again using constants and labels makes this very clear:

# RPIO interrupt callbacks; when buttons are pressed (or limits triggered)
# the respective function will called
RPIO.add_interrupt_callback(LIMIT_SWITCH_DOWN, limit_switch_down)
RPIO.add_interrupt_callback(BUTTON_CALL_GROUND, button_call_ground)
RPIO.add_interrupt_callback(BUTTON_UP_GROUND, button_up_ground)

RPIO.add_interrupt_callback(BUTTON_DOWN_1ST, button_down_1st)
RPIO.add_interrupt_callback(BUTTON_CALL_1ST, button_call_1st)
RPIO.add_interrupt_callback(LIMIT_SWITCH_UP, limit_switch_up)

The last step is to start the event loop. This will 'sit' in a loop, waiting for events from the defined interrupts (buttons/switches) and call the relevant functions when those states are found:

# Start the event loop
RPIO.wait_for_interrupts()

Note that the above code doesn't apply any sense checks, or use state. So for example, you can call the lift up when it's at the first floor. Have a think about how you would implement that.

A general style hint about your own code: Don't call back into the calling function (i.e. calling button_select() from inside down() which was itself called from button_select()). This is uncontrolled recursion and will eventually crash - if your code runs long enough. Instead, structure your code to have a 'main loop' and 'fall' back out of your subroutines into that.