Regression from PR #4 (changes semantics of displayio)

[bablokb]

The (merged) pull-request #4 changes the semantics on how displayio works (and breaks many of my programs): If auto_refresh is true, changes to the displayio.Group are normally reflected at once. Since the pr you explicitly have to call display.running (i.e. refresh()).

Now I wonder why this was changed. Removing threading is not a value for itself. The version prior to pr #4 worked fine with one exception: toggling auto_refresh causes an exception, but this is an error in Blinka thread-handling and I will create a pull request for that to fix it.

So my suggestion is to remove the last commit. Unless there were additional problems of course.

[FoamyGuy]

@bablokb It's true this change does change how displayio works and I am definitely hopeful that a better solution to be possible.

What I found was that without the changes from this PR the library would no longer work for me. I don't recall the exact behavior, I'll try again tonight and post more details about how it behaves. I believe I was streaming during some of the work on that PR as well so there is likely a VOD showing what was going on, I'll try to dig that up to and link it if there is a clear section showing the issue I was having.

The crux of it for me though was that in my now current development environment the library wouldn't draw anything for me. Digging into it a bit I found that Blinka displayio uses a seperate thread to call refresh and that if I moved that call back to the main thread it allowed it to work for me again. The mechanism I landed on for moving it to the main thread is definitely less than ideal though. Perhaps there is some room for Blinka_DisplayIO to have a configurable input to enable or disable the multi-threading behavior.

Which version of Python are you running under? I am using 3.10 in the environment where I started seeing that problem occur. It was still working properly on my old computer, but it was a lower version of python on that one.

[bablokb]

I'm on 3.9. I will see if I can somehow manage to install 3.10.

[bablokb]

I checked my old notes from the days when I created a GUI-framework based on pygame (https://github.com/bablokb/pygame-fbgui). I remember having a number of problems.

Some insights:

• pygame event-polling has to be on the main thread
• update of the pygame-display has to be on the same thread where the display was created

Before #4 the code violated both rules. I will try an alternative solution, which decouples Blinka display-refresh from pygame display-refresh. And which pulls out event-processing to the main loop (i.e. user-code).

The goal should be to preserve as much CircuitPython-semantics as possible. But I think the following typical MCU-code will never work:

while True:
  read_sensor
  update display
  sleep five minutes

[bablokb]

If you find time, please check: https://github.com/bablokb/blinka_displayio_pygamedisplay/tree/new-arch

Instead of a while True: loop at the end of the main application program, you now write:

def on_time():
    do_something

display.event_loop(interval=1,on_time=on_time)

[FoamyGuy]

@bablokb thank you for working on this! The way you've got the threading set up now does allow this to work in my environment with auto_refresh more like how it did before the previous PR. I'm really happy to see that. When I made #4 I wasn't too thrilled with the loss of auto_refresh functionality and subsequent "faking it" by making running also do a refresh. However I was eager to get something that allowed me to run successfully at least.

I'd love it if there were some way to keep the while True: loop for the main program structure. Even if it ends up being optional and user code could opt instead for the event loop with callback like you've proposed there.

I cloned your branch and modified by adding this:

    def check_quit(self):
        try:
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    # stop and leave method
                    self._pygame_display_tevent.set()
                    self._pygame_display_thread.join()
                    pygame.quit()
                    return True
        except pygame.error:
            return True
        return False

which allows me to run something like this:

# ...
# setup and show a label or whatever you want
# ...

while True:
    text_area.color = rainbowio.colorwheel(color_num)
    color_num += 1
    if color_num > 255:
        color_num = 0
    print(time.monotonic())
    time.sleep(0.05)

    if display.check_quit():
        break

I do kind of prefer this structure over the display.event_loop(interval=1,on_time=on_time) and then putting all application logic inside of the on_time callback.

I haven't done thorough testing, but the simpletest does seem to run and I tweaked it to change the text colors over time to verify that the display is updating.

That being said I can't really say that I have a super strong grasp on the python threading or even pygame concepts involved, this did run for me and allows closing with the X button, but there may be some reason that I'm unaware of why it could be problematic.

Do you see any issues with having the quit event be "one shot" poll-able function that user code calls once per iteration like this? It makes it very similar to the old behavior with display.running, but does now work with the separate thread for auto_refresh thanks to your changes.

I'll look into this more over the upcoming weekend.

[bablokb]

Having the option to keep while True: is fine. But it should be clear from the documentation that display.check_quit() must be called within short intervals. Something like

while True:
  do_something_every_five_seconds
  time.sleep(5)
  if display.check_quit():
    break

will make your program/window non-reactive and will give you nasty questions from the OS about your window not responding.

I think we should keep both options. Your use-case is more efficiently implemented with the while True-loop since it prevents these 20 callbacks per second. The event-loop method works fine with larger intervals and has the added benefit that you can also have a callback for click/touch-events, allowing for interactive applications.

[bablokb]

Another idea: instead of check_quit() we could have a method sleep_and_check(duration=0,exit=True). Instead of sleeping the method would loop over the pygame-event-queue for the specified amount of time. If the exit-parameter is set, the code would simply raise a SystemExit exception.

With this implementation, the migration of code would be really simple: replace time.sleep with display.sleep_and_check and that should take care of the pygame-specifics.

[FoamyGuy]

I made and merged #7 that is mostly from your branch with a few tweaks for pylint and docs building and the check_quit() function.

I didn't see your last message until I came back to this page afterward to comment so for now it does currently have the check_quit() function. I did think briefly about raising an exception when the X button is clicked rather than returning True/False. That does seem like a good way to do it, it's pretty analogous to the KeyboardInterrupt raised by ctrl-c in python.

I'll think about it a bit more in the coming days and may go ahead and make that change as well.

Closing this issue for now as currently released version should be back to pre #4 functionality.