Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
KurtE opened this issue May 1, 2022 · 8 comments · Fixed by #6328
Closed

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

KurtE opened this issue May 1, 2022 · 8 comments · Fixed by #6328
Labels
bug mimxrt10xx iMX RT based boards such as Teensy 4.x
Milestone

Comments

@KurtE
Copy link

KurtE commented May 1, 2022

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
Copy link
Author

KurtE commented May 1, 2022

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
Copy link
Member

jepler commented May 3, 2022

@dhalbert
Copy link
Collaborator

dhalbert commented May 3, 2022

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
Copy link
Member

jepler commented May 3, 2022

.. 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
Copy link
Member

jepler commented May 3, 2022

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.

@jepler jepler closed this as completed May 3, 2022
@dhalbert
Copy link
Collaborator

dhalbert commented May 3, 2022

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
Copy link
Member

jepler commented May 3, 2022

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

@jepler jepler reopened this May 3, 2022
@jepler jepler added bug mimxrt10xx iMX RT based boards such as Teensy 4.x and removed enhancement labels May 3, 2022
@jepler jepler changed the title Inconsistent return results when uart.read(n) times out. mimxrt should return None when uart.read(n) times out with 0 bytes read May 3, 2022
@jepler jepler added this to the Long term milestone May 3, 2022
KurtE added a commit to KurtE/circuitpython that referenced this issue May 3, 2022
there return of a read operation that times out with no data received
is inconsistent:
```
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''
```

Since I have a PR on this file anyway, I thought I would put in the change to make it consistent
with the other 3 board types I tried.  Can not say about any of the others.
@KurtE
Copy link
Author

KurtE commented May 3, 2022

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:
#6328

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mimxrt10xx iMX RT based boards such as Teensy 4.x
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants