From 56f2152dafc89210c688b7aa5f4fad61d2b1ff40 Mon Sep 17 00:00:00 2001 From: Dave Jones Date: Sun, 25 Jun 2017 20:44:13 +0100 Subject: [PATCH] Change CompositeDevice to reject invalid identifiers Also updated StatusBoard and StatusZero to reject duplicate identifiers (namedtuple doesn't pick 'em up because they're passed in a dict and thus the dups are squashed prior to the call). Added tests for all the relevant stuff. --- gpiozero/boards.py | 26 +++++++++++++++++--------- gpiozero/devices.py | 3 +-- tests/test_boards.py | 32 ++++++++++++++++++++++++-------- tests/test_devices.py | 2 +- 4 files changed, 43 insertions(+), 20 deletions(-) diff --git a/gpiozero/boards.py b/gpiozero/boards.py index 4423fdc..1ec8028 100644 --- a/gpiozero/boards.py +++ b/gpiozero/boards.py @@ -12,7 +12,7 @@ except ImportError: from time import sleep from itertools import repeat, cycle, chain from threading import Lock -from collections import OrderedDict +from collections import OrderedDict, Counter from .exc import ( DeviceClosed, @@ -819,6 +819,8 @@ class StatusZero(LEDBoard): .. _STATUS Zero: https://thepihut.com/statuszero """ + default_labels = ('one', 'two', 'three') + def __init__(self, *labels, **kwargs): pins = ( (17, 4), @@ -826,12 +828,14 @@ class StatusZero(LEDBoard): (9, 10), ) if len(labels) == 0: - labels = ['one', 'two', 'three'] + labels = self.default_labels elif len(labels) > len(pins): - raise ValueError + raise ValueError("StatusZero doesn't support more than three labels") + dup, count = Counter(labels).most_common(1)[0] + if count > 1: + raise ValueError("Duplicate label %s" % dup) strips = OrderedDict() - for index, label in enumerate(labels): - green, red = pins[index] + for (green, red), label in zip(pins, labels): strips[label] = LEDBoard(red=red, green=green, _order=('red', 'green'), **kwargs) super(StatusZero, self).__init__(_order=strips.keys(), **strips) @@ -861,6 +865,8 @@ class StatusBoard(CompositeOutputDevice): .. _STATUS: https://thepihut.com/status """ + default_labels = ('one', 'two', 'three', 'four', 'five') + def __init__(self, *labels, **kwargs): pins = ( (17, 4, 14), @@ -870,12 +876,14 @@ class StatusBoard(CompositeOutputDevice): (13, 6, 18), ) if len(labels) == 0: - labels = ['one', 'two', 'three', 'four', 'five'] + labels = self.default_labels elif len(labels) > len(pins): - raise ValueError + raise ValueError("StatusBoard doesn't support more than five labels") + dup, count = Counter(labels).most_common(1)[0] + if count > 1: + raise ValueError("Duplicate label %s" % dup) strips = OrderedDict() - for index, label in enumerate(labels): - green, red, button = pins[index] + for (green, red, button), label in zip(pins, labels): strips[label] = CompositeOutputDevice( button=Button(button), lights=LEDBoard( diff --git a/gpiozero/devices.py b/gpiozero/devices.py index e993b0d..bec651b 100644 --- a/gpiozero/devices.py +++ b/gpiozero/devices.py @@ -330,8 +330,7 @@ class CompositeDevice(Device): raise CompositeDeviceBadDevice("%s doesn't inherit from Device" % dev) self._named = frozendict(kwargs) self._namedtuple = namedtuple('%sValue' % self.__class__.__name__, chain( - (str(i) for i in range(len(args))), self._order), - rename=True) + ('device_%d' % i for i in range(len(args))), self._order)) def __getattr__(self, name): # if _named doesn't exist yet, pretend it's an empty dict diff --git a/tests/test_boards.py b/tests/test_boards.py index e983b13..983bfee 100644 --- a/tests/test_boards.py +++ b/tests/test_boards.py @@ -783,13 +783,21 @@ def test_energenie(): def test_statuszero_init(): with StatusZero() as sz: - assert sz + assert sz.namedtuple._fields == ('one', 'two', 'three') with StatusZero('a') as sz: - assert sz + assert sz.namedtuple._fields == ('a',) with StatusZero('a', 'b') as sz: - assert sz + assert sz.namedtuple._fields == ('a', 'b') with StatusZero('a', 'b', 'c') as sz: - assert sz + assert sz.namedtuple._fields == ('a', 'b', 'c') + with pytest.raises(ValueError): + StatusZero('a', 'b', 'c', 'd') + with pytest.raises(ValueError): + StatusZero('0') + with pytest.raises(ValueError): + StatusZero('foo', 'hello world') + with pytest.raises(ValueError): + StatusZero('foo', 'foo') def test_statuszero(): with StatusZero() as sz: @@ -824,13 +832,21 @@ def test_statuszero_named(): def test_statusboard_init(): with StatusBoard() as sb: - assert sb + assert sb.namedtuple._fields == ('one', 'two', 'three', 'four', 'five') with StatusBoard('a') as sb: - assert sb + assert sb.namedtuple._fields == ('a',) with StatusBoard('a', 'b') as sb: - assert sb + assert sb.namedtuple._fields == ('a', 'b',) with StatusBoard('a', 'b', 'c', 'd', 'e') as sb: - assert sb + assert sb.namedtuple._fields == ('a', 'b', 'c', 'd', 'e') + with pytest.raises(ValueError): + StatusBoard('a', 'b', 'c', 'd', 'e', 'f') + with pytest.raises(ValueError): + StatusBoard('0') + with pytest.raises(ValueError): + StatusBoard('foo', 'hello world') + with pytest.raises(ValueError): + StatusBoard('foo', 'foo') def test_statusboard(): with StatusBoard() as sb: diff --git a/tests/test_devices.py b/tests/test_devices.py index de5fcd0..7293741 100644 --- a/tests/test_devices.py +++ b/tests/test_devices.py @@ -91,7 +91,7 @@ def test_composite_device_sequence(): assert len(device) == 2 assert device[0].pin.number == 4 assert device[1].pin.number == 5 - assert device.namedtuple._fields == ('_0', '_1') + assert device.namedtuple._fields == ('device_0', 'device_1') def test_composite_device_values(): with CompositeDevice(