adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
MIT License
3.96k stars 1.16k forks source link

CircuitPython refreshes screen on first REPL or code completion, ignorring `CIRCUITPY_TERMINALIO = 0` #8721

Open bradanlane opened 6 months ago

bradanlane commented 6 months ago

hardware: raspberrypi RP2040 board with integrated ePaper display situation: unable to prevent low level refresh of display whenever REPL is reached (error, CTRL-C, or end of code.py) version: building with 8.2.x branch

I am using the build flag CIRCUITPY_TERMINALIO = 0 and the compiler definition CIRCUITPY_REPL_LOGO 0 to prevent REPL content from appearing on an ePaper display. Howerver, the display still refreshes (to a blank screen with a border) by something in the circuitpython code

What method will prevent the board ePaper display from refreshing outside of the user explicit code?

Note: This is separate from issue #8675

tannewt commented 6 months ago

I don't think you should prevent errors from showing on the display. It's helpful.

tannewt commented 6 months ago

Is the real issue that you are getting refreshes more often than you should? It shouldn't refresh if code is reloading.

bradanlane commented 6 months ago

I keep reading "you shouldn't prevent errors from showing on the display". It's on several similar issues.

There are several cases where the display is of little use for error messages, including very low resolution "displays" like pixel matrices, tiny OLED displays, and small very slow ePaper displays.

In my use case, the board has a small very slow ePaper display which is designed for infrequent use to display a static image.

Using the two build-time settings - the build flag CIRCUITPY_TERMINALIO = 0 and the compiler definition CIRCUITPY_REPL_LOGO 0 - does prevent errors from rendering on the ePaper. However, "something" is still causing the ePaper display refresh with solid field and a boarder.

bradanlane commented 6 months ago

Here is a simple test:

import time

print("delay 10 seconds ...")
time.sleep(10.0) # allow some time for user to start serial terminal
print("start test")
time.sleep(1.0) # just a simple delay to separate the print statements
print("end test")
# end of program

The first time this code runs (after the board is powered up), the display refreshes at the end of the code execution. If the code has a while True: loop at the end, then the board will refresh when the code is edited and saved.

Here is an annotated video of the issue ...

https://github.com/adafruit/circuitpython/assets/14001599/3f267ca2-4960-4d1c-a93e-68a7b898fe30

bradanlane commented 6 months ago

Do you agree that, given the test example and video results, this as a "bug" and not as an "enhancement" request ?

dhalbert commented 6 months ago

My understanding is that you've added the display as board.DISPLAY on this board. Our assumption was that board.DISPLAY was usable for status information. It is made available continuously, across Python VM instantiations. If you removed board.DISPLAY, and treated this like any other external display, initializing it with Python code, would you see the refreshes you are reporting?

We can track down the extra refreshes by examining the code, and maybe you can too.

Maybe this should be called something other than board.DISPLAY (e.g., board.NON_CONSOLE_DISPLAY, though that's not a very good name), to avoid people thinking that they will see console messages.

bradanlane commented 6 months ago

Yes, I have an integrated ePaper display on the board. I have explicitly compiled CircuitPython to not use it for terminal output.

That is what CIRCUITPY_TERMINALIO = 0 is meant to do - not use the display for console output.

While it is no longer getting console output, it is still being refreshed on the first REPL (or code completion).

Addendum: I attempted to track down the extra refresh. I think it has to do with epaperdisplay_epaperdisplay_reset() which changes the root_group to circuitpython_splash. (but I could be totally off the mark)

dhalbert commented 6 months ago

I attempted to track down the extra refresh. I think it has to do with epaperdisplay_epaperdisplay_reset() which changes teh root_group to circuitpython_splash.

So if you just guard that with #if TERMINAL_IO, does that solve the problem? We would take a PR.

dhalbert commented 6 months ago

It sounds like you want board.DISPLAY to be available, but you want no writing to be done to it ever, except by the user's CircuitPython code. Is that correct?

bradanlane commented 6 months ago

I will add the guard preprocessing and give that a try. I am already working in a fork and branch and I'm not the most talented GIT user so please understand if I fail miserably on the PR.

It sounds like you want board.DISPLAY to be available, but you want no writing to be done to it ever, except by the user's CircuitPython code. Is that correct?

yes.

dhalbert commented 6 months ago

If you have not already seen it, this may be helpful: https://learn.adafruit.com/contribute-to-circuitpython-with-git-and-github

tannewt commented 6 months ago

Do you agree that, given the test example and video results, this as a "bug" and not as an "enhancement" request ?

I don't think you should diverge from standard CircuitPython behavior of showing status on epaper after code finishes. Even if it finishes cleanly, it is important to know that user code is no longer running. It will also include the status bar. We've done this since the MagTag release in November 2020 and no one has complained about it.

The refresh is coming from here: https://github.com/adafruit/circuitpython/blob/main/main.c#L702

bradanlane commented 6 months ago

When I add the traditional while True: loop to the end and the user performs CTRL-C to get to REPL, there is no reason for the display to refresh but it does.

ePaper "displays" are very different from other displays. They are used for persistent information. Having they cleared outside the users code is the issue I am attempting to address.

I feel, if a board developer wants the ePaper display to be treated as a "persistent information device" and not as a console device, then there should be a way for their board version of CircuitPython to achieve this.

The existence of CIRCUITPY_TERMINALIO = 0 suggests there is the intent to allow the board developer to not have their display used outside of the users code.

dglaude commented 6 months ago

Usually you want code for the MagTag to go to sleep once the display content has been rendered. You then wake up on a time alarm, a button press (or even orientation change). So the code never reach at the end the REPL) because at wake-up it restart (with the option to save state information between runs). The only time it reach the REPL would be if you don't catch exception and there is a problem (mostly network issue like WIFI being out of reach or the internet API not responding). This is why there never was a lot of complain about displaying the REPL, when it occurs it can be ugly but it is usefull.

