STM32: UART support

[hierophect]

This PR adds UART support to the STM32 series of development boards. As required by boards such as the GPS Featherwing, the current implementation allows for unexpected reception of data via a ring buffer. Transmits, however, are blocking. Since the UART HAL does not allow for passing context pointers, UART objects are tracked in global state.

This iteration is currently a bit hacked together. I'd probably be a good idea to revisit the module in the future to remove unnecessary HAL features and streamline the code for improved speed/maintainability.

[dhalbert]

Maybe look at the STM32 Arduino core for inspiration? https://github.com/stm32duino/Arduino_Core_STM32/blob/master/cores/arduino/stm32/uart.c et al

[dhalbert]

Or the MicroPython STM32 port?

[hierophect]

@dhalbert I'll see what I can dig out of micropython. It has a very different interface but maybe I can glean their strategy on the interrupts.

[tannewt]

Here are some initial comments. I'm headed out now for the day but will look at the rest tonight.

[hierophect]

firmware.bin.zip
bin for @ladyada

[tannewt]

Please double check that this PR doesn't change the version of TinyUSB. The Spresense build is failing because tinyusb is an older version that what is checked in.

I don't know why it's crashing after soft reset. The backtrace of the crash should provide info to help hunt it down. The busio_uart_obj_t data will be left alone until the next vm run where it may be allocated for something else and cleared. I'd suggest clearing the pointer array in uart_reset as well to make sure other memory isn't accidentally referenced on a subsequent run.

[tannewt]

Want to use MAX_UART here?

[hierophect]

See above. I could just have this definition in every mpconfigboard.h file if that's better, though.

[hierophect]

@tannewt on the subject of tinyusb, I often find I have a hard time keeping it synced when merging in upstream/master. It only every happens with tinyusb, too. There are no messages or conflicts or anything to signify that the wrong version is taking priority during the merge.

Do you have any idea what that might be?

[tannewt]

@hierophect Merging and changing branches doesn't automatically update submodules. After merging and before committing I recommend running git status to make sure everything is as you expect. After merging you'll want to git submodule update --init --recursive . to make sure you have the correct submodule.

[tannewt]

To get TinyUSB back to the checked in version I recommend looking on github for the hash and then locally change to lib/tinyusb and check it out. The commit change should show on git status and then you can commit it and push.

[dhalbert]

To get TinyUSB back to the checked in version I recommend looking on github for the hash and then locally change to lib/tinyusb and check it out. The commit change should show on git status and then you can commit it and push.

Yes, I usually do:

$ cd lib/tinyusb
$ git checkout master
$ git pull
$ git log
# check that the commit matches the commit in adafruit/circuitpython
# and checkout the wanted commit or just leave as is if it's already correct.

[hierophect]

@ladyada Do you think you could give this latest binary a run? I'm trying to get a better handle on what's wrong with it, which is proving tricky without better python experience. It's structurally very similar to stm32duino, but it just doesn't seem to be delivering on the basics these libraries need from it.

[hierophect]

bin if git CI is still down
firmware.bin.zip

[hierophect]

@ladyada New bin, should have fixed all crashes and buffer errors.
firmware.bin.zip

[ladyada]

yay verified this works a lot better, this is ready to merge/release to folks. open an issue to add dma/non-blocking writes and we can get feedback ask well

[ladyada]

tested functionality on STM32F405

[tannewt]

Good job getting this going using the HAL. I think it'd be simpler to not use the HAL for the RX interrupt but don't want to block if others disagree. The main thing will be to make sure the interrupt bits are cleared after you handle it. (The peripheral interrupt bits get merged into the interrupt line to the CPU.)

[hierophect]

Hey @tannewt, my current approach is inspired by stm32duino, which is completely HAL based. I opted to follow the way they did things as I felt it'd make stuff more readable to contributors who are familiar with that system. It's also a little closer to NRF, and avoids exposing all the parity and wordlength management in UART_Receive_IT in the uart.c file. But I can revisit a custom register approach if you feel strongly that it's more maintainable/appropriate.

[dhalbert]

Some q's and a few things to fix.

[tannewt]

I have one non-blocking comment. Will change review to comment so it can be merged after approval from @dhalbert

stale

[dhalbert]

OK, let's go with this now! I agree with @tannewt that there is a chance you may lose characters due to disabling/enabling, but we can revisit that after some user testing.