Add CIRCUITPY_TERMINAL_SCALE settings.toml parameter

[RetiredWizard]

As discussed in #10084 this adds a settings.toml parameter to override the default terminalio scaling factor for displayio.CIRCUITPYTHON_TERMINAL.

This doesn't resolve the reported hard fault but is hopefully a more robust method of performing the scaling than the technique currently causing the fault.

[samblenny]

Did you happen to come across the code that is supposed to make sure that display.root_group is not set to a heap allocated group that will go away when the supervisor does a soft reset? From what tannewt said in #10084 (comment), I think something is supposed to make sure the root group gets set to displayio.CIRCUITPYTHON_TERMINAL?

[RetiredWizard]

I did not, as I try and work out the memory issues here, I'll keep an eye open for it, but I read the comment a little differently. I thought tannewt was just letting you know that by appending the scaled displayio.CIRCUITPYTHON_TERMINAL to a VM heap allocated group it wouldn't survive a reset.

[samblenny]

There's something that's allowing a hard fault, which is not supposed to happen.

[RetiredWizard]

I'm not sure if I added this to the proper location and the use of a new compile parameter may also not be acceptable, however I figured that the M0 boards are running into build size issues more and more frequently and perhaps a generic option to exclude newly added features would be a solution that could work for more than this PR.

[tannewt]

Thanks for adding this! One question.

[dhalbert]

The M0 boards now have many new setting lines that were copy/pasted identically (?) them. What did you turn off that was on previously for M0 boards? I'd prefer that changes were made in mpconfigport.mk for M0 boards, so they could share the settings. You can adjust what CIRCUITPY_FULL_BUILD does there rather than in each board's mpconfigboard.mk.

[RetiredWizard]

I went through the logic in py/circuitpy_mpconfig.mk and tried to identify everything that was turned off when FULL_BUILD was turned off which is the list of parameters I added to each of the boards that had overflowed by adding the new TERMINAL_SCALE logic. Not every M0 board needed to be updated although it's possible that all the boards I didn't update already had the FULL_BUILD turned off.

Other than the CIRCUITPY_FULL_BUILD option I didn't turn anything else off on the boards, the list of parameters I added shows the things I turned back on that FULL_BUILD turns off. I thought it was helpful to have the list in the boards as some of the things I turned back on may need to be disabled again as CircuitPython continues to grow.

I did essentially make the same changes to all the boards I updated, however I believe most of the boards that used the CIRCUITPY_FULL_BUILD=0 option prior to this simply left everything disabled (the pewpew boards being one exception) so moving the features I left enabled to the mpconfigport.mk file could require some tweaking on boards which are currently working fine.

I can either try and get my list of re-enabled features to work from mpconfigport.mk or another option would be to remove all the re-enabled parameters from the boards I updated and just set CIRCUITPY_FULL_BUILD=0. If all of the new settings lines in the mpconfigboard.mk files are a problem, I can also remove all the parameters that are set to 0 in each of the boards I updated as I only left them in there for documentation of what CIRCUITPY_FULL_BUILD=0 was disabling and they actually don't do anything.

[RetiredWizard]

Would something like this be a more acceptable approach?

SB_VID = 0x239A
USB_PID = 0x8014
USB_PRODUCT = "Metro M0 Express"
USB_MANUFACTURER = "Adafruit Industries LLC"

CHIP_VARIANT = SAMD21G18A
CHIP_FAMILY = samd21

SPI_FLASH_FILESYSTEM = 1
EXTERNAL_FLASH_DEVICES = "S25FL216K, GD25Q16C, W25Q16JVxQ"
LONGINT_IMPL = MPZ

CIRCUITPY_RAINBOWIO = 0

CIRCUITPY_FULL_BUILD = 0
# Re-enable some of the features that CIRCUITPY_FULL_BUILD=0 disables
MICROPY_PY_ASYNC_AWAIT = 1
CIRCUITPY_ATEXIT = 1
CIRCUITPY_BITBANGIO = 1
CIRCUITPY_BUSDEVICE = 1
CIRCUITPY_COUNTIO = 1
CIRCUITPY_DISPLAYIO = 1
CIRCUITPY_OS_GETENV = 1
CIRCUITPY_ERRNO = 1
CIRCUITPY_FREQUENCYIO = 1
CIRCUITPY_LOCALE = 1
CIRCUITPY_OPT_MAP_LOOKUP_CACHE = 1
CIRCUITPY_PULSEIO = 1
CIRCUITPY_SDCARDIO = 1
CIRCUITPY_TRACEBACK = 1
CIRCUITPY_WARNINGS = 1
# End of re-enabled FULL_BUILD settings

[dhalbert]

For the small M0 builds, instead of the original idea of CIRCUITPY_NOT_FLASH_LIMITED, I would just do CIRCUITPY_TERMINAL_SCALING or CIRCUITPY_TERMINAL_SCALE. Then turn that off for the boards where it doesn't fit.

I know you are trying to figure out how to solve this problem more generally. When a board overflows, we only want to turn off one or a few features to get it to fit. Either we should do that per board, or we should make up a CIRCUITPY_MEDIUM_BUILD (I don't like the name, but something to that effect) that turns off/on a specific set of features. What I'm trying to avoid is having a long list of features listed in a large number of mpconfigboard.mk files.

In the past, I've tried to turn off modules that were less important for a particular board. Maybe it doesn't have many pins, maybe it has a special use case, etc. That can help guide the decisions about the modules.

Could you back out the most recent changes and go back to build that shows which boards are overflowing? Then I'll take a look at what's failing and we can discuss how to proceed from there.

As a discussion point, note that MicroPython has this in py/mpconfig.h

#define MICROPY_CONFIG_ROM_LEVEL_MINIMUM (0)
#define MICROPY_CONFIG_ROM_LEVEL_CORE_FEATURES (10)
#define MICROPY_CONFIG_ROM_LEVEL_BASIC_FEATURES (20)
#define MICROPY_CONFIG_ROM_LEVEL_EXTRA_FEATURES (30)
#define MICROPY_CONFIG_ROM_LEVEL_FULL_FEATURES (40)
#define MICROPY_CONFIG_ROM_LEVEL_EVERYTHING (50)
...

I'm not too fond of the names, but we could think about a level-based system instead of just on/off.

[dhalbert]

Thanks for the reversion. I now see that the build failures due to size are >1kB, which is way more than the code you added. I think this is because adding CIRCUITPY_TERMINAL_SCALE is the first instance of using common_hal_os_getenv_int() in these builds, which causes all the getenv mechanism to be linked in. Previously the build did not use getenv at all, and that code was omitted at link time.

I think the most straightforward fix here is to guard the code you added with

#if CIRCUITPY_OS_GETENV
...
#endif

shared-module/os/getenv.c is being always being compiled (but not always linked) because CIRCUITPY_OS = 1. It could be removed through some trickiness in py/circuitpy_defns.mk that checks CIRCUITPY_OS_GETENV, but I think the easier thing to do is to guard the entire contents of shared-module/os/getenv.c with an #if CIRCUITPY_OS_GETENV after the includes, adding #endif at the end.

That will make sure that the getenv routines are never compiled, and will cause a compile error if someone tries to use them when CIRCUITPY_OS_GETENV = 0.

[RetiredWizard]

I think the most straightforward fix here is to guard the code you added with

I love it ❤️, Thanks!!!

[RetiredWizard]

Oops, I just re-read you comment and see I was so excited at the solution I missed the details of your suggestion. I'll fix and resubmit 😁

[RetiredWizard]

It looks like I still have to guard the call I added to common_hal_os_getenv_int or I get an undefined reference. Since the call has to be guarded and by guarding the call and the include at the top, getenv is apparently not linked in, is there still any added value in guarding the contents of shared-module/os/getenv.c?

[dhalbert]

I meant add guards both places. Guard your own call to common_hal_os_getenv_int() that you added in this PR, and guard the contents of getenv.c so this won't happen again. That's the advantage of the latter.

[dhalbert]

Looks good - thanks for your patience on this!

resolved in another way