Check "pin" parameter type in GPIODevice class

[bennuttall]

If you try to create a GPIO device object with a string as the pin number, you get a strange error:

>>> led = LED("2")
[abridged traceback...]
--> 135             self = super(GPIOMeta, cls).__call__(*args, **kwargs)
--> 131         super(DigitalOutputDevice, self).__init__(pin, active_high, initial_value)
---> 47             self.pin.output_with_state(self._value_to_state(initial_value))
AttributeError: 'str' object has no attribute 'output_with_state'

First of all, it would be better if the error was caught straight away, not coincidentally that something later tried to operate on a string and failed.

Secondly, this causes an issue when closing the device (e.g. when closing the shell):

_PINS.pop().close()
AttributeError: 'str' object has no attribute 'close'
[snip]
AttributeError: 'LED' object has no attribute '_controller'

This is because GPIODevice only checks if pin is an int, and converts it to a Pin object. Either way, it continues from this point assuming pin is a Pin object, and adds it to the _PINS set. Then when it comes to close on exit, it fails to close this pin, because it's a string not a pin.

I will PR a suggested fix but there's every chance I'll trivialise it and it'll need to be done differently.

[lurch]

Just out of curiosity, what caused you to try feeding strings to GpioZero?
(I guess this has slight overlaps with #467 )

[bennuttall]

It came up while implementing #548. Reading args in from a file meant I ended up with pin as a string.

[waveform80]

As @lurch notes, this dovetails quite neatly with #467. After the fix for #279 we now have a very neat system for devices obtaining pins:

1. Decide what pin factory we're using; if pin_factory is specified in the constructor use that, otherwise use the current value of Device.pin_factory
2. Call the pin_factory.pin method with whatever we've been given for our pin argument.

Note that GPIODevice doesn't care one jot what "pin" is, what types are valid, etc. That's entirely up to the pin-factory.

Currently all implemented factories only accept integer numbers which we treat as GPIO numbers. If the PR for #467 is merged (it needs a rebase after the #279 changes), that changes to allow a variety of things in all extant factories (integers for GPIO numbers, and a variety of string formats for different numbering systems, up to and including the ability to treat, e.g. "2" as GPIO2).

As it stands (since the merge of the #279 changes), you actually get a more sensible (although still not entirely obvious) error:

>>> led = LED("2")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/dave/gpio-zero/gpiozero/devices.py", line 95, in __call__
    self = super(GPIOMeta, cls).__call__(*args, **kwargs)
  File "/home/dave/gpio-zero/gpiozero/output_devices.py", line 141, in __init__
    pin, active_high, initial_value, pin_factory=pin_factory
  File "/home/dave/gpio-zero/gpiozero/output_devices.py", line 47, in __init__
    super(OutputDevice, self).__init__(pin, pin_factory=pin_factory)
  File "/home/dave/gpio-zero/gpiozero/mixins.py", line 69, in __init__
    super(SourceMixin, self).__init__(*args, **kwargs)
  File "/home/dave/gpio-zero/gpiozero/devices.py", line 374, in __init__
    pin = self.pin_factory.pin(pin)
  File "/home/dave/gpio-zero/gpiozero/pins/pi.py", line 69, in pin
    n = self._to_gpio(spec)
  File "/home/dave/gpio-zero/gpiozero/pins/pi.py", line 81, in _to_gpio
    if not 0 <= spec < 54:
TypeError: unorderable types: int() <= str()

This is because it's trying to decide if "2" is between 0 and 53, the valid GPIO numbers and ints can't be ordered with respect to strings. As stated, not entirely obvious but at least int and str are mentioned. Also, the issue with closing the pins afterward is gone (because the pin never gets constructed).

Ultimately, I'd call this a duplicate of #467 as (absent the closing issue), it's about expanding the definition of a valid pin-spec (albeit minimally).

[bennuttall]

This is fixed now!