More tidying up

Ensure LEDCollection cleans up upon construction failure, rename some
internals to be a bit more obvious, rename PinGPIOUnsupported to
PinUnsupported, and some other stuff I've forgotten!

docs/api_exc.rst

@@ -105,7 +105,7 @@ Errors
 
 .. autoexception:: PinEdgeDetectUnsupported
 
-.. autoexception:: PinGPIOUnsupported
+.. autoexception:: PinUnsupported
 
 .. autoexception:: PinSPIUnsupported
 

gpiozero/boards.py

@@ -211,20 +211,24 @@ class LEDCollection(CompositeOutputDevice):
         initial_value = kwargs.pop('initial_value', False)
         order = kwargs.pop('_order', None)
         LEDClass = PWMLED if pwm else LED
-        super(LEDCollection, self).__init__(
-            *(
-                pin_or_collection
-                if isinstance(pin_or_collection, LEDCollection) else
-                LEDClass(pin_or_collection, active_high, initial_value)
-                for pin_or_collection in args
-                ),
-            _order=order,
-            **{
-                name: pin_or_collection
-                if isinstance(pin_or_collection, LEDCollection) else
-                LEDClass(pin_or_collection, active_high, initial_value)
-                for name, pin_or_collection in kwargs.items()
-                })
+        try:
+            super(LEDCollection, self).__init__(
+                *(
+                    pin_or_collection
+                    if isinstance(pin_or_collection, LEDCollection) else
+                    LEDClass(pin_or_collection, active_high, initial_value)
+                    for pin_or_collection in args
+                    ),
+                _order=order,
+                **{
+                    name: pin_or_collection
+                    if isinstance(pin_or_collection, LEDCollection) else
+                    LEDClass(pin_or_collection, active_high, initial_value)
+                    for name, pin_or_collection in kwargs.items()
+                    })
+        except:
+            self.close()
+            raise
         leds = []
         for item in self:
             if isinstance(item, LEDCollection):

gpiozero/devices.py

@@ -11,7 +11,7 @@ import os
 import atexit
 import weakref
 import warnings
-from collections import namedtuple
+from collections import namedtuple, defaultdict
 from itertools import chain
 from types import FunctionType
 from threading import Lock
@@ -194,7 +194,7 @@ class Device(ValuesMixin, GPIOBase):
     property, the :attr:`value` property, and the :meth:`close` method).
     """
     _pin_factory = None # instance of a Factory sub-class
-    _reservations = {} # maps pin addresses to lists of devices
+    _reservations = defaultdict(list) # maps pin addresses to lists of devices
     _res_lock = Lock()
 
     def __repr__(self):
@@ -226,19 +226,14 @@ class Device(ValuesMixin, GPIOBase):
             )
         with self._res_lock:
             for address in addresses:
-                try:
-                    conflictors = self._reservations[address]
-                except KeyError:
-                    conflictors = []
-                    self._reservations[address] = conflictors
-                for device_ref in conflictors:
+                for device_ref in Device._reservations[address]:
                     device = device_ref()
                     if device is not None and self._conflicts_with(device):
                         raise GPIOPinInUse(
                             'pin %s is already in use by %r' % (
                                 '/'.join(address), device)
                         )
-                conflictors.append(weakref.ref(self))
+                Device._reservations[address].append(weakref.ref(self))
 
     def _release_pins(self, *pins_or_addresses):
         """
@@ -254,7 +249,7 @@ class Device(ValuesMixin, GPIOBase):
             )
         with self._res_lock:
             for address in addresses:
-                self._reservations[address] = [
+                Device._reservations[address] = [
                     ref for ref in self._reservations[address]
                     if ref() not in (self, None) # may as well clean up dead refs
                     ]
@@ -265,13 +260,13 @@ class Device(ValuesMixin, GPIOBase):
         :meth:`_release_pins` for further information).
         """
         with self._res_lock:
-            Device._reservations = {
+            Device._reservations = defaultdict(list, {
                 address: [
                     ref for ref in conflictors
                     if ref() not in (self, None)
                     ]
                 for address, conflictors in self._reservations.items()
-                }
+                })
 
     def _conflicts_with(self, other):
         """

