Skip to content

allow missing vectorio #57

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

Merged
merged 3 commits into from
Dec 16, 2021
Merged

Conversation

FoamyGuy
Copy link
Contributor

resolves: #51

These changes include:

  • making vectorio requirement optional
  • fix docstring of older example
  • add new example that wouldn't be possible to run without this change.

@rsbohn
Copy link
Contributor

rsbohn commented Nov 27, 2021

When I tried running with Blinka it complained that I was trying to put an invalid color into a palette.

switch_round.py line 724 self._switch_circle.fill was {'transparent': False, 'rgb888': 16777215, 'rgba': (255, 255, 255, 255)}. If I used ..fill['rgba'] it would run.

@FoamyGuy
Copy link
Contributor Author

FoamyGuy commented Nov 28, 2021

When I tried running with Blinka it complained that I was trying to put an invalid color into a palette.

@rsbohn Strange. I am seeing that error now to. I am fairly positive that I ran this example successfully before making the PR. I must have had a different version of something at that time that allowed it to work.

I'm not sure what you mean when you mentioned "If I used ..fill['rgba'] it would run". Is this a change you made inside of the example? or in the switch_round class or somewhere else? Can you post the code that you changed to when you got it to run?

@FoamyGuy
Copy link
Contributor Author

Interestingly I was able to test both displayio_layout_switch_multiple.py and displayio_layout_switch_simpletest.py on a PyPortal. So whatever the root of this issue is, it's either only an issue in the Blinka_DisplayIO context, or else caused by something in the new example that isn't present in the others.

@FoamyGuy
Copy link
Contributor Author

@rsbohn I figured out what you mean I think. I see that Palettes in Blinka_DisplayIO are containing dictionaries for each item, whereas in the core the palette items are literal ints. So I can see how using fill['rgba'] inside of SwitchRound class would allow the code to run in Blinka_DisplayIO, but I think it would crash if/when it ran on a microcontroller with core displayio.

I think the fix will be to try to change Blinka_DisplayIO to make it return the same value as core displayio when using square brackets to get_item out of a Palette.

I am still a bit perplexed as to when this difference started occurring. I'm pretty sure I've had this code execute successfully before, but I don't think it could with the Palette behavior is like this. I took a peek at recent PRs in Blinka_DisplayIO but don't see anything that seems like it would have been relevant to this difference so I'm still unsure what could have changed in my local environment to cause this to start happening.

@FoamyGuy
Copy link
Contributor Author

FoamyGuy commented Nov 28, 2021

I did find a fix inside of Blinka_DisplayIO for this. I changed __getitem__ inside of Palette to this:

    def __getitem__(self, index):
        if not 0 <= index < len(self._colors):
            raise ValueError("Palette index out of range")
        return self._colors[index]["rgb888"]

and it allows this example to run successfully (and equalizes the behavior between core displayio and Blinka_DisplayIO). I think that is the best solution for this issue, but I would like to wait for input from Melissa to see if there was a specific reason for the difference, or if changing it as above could cause other issues.

#
# SPDX-License-Identifier: MIT
"""
Make a GridLayout with some Labels in it's cells.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the docstring to match the code (SwitchRound demo for PyGameDisplay).

@rsbohn
Copy link
Contributor

rsbohn commented Nov 29, 2021

Sorry I didn't respond sooner. This should be okay to merge, but I won't be able to test it until #Adafruit_Blinka_Displayio #73 is resolved.
It looks like the one function in widgets/init.py is only used in the cartesian.py widget, but there may be other uses outside of this module.

@FoamyGuy FoamyGuy merged commit 5daad48 into adafruit:main Dec 16, 2021
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Dec 16, 2021
Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_Layout to 1.15.1 from 1.15.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_Layout#60 from FoamyGuy/cartesian_vectorio_fix
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_Layout#57 from FoamyGuy/allow_missing_vectorio
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.

vectorio import should be optional inside the widget module
2 participants