pimoroni / inky

Combined library for V2/V3 Inky pHAT and Inky wHAT.
https://shop.pimoroni.com/?q=inky
MIT License
594 stars 122 forks source link

Inky Impression set_image breaks subsequent calls to set_pixel #93

Closed ilyearer closed 2 years ago

ilyearer commented 3 years ago

On line 161 of inky_uc8159.py, the image buffer is initialized in init as follows: self.buf = numpy.zeros((self.rows, self.cols), dtype=numpy.uint8)

On line 392 of inky_uc8159.py, set_image reassigns the buffer with the image and reshapes: self.buf = numpy.array(image, dtype=numpy.uint8).reshape((self.cols, self.rows))

The (self.rows, self.cols) vs (self.cols, self.rows) breaks further calls to set pixel as show in the examples (cycle.py and clear.py). I created the following by merging logic in clear.py and image.py:

#!/usr/bin/env python3
import time
import sys

from PIL import Image
from inky.inky_uc8159 import Inky, CLEAN

inky = Inky()

if len(sys.argv) == 1:
    print("""
Usage: {file} image-file
""".format(file=sys.argv[0]))
    sys.exit(1)

print("Clearing display...")
for _ in range(2):
    for y in range(inky.height - 1):  # Should actually just be inky.height
        for x in range(inky.width - 1):  # Should actually just be inky.width
            inky.set_pixel(x, y, CLEAN)
    inky.show()
    time.sleep(1.0)

print("Loading image...")
image = Image.open(sys.argv[1])
inky.set_image(image, saturation=0.5)
inky.show()
time.sleep(5.0)

print("Clearing display...")
for _ in range(2):
    for y in range(inky.height - 1):
        for x in range(inky.width - 1):
            inky.set_pixel(x, y, CLEAN)
    inky.show()
    time.sleep(1.0)

This results in the following output:

Clearing display...
Loading image...
Clearing display...
Traceback (most recent call last):
  File "inky_bug.py", line 39, in <module>
    inky.set_pixel(x, y, CLEAN)
  File "/usr/local/lib/python3.5/dist-packages/inky/inky_uc8159.py", line 342, in set_pixel
    self.buf[y][x] = v & 0x07
IndexError: index 448 is out of bounds for axis 0 with size 448

On the first CLEAN loop, the display buffer shape was (448, 600), but after any image is displayed, it has been reshaped to (600, 448). So after a call to set_image, inky.height/width no longer matches buf.shape.

The easy workaround is to just use the buffer shape instead of the height/width attributes, but it would be nice if the two could remain consistent.

Gadgetoid commented 3 years ago

Managed to replicate this by shimming up RPi.GPIO, for added confusion the Simulator doesn't have this bug.

Does the following work for you? I've not no access to hardware for testing right now, but some basic tests suggest this at least keeps the buffer shape consistent:

diff --git a/library/inky/inky_uc8159.py b/library/inky/inky_uc8159.py
index 0892070..fbed0c7 100644
--- a/library/inky/inky_uc8159.py
+++ b/library/inky/inky_uc8159.py
@@ -390,6 +390,7 @@ class Inky:
             image.load()
             image = image.im.convert("P", True, palette_image.im)
         self.buf = numpy.array(image, dtype=numpy.uint8).reshape((self.cols, self.rows))
+        self.buf = numpy.rot90(self.buf, 1)

     def _spi_write(self, dc, values):
         """Write values over SPI.
ilyearer commented 3 years ago

This does keep the buffer shape consistent where I no longer get an exception on the second clean loop, but now displaying any image with set_image rotates the image such that it is distorted. Attached is picture of the effect on the girldoomscrolling image included in the repo: girldoomscrolling_bad

In the __init__ method on line 161, the buffer is initialized as follows:

self.buf = numpy.zeros((self.rows, self.cols), dtype=numpy.uint8)

The line above your change does a reshape to the buffer using the shape (self.cols, self.rows) instead of matching the shape used in __init__. If instead of adding the call to numpy.rot90, you could swap the order in the reshape call:

self.buf = numpy.array(image, dtype=numpy.uint8).reshape((self.rows, self.cols))

This fixes the issue causing the exception I encountered and still displays images properly.

I'm not sure if there is a particular reason for the shape differences between __init__ and set_image or if it is just a typo, but it would seem to me that such a change would be more consistent. I did try to run the tests locally on my pi zero w, but I seem to get stuck on the 9th or 10th test in test_init.py, but that happens regardless of my code change.

Gadgetoid commented 3 years ago

You're right- it looks like there was a typo :facepalm:

Sorry this issue had slipped through the cracks a little while I moved house and prepped an office!

Have now tweaked, tested on hardware and raised a PR: https://github.com/pimoroni/inky/pull/120

helgibbons commented 2 years ago

Closing as I think this bug should now be sorted.