Skip to content

Instantation error if display is disabled via displayio.release_displays() #35

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.

Sign up for GitHub

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

Jump to bottom
Closed
fivesixzero opened this issue Jan 22, 2022 · 6 comments
Closed

Instantation error if display is disabled via displayio.release_displays() #35

fivesixzero opened this issue Jan 22, 2022 · 6 comments

Comments

@fivesixzero
Copy link
Contributor

In the early stages of a keeb type project I try to start simple by disabling displays/NeoPixels for the first prototype builds.

This causes an error when instantiating a MacroPad.

code.py output:
Traceback (most recent call last):
  File "code.py", line 6, in <module>
  File "adafruit_macropad.py", line 155, in __init__
AttributeError: 'NoneType' object has no attribute 'rotation'

Here's where its coming from in the current lib version:

Adafruit_CircuitPython_MacroPad/adafruit_macropad.py

Lines 153 to 155 in 89b5cc7

# Define display:
self.display = board.DISPLAY
self.display.rotation = rotation

Off the top of my head I can think of a few possible ways to address this, if addressing it with a code change is actually warranted.

  1. Offer an optional instantiation kwarg allowing the user to disable display init.
  2. Do a null check on board.DISPLAY before doing a display init

This could be considered "not a bug, but a feature" if my more adavanced use case for the library doesn't line up with the lib's intent. It does make sense to keep these 'all in one' libs as simple as possible. So if that's the case, closing this issue with a "won't fix" is totally fine. 😄

For now I've been working around it easily by just making my own slimmed-down MacroPad class using this lib as an example. But its just so convenient to use the 'all-in-one' lib for quick-start prototyping that I find myself wishing for a bit more granular control (or null safety) during instantiation to allow for more slimmed-down use cases.
@fivesixzero
Copy link
Contributor Author

fivesixzero commented Jan 22, 2022 •
edited
Loading

Looks like this may be similar to #33 and this lib has an outstanding type annotations issue (#30) so I'll cook up a quick PR incorporating fixes for those as well as this one.

In the interest of keeping things as simple as possible I'll implement option "2" - just using null checks to allow MacroPad instantiation without adding any additional complexity in the form of kwargs.

edit: Type annotations took a bit more time than I thought they would and were complicated enough that I don't think I'll be tackling additonal fixes until that PR is done

@ladyada
Copy link
Member

ladyada commented Jan 26, 2022

just to check, are you using Adafruit MacroPad hardware or is this a DIY thing?

@fivesixzero
Copy link
Contributor Author

@ladyada Using a standard issue Adafruit MacroPad RP2040. The only difference is that I had done a displayio.release_dispays() prior to instantiating the MacroPad() object.

@ladyada
Copy link
Member

ladyada commented Jan 26, 2022

oh i see you want to practice not having the display exist...i guess for each time we access the display you can verify its not None - it isnt a standard behavior (most folks use the hardware as-is!) so probably the best way to get this implemented is if ya can submit a PR :)

@fivesixzero fivesixzero mentioned this issue Jan 26, 2022
Merged
@fivesixzero
Copy link
Contributor Author

fivesixzero commented Jan 26, 2022 •
edited
Loading

I was going to do a quick if board.DISPLAY is not None check for a PR but in testing with the REPL on a MacroPad RP2040 with 7.1.1 that always returned True, even if board.DISPLAY was None. That had me scratching my head a bit, probably since I'm not well versed (yet) on how board.DISPLAY gets assigned/instantiated.

Here are some permutations I tried:

  • if board.DISPLAY
    • Always True, even after displayio.release_displays()
  • if board.DISPLAY is not None
    • Always True, even after displayio.release_displays()
  • if type(board.DISPLAY) is displayio.Display
    • Works as expected but may be sub-optimal since it relies on displayio internal structure remaining consistent. Which might be a safe bet but creates a structural dependency that just doesn't feel right.
  • if type(board.DISPLAY) is not type(None)
    • Works as expected but looks a bit weird, although nothing too crazy.
  • if not isinstance(board.DISPLAY, type(None))
    • Works as expected, makes pylint/pre-commit a lot happier.

I also considered using a try block with a pass but that nuclear option seems like a bit much.

Here's a quick example of in-situ REPL testing results.

Adafruit CircuitPython 7.1.1 on 2022-01-14; Adafruit Macropad RP2040 with rp2040
>>> import board
>>> if not isinstance(board.DISPLAY, type(None)):
...     print("board.DISPLAY is ready to go")
...     
...     
... 
board.DISPLAY is ready to go
>>> import displayio
>>> displayio.release_displays()
>>> if not isinstance(board.DISPLAY, type(None)):
...     print("board.DISPLAY is ready to go")
...     
...     
... 
>>>

Since its a tiny change (famous last words, I know) I just stacked it on to my type annotation changes in #36 (in a76f87e) since that PR is still open. If having two unrelated fixes in a single PR is an issue I can pull the commit back out and submit a separate PR though.

@fivesixzero
Copy link
Contributor Author

PR #36 is merged so this is all wrapped up.

With this fix applied, instantiating a MacroPad() with the display previously disabled (via boot.py or a prior line in code.py) no longer fails.

@fivesixzero fivesixzero mentioned this issue Jan 27, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ladyada @fivesixzero