RP2040: Support for UART

[microdev1]

No description provided.

[jerryneedell]

Tested by connecting a gps Breakout to TX0/RX0 on Pins GP12/13 and running the gps_echotest.py script. worked normally.
Edited to add -- gps_echotest works ok, but gps_simpletest is not working -- looking into it.

[ladyada]

oh wow, this is awesome. let me try it too :)

[ladyada]

ok GPS tested, 9600 baud and works as expected. will try some RX/TX loopback tests

[ladyada]

tested all common baud rates from 1200 to 115200 with

import time
import board
import busio

uart = busio.UART(board.TX, board.RX, baudrate=115200, timeout=0.1)

char = 0
while True:
    uart.write(bytearray([char]))
    char = (char + 1) % 256
    data = uart.read(1) 
    print(data)  # this is a bytearray type

[jerryneedell]

@ladyada Did gps_simpletest work for you? It is not seeing a fix for me.... echotest works fine.....

[ladyada]

i didnt test fix cause thats a 'sit it outdoors' thing and its really cold out :D

[jerryneedell]

hopefully not related to UART -- trying to figure it out....

[jerryneedell]

There may be an issue -- the gps.update is faiong in _read_sentence here:

    def _read_sentence(self):
        # Parse any NMEA sentence that is available.
        # pylint: disable=len-as-condition
        # This needs to be refactored when it can be tested.

        # Only continue if we have at least 32 bytes in the input buffer
        print(self.in_waiting)        
        if self.in_waiting < 32:
            return None

https://github.com/adafruit/Adafruit_CircuitPython_GPS/blob/master/adafruit_gps.py#L200
self.in_waiting seems to be 1 all the time

[jerryneedell]

is there potentially an issue with this not returning the correct value for bytes in the uart?

circuitpython/ports/raspberrypi/common-hal/busio/UART.c

Line 231 in b12ccef

return uart_is_readable(self->uart);

[microdev1]

Thanks! @ladyada and @jerryneedell for the extensive testing... 👍

is there potentially an issue with this not returning the correct value for bytes in the uart?

Seems like uart_is_writable can only return if data is available or not and not the actual number of bytes.

/*! \brief Determine whether data is waiting in the RX FIFO
 *  \ingroup hardware_uart
 *
 * \param uart UART instance. \ref uart0 or \ref uart1
 * \return 0 if no data available, otherwise the number of bytes, at least, that can be read
 *
 * \note HW limitations mean this function will return either 0 or 1.
 */
static inline bool uart_is_readable(uart_inst_t *uart) {
    // PL011 doesn't expose levels directly, so return values are only 0 or 1
    return !(uart_get_hw(uart)->fr & UART_UARTFR_RXFE_BITS);
}

[ladyada]

yeha we should be buffering the UART data internally

[microdev1]

yeha we should be buffering the UART data internally

yup... that's what I was thinking for a workaround

[jerryneedell]

just "for fun" I changed the gps code to continue as long as anything was in the UART and now both the gps_simpltest.py and gps_time_source.py examples work.
This is what I changed at https://github.com/adafruit/Adafruit_CircuitPython_GPS/blob/master/adafruit_gps.py#L200

        #if self.in_waiting < 32:
        if not self.in_waiting:
            return None

Not a fix, but it's nice to see it running

[tannewt]

Thank you for this! I think we do need to add buffering before we merge this because the API explicitly allows setting the buffer size.

We can either copy other ports and use an interrupt to write into a ring buffer. Or, we can use the RP2040 ring DMA while constraining the buffer sizes to powers of two.

I'd be happy to add the buffering next week if you'd like to hand it off.

[microdev1]

the API explicitly allows setting the buffer size

I see. I have added internal buffering now.
@jerryneedell can you do a retest of gps_simpletest.

[jerryneedell]

@microdev - sorry... It does not work at all. Even the "echotest" now just returns a steady stream of "HHHHHHH" characters. the "simpletest" crashes due to a memory allocation error.....

[jerryneedell]

did a simple test -- with nothing connected -- same result with GPS connected.

Adafruit CircuitPython 6.2.0-beta.2-48-gefe6aab5f on 2021-02-23; Raspberry Pi Pico with rp2040
>>> import time
>>> import board
>>> import busio
>>> 
>>> uart = busio.UART(board.GP12, board.GP13, baudrate=9600, timeout=10)
>>> 
>>> uart.in_waiting
235
>>> uart.read(1)
b'H'
>>> uart.read(1)
b'H'
>>> uart.in_waiting
235
>>> 

[jerryneedell]

FYI _ I built CP with this PR and tested it with the GPS -- both the echotest and simpletest now work normally.

[ajs256]

Tested using:

import board
import busio
import random

uart = busio.UART(board.GP0, None, baudrate=9600)

uart.write("|-".encode("UTF-8")) # Clear display

uart.write("Hello world!    ".encode("UTF-8")) # Show some text.

uart.write("Rand: ".encode("UTF-8")) 
rand = str(random.randint(0, 100))
uart.write(rand.encode("UTF-8")) # So I can see that it's working every time, print a random number.

and a SparkFun Serial LCD. I can verify that it is reliably writing, and that the random value is different every time.

[tannewt]

Thank you for the improvements. Unfortunately, UART has a lot of corner cases. Let me know if you'd like to hand off the PR to me.

[lesamouraipourpre]

just "for fun" I changed the gps code to continue as long as anything was in the UART and now both the gps_simpltest.py and gps_time_source.py examples work.
This is what I changed at https://github.com/adafruit/Adafruit_CircuitPython_GPS/blob/master/adafruit_gps.py#L200

        #if self.in_waiting < 32:
        if not self.in_waiting:
            return None

Not a fix, but it's nice to see it running

As an aside the 32 is too large, new issue is #56. I'll do a PR for it, once my previous PR gets past PyLint and the examples duplicate-code annoyance.

[microdev1]

Unfortunately, UART has a lot of corner cases. Let me know if you'd like to hand off the PR to me.

@tannewt Thanks! for the review. I have addressed the current set of suggestions.
If there is still a chance of improvement you can go ahead push directly to my branch. :)

[tannewt]

Ok, I've fixed a couple more issues I spotted. If folks could test and review, that'd be awesome.

[jerryneedell]

Ok, I've fixed a couple more issues I spotted. If folks could test and review, that'd be awesome.

Reran gsp_echotest and gps_simpletest -- both still work normally.

[tannewt]

Gah, I had my debug text files in there for a bit. re-did that last commit and the CI needs to run again.

[tannewt]

Thank you @microdev1

[microdev1]

Thanks! everyone for collaborating on this. 👍

[ladyada]

Thank you for working hard on the PR, its a wonderful and helpful addition!