Merge pull request #39 from waveform80/explicit-cleanup

Explicit cleanup

gpiozero/__init__.py

@@ -1,11 +1,6 @@
 from __future__ import absolute_import
 
-import atexit
-
-from RPi import GPIO
-
 from .devices import (
-    _gpio_threads_shutdown,
     GPIODeviceError,
     GPIODevice,
 )
@@ -35,11 +30,3 @@ from .boards import (
     TrafficHat,
 )
 
-
-def gpiozero_shutdown():
-    _gpio_threads_shutdown()
-    GPIO.cleanup()
-
-atexit.register(gpiozero_shutdown)
-GPIO.setmode(GPIO.BCM)
-GPIO.setwarnings(False)

gpiozero/devices.py

@@ -1,31 +1,96 @@
+import atexit
 import weakref
-from threading import Thread, Event
+from threading import Thread, Event, RLock
 from collections import deque
 
 from RPi import GPIO
 
 
+_GPIO_THREADS = set()
+_GPIO_PINS = set()
+# Due to interactions between RPi.GPIO cleanup and the GPIODevice.close()
+# method the same thread may attempt to acquire this lock, leading to deadlock
+# unless the lock is re-entrant
+_GPIO_PINS_LOCK = RLock()
+
+def _gpio_threads_shutdown():
+    while _GPIO_THREADS:
+        for t in _GPIO_THREADS.copy():
+            t.stop()
+    with _GPIO_PINS_LOCK:
+        while _GPIO_PINS:
+            GPIO.remove_event_detect(_GPIO_PINS.pop())
+        GPIO.cleanup()
+
+atexit.register(_gpio_threads_shutdown)
+GPIO.setmode(GPIO.BCM)
+GPIO.setwarnings(False)
+
+
 class GPIODeviceError(Exception):
     pass
 
+class GPIODeviceClosed(GPIODeviceError):
+    pass
+
 
 class GPIODevice(object):
     """
     Generic GPIO Device.
     """
     def __init__(self, pin=None):
+        # self._pin must be set before any possible exceptions can be raised
+        # because it's accessed in __del__. However, it mustn't be given the
+        # value of pin until we've verified that it isn't already allocated
+        self._pin = None
         if pin is None:
             raise GPIODeviceError('No GPIO pin number given')
+        with _GPIO_PINS_LOCK:
+            if pin in _GPIO_PINS:
+                raise GPIODeviceError(
+                    'pin %d is already in use by another gpiozero object' % pin)
+            _GPIO_PINS.add(pin)
         self._pin = pin
         self._active_state = GPIO.HIGH
         self._inactive_state = GPIO.LOW
 
+    def __del__(self):
+        self.close()
+
     def _read(self):
-        return GPIO.input(self.pin) == self._active_state
+        try:
+            return GPIO.input(self.pin) == self._active_state
+        except TypeError:
+            self._check_open()
+            raise
 
     def _fire_events(self):
         pass
 
+    def _check_open(self):
+        if self.closed:
+            raise GPIODeviceClosed(
+                '%s is closed or uninitialized' % self.__class__.__name__)
+
+    @property
+    def closed(self):
+        return self._pin is None
+
+    def close(self):
+        with _GPIO_PINS_LOCK:
+            pin = self._pin
+            self._pin = None
+            if pin in _GPIO_PINS:
+                _GPIO_PINS.remove(pin)
+                GPIO.remove_event_detect(pin)
+                GPIO.cleanup(pin)
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exc_type, exc_value, exc_tb):
+        self.close()
+
     @property
     def pin(self):
         return self._pin
@@ -35,17 +100,11 @@ class GPIODevice(object):
         return self._read()
 
     def __repr__(self):
-        return "<gpiozero.%s object on pin=%d, is_active=%s>" % (
-            self.__class__.__name__, self.pin, self.is_active)
-
-
-_GPIO_THREADS = set()
-
-
-def _gpio_threads_shutdown():
-    while _GPIO_THREADS:
-        for t in _GPIO_THREADS.copy():
-            t.stop()
+        try:
+            return "<gpiozero.%s object on pin=%d, is_active=%s>" % (
+                self.__class__.__name__, self.pin, self.is_active)
+        except GPIODeviceClosed:
+            return "<gpiozero.%s object closed>" % self.__class__.__name__
 
 
 class GPIOThread(Thread):

