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 (teensy) Allow Any GPIO pin for RS485 pin #6328

Merged
merged 3 commits into from
May 4, 2022

Conversation

KurtE
Copy link

@KurtE KurtE commented Apr 30, 2022

Fixes #6310
Fixes #6332

The existing code was setup that allowed you to specify an RTS
pin to be used as an RS485 direction pin, however there are no
RTS pins that are exposed on any of the Teensy 4.x boards.

Instead Arduino code base allowed you to specify any GPIO pin to
work instead. So I added the code in to facilitate this.

In addition the alternative code to wrap your own GPIO pin set high and low
around call(s) to uart.write() will not currently work, unless maybe you
fudge it and add your own delays as the write will return after the last
byte was pushed onto the UART’s hardware FIFO queue and as such if you
then immediately set the IO pin low, it will corrupt your output stream.

The code I added detects that you are setup to use the RS485 pin and
before it returns will wait for the UART’s Transfer complete status flag
to be set.

We (@mjs513 and myself) have done a little playing with Dynamixel Servos connected up to a Teensy 4.1

I have hacked up the Python version of the DynamixelSDK to run on CPY.

Will continue to play some more, with it...

The existing code was setup that allowed you to specify an RTS
pin to be used as an RS485 direction pin, however there are no
RTS pins that are exposed on any of the Teensy 4.x boards.

Instead Arduino code base allowed you to specify any GPIO pin to
work instead.  So I added the code in to facilitate this.

In addition the alternative code to wrap your own GPIO pin set high and low
around call(s) to uart.write() will not currently work, unless maybe you
fudge it and add your own delays as the write will return after the last
byte was pushed onto the UART’s hardware FIFO queue and as such if you
then immediately set the IO pin low, it will corrupt your output stream.

The code I added detects that you are setup to use the RS485 pin and
before it returns will wait for the UART’s Transfer complete status flag
to be set.
@KurtE
Copy link
Author

KurtE commented Apr 30, 2022

Note: This is to fix the issues in #6310

@KurtE
Copy link
Author

KurtE commented Apr 30, 2022

As a slightly off topic - in that other ports have similar limitations, like RP2040, I am playing with a first cut of making the same logical code work more or less the same on it...

That is the construct code has added (also remove the error message)

    if (rs485_dir != NULL) {
        uint8_t pin = rs485_dir->number;
        self->rs485_dir_pin = pin;
        self->rs485_invert = rs485_invert;

        gpio_init(pin);

        claim_pin(rs485_dir);

        gpio_disable_pulls(pin);

        // Turn on "strong" pin driving (more current available).
        hw_write_masked(&padsbank0_hw->io[pin],
            PADS_BANK0_GPIO0_DRIVE_VALUE_12MA << PADS_BANK0_GPIO0_DRIVE_LSB,
            PADS_BANK0_GPIO0_DRIVE_BITS);

        gpio_put(self->rs485_dir_pin, rs485_invert);
        gpio_set_dir(self->rs485_dir_pin, GPIO_OUT);
    } else {
        self->rs485_dir_pin = NO_PIN;
    }

And the write code:

size_t common_hal_busio_uart_write(busio_uart_obj_t *self, const uint8_t *data, size_t len, int *errcode) {
    if (self->tx_pin == NO_PIN) {
        mp_raise_ValueError(translate("No TX pin"));
    }

    if (self->rs485_dir_pin != NO_PIN) {
        uart_tx_wait_blocking(self->uart);
        gpio_put(self->rs485_dir_pin, !self->rs485_invert);
    }

    size_t left_to_write = len;
    while (left_to_write > 0) {
        while (uart_is_writable(self->uart) && left_to_write > 0) {
            // Write and advance.
            uart_get_hw(self->uart)->dr = *data++;
            // Decrease how many chars left to write.
            left_to_write--;
        }
        RUN_BACKGROUND_TASKS;
    }
    if (self->rs485_dir_pin != NO_PIN) {
        uart_tx_wait_blocking(self->uart);
        gpio_put(self->rs485_dir_pin, self->rs485_invert);
    }
    return len;
}

And I am getting it to drive that pin 

Adafruit CircuitPython 7.3.0-beta.1-31-g73f6b4867-dirty on 2022-04-30; Adafruit Feather RP2040 with rp2040

import board, busio
board.UART().deinit()
uart = busio.UART(board.TX, board.RX, rs485_dir=board.D5, baudrate=1000000)
uart.write(b'abcdeef')
7