gpiozero/exc.py

@@ -118,8 +118,8 @@ class PinFixedPull(PinError, AttributeError):
 class PinEdgeDetectUnsupported(PinError, AttributeError):
     "Error raised when attempting to use edge detection on unsupported pins"
 
-class PinGPIOUnsupported(PinError, NotImplementedError):
-    "Error raised when attempting to obtain a GPIO interface on unsupported pins"
+class PinUnsupported(PinError, NotImplementedError):
+    "Error raised when attempting to obtain a pin interface on unsupported pins"
 
 class PinSPIUnsupported(PinError, NotImplementedError):
     "Error raised when attempting to obtain an SPI interface on unsupported pins"

gpiozero/mixins.py

@@ -71,9 +71,9 @@ class SourceMixin(object):
     def close(self):
         try:
             super(SourceMixin, self).close()
+            self.source = None
         except AttributeError:
             pass
-        self.source = None
 
     def _copy_values(self, source):
         for v in source:

gpiozero/pins/__init__.py

@@ -12,6 +12,7 @@ from ..exc import (
     PinInvalidFunction,
     PinSetInput,
     PinFixedPull,
+    PinUnsupported,
     PinSPIUnsupported,
     PinPWMUnsupported,
     PinEdgeDetectUnsupported,
@@ -58,7 +59,7 @@ class Factory(object):
             :meth:`pin` for the same pin specification must return the same
             object.
         """
-        raise PinGPIOUnsupported("GPIO not supported by this pin factory")
+        raise PinUnsupported("Individual pins are not supported by this pin factory")
 
     def pin_address(self, spec):
         """

gpiozero/pins/local.py

@@ -32,10 +32,12 @@ class LocalPiFactory(PiFactory):
 
     def __init__(self):
         super(LocalPiFactory, self).__init__()
-        self.spi_hardware_class = LocalPiHardwareSPI
-        self.spi_software_class = LocalPiSoftwareSPI
-        self.shared_spi_hardware_class = LocalPiHardwareSPIShared
-        self.shared_spi_software_class = LocalPiSoftwareSPIShared
+        self.spi_classes = {
+            ('hardware', 'exclusive'): LocalPiHardwareSPI,
+            ('hardware', 'shared'):    LocalPiHardwareSPIShared,
+            ('software', 'exclusive'): LocalPiSoftwareSPI,
+            ('software', 'shared'):    LocalPiSoftwareSPIShared,
+            }
         # Override the pins dict to be this class' pins dict. This is a bit of
         # a dirty hack, but ensures that anyone evil enough to mix pin
         # implementations doesn't try and control the same pin with different
@@ -74,7 +76,7 @@ class LocalPiHardwareSPI(SPI, Device):
             raise ImportError('failed to import spidev')
         self._port = port
         self._device = device
-        self._intf = None
+        self._interface = None
         self._address = factory.address + ('SPI(port=%d, device=%d)' % (port, device),)
         super(LocalPiHardwareSPI, self).__init__()
         self._reserve_pins(
@@ -83,9 +85,9 @@ class LocalPiHardwareSPI(SPI, Device):
             factory.pin_address(9),
             factory.pin_address((8, 7)[device])
             )
-        self._intf = SpiDev()
-        self._intf.open(port, device)
-        self._intf.max_speed_hz = 500000
+        self._interface = SpiDev()
+        self._interface.open(port, device)
+        self._interface.max_speed_hz = 500000
 
     def _conflicts_with(self, other):
         return not (
@@ -94,17 +96,17 @@ class LocalPiHardwareSPI(SPI, Device):
             )
 
     def close(self):
-        if self._intf:
+        if self._interface:
             try:
-                self._intf.close()
+                self._interface.close()
             finally:
-                self._intf = None
+                self._interface = None
         self._release_all()
         super(LocalPiHardwareSPI, self).close()
 
     @property
     def closed(self):
-        return self._intf is None
+        return self._interface is None
 
     def __repr__(self):
         try:
@@ -119,31 +121,31 @@ class LocalPiHardwareSPI(SPI, Device):
         :attr:`bits_per_word` bits or less) to the SPI interface, and reads an
         equivalent number of words, returning them as a list of integers.
         """
-        return self._intf.xfer2(data)
+        return self._interface.xfer2(data)
 
     def _get_clock_mode(self):
-        return self._intf.mode
+        return self._interface.mode
 
     def _set_clock_mode(self, value):
-        self._intf.mode = value
+        self._interface.mode = value
 
     def _get_lsb_first(self):
-        return self._intf.lsbfirst
+        return self._interface.lsbfirst
 
     def _set_lsb_first(self, value):
-        self._intf.lsbfirst = bool(value)
+        self._interface.lsbfirst = bool(value)
 
     def _get_select_high(self):
-        return self._intf.cshigh
+        return self._interface.cshigh
 
     def _set_select_high(self, value):
-        self._intf.cshigh = bool(value)
+        self._interface.cshigh = bool(value)
 
     def _get_bits_per_word(self):
-        return self._intf.bits_per_word
+        return self._interface.bits_per_word
 
     def _set_bits_per_word(self, value):
-        self._intf.bits_per_word = value
+        self._interface.bits_per_word = value
 
 
 class LocalPiSoftwareSPI(SPI, OutputDevice):

gpiozero/pins/pi.py

@@ -35,10 +35,12 @@ class PiFactory(Factory):
         self._info = None
         self.pins = {}
         self.pin_class = None
-        self.spi_hardware_class = None
-        self.spi_software_class = None
-        self.shared_spi_hardware_class = None
-        self.shared_spi_software_class = None
+        self.spi_classes = {
+            ('hardware', 'exclusive'): None,
+            ('hardware', 'shared'):    None,
+            ('software', 'exclusive'): None,
+            ('software', 'shared'):    None,
+            }
 
     def close(self):
         for pin in self.pins.values():
@@ -93,7 +95,7 @@ class PiFactory(Factory):
         doesn't matter).
         """
         spi_args, kwargs = self._extract_spi_args(**spi_args)
