Repository: rm-hull/luma.core

Issue:
Deprecate SPI params bcm_DC, bcm_RST in favour of gpio_DC, gpio_RST
See https://github.com/rm-hull/luma.core/blob/master/luma/core/serial.py#L96

*bcm* is shorthand for 'broadcom' pin assignment, and is specific to RPi implementations, whereas *gpio* is a more general term, and should be used if ever a generic GPIO library replaces RPi.GPIO 

Discussion:
thijstriemstra: work in progress..
thijstriemstra: Enabling warnings displays a lot of resource warnings: https://travis-ci.org/rm-hull/luma.core/jobs/210702792#L472. I'll fix those as well.
thijstriemstra: @rm-hull the resource warnings coming from `test_spritesheet` could be considered bugs because the luma code doesn't close the image for you. But I'm not sure where to close the image in the `spritesheet` class..

```
WW0 in tests/test_spritesheet.py:42 the following warning was recorded:
 .tox/py35/lib/python3.5/site-packages/PIL/Image.py:1028: ResourceWarning: unclosed file <_io.BufferedReader name='tests/reference/runner.png'>
  self.load()
```
rm-hull: Think you're right about the spritesheet - maybe adding a `__del__()` destructor method to the spritesheet class that calls `self.image.close()` ?
thijstriemstra: @rm-hull spritesheet probably needs a `close` method. This can be called manually and if spritesheet would support context manager usage, `close` could be called there.
thijstriemstra: Should be ready for review.
rm-hull: Just reading the docs for https://pillow.readthedocs.io/en/4.0.x/reference/Image.html#PIL.Image.Image.load

This implies as soon as an image is accessed, `load` is called and the underlying file pointer is closed. Are we unnecessarily complicating things? Are the warnings actually just false alarms?
thijstriemstra: Pillow seems buggy with closing them. Pillow should also support the `with` keyword but it's not working. Also see http://stackoverflow.com/a/38542329
rm-hull: Can you squash & merge when everything goes green pls ?

Patch:
diff --git a/luma/core/serial.py b/luma/core/serial.py
index f908e53..13d4c6f 100644
--- a/luma/core/serial.py
+++ b/luma/core/serial.py
@@ -12,6 +12,7 @@ import errno
 import luma.core.error
 
 from luma.core import lib
+from luma.core.util import deprecation
 
 
 __all__ = ["i2c", "spi"]
@@ -135,17 +136,30 @@ class spi(object):
         only support maxium of 64 or 128 bytes, whereas RPi/py-spidev supports
         4096 (default).
     :type transfer_size: int
-    :param bcm_DC: The BCM pin to connect data/command select (DC) to (defaults to 24).
+    :param gpio_DC: The GPIO pin to connect data/command select (DC) to (defaults to 24).
+    :type gpio_DC: int
+    :param gpio_RST: The GPIO pin to connect reset (RES / RST) to (defaults to 24).
+    :type gpio_RST: int
+    :param bcm_DC: Deprecated. Use ``gpio_DC`` instead.
     :type bcm_DC: int
