search

adafruit / Adafruit_CircuitPython_DisplayIO_SSD1306

DisplayIO driver for SSD1306 monochrome OLED displays
MIT License
57 stars 24 forks source link

Fix sleep/wake, again #44

Open regicidalplutophage opened 2 months ago

regicidalplutophage commented 2 months ago

Not sure about wake(), but sleep() were throwing TypeError: object with buffer protocol required at me.

regicidalplutophage commented 2 months ago

It seems like the actual issue is incompatibility between i2cdisplaybus.I2CDisplayBus and I2CDisplay.I2CDisplayBus and my misguided fix would break it for the folks who needed the recent fix that broke it for me.

Converting it to draft for now.

regicidalplutophage commented 2 months ago

@EAGrahamJr can you test if this works for you? I think your issue was caused by Blinka, but your fix broke it for me on CircuitPython 9.0.5

EAGrahamJr commented 2 months ago

This seems like the wrong place to fix incompatibilities as this code should not be "blinka" aware. If there's an incompatibility, it should be fixed at the bus level.

regicidalplutophage commented 2 months ago

This code is kind of aware though, as typing module would only be available in Python, not CircuitPython. If I understand the issue correctly, what I think should happen is that Blinka implementation of i2cdisplaybus should be brought in line with CircuitPython and sleep/wake fixes for SSD1306 and SH1106 should be reverted. I have zero investment in Blinka so I'm trying to figure out if my theory is even correct before I proceed. Though it's better if someone else fixes Adafruit_Blinka_Displayio's i2cdisplaybus.

EAGrahamJr commented 2 months ago

This code is kind of aware though, as typing module would only be available in Python, not CircuitPython.

That's not quite correct: the imports are "aware" but the rest of the code should not be: whatever bus is being used should have a consistent interface so that the display code doesn't have to make a choice when calling the method.

mboehn commented 2 months ago

Sorry for barging in. I have the same problem of sleep() throwing TypeError: object with buffer protocol required. I'm new to CircuitPython and I'm pretty sure I don't understand the hierarchy with Adafruit_Blinka_Displayio and so yet.

Do I understand correctly if this issue should be handled in Adafruit_Blinka_Displayio? If so, have an issue been raised there?

I tried looking for one but I'm not sure what I would be looking for :-)

regicidalplutophage commented 2 months ago

Basically, the way I understand it, the latest commit should be reverted and the stuff that commit was fixing should be fixed elsewhere. You can use the 2.0.1 release, @mboehn

EAGrahamJr commented 1 month ago

Part of the problem here is that this driver isn't quite the same as the other SSD drivers: they all seem to only take a displayio constructor, which should have a consistent interface at this point and should fix (or break :smiling_imp: ) all the issues here.

I only have I2C, so that's all I can test/verify.

(see https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SH1106/pull/19#issuecomment-2248198557 for similar comments)

EAGrahamJr commented 1 month ago

It seems like the actual issue is incompatibility between i2cdisplaybus.I2CDisplayBus and I2CDisplay.I2CDisplayBus and my misguided fix would break it for the folks who needed the recent fix that broke it for me.

Converting it to draft for now.

Since this was mentioned in the forums recently, I thought I would re-comment on this: as noted this is a conflict between the two I2CDisplayBus interfaces and and it is my opinion that should be resolved first. Since that is more of a core issue and a conflict between the two libraries, I'm not sure who the appropriate person is to get involved and I'm hesitant to blindly "at" someone.

EAGrahamJr commented 1 month ago

Yes, I can confirm that there is an issue between the two implementations. The fix I submitted apparently only works on Blinka.

Library Comparisons

Raspberry Pi (Blinka) QTPY RP2040 (error)
adafruit-circuitpython-displayio-ssd1306 2.0.2 adafruit_displayio_ssd1306 2.0.2
adafruit-blinka-displayio 2.1.0
adafruit-circuitpython-busdevice 5.2.9 adafruit_bus_device 5.2.9
EAGrahamJr commented 1 month ago

@tannewt This was mentioned in forum post https://forums.adafruit.com/viewtopic.php?t=212512 -- since this involves a conflict between two core drivers, I still think that the resolution should be attempted there and not at this driver level.

tannewt commented 1 month ago

I still think that the resolution should be attempted there and not at this driver level.

I agree. This is a Blinka bug. A list isn't a readable buffer because it isn't linear memory. bytes, bytearray and array are. Blinka should be updated to match CP.