adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4k stars 1.18k forks source link

mimxrt should return None when uart.read(n) times out with 0 bytes read #6332

Closed KurtE closed 2 years ago

KurtE commented 2 years ago

Not sure if best to ask/mention here or discord or forum...

I have been playing around with adding ability to talk to Dynamixel Servos, as mentioned in the couple of PR's and earlier issue. For the heck of it I ported over the DynamixelSDK python code to talk to servos using the rs485_dir

When I tried it on the RP2040 with my mods to support the direction pin, the python code error happened in the dynamixel library code that None class is not itterable... In the function: https://github.com/ROBOTIS-GIT/DynamixelSDK/blob/master/python/src/dynamixel_sdk/protocol1_packet_handler.py#L130

On the line:

 rxpacket.extend(port.readPort(wait_length - rx_length))

It turns out that several of the current boards return None when a read times out with no data. Note: The Teensy MIMXRT port returns empty byte array. As you can see with the quick and dirty tests using REPL:

Adafruit CircuitPython 7.3.0-beta.1-31-g73f6b4867-dirty on 2022-04-30; Adafruit Feather RP2040 with rp2040
>>> 
>>> import board, busio
>>> print(board.UART().read(5))
None

Adafruit CircuitPython 6.3.0 on 2021-06-01; FeatherS2 with ESP32S2
>>> import board,busio
>>> print(board.UART().read(5))
None

Adafruit CircuitPython 7.3.0-beta.1 on 2022-04-07; Adafruit Feather STM32F405 Express with STM32F405RG
>>> import board, busio
>>> print(board.UART().read(5))
None

Adafruit CircuitPython 7.3.0-beta.1-31-g73f6b4867-dirty on 2022-04-28; Teensy 4.1 with IMXRT1062DVJ6A
>>> import board, busio
>>> print(board.UART().read(5))
b''

This may be by design, in which case probably close this one out. The main CircuitPython page for UARTS https://docs.circuitpython.org/en/1.0.0/docs/library/machine.UART.html did not exist, but I did find... https://circuitpython-jake.readthedocs.io/en/latest/shared-bindings/busio/UART.html#busio.UART.read

Which says it can return bytes or None

When I looked at PySerial doc: https://pyserial.readthedocs.io/en/latest/pyserial_api.html#serial.Serial.read

It just shows it returns bytes.

Other difference in docs is that number of bytes to read. The Jake version says defaults to None, the PySerial says default to 1.

Again sorry if this is the wrong place to mention this.

KurtE commented 2 years ago

Note as mentioned on discord: I changed the code from: rxpacket.extend(port.readPort(wait_length - rx_length))

to:

            rdata = port.readPort(wait_length - rx_length)
            if rdata:
                rxpacket.extend(rdata)

Which is working

jepler commented 2 years ago

pyserial has the return type as 'bytes'. https://pyserial.readthedocs.io/en/latest/pyserial_api.html?highlight=read#serial.Serial.read

dhalbert commented 2 years ago

The main CircuitPython page for UARTS https://docs.circuitpython.org/en/1.0.0/docs/library/machine.UART.html did not exist, but I did find... https://circuitpython-jake.readthedocs.io/en/latest/shared-bindings/busio/UART.html#busio.UART.read

This is the doc page you want; not sure where you got the 1.0.0 page URL, that is ancient. https://docs.circuitpython.org/en/latest/shared-bindings/busio/index.html#busio.UART

RTD is linked several places, e.g., see along the top of the page: https://circuitpython.org/

jepler commented 2 years ago

.. but the core says (at least according to source code comments inherited from micropython development):

        if (mp_is_nonblocking_error(error)) {
            // https://docs.python.org/3.4/library/io.html#io.RawIOBase.read
            // "If the object is in non-blocking mode and no bytes are available,
            // None is returned."
            // This is actually very weird, as naive truth check will treat
            // this as EOF.

The Python docs make the distinction between a zero-length return and a None return:

If 0 bytes are returned, and size was not 0, this indicates end of file. If the object is in non-blocking mode and no bytes are available, None is returned.

so it's about distinguishing EOF and timeout.

jepler commented 2 years ago

I'm going to go ahead and close it up. It appears to me that the behavior is deliberate, and matches the documented behavior of io.RawIOBase.read.

dhalbert commented 2 years ago

So then this does point out that:

Adafruit CircuitPython 7.3.0-beta.1-31-g73f6b4867-dirty on 2022-04-28; Teensy 4.1 with IMXRT1062DVJ6A
>>> import board, busio
>>> print(board.UART().read(5))
b''

should really be None.

jepler commented 2 years ago

You're right, I missed the part where it was inconsistent within our various ports, after I thought I'd seen that we had one consistent block of code to handle the condition. Reopening, as apparently mimxrt needs a block similar to

    if (total_read == 0) {
        *errcode = EAGAIN;
        return MP_STREAM_ERROR;
    }

    return total_read;
}

from RP2040

KurtE commented 2 years ago

Note as I discovered the differences in these return values as part of the code to try to handle Dynnamixel messges

I added the changes for this into that PR: https://github.com/adafruit/circuitpython/pull/6328