-    :param bcm_RST: The BCM pin to connect reset (RES / RST) to (defaults to 24).
+    :param bcm_RST:  Deprecated. Use ``gpio_RST`` instead.
     :type bcm_RST: int
     :raises luma.core.error.DeviceNotFoundError: SPI device could not be found.
     :raises luma.core.error.UnsupportedPlatform: GPIO access not available.
     """
     def __init__(self, spi=None, gpio=None, port=0, device=0,
                  bus_speed_hz=8000000, transfer_size=4096,
-                 bcm_DC=24, bcm_RST=25):
+                 gpio_DC=24, gpio_RST=25, bcm_DC=None, bcm_RST=None):
         assert(bus_speed_hz in [mhz * 1000000 for mhz in [0.5, 1, 2, 4, 8, 16, 32]])
+
+        if bcm_DC is not None:
+            deprecation('bcm_DC argument is deprecated in favor of gpio_DC and will be removed in 1.0.0')
+            gpio_DC = bcm_DC
+
+        if bcm_RST is not None:
+            deprecation('bcm_RST argument is deprecated in favor of gpio_RST and will be removed in 1.0.0')
+            gpio_RST = bcm_RST
+
         self._gpio = gpio or self.__rpi_gpio__()
         self._spi = spi or self.__spidev__()
 
@@ -159,16 +173,16 @@ class spi(object):
 
         self._transfer_size = transfer_size
         self._spi.max_speed_hz = bus_speed_hz
-        self._bcm_DC = bcm_DC
-        self._bcm_RST = bcm_RST
+        self._gpio_DC = gpio_DC
+        self._gpio_RST = gpio_RST
         self._cmd_mode = self._gpio.LOW    # Command mode = Hold low
         self._data_mode = self._gpio.HIGH  # Data mode = Pull high
 
         self._gpio.setmode(self._gpio.BCM)
-        self._gpio.setup(self._bcm_DC, self._gpio.OUT)
-        self._gpio.setup(self._bcm_RST, self._gpio.OUT)
-        self._gpio.output(self._bcm_RST, self._gpio.LOW)   # Reset device
-        self._gpio.output(self._bcm_RST, self._gpio.HIGH)  # Keep RESET pulled high
+        self._gpio.setup(self._gpio_DC, self._gpio.OUT)
+        self._gpio.setup(self._gpio_RST, self._gpio.OUT)
+        self._gpio.output(self._gpio_RST, self._gpio.LOW)   # Reset device
+        self._gpio.output(self._gpio_RST, self._gpio.HIGH)  # Keep RESET pulled high
 
     def command(self, *cmd):
         """
@@ -177,7 +191,7 @@ class spi(object):
         :param cmd: a spread of commands
         :type cmd: int
         """
-        self._gpio.output(self._bcm_DC, self._cmd_mode)
+        self._gpio.output(self._gpio_DC, self._cmd_mode)
         self._spi.writebytes(list(cmd))
 
     def data(self, data):
@@ -188,7 +202,7 @@ class spi(object):
         :param data: a data sequence
         :type data: list, bytearray
         """
-        self._gpio.output(self._bcm_DC, self._data_mode)
+        self._gpio.output(self._gpio_DC, self._data_mode)
         i = 0
         n = len(data)
         tx_sz = self._transfer_size
diff --git a/luma/core/sprite_system.py b/luma/core/sprite_system.py
index 49d1a01..5c68280 100755
--- a/luma/core/sprite_system.py
+++ b/luma/core/sprite_system.py
@@ -10,6 +10,7 @@ Simplified sprite animation framework.
 """
 
 import time
+
 from PIL import Image
 
 
@@ -66,7 +67,9 @@ class spritesheet(object):
     Loosely based on http://www.createjs.com/docs/easeljs/classes/SpriteSheet.html
     """
     def __init__(self, image, frames, animations):
-        self.image = Image.open(image)
+        with open(image, 'rb') as fp:
+            self.image = Image.open(fp)
+            self.image.load()
         self.frames = dict_wrapper(frames)
         self.animations = dict_wrapper(animations)
         # Reframe the sprite map in terms of the registration point (if set)
diff --git a/luma/core/util.py b/luma/core/util.py
new file mode 100644
index 0000000..b570518
--- /dev/null
+++ b/luma/core/util.py
@@ -0,0 +1,12 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2017 Richard Hull and contributors
+# See LICENSE.rst for details.
+
+import warnings
+
+
+__all__ = ["deprecation"]
+
+
+def deprecation(message):
+    warnings.warn(message, DeprecationWarning, stacklevel=2)
diff --git a/setup.py b/setup.py
index 14fb26e..aee7ad3 100644
--- a/setup.py
+++ b/setup.py
@@ -18,7 +18,7 @@ version = read_file("VERSION.txt").strip()
 
 needs_pytest = {'pytest', 'test', 'ptr'}.intersection(sys.argv)
 pytest_runner = ['pytest-runner'] if needs_pytest else []