tannewt commented 6 months ago

When I add the traditional while True: loop to the end and the user performs CTRL-C to get to REPL, there is no reason for the display to refresh but it does.

It is helpful to know user code is finished. Generally it should keep running. ctrl-c into the repl is a corner case. Autoreload will already skip the refresh to run a new version of code.py. We could skip the refresh if code ended from a keyboard interrupt too. That's a much smaller change than what you are attempting.

ePaper "displays" are very different from other displays. They are used for persistent information. Having they cleared outside the users code is the issue I am attempting to address.

Our epaper support is not new in circuitpython. We've spent a lot of time thinking about how it should work and many folks have already used it that way.

I feel, if a board developer wants the ePaper display to be treated as a "persistent information device" and not as a console device, then there should be a way for their board version of CircuitPython to achieve this.

I'm pushing back on this change because it makes the workflow of CircuitPython inconsistent on your board. Consistency is incredibly valuable.

I disagree with you that it prevents the "persistent information device" scenario. It should be done by leaving user code running or using deep sleep. Only on error should it exit user code and then it is really helpful to know why.

The existence of CIRCUITPY_TERMINALIO = 0 suggests there is the intent to allow the board developer to not have their display used outside of the users code.

The main reason to have this configurable is to save space on translations that don't have good font coverage. Generally, is should be on when displays are present because they make them more useful.

bradanlane commented 6 months ago

Let's bubble back up a few levels ...

As a board developer, if I have set CIRCUITPY_TERMINALIO = 0 and #define CIRCUITPY_REPL_LOGO 0 then I would expect no terminal activity to affect the display.

That is not currently the case.

The code @tannewt pointed to bases its decision to refresh off of epaper_display.core.current_group != &circuitpython_splash. Since this has not been explicitly set, then something has implicitly set it.

bradanlane commented 6 months ago

Consistency is incredibly valuable.

I don't disagree.

However, if there are established preprocessor and build flags to prevent the display from being used by the core, and I used those build options, then I do not understand why something is refreshing the display.

addendum: FYI: the refresh only happens on the first REPL following reset or power up. subsequent errors do not trigger the refresh. This suggests some flag is set after the first error.

tannewt commented 6 months ago

However, if there are established preprocessor and build flags to prevent the display from being used by the core, and I used those build options, then I do not understand why something is refreshing the display.

I don't think boards should set these both so I'd consider it unsupported.

bradanlane commented 6 months ago

The reason for setting them both is because setting just CIRCUITPY_TERMINALIO = 0 resulted in a clipped version of the python logo displaying.

bradanlane commented 6 months ago

There does seems to be a forward looking precedent for board developers to indicate a display should not be used for core output.

This is being considered as part of #8675 and the creation of supervisor.runtime.display.

I am only trying to find a method that works today. If supervisor.runtime.display is added in the future, then I would adopt that method.