12

I currently have 2 buttons hooked up to my Raspberry Pi (these are the ones with ring LED's in them) and I'm trying to perform this code

#!/usr/bin/env python
import RPi.GPIO as GPIO
import time

GPIO.setmode(GPIO.BCM)
GPIO.setwarnings(False)
GPIO.setup(17, GPIO.OUT) #green LED
GPIO.setup(18, GPIO.OUT) #red LED
GPIO.setup(4, GPIO.IN, GPIO.PUD_UP) #green button
GPIO.setup(27, GPIO.IN, GPIO.PUD_UP) #red button

def remove_events():
        GPIO.remove_event_detect(4)
        GPIO.remove_event_detect(27)

def add_events():
        GPIO.add_event_detect(4, GPIO.FALLING, callback=green, bouncetime=800)
        GPIO.add_event_detect(27, GPIO.FALLING, callback=red, bouncetime=800)

def red(pin):
        remove_events()
        GPIO.output(17, GPIO.LOW)
        print "red pushed"
        time.sleep(2)
        GPIO.output(17, GPIO.HIGH)
        add_events()

def green(pin):
        remove_events()
        GPIO.output(18, GPIO.LOW)
        print "green pushed"
        time.sleep(2)
        GPIO.output(18, GPIO.HIGH)
        add_events()

def main():
    while True:
        print "waiting"
        time.sleep(0.5)

GPIO.output(17, GPIO.HIGH)
GPIO.output(18, GPIO.HIGH)
GPIO.add_event_detect(4, GPIO.FALLING, callback=green, bouncetime=800)
GPIO.add_event_detect(27, GPIO.FALLING, callback=red, bouncetime=800)

if __name__ == "__main__":
    main()

On the surface it looks like a fairly easy script. When a button press is detected:

  1. remove the events
  2. print the message
  3. wait 2 seconds before adding the events and turning the LED's back on

Which normally works out great when I press the green button. I tried it several times in succession and it works without fail. With the red, however, it works well the first time, and the second time, but after it has completed it second red(pin) cycle the script just stops.

Considering both events are fairly similar, I can't explain why it fails on the end of the 2nd red button.

EDIT: I have changed the pins from red and green respectively (either to different pin's completely or swap them). Either way, it's always the red button code (actually now green button) causes an error. So it seems its' not a physical red button problem, nor a pin problem, this just leaves the code to be at fault...

sohnryang
  • 725
  • 2
  • 13
  • 27
user5740843
  • 1,540
  • 5
  • 22
  • 42
  • Perhaps one of the `GPIO.output` calls raised an exception and then `add_events()`was never called again? – zvone Sep 30 '16 at 07:08
  • Thanks for your thought on the matter. I've added except clauses but they weren't triggered. Looks like this wasn't it. – user5740843 Sep 30 '16 at 12:51
  • It also wouldn't explain why it works well once but always fails at the end of the second cycle... – user5740843 Sep 30 '16 at 12:51
  • This code does not show any other possibility that I can see. Is that all the code you have? Did you try other output pins (other than 17 and 18)? Did you try other input pins (other than 4 and 27)? What happens if you alternate 1 green click - 1 red click etc. Does it still stop responding after second red click, or after second green click? Can you see the prints on some serial monitor? If yes, add more (e.g. after every line), so that you know the exact path before the problem. – zvone Sep 30 '16 at 20:32
  • This is seriously all the code that I have. I have also tried different pins, yes. The result seems stunningly the same. I've also alternated, it always seems to stop at the second cycle of the red no matter what I try (i.e. 1 red, 10 green and then 1 more red) I've also add print lines to see where it goes wrong and it does it up until the last line of code, so I'm really baffled... – user5740843 Sep 30 '16 at 21:37
  • So, what happens when you change pins? If you swap red and green lights, does green cause problems? And if you switch red and green buttons? And if you switch 17 and 18 in the code? And if you swap 4 and 27 in the code? Which light causes problems if you use completely different pins? The one which is physically red, or the one you call "red" in the code? If you add prints after each line, which print is printed last? Does it still print "waiting" every 0.5 seconds after it stops working? – zvone Sep 30 '16 at 21:48
  • It looks like if I change (completely or swap them) the pins from red and green respectively, the red button code (actually now green button) still causes an error. So it seems its' not a physical red button problem, nor a pin problem, this just leaves the code to be at fault... – user5740843 Oct 02 '16 at 14:47
  • Does the code only hang if you are pressing both buttons alternatively? Does the faulty button still fail if you do not mix it with pressing the working button? Perhaps also try to setWarnings to True so you can see errors as they appear to be disabled from code above. – Matz Oct 02 '16 at 23:51
  • `GPIO.add_event_detect(4, GPIO.FALLING, callback=lambda pin=4 : green(pin), bouncetime=800)` AND `GPIO.add_event_detect(27, GPIO.FALLING, callback=lambda pin=27: green(pin), bouncetime=800)` So **how to call function with an argument ?** – dsgdfg Oct 08 '16 at 13:30

1 Answers1

9

I was able to reproduce your problem on my Raspberry Pi 1, Model B by running your script and connecting a jumper cable between ground and GPIO27 to simulate red button presses. (Those are pins 25 and 13 on my particular Pi model.)

The python interpreter is crashing with a Segmentation Fault in the thread dedicated to polling GPIO events after red returns from handling a button press. After looking at the implementation of the Python GPIO module, it is clear to me that it is unsafe to call remove_event_detect from within an event handler callback, and this is causing the crash. In particular, removing an event handler while that event handler is currently running can lead to memory corruption, which will result in crashes (as you have seen) or other strange behaviors.

I suspect you are removing and re-adding the event handlers because you are concerned about getting a callback during the time when you are handing a button press. There is no need to do this. The GPIO module spins up a single polling thread to monitor GPIO events, and will wait for one callback to return before calling another, regardless of the number of GPIO events you are watching.

I suggest you simply make your calls to add_event_detect as your script starts, and never remove the callbacks. Simply removing add_events and remove_events (and their invocations) from your script will correct the problem.

If you are interested in the details of the problem in the GPIO module, you can take a look at the C source code for that module. Take a look at run_callbacks and remove_callbacks in the file RPi.GPIO-0.6.2/source/event_gpio.c. Notice that both of these functions use a global chain of struct callback nodes. run_callbacks walks the callback chain by grabbing one node, invoking the callback, and then following that node's link to the next callback in the chain. remove_callbacks will walk the same callback chain, and free the memory associated with the callbacks on a particular GPIO pin. If remove_callbacks is called in the middle of run_callbacks, the node currently held by run_callbacks can be freed (and have its memory potentially reused and overwritten) before the pointer to the next node is followed.

The reason you see this problem only for the red button is likely due to the order of calls to add_event_detect and remove_event_detect causes the memory previously used by the callback node for the red button to be reclaimed for some other purpose and overwritten earlier than the memory used from the green button callback node is similarly reclaimed. However, be assured that the problem exists for both buttons -- it is just luck that that the memory associated with the green button callback isn't changed before the pointer to the next callback node is followed.

More generally, there is a concerning lack of thread synchronization around the callback chain use in the GPIO module in general, and I suspect similar problems could occur if remove_event_detect or add_event_detect are called while an event handler is running, even if events are removed from another thread! I would suggest that the author of the RPi.GPIO module should use some synchronization to ensure that the callback chain can't be modified while callbacks are being made. (Perhaps, in addition to checking whether the chain is being modified on the polling thread itself, pthread_mutex_lock and pthread_mutex_unlock could be used to prevent other threads from modifying the callback chain while it is in use by the polling thread.)

Unfortunately, that is not currently the case, and for this reason I suggest you avoid calling remove_event_detect entirely if you can avoid it.

mkimball
  • 764
  • 4
  • 9
  • Good debugging. You might mention how you were able to catch the interpreter in a segfault, but otherwise it is very complete information. In part because GPIO only services one event at a time, it is not a good idea to sleep in an event handler. The solution is to set an event end-time and then keep scanning. The way to avoid multiple events is to use the test-and-swap commands, which on ARMv6 (Raspberry Pi 1) and later are STREX and LDREX: http://infocenter.arm.com/help/topic/com.arm.doc.dht0008a/DHT0008A_arm_synchronization_primitives.pdf Maybe for the Pi and Python there is a wrapper. – Daniel Wisehart Oct 09 '16 at 14:05