-test_deps = ["mock", "pytest", "pytest-cov"]
+test_deps = ["mock", "pytest", "pytest-cov", "pytest-warnings"]
 
 setup(
     name="luma.core",
diff --git a/tox.ini b/tox.ini
index 96c74b0..e759594 100644
--- a/tox.ini
+++ b/tox.ini
@@ -8,7 +8,7 @@ skip_missing_interpreters = True
 [testenv]
 commands =
     python setup.py install
-    coverage run -m py.test -v
+    coverage run -m py.test -v -r w
     coverage report
 deps =
     .[test]


Test patch:
diff --git a/tests/helpers.py b/tests/helpers.py
new file mode 100644
index 0000000..cf41d46
--- /dev/null
+++ b/tests/helpers.py
@@ -0,0 +1,17 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2017 Richard Hull and contributors
+# See LICENSE.rst for details.
+
+"""
+Test helpers.
+"""
+
+
+import os.path
+
+
+def get_reference_image(fname):
+    return os.path.abspath(os.path.join(
+        os.path.dirname(__file__),
+        'reference',
+        fname))
diff --git a/tests/test_dummy.py b/tests/test_dummy.py
index 1f90103..00dd2a8 100644
--- a/tests/test_dummy.py
+++ b/tests/test_dummy.py
@@ -7,13 +7,13 @@
 Tests for the :py:class:`luma.core.device.dummy` class.
 """
 
-import os.path
 from PIL import Image, ImageChops
 
 from luma.core.render import canvas
 from luma.core.device import dummy
 
 import baseline_data
+from helpers import get_reference_image
 
 
 def test_capture_noops():
@@ -28,36 +28,33 @@ def test_capture_noops():
 
 
 def test_portrait():
-    reference = Image.open(
-        os.path.abspath(os.path.join(
-            os.path.dirname(__file__),
-            'reference',
-            'portrait.png')))
+    img_path = get_reference_image('portrait.png')
 
-    device = dummy(rotate=1)
+    with open(img_path, 'rb') as p:
+        reference = Image.open(p)
 
-    # Use the same drawing primitives as the demo
-    with canvas(device) as draw:
-        baseline_data.primitives(device, draw)
+        device = dummy(rotate=1)
 
-    bbox = ImageChops.difference(reference, device.image).getbbox()
-    assert bbox is None
+        # Use the same drawing primitives as the demo
+        with canvas(device) as draw:
+            baseline_data.primitives(device, draw)
+
+        bbox = ImageChops.difference(reference, device.image).getbbox()
+        assert bbox is None
 
 
 def test_dither():
-    reference = Image.open(
-        os.path.abspath(os.path.join(
-            os.path.dirname(__file__),
-            'reference',
-            'dither.png')))
-
-    device = dummy(mode="1")
-
-    with canvas(device, dither=True) as draw:
-        draw.rectangle((0, 0, 64, 32), fill="red")
-        draw.rectangle((64, 0, 128, 32), fill="yellow")
-        draw.rectangle((0, 32, 64, 64), fill="blue")
-        draw.rectangle((64, 32, 128, 64), fill="white")
-
-    bbox = ImageChops.difference(reference, device.image).getbbox()
-    assert bbox is None
+    img_path = get_reference_image('dither.png')
+
+    with open(img_path, 'rb') as p:
+        reference = Image.open(p)
+        device = dummy(mode="1")
+
+        with canvas(device, dither=True) as draw:
+            draw.rectangle((0, 0, 64, 32), fill="red")
+            draw.rectangle((64, 0, 128, 32), fill="yellow")
+            draw.rectangle((0, 32, 64, 64), fill="blue")
+            draw.rectangle((64, 32, 128, 64), fill="white")
+
+        bbox = ImageChops.difference(reference, device.image).getbbox()
+        assert bbox is None
diff --git a/tests/test_serial.py b/tests/test_serial.py
index 9a76f6e..0d78472 100644
--- a/tests/test_serial.py
+++ b/tests/test_serial.py
@@ -138,14 +138,42 @@ def verify_spi_init(port, device, bus_speed_hz=8000000, dc=24, rst=25):
 
 
 def test_spi_init():
-    spi(gpio=gpio, spi=spidev, port=5, device=2, bus_speed_hz=16000000, bcm_DC=17, bcm_RST=11)
-    verify_spi_init(5, 2, 16000000, 17, 11)
+    port = 5
+    device = 2
+    bus_speed = 16000000
+    dc = 17
+    rst = 11
+
+    spi(gpio=gpio, spi=spidev, port=port, device=device, bus_speed_hz=16000000,
+        gpio_DC=dc, gpio_RST=rst)
+    verify_spi_init(port, device, bus_speed, dc, rst)
     gpio.output.assert_has_calls([
-        call(11, gpio.LOW),
-        call(11, gpio.HIGH)
+        call(rst, gpio.LOW),
+        call(rst, gpio.HIGH)
     ])
 
 
+def test_spi_init_params_deprecated():
+    port = 5
+    device = 2
+    bus_speed = 16000000
+    dc = 80
+    rst = 90
+    msg1 = 'bcm_DC argument is deprecated in favor of gpio_DC and will be removed in 1.0.0'
+    msg2 = 'bcm_RST argument is deprecated in favor of gpio_RST and will be removed in 1.0.0'
+
+    with pytest.deprecated_call() as c:
+        spi(gpio=gpio, spi=spidev, port=port, device=device,
+            bus_speed_hz=bus_speed, bcm_DC=dc, bcm_RST=rst)
+        verify_spi_init(port, device, bus_speed, dc, rst)
+        gpio.output.assert_has_calls([
+            call(rst, gpio.LOW),
+            call(rst, gpio.HIGH)
+        ])
+        assert str(c.list[0].message) == msg1
+        assert str(c.list[1].message) == msg2
+
+
 def test_spi_init_invalid_bus_speed():
     with pytest.raises(AssertionError):
         spi(gpio=gpio, spi=spidev, port=5, device=2, bus_speed_hz=942312)
diff --git a/tests/test_spritesheet.py b/tests/test_spritesheet.py
index 56a5636..fce267a 100644
--- a/tests/test_spritesheet.py
+++ b/tests/test_spritesheet.py
@@ -12,6 +12,7 @@ import PIL
 
 from luma.core.sprite_system import spritesheet
 
+
 data = {
     'image': "tests/reference/runner.png",
     'frames': {
diff --git a/tests/test_terminal.py b/tests/test_terminal.py
index 6a7f06c..fbfd8cf 100644
--- a/tests/test_terminal.py
+++ b/tests/test_terminal.py
@@ -7,112 +7,101 @@
 Tests for the :py:class:`luma.core.virtual.terminal` class.
 """
 
-import os.path
 from PIL import Image, ImageChops
 
 from luma.core.device import dummy
 from luma.core.virtual import terminal
 
+from helpers import get_reference_image
+
 
 def test_wrapped_text():
-    reference = Image.open(
-        os.path.abspath(os.path.join(
-            os.path.dirname(__file__),
-            'reference',
-            'quick_brown_fox.png')))
+    img_path = get_reference_image('quick_brown_fox.png')
 
-    device = dummy()
-    term = terminal(device)
+    with open(img_path, 'rb') as p:
+        reference = Image.open(p)
+        device = dummy()
+        term = terminal(device)
 
-    term.println("The quick brown fox jumps over the lazy dog")
+        term.println("The quick brown fox jumps over the lazy dog")
 
-    bbox = ImageChops.difference(reference, device.image).getbbox()
-    assert bbox is None
+        bbox = ImageChops.difference(reference, device.image).getbbox()
+        assert bbox is None
 
 
 def test_tab_alignment():
-    reference = Image.open(
-        os.path.abspath(os.path.join(
-            os.path.dirname(__file__),
-            'reference',
-            'tab_align.png')))
+    img_path = get_reference_image('tab_align.png')
 
-    device = dummy()
-    term = terminal(device, animate=False)
+    with open(img_path, 'rb') as p:
+        reference = Image.open(p)
+        device = dummy()
+        term = terminal(device, animate=False)
 
-    term.println("1\t32\t999")
-    term.println("999\t1\t32")
+        term.println("1\t32\t999")
+        term.println("999\t1\t32")
 
-    bbox = ImageChops.difference(reference, device.image).getbbox()
-    assert bbox is None
+        bbox = ImageChops.difference(reference, device.image).getbbox()
+        assert bbox is None
 
 
 def test_control_chars():
-    reference = Image.open(
-        os.path.abspath(os.path.join(
-            os.path.dirname(__file__),
-            'reference',
-            'control_chars.png')))
+    img_path = get_reference_image('control_chars.png')
 
-    device = dummy()
-    term = terminal(device, animate=False)
+    with open(img_path, 'rb') as p:
+        reference = Image.open(p)
+        device = dummy()
+        term = terminal(device, animate=False)
 
-    term.println('foo\rbar\bspam\teggs\n\nham and cheese on rye')
+        term.println('foo\rbar\bspam\teggs\n\nham and cheese on rye')
 
-    bbox = ImageChops.difference(reference, device.image).getbbox()
-    assert bbox is None
+        bbox = ImageChops.difference(reference, device.image).getbbox()
+        assert bbox is None
 
 
 def test_scrolling():
-    reference = Image.open(
-        os.path.abspath(os.path.join(
-            os.path.dirname(__file__),
-            'reference',
-            'scroll_text.png')))
+    img_path = get_reference_image('scroll_text.png')
 
-    device = dummy()
-    term = terminal(device, animate=False)
+    with open(img_path, 'rb') as p:
+        reference = Image.open(p)
+        device = dummy()
+        term = terminal(device, animate=False)
 
-    term.println(
-        "it oozed over the blackness, and heard Harris's sleepy voice asking " +
-        "where we drew near it, so they spread their handkerchiefs on the back " +
-        "of Harris and Harris's friend as to avoid running down which, we managed " +
-        "to get out of here while this billing and cooing is on. We'll go down " +
-        "to eat vegetables. He said they were demons.")
+        term.println(
+            "it oozed over the blackness, and heard Harris's sleepy voice asking " +
+            "where we drew near it, so they spread their handkerchiefs on the back " +
+            "of Harris and Harris's friend as to avoid running down which, we managed " +
+            "to get out of here while this billing and cooing is on. We'll go down " +
+            "to eat vegetables. He said they were demons.")
 
-    bbox = ImageChops.difference(reference, device.image).getbbox()
-    assert bbox is None
+        bbox = ImageChops.difference(reference, device.image).getbbox()
+        assert bbox is None
 
 
 def test_alt_colors():
-    reference = Image.open(
-        os.path.abspath(os.path.join(
-            os.path.dirname(__file__),
-            'reference',
-            'alt_colors.png')))
+    img_path = get_reference_image('alt_colors.png')
 
-    device = dummy()
-    term = terminal(device, color="blue", bgcolor="grey", animate=False)
+    with open(img_path, 'rb') as p:
+        reference = Image.open(p)
+        device = dummy()
+        term = terminal(device, color="blue", bgcolor="grey", animate=False)
 
-    term.println("blue on grey")
+        term.println("blue on grey")
 
-    bbox = ImageChops.difference(reference, device.image).getbbox()
-    assert bbox is None
+        bbox = ImageChops.difference(reference, device.image).getbbox()
+        assert bbox is None
 
 
 def test_ansi_colors():
-    reference = Image.open(
-        os.path.abspath(os.path.join(
-            os.path.dirname(__file__),
-            'reference',
-            'ansi_colors.png')))
+    img_path = get_reference_image('ansi_colors.png')
 
-    device = dummy()
-    term = terminal(device)
+    with open(img_path, 'rb') as p:
+        reference = Image.open(p)
+        device = dummy()
+        term = terminal(device)
 
-    term.println("hello \033[31mworld\33[0m")
-    term.println("this is \033[7mreversed\033[7m!")
-    term.println("\033[45;37mYellow\033[0m \033[43;30mMagenta")
+        term.println("hello \033[31mworld\33[0m")
+        term.println("this is \033[7mreversed\033[7m!")
+        term.println("\033[45;37mYellow\033[0m \033[43;30mMagenta")
 
-    bbox = ImageChops.difference(reference, device.image).getbbox()
-    assert bbox is None
+        bbox = ImageChops.difference(reference, device.image).getbbox()
+        assert bbox is None
diff --git a/tests/test_viewport.py b/tests/test_viewport.py
index 88f6e6d..be7060e 100644
--- a/tests/test_viewport.py
+++ b/tests/test_viewport.py
@@ -9,7 +9,7 @@ helpers.
 """
 
 import time
-import os.path
+
 from PIL import Image, ImageChops
 
 from luma.core.device import dummy
@@ -17,6 +17,7 @@ from luma.core.render import canvas
 from luma.core.virtual import range_overlap, hotspot, snapshot, viewport
 
 import baseline_data
+from helpers import get_reference_image
 
 
 def overlap(box1, box2):
@@ -107,42 +108,38 @@ def test_snapshot_last_updated():
 
 
 def test_viewport_set_position():
-    reference = Image.open(
-        os.path.abspath(os.path.join(
-            os.path.dirname(__file__),
-            'reference',
-            'set_position.png')))
+    img_path = get_reference_image('set_position.png')
 
-    device = dummy()
-    virtual = viewport(device, 200, 200)
+    with open(img_path, 'rb') as p:
+        reference = Image.open(p)
+        device = dummy()
+        virtual = viewport(device, 200, 200)
 
-    # Use the same drawing primitives as the demo
-    with canvas(virtual) as draw:
-        baseline_data.primitives(virtual, draw)
+        # Use the same drawing primitives as the demo
+        with canvas(virtual) as draw:
+            baseline_data.primitives(virtual, draw)
 
-    virtual.set_position((20, 30))
-    bbox = ImageChops.difference(reference, device.image).getbbox()
-    assert bbox is None
+        virtual.set_position((20, 30))
+        bbox = ImageChops.difference(reference, device.image).getbbox()
+        assert bbox is None
 
 
 def test_viewport_hotspot():
-    reference = Image.open(
-        os.path.abspath(os.path.join(
-            os.path.dirname(__file__),
-            'reference',
-            'hotspot.png')))
+    img_path = get_reference_image('hotspot.png')
 
-    device = dummy()
-    virtual = viewport(device, 200, 200)
+    with open(img_path, 'rb') as p:
+        reference = Image.open(p)
+        device = dummy()
+        virtual = viewport(device, 200, 200)
 
-    def draw_fn(draw, width, height):
-        baseline_data.primitives(device, draw)
+        def draw_fn(draw, width, height):
+            baseline_data.primitives(device, draw)
 
-    widget = hotspot(device.width, device.height, draw_fn)
+        widget = hotspot(device.width, device.height, draw_fn)
 
-    virtual.add_hotspot(widget, (19, 56))
-    virtual.set_position((28, 30))
-    virtual.remove_hotspot(widget, (19, 56))
+        virtual.add_hotspot(widget, (19, 56))
+        virtual.set_position((28, 30))
+        virtual.remove_hotspot(widget, (19, 56))
 
-    bbox = ImageChops.difference(reference, device.image).getbbox()
-    assert bbox is None
+        bbox = ImageChops.difference(reference, device.image).getbbox()
+        assert bbox is None