-        shared = kwargs.pop('shared', False)
+        shared = 'shared' if kwargs.pop('shared', False) else 'exclusive'
         if kwargs:
             raise SPIBadArgs(
                 'unrecognized keyword argument %s' % kwargs.popitem()[0])
@@ -108,10 +110,7 @@ class PiFactory(Factory):
                     'port': 0,
                     'device': {8: 0, 7: 1}[spi_args['select_pin']],
                     }
-                if shared:
-                    return self.shared_spi_hardware_class(self, **hardware_spi_args)
-                else:
-                    return self.spi_hardware_class(self, **hardware_spi_args)
+                return self.spi_classes[('hardware', shared)](self, **hardware_spi_args)
             except Exception as e:
                 warnings.warn(
                     SPISoftwareFallback(
@@ -125,10 +124,7 @@ class PiFactory(Factory):
             key: pin.number if isinstance(pin, Pin) else pin
             for key, pin in spi_args.items()
             }
-        if shared:
-            return self.shared_spi_software_class(self, **spi_args)
-        else:
-            return self.spi_software_class(self, **spi_args)
+        return self.spi_classes[('software', shared)](self, **spi_args)
 
     def _extract_spi_args(self, **kwargs):
         """

gpiozero/pins/pigpiod.py

@@ -76,10 +76,12 @@ class PiGPIOFactory(PiFactory):
             port=int(os.getenv('PIGPIO_PORT', 8888))):
         super(PiGPIOFactory, self).__init__()
         self.pin_class = PiGPIOPin
-        self.spi_hardware_class = PiGPIOHardwareSPI
-        self.spi_software_class = PiGPIOSoftwareSPI
-        self.shared_spi_hardware_class = PiGPIOHardwareSPIShared
-        self.shared_spi_software_class = PiGPIOSoftwareSPIShared
+        self.spi_classes = {
+            ('hardware', 'exclusive'): PiGPIOHardwareSPI,
+            ('hardware', 'shared'):    PiGPIOHardwareSPIShared,
+            ('software', 'exclusive'): PiGPIOSoftwareSPI,
+            ('software', 'shared'):    PiGPIOSoftwareSPIShared,
+            }
         self._connection = pigpio.pi(host, port)
         self._host = host
         self._port = port