A work-around for using byte-string as the data

[EAGrahamJr]

Fixes #137 -- a byte-string is not representable as a list for adding, so this just re-wraps the iterable in a "real" list

This is the code used to verify that the outputs are similar for both 4-Wire and I2C (as that was the easiest to compare to).

from circuitpython_typing import ReadableBuffer

def pretty_print(prefix, data):
    hex_code = "".join(["%02X " % x for x in data])
    print(f"{prefix} {hex_code}\n")

def i2c_usage(command: int, data: ReadableBuffer):
    buffer = list(data) if isinstance(data, bytes) else data
    sent_this = bytes([command] + buffer)
    pretty_print("i2c", sent_this)

def four_wire(command: int, data: ReadableBuffer):
    b = bytes([command])
    pretty_print("4-cmd", b)
    pretty_print("4-buffer", data)

# SSD1036 "sleep"
# 2.0.2
print("My change-----------------")
four_wire(0xAE, [])
i2c_usage(0xAE, [])

four_wire(0xAE, [1,2,3])
i2c_usage(0xAE, [1,2,3])


# 2.0.1
print("Old usage-----------------")
four_wire(0xAE,b"")
# i2c_usage(0xAE,b"")

four_wire(0xAE, b"123")
i2c_usage(0xAE, b"123")

[deshipu]

This is going to be super slow. Maybe instead _send could take an optional command argument, and add it to command_bytes before the rest of the data if it's specified?

[EAGrahamJr]

There are very few instances where a binary-character literal is actually used, since most command/buffer combos are described using real numeric arrays, thus only occurring the overhead of the isinstance check.

Perhaps the more interesting question should be is why did my change on the SSD1306 driver break non-Blinka devices? They should already be able to take numeric-arrays since all the other command-buffer combinations work, what's so special about an empty array vs an empty binary-character literal?

[tannewt]

I agree with @deshipu, just change _send() to take command directly and skip the extra conversion.

Note that data should not be a list. It should always be a bytes, bytearray or array.

[EAGrahamJr]

Ok, it took a bit of back-tracking to see what y'all were referring to (the usage in _displaycore vs i2cdisplaybus). Changes forthcoming...

[tannewt]

Instead of creating a bytearray in send, add an argument to _send(). _send() is allocating a bytearray as well that it can put command into. That way you'll only make one new bytearray, not two.

[EAGrahamJr]

I agree with @deshipu, just change _send() to take command directly and skip the extra conversion.

Note that data should not be a list. It should always be a bytes, bytearray or array.

Not to be (too overly) pedantic, but I think that's part of the initial issue -- bytes don't work:

>>> from os import urandom
>>> data = urandom(8)
>>> type(data)
<class 'bytes'>
>>> cmd = 0x80
>>> type(cmd)
<class 'int'>
>>> [cmd] + data
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: can only concatenate list (not "bytes") to list

[makermelissa]

What happens when you send it in 2 calls like in FourWire? This may have been a conversion error when I was updating this class that just happened to test fine.

[EAGrahamJr]

Just walking through this again, the difference, I believe, is that I2C uses buffered writes, while it appears the FourWire is just jamming byte-at-a-time.

If it's changed to two _send commands (within the same transaction):

# create an array to create a `ReadableBuffer`
self._send(DISPLAY_COMMAND, CHIP_SELECT_UNTOUCHED, [command])
# create buffer and write
self._send(DISPLAY_COMMAND, CHIP_SELECT_UNTOUCHED, data)
# create buffer and write

That appears to require a lot more buffer creation, so even more overhead.

My surmise is that this is because I2C is an addressable format, thus easier to lock and send a bunchastuff at once, while FourWire is chip-select and can tolerate interleaving of bytes.

(Sorry if I'm getting windy, but I recently retired from software development and this the best conversation I've had along those lines in months!)

[tannewt]

It also avoids any changes to method signatures and buffer construction elsewhere.

_send() is a private function (designated by the _) used by just this function. It's not a big deal to change its method signature. So, please add command to the signature and then pack it into a buffer inside _send().

[EAGrahamJr]

Ok, I'm not comfortable with that change, thanks.

[FoamyGuy]

I tested the functionality of this change successfully on a Raspberry Pi 3 B+ In these 3 scenarios:

• With a SPI display to ensure this change doesn't cause any trouble with them (it didn't)
• With a SSD1306 without using an sleep() functionality to ensure the default behavior is still working as intended (it is)
• With a SSD1306 and using sleep() and wake() functionality.

All appear to be working as intended.

[tannewt]

@makermelissa Would you mind picking this up? Thanks!