search

adafruit / Adafruit_Blinka_Displayio

Displayio for Blinka
MIT License
14 stars 20 forks source link

return rgba888 value from palette getitem #73

Closed FoamyGuy closed 2 years ago

FoamyGuy commented 2 years ago

This change makes the behavior of Palette.__getitem__() the same as it is in core displayio returning the literal int rgb888 value instead of a dictionary containing that int and a few other fields.

I tested the change successfully on Raspberry Pi 3 B+ with an ili9341 display using a "hello world" simpletest, the simpletest from Display_Shapes library, and the SwitchRound simpletest from DisplayIO_Layout library. I also verified that make_transparent() on the Palette object works as expected with this changes.

Also tested successfully with SwitchRound simpletest and another self-contained palette item printing example on Linux PC with my PyGameDisplay library.

FoamyGuy commented 2 years ago

I don't really understand the root of the issue causing the CI to fail. I found this post which seems to be discussing the problem: https://linux.debian.bugs.dist.narkive.com/BYQcZVKW/bug-997857-python-debian-0-1-42-broken-with-python-3-5-3-6

And I noticed the version of Python in use by the actions in this repo is 3.6: https://github.com/adafruit/Adafruit_Blinka_Displayio/blob/ac309ee7e350e30cdbab4e1f508a2551423077bb/.github/workflows/build.yml#L25

Whereas the libraries are currently set up to use 3.7: https://github.com/adafruit/Adafruit_CircuitPython_Display_Shapes/blob/bafa35c18a610660bdce68784195c4a7fbdf1189/.github/workflows/build.yml#L28

I haven't tried it yet, but I think that using python version 3.7 in the actions container might resolve this issue.

@makermelissa are you open to using python 3.7 for actions in this repo, or do you think that would potentially cause other problems? If you're alright with that I can make that change, if so let me know if you prefer that I make a separate PR or add it to this one.

makermelissa commented 2 years ago

@makermelissa are you open to using python 3.7 for actions in this repo, or do you think that would potentially cause other problems? If you're alright with that I can make that change, if so let me know if you prefer that I make a separate PR or add it to this one.

Sure, that works for me. We probably should update Blinka to use 3.7 as well.

FoamyGuy commented 2 years ago

Changed to python 3.7 and it is passing actions now. I'll PR the Blinka repo as well.

FoamyGuy commented 2 years ago

I wonder if the version would need to be specified here as well: https://github.com/adafruit/Adafruit_Blinka_Displayio/blob/ac309ee7e350e30cdbab4e1f508a2551423077bb/.github/workflows/release.yml#L25

I'm not sure if it's possible to test that without making a release though.

makermelissa commented 2 years ago

I wonder if the version would need to be specified here as well:

https://github.com/adafruit/Adafruit_Blinka_Displayio/blob/ac309ee7e350e30cdbab4e1f508a2551423077bb/.github/workflows/release.yml#L25

I'm not sure if it's possible to test that without making a release though.

Yeah, I'm not sure. I don't think it's so rigorously tested in the release, so it probably doesn't matter.