adafruit / Adafruit_Blinka_Displayio

Displayio for Blinka
MIT License
14 stars 20 forks source link

continue if i2cdisplay failed to import #63

Closed FoamyGuy closed 3 years ago

FoamyGuy commented 3 years ago

this resolves #62

By catching this exception we can allow much of the rest of the functionality of this library to work in environments that cannot successfully import microcontroller

makermelissa commented 3 years ago

We may want to fix the problem inside i2cdisplay for the import microcontroller instead of a try/except block for importing the entire i2cdisplay. It seems like it's kind of a band-aid fix. We could also just remove the Optional[microcontroller.Pin] part for now as it should work just fine without that or even better fix the microcontroller module in Blinka so it doesn't bomb.

FoamyGuy commented 3 years ago

I will work on the changes inside i2cdisplay tomorrow to allow it from there instead of high level like this PR is currently.

I think it makes sense to catch it inside this library instead of Blinka since there are far fewer if any things that are likely to be working with Blinka in the type of environment that this use case is for (CPython on standard PC).

The classes from Blinka Displayio are decoupled nicely in such a way that allows much of the displayio stuff to function even in the absence of a traditional hardware display, but I do understand the more traditional hardware display (and other general physical gadgets) are really what Blinka and Blinka Displayio are more aimed at so I can see how it can help avoid confusion for Blinka to raise the exception once it knows that it's not really running in the primarily intended type of environment.

lesamouraipourpre commented 3 years ago

I've been testing the following changes today and they seem to work fine. We do lose type information with this change but it's probably the easiest thing to do.

diff --git a/displayio/__init__.py b/displayio/__init__.py
index 7489966..343ffa1 100644
--- a/displayio/__init__.py
+++ b/displayio/__init__.py
@@ -23,11 +23,7 @@ from displayio.display import Display
 from displayio.epaperdisplay import EPaperDisplay
 from displayio.fourwire import FourWire
 from displayio.group import Group
-
-try:
-    from displayio.i2cdisplay import I2CDisplay
-except NotImplementedError:
-    print("WARNING: I2CDisplay is not supported on this device.")
+from displayio.i2cdisplay import I2CDisplay
 from displayio.ondiskbitmap import OnDiskBitmap
 from displayio.palette import Palette
 from displayio.parallelbus import ParallelBus
diff --git a/displayio/i2cdisplay.py b/displayio/i2cdisplay.py
index 409e0a5..d7d6f90 100644
--- a/displayio/i2cdisplay.py
+++ b/displayio/i2cdisplay.py
@@ -27,12 +27,6 @@ __repo__ = "https://github.com/adafruit/Adafruit_Blinka_displayio.git"
 import time
 import busio
 import digitalio
-import microcontroller
-
-try:
-    from typing import Optional
-except ImportError:
-    pass

 class I2CDisplay:
@@ -40,13 +34,7 @@ class I2CDisplay:
     It doesn’t handle display initialization.
     """

-    def __init__(
-        self,
-        i2c_bus: busio.I2C,
-        *,
-        device_address: int,
-        reset: Optional[microcontroller.Pin] = None
-    ):
+    def __init__(self, i2c_bus: busio.I2C, *, device_address: int, reset=None):
         """Create a I2CDisplay object associated with the given I2C bus and reset pin.

         The I2C bus and pins are then in use by the display until displayio.release_displays() is
FoamyGuy commented 3 years ago

Thanks @lesamouraipourpre. The newest commit contains those changes. I tested it successfully as well with a PyGameDisplay script.

shulltronics commented 2 years ago

Hi All,

Sorry to revive an old thread, but was hoping to get some help on this: I'd like to use Blinka and displayio to create a UI framework that could work across devices (normal pc, Raspberry Pi with PiTFT, or Feather RP2040). I'm running into the issue that this thread is about, that when I try to import displayio I get a "NotImplementedError". It seems like this merge was supposed to fix that, but perhaps I am not understanding correctly. Any help would be appreciated. Thank you.

FoamyGuy commented 2 years ago

@shulltronics no need for apologies to post on an older issue if you think it's relevant to something again.

I created this library: https://github.com/FoamyGuy/Blinka_Displayio_PyGameDisplay that allows displayio code to run in the CPython context (PC primarily) using PyGame for the rendering. Perhaps there is some inspiration to be drawn, or possible re-use from here that could help you with your project.

It's not necessarily an officially supported usage of Blinka and Blinka_DisplayIO so there are times that updates can cause it to stop working. I noticed recently that it is not working atm with latest versions of Blinka_DisplayIO, but I have not dug far enough to find and fix all of the places needed to get it back to running in the GENERIC_LINUX environment which is where this usage falls under.

If you're interested in helping out and potentially submitting a PR I could try to help get you pointed in the right direction.

shulltronics commented 2 years ago

Hi @FoamyGuy,

Thanks again for your willingness to help. I was able to use your code to get a pygame window to work as a displayio.Display! It's awesome, and already accomplishes my vision of having a displayio based UI framework that can work on Windows, Linux desktop, Raspberry Pi (desktop with pygame, and with PiTFT), and also on a Feather RP2040 (or any other CircuitPython board).

The one thing that doesn't work out of the box -- and that I would be interested in working on a solution for -- is that Blinka throws a NotImplementedError if it can't detect an appropriate chip_id and board_id. My work-around involved modifying pin.py and __init__.py in the microcontroller directory of Adafruit_Blinka. You can see the diff here: https://github.com/shulltronics/Adafruit_Blinka/commit/59d29634acb5c2489f9c8fd38770d67f302d9c78. I realize that this isn't the most elegant solution because I'm importing the Pin definitions for the RP2040 even though I'm not using that chip. The __init__.py part might be ok, I'm just printing a warning instead of throwing the exception.

If you have any specific ideas about how I could do this in a way that you would accept a PR for, please give me some guidance! I would love to contribute to this awesome project. Cheers!

FoamyGuy commented 2 years ago

@shulltronics Thanks for looking into that and sharing the working solution!

Catching those exceptions and printing a warning that not all features are supported in the GENERIC_LINUX environment is how I have resolved it in the past, so I think that is an okay solution.

I would say go ahead and PR what you have and we can take a look and get feedback from the other project maintainers to see if anyone has a different idea of how to handle it.

shulltronics commented 2 years ago

@FoamyGuy just pinging you to let you know the PR is now open! Sorry it took so long.