gpiozero/input_devices.py

@@ -9,7 +9,7 @@ from threading import Event
 from RPi import GPIO
 from w1thermsensor import W1ThermSensor
 
-from .devices import GPIODeviceError, GPIODevice, GPIOQueue
+from .devices import GPIODeviceError, GPIODeviceClosed, GPIODevice, GPIOQueue
 
 
 def _alias(key):
@@ -33,8 +33,12 @@ class InputDevice(GPIODevice):
                 'GPIO pins 2 and 3 are fitted with physical pull up '
                 'resistors; you cannot initialize them with pull_up=False'
             )
-        super(InputDevice, self).__init__(pin)
+        # _pull_up should be assigned first as __repr__ relies upon it to
+        # support the case where __repr__ is called during debugging of an
+        # instance that has failed to initialize (due to an exception in the
+        # super-class __init__)
         self._pull_up = pull_up
+        super(InputDevice, self).__init__(pin)
         self._active_edge = GPIO.FALLING if pull_up else GPIO.RISING
         self._inactive_edge = GPIO.RISING if pull_up else GPIO.FALLING
         self._active_state = GPIO.LOW if pull_up else GPIO.HIGH
@@ -59,8 +63,11 @@ class InputDevice(GPIODevice):
         return self._pull_up
 
     def __repr__(self):
-        return "<gpiozero.%s object on pin=%d, pull_up=%s, is_active=%s>" % (
-            self.__class__.__name__, self.pin, self.pull_up, self.is_active)
+        try:
+            return "<gpiozero.%s object on pin=%d, pull_up=%s, is_active=%s>" % (
+                self.__class__.__name__, self.pin, self.pull_up, self.is_active)
+        except:
+            return super(InputDevice, self).__repr__()
 
 
 class WaitableInputDevice(InputDevice):
@@ -174,9 +181,6 @@ class DigitalInputDevice(WaitableInputDevice):
         # Call _fire_events once to set initial state of events
         super(DigitalInputDevice, self)._fire_events()
 
-    def __del__(self):
-        GPIO.remove_event_detect(self.pin)
-
     def _fire_events(self, channel):
         super(DigitalInputDevice, self)._fire_events()
 
@@ -188,27 +192,50 @@ class SmoothedInputDevice(WaitableInputDevice):
     def __init__(
             self, pin=None, pull_up=False, threshold=0.5,
             queue_len=5, sample_wait=0.0, partial=False):
+        self._queue = None
         super(SmoothedInputDevice, self).__init__(pin, pull_up)
         self._queue = GPIOQueue(self, queue_len, sample_wait, partial)
         self.threshold = float(threshold)
 
+    def close(self):
+        try:
+            self._queue.stop()
+        except AttributeError:
+            if self._queue is not None:
+                raise
+        except RuntimeError:
+            # Cannot join thread before it starts; we don't care about this
+            # because we're trying to close the thread anyway
+            pass
+        else:
+            self._queue = None
+        super(SmoothedInputDevice, self).close()
+
     def __repr__(self):
-        if self.partial or self._queue.full.wait(0):
+        try:
+            self._check_open()
+        except GPIODeviceClosed:
             return super(SmoothedInputDevice, self).__repr__()
         else:
-            return "<gpiozero.%s object on pin=%d, pull_up=%s>" % (
-                self.__class__.__name__, self.pin, self.pull_up)
+            if self.partial or self._queue.full.wait(0):
+                return super(SmoothedInputDevice, self).__repr__()
+            else:
+                return "<gpiozero.%s object on pin=%d, pull_up=%s>" % (
+                    self.__class__.__name__, self.pin, self.pull_up)
 
     @property
     def queue_len(self):
+        self._check_open()
         return self._queue.queue.maxlen
 
     @property
     def partial(self):
+        self._check_open()
         return self._queue.partial
 
     @property
     def value(self):
+        self._check_open()
         return self._queue.value
 
     def _get_threshold(self):
@@ -284,9 +311,6 @@ class LightSensor(SmoothedInputDevice):
         )
         self._queue.start()
 
-    def __del__(self):
-        GPIO.remove_event_detect(self.pin)
-
     @property
     def charge_time_limit(self):
         return self._charge_time_limit