![image](https://user-images.githubusercontent.com/1558080/166118548-d1749b01-68f3-4a87-bb0c-5a3bae519c98.png)

But as you can see, it holds the DIR pin high for longer than I would like... Not sure if
` uart_tx_wait_blocking(self->uart);
`
Is the best way to detect when the write completes?  Sorry I am not much of expert on these chips... But the spec sort of sounded like the busy flag would be held asserted until the TX completes...

Suggestions?  Or maybe just punt? 

KurtE added a commit to KurtE/circuitpython that referenced this pull request Apr 30, 2022
As I mentioned in issue adafruit#6310 while investigating that the Teensy port
did not support RS485_dir pin on normal GPIO pins, I found that it
was not implemented either as well on some other ports.

So was curious to implement it for RP2040 using same approach as I did
for the MIMXRT in the Pull Request adafruit#6328

That is I setup the specified pin as a normal GPIO pin in output mode
and then when you do a write operation it sets the GPIO pin logically
high, and when the write completes I set it logically low.

Note: knowing when I can set it low can be tricky, as you need to make
sure the full output has completed otherwise the data will be corrupted.

I am using:         uart_tx_wait_blocking(self->uart);
Which looks like it is supposed to wait until the busy status is no
longer set, which the Reference manual mentioned, but this is leaving
the line logically set longer than I would like.

however I have tried running it with my hacked up version of the
Python Robotis DynamixelSDK and was able to talk to some AX servos.

I did have to change the library slightly for the RP2040, as the
library was erroring out when you did something like uart.read(5)
and it timed out without receiving anything.  The RP2040 returned
None whereas I think the Teensy returned an empty set, which is what
it looks like the PySerial original code expects.

Not sure if anyone is interested in this, but thought i would
put it out as PR and see.
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.
jepler
jepler previously approved these changes May 3, 2022
Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't spot any problems, but no testing performed.

Note: This is to fix the issues in #6310

Please use the special syntax (by editing the initial comment on the PR) to 'link' issues that will be fixed by a pull request. It's super helpful. You can do this anytime before the PR is merged. (edited to add the link to the docs)

@jepler jepler self-requested a review May 4, 2022 13:57
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi - my understanding is that you are simulating RS485 serial hardware, which guarantees certain timing on the direction pin, as described, for instance, in https://www.st.com/resource/en/application_note/cd00249778-managing-the-driver-enable-signal-for-rs485-and-iolink-communications-with-the-stm32s-usart-stmicroelectronics.pdf (linke courtesy @jepler).

if, say, an interrupt occurred before the pin was toggled, the signal might get delayed, or it might get lengthened if the interrupt occurred during the toggle. Then the timing might break RS485 rules.

Up to now, we have only implemented RS485 when the hardware provided it, e.g. SAMD51 vs SAMD21. Did the i.MX port use hw RS485 before, and now it optionally doesn't? I'm not sure even after reading the code.

I'm concerned that this RS485 might work "most" of the time, but would randomly fail occasionally due to timing issues. For casual servo work that might be fine, but not in other cases.

@KurtE
Copy link
Author

KurtE commented May 4, 2022

I'm concerned that this RS485 might work "most" of the time, but would randomly fail occasionally due to timing issues. For casual servo work that might be fine, but not in other cases.

Understood!

However today on all of the Teensy 4.x board you two choices. Either it works most of the time, or works none of the time. None of the Uarts have any CTS pins exposed, as such you do not have a way to synchronize a pin with when transmission completes.

Your Uart Write code here is neither synchronous nor asynchronous as it returns as soon as it places the last byte in the output queue of the IMXRT processor. As I showed in the issue mentioned in first post, if your code then tries to emulate it, it may easily drop the IO level before the transfer completes.

And your interface does not provide anything like Arduino: Serial1.flush()
or Linux(FTDI) termios tcDrain() which I found you really want to avoid except for FTDI...

So If you are uncomfortable with this approach, you should probably close out this PR as well as #6330

Alternatively you could choose to do something different like:

a) Add a flush() method which waits until transfer is complete. Note PySerial has this. At which case the user code code do the same stuff as I did in this PR.

b) Do a major rework of your BUSIO code base, and for example have software output queue, where the UART code will fill up the hardware queue from software queue, and when it completes, and you get the interrupt for Transfer complete, the interrupt code is the one that then transfer has completed, at which point it handles setting the state of the RS485 direction pin... It also handles changing the appropriate registers to allow the Serial port to work in half duplex mode.

Until then I will stop experimenting of trying to use CircuitPython on Robot controller that outputs to servos.

@dhalbert
Copy link
Collaborator

dhalbert commented May 4, 2022

Your Uart Write code here is neither synchronous nor asynchronous as it returns as soon as it places the last byte in the output queue of the IMXRT processor.

Both the atmel-samd and the nrf ports wait until the UART peripheral is no longer busy to return. I think, but am not sure, that mean that the last byte has been transmitted out of the FIFO. If the i.MX port is not doing that, then it should, and we would welcome that fix.

(As I think may be clear already, the current i.MX code is in no way sacred, and issues you find are not necessarily based on deliberate choices when the code was written.)

Re RS485. We could place a warning in the documentation that the RS485 support is "best effort, use with caution", and that seems fine to me.

@dhalbert
Copy link
Collaborator

dhalbert commented May 4, 2022

Re RS485. We could place a warning in the documentation that the RS485 support is "best effort, use with caution", and that seems fine to me.

Added a doc note. I'll approve this and the RP2040 one after that.

@KurtE
Copy link
Author

KurtE commented May 4, 2022

Sounds good

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on this!

@dhalbert dhalbert merged commit a7ec8b0 into adafruit:main May 4, 2022
@KurtE KurtE deleted the mixrt_uart_rs485 branch May 4, 2022 18:07
@dhalbert
Copy link
Collaborator

Both the atmel-samd and the nrf ports wait until the UART peripheral is no longer busy to return. I think, but am not sure, that mean that the last byte has been transmitted out of the FIFO. If the i.MX port is not doing that, then it should, and we would welcome that fix.

There is an issue I found about this for atmel-samd: #1770

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants