Add support for a debug console, such as ST-Link VCP.

[k0d]

This is a work in progress!

Todo:
Make more generic
Enable debug_console when entering REPL

[k0d]

@tannewt @hierophect
This works now...the only thing is that it's not generic, meaning that we should decide where we set the config and how to enable/disable it.

Could you have a look through the commit, ignore where it says "TODO"...but I'd like your general comments on the solution.

[hierophect]

@k0d it's an interesting idea to have the serial console simultaneously to USB - is there a particular application that you have this in mind for? Did you check out the port level supervisor/serial.c files that exist in most ports? Most of the time, it's automatic to switch to a serial connection when USB is set to 0 (which you can do from the build command as USB=0).

Existing implementations aren't very sophisticated because the use is kind of rare, and they don't use the UART module (by design), so this is definitely a more complete implementation. Perhaps you could work it into the existing model somehow, so if the UART module is available it will default to this debug implementation when USB is disabled?

[k0d]

The serial.c that exists already works if you don't have USB, but...if you want to continue to use the VCP even when USB is enabled then that does't work. When I'm developing it's much easier to have the VCP instead of the USB.

I understand it's not everyone would use this when developing, but I use it all the time.

[k0d]

@hierophect I thought of using the serial.c implementation, which I could do...but I'd have to tweak some things a bit because if USB is enabled, it uses supervisor/shared/serial.c.

I think it'd be better to call it console.c, which in turn uses serial.c/usb.c/display.c depending on what's available?

[k0d]

@hierophect This is ready for review now, the CI checks are failing due to 'Github timeouts', if there are any real issues...I'll fix them.

[tannewt]

Thank you for the PR!

I think that these changes are too invasive to take as is. (All of the _UNAVAILABLE defines are changing a bunch of stuff.) Instead, I'd like to see this build on top of UART by calling the common_hal constructor directly. The only thing UART will need to do it is a "alert me on a character" callback like tinusb has.

Leveraging the existing UART code will:

1. Simplify the configuration in mpconfigboard.h to the pins and maybe a baudrate.
2. Simplify the init code in debug_init.
3. Work on more than just STM out of the box except for ctrl-c which requires the callback.

[k0d]

I think that these changes are too invasive to take as is. (All of the _UNAVAILABLE defines are changing a bunch of stuff.) Instead, I'd like to see this build on top of UART by calling the common_hal constructor directly. The only thing UART will need to do it is a "alert me on a character" callback like tinusb has.

Leveraging the existing UART code will:

1. Simplify the configuration in mpconfigboard.h to the pins and maybe a baudrate.
2. Simplify the init code in debug_init.
3. Work on more than just STM out of the box except for ctrl-c which requires the callback.

The common_hal implementation of STM32_UART requires there to be an active 'python session', so it doesn't work in certain situations, this is kind of a higher level than the python uart modules. I find it hard to explain this, but it didn't work...I tired it...that's why this PR took so long to make.

[tannewt]

The common_hal implementation of STM32_UART requires there to be an active 'python session', so it doesn't work in certain situations, this is kind of a higher level than the python uart modules. I find it hard to explain this, but it didn't work...I tired it...that's why this PR took so long to make.

Did you try statically allocating the struct for it? You should be able to pass in a statically allocated struct to the construct and then never_reset it to keep it alive throughout VM runs.

[dhalbert]

Did you try statically allocating the struct for it? You should be able to pass in a statically allocated struct to the construct and then never_reset it to keep it alive throughout VM runs.

There are examples of that technique for displayio, such as the SPI object it uses.

[k0d]

Did you try statically allocating the struct for it? You should be able to pass in a statically allocated struct to the construct and then never_reset it to keep it alive throughout VM runs.

There are examples of that technique for displayio, such as the SPI object it uses.

@dhalbert link to an example please? :)

[dhalbert]

@dhalbert link to an example please? :)

shared-module/displayio/__init__.c and .h. The primary_display_t objects in displays are allocated statically, and include displayio_fourwire_obj_t in a union. Fourwire includes an SPI object: see shared-module/displayio/FourWire.h.

Also see common_hal_busio_spi_never_reset() calls in shared-module/displayio/FourWire.c.

It's a bit complicated because dynamically allocated heap objects can get copied into the static space: see reset_displays() in __init__.c.

Another example is the internal connection objects in ports/nrf/common-hal/_bleio/Adapter.*

[k0d]

@tannewt @hierophect @dhalbert
This PR has been rewritten from scratch, it now uses the common_hal to setup the UART. Much nicer, cleaner and logical! Many thanks to the pointers.

There is a TODO note in supervisor/shared/serial.c, I actually don't know if we need to fix it. Please advise.

[tannewt]

Looks much much better! Thank you! Just a few more things.

[k0d]

Strange, the failed checks don't happen when I build locally...it's also not related to my changes. Known issue?

[dhalbert]

Strange, the failed checks don't happen when I build locally...it's also not related to my changes. Known issue?

The errors may be due to some kind of submodule sync problem: one is from STM, and the other has to do with protomatter. Could you merge from upstream and also do git submodule sync and git submodule update? Also make sure your pushes are from a branch with everything added.

[k0d]

@dhalbert That didn't fix it, I also get it when I checkout master.

[tannewt]

I think master is broken. It isn't a problem with this PR.

[k0d]

Yeah, I saw that the RGBMatrix PR broke things...so I'm just ignoring the CI checks now... :)

[tannewt]

Thank you for the updates to this! It's getting close. Just a few more comments about the UART functionality that needs to work outside of the debug use too.

[hierophect]

Looks good to me. I chimed in a bit on the VCP TX/RX pin exposure discussion but it seems to already be resolved. Anything to do with Morpho pins is in the territory of "just change the build yourself" anyway.

[tannewt]

One small bug to fix and then this will be ready. Thank you!

[k0d]

@tannewt Can we ignore the failed checks? or should we run it again?

[tannewt]

We can ignore it. Thanks!