0

I'm using a Raspberry Pi to run a motor off a button and some LEDs. The LEDs have to perform a flashing pattern based on certain variables, and I have them running in their own thread to allow the code to keep working (I got the idea from here, of course).

The problem is, depending on how fast the user pushes the button (a sequence requires multiple pushes to set a speed), the LED thread, which is stopped when the button is released, is sometimes called again before the last one has terminated.

This means, I think, there are two threads now, so when the command to stop the run() action in the LED thread is sent, I believe one of the threads (or both, maybe) doesn't get the stop command, so the LED stays flashing (as the Thread calls for) after the button is released for good.

So I think what I'm looking for is a way to ensure only one instance of the LED thread runs at a time. I think the easiest way would be for the LED Thread to, as it's first step, terminate all other instances of the LED Thread that might be running. I like this because no matter which light is flashing from the Thread, it just stops, and a new thread from the most recent action calling for a flashing light takes over.

How do I do this? I've been looking at the isActive() function, but I'm not able to conceptualize it I guess, so I don't know how to use it.

Here is the LED thread for reference. It is called by other .py scripts using the flash_led() function and stopped with the stop_led() function:

## leds.py. This is imported into the scripts that run the motors and buttons

# LED control Thread (Which-LED, Desired-State On/Off/Flash, Flash-On-Duration, Flash-Off-Duration, Number-Of-Flashes)

class LEDFlash(Thread):

    #Initial parameters for an LED Flash command, overwritten by led_flash()
    led_pin = led_dict["red_led_1"] #from a dict earlier in the script
    flash_on_duration = 1.0
    flash_off_duration = 1.0

    def __init__(self):
        super(LEDFlash, self).__init__()
        self.keep_flashing = True

    def run(self):
        while self.keep_flashing:
            GPIO.output(self.led_pin, GPIO.HIGH)
            time.sleep(self.flash_on_duration)
            GPIO.output(self.led_pin, GPIO.LOW)
            time.sleep(self.flash_off_duration)

    def stop_flash(self):
        self.keep_flashing = False


def flash_led(led, on_time, off_time): # 'flash_led.' added to make flash_thread accessible from the stop_led() function
    flash_led.flash_thread = LEDFlash()
    flash_led.flash_thread.led_pin = led_dict[str(led)]
    flash_led.flash_thread.flash_on_duration = on_time
    flash_led.flash_thread.flash_off_duration = off_time
    flash_led.flash_thread.start()

def stop_led():
    flash_led.flash_thread.stop_flash()
max
  • 49,282
  • 56
  • 208
  • 355

2 Answers2

1

If I understand correctly, you are asking how to stop a thread in python. I like this existing post for that topic: Is there any way to kill a Thread in Python? .

Having said that, you may also want to consider the option of not stopping the thread when the button is released, because creating threads is a relatively expensive operation, and, as mentioned in the above thread, you probably shouldn't outright kill the thread anyway -- you will want to wait for it to respond to an Event instead. If you go down that route, you may as well have an event for a state change, and just inform that existing thread of what state it should be in, rather than creating a whole new thread that would just end up being in that same state.

Michael Nelson
  • 370
  • 2
  • 11
  • Thanks for the reply. I learned Python (started to, at least) a grand total of three days ago, so I've been winging it. I don't know much about events, or how to pass an event from one script (the script that watches the buttons) to leds.py (the script that controls the flashing of the lights). I understand the idea, I think, I just don't know how to make that happen – Seth Ratner Apr 16 '17 at 22:39
  • @SethRatner You're doing very well for 3 days of Python. max's answer will get you where you want to be, and has good suggestions for code structure too. – Michael Nelson Apr 16 '17 at 23:22
  • @SethRatner I can't comment on max's post, but if your app is mission critical, you might want to make sure nothing bad happens to the system if you press the button rapidly for a minute or two -- the cost of all the threads might start to add up. If you do find a need to keep the number of a threads to a minimum, the simplest thing is to change while self.keep_flashing: to `while True:\n if self.keep_flashing:` and have flash_led and stop_led functions modify the flash_thread properties. Depending how your app works, that might be enough, or you might need a way to stop the thread at exit – Michael Nelson Apr 16 '17 at 23:51
1

The advice from @MichaelNelson to only have one permanent thread that deals with the LED is really the best approach.

But to solve your problem directly, you can add this code at the start of flash_led():

if hasattr(flash_led, 'flash_thread'):
  stop_led()

This would ensure that the old thread, if any, is destroyed before the new thread is created.

Edit: there is still a chance that both the old and the new threads are active for a brief time; it shouldn't be a problem in your case, but if it is, you can say:

if hasattr(flash_led, 'flash_thread'):
  stop_led()
  flash_led.flash_thread.join() # wait until old thread completes

You definitely do NOT want to kill a thread; as explained here, terminating a thread is extremely dangerous and messy, and should only be done in extremely rare circumstances where no other approach works. Your situation is completely standard, and can be handled with perfectly clean code, without resorting to any ugly stuff.

In addition, I'd suggest to replace the function attribute with a global variable; they are roughly equivalent, but global variables are much easier to keep track of when someone reads your code. In other words, just write:

flash_thread = None
def flash_led(led, on_time, off_time):
    # you need global statement at the beginning of every function
    # in which you modify a global variable
    global flash_thread
    if flash_thread is not None:
      flash_thread.stop_flash() # probably no need to define stop_led() 
    flash_thread = LEDFlash()
    flash_thread.led_pin = ...
    ...

Even better is to refactor the code to store it as an instance attribute of some controller object, but that would require more work.

Community
  • 1
  • 1
max
  • 49,282
  • 56
  • 208
  • 355
  • Ok, just so I have it straight: The code you provided isn't "killing" a thread, because it's actually just ending the previous thread using the stop function. Which makes the existing thread vanish after the run() has completed... right? Therefore it will be gone before the new one is created? – Seth Ratner Apr 16 '17 at 22:34
  • Exactly. You were basically already doing that, except sometimes the new thread was created before the old one was sent the instruction to stop. – max Apr 16 '17 at 22:39
  • This worked great. Thanks! Any chance you could elaborate on making the function attribute global? I started with python a few days ago an am still rather weak. Thanks! – Seth Ratner Apr 16 '17 at 23:19
  • I updated the answer to clarify. Enjoy python with raspberry pi! – max Apr 16 '17 at 23:33