Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sleep/wake, again #44

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

regicidalplutophage
Copy link

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

@regicidalplutophage
Copy link
Author

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 regicidalplutophage marked this pull request as draft July 1, 2024 10:46
@regicidalplutophage
Copy link
Author

@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
Copy link
Contributor

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
Copy link
Author

regicidalplutophage commented Jul 1, 2024

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
Copy link
Contributor

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
Copy link

mboehn commented Jul 11, 2024

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
Copy link
Author

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
Copy link
Contributor

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 😈 ) all the issues here.

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

(see adafruit/Adafruit_CircuitPython_DisplayIO_SH1106#19 (comment) for similar comments)

@EAGrahamJr
Copy link
Contributor

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
Copy link
Contributor

EAGrahamJr commented Aug 1, 2024

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

  • Adafruit CircuitPython 9.1.1 on 2024-07-22; Adafruit QT Py RP2040 with rp2040
  • Raspberry Pi Zero; Bookworm

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
Copy link
Contributor

@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
Copy link
Member

tannewt commented Aug 5, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants