Skip to content

ESP UART pins misnamed on some boards #3770

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

Merged
merged 2 commits into from
Nov 30, 2020
Merged

Conversation

dhalbert
Copy link
Collaborator

@dhalbert dhalbert commented Nov 28, 2020

I was helping someone with a PyPortal-using-BLE issue, and it turns out the ESP TX/RX pins on a few Airlift-style boards are labeled only RX and TX, even though they are only connected to the ESP32. This makes them incompatible with the adafruit-airlift library right now without giving different pin names.

This PR adds ESP_TX and ESP_RX names to the existing TX and RX pins, which is upward compatible.

However, I think that it would actually be better to remove the TX and RX names and also board.UART(), since these are not externally available pins.

If you agree, let me know and I'll revise this draft PR. One or two Guides would have to be updated as well.

@dhalbert dhalbert requested review from tannewt and ladyada November 28, 2020 20:00
@ladyada
Copy link
Member

ladyada commented Nov 28, 2020

@brentru - please take a look at your convenience!

@brentru
Copy link
Member

brentru commented Nov 30, 2020

@dhalbert @ladyada

However, I think that it would actually be better to remove the TX and RX names and also board.UART(), since these are not externally available pins.

Agreed! I'm ok with renaming the internally connected pins to ESP_ as well.

One or two Guides would have to be updated as well.

Which guides are effected by this change?

@dhalbert
Copy link
Collaborator Author

This is the one I found so far: https://learn.adafruit.com/adafruit-pyportal/pinouts#step-3020486
image

@brentru
Copy link
Member

brentru commented Nov 30, 2020

@dhalbert dhalbert requested review from brentru and removed request for ladyada and tannewt November 30, 2020 16:13
@dhalbert dhalbert marked this pull request as ready for review November 30, 2020 16:13
Copy link
Member

@brentru brentru left a comment

Choose a reason for hiding this comment

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

Looks good - uart removed, new ESP_ pins are correctly labelled for esp control.

@dhalbert
Copy link
Collaborator Author

May also need to update MiniESPTool examples (https://github.com/adafruit/Adafruit_CircuitPython_miniesptool/blob/master/examples/miniesptool_simpletest.py)

This looks like it will not need revision because it will just pick up the new names, e.g.:

tx = getattr(board, "ESP_TX", board.TX)
rx = getattr(board, "ESP_RX", board.RX)

@dhalbert
Copy link
Collaborator Author

1 failure is an upload failure, unrelated to PR.

@dhalbert dhalbert merged commit 0f0bbd8 into adafruit:main Nov 30, 2020
@dhalbert dhalbert deleted the esp-uart-pins branch November 30, 2020 17:17
@dhalbert
Copy link
Collaborator Author

dhalbert commented Nov 30, 2020

I updated https://learn.adafruit.com/adafruit-pyportal/pinouts#step-3020486, which mentions the pin names, and also the sample AirLift BLE program here: https://learn.adafruit.com/adafruit-airlift-breakout/circuitpython-ble#copy-and-adjust-the-example-program-3076274-17. They mention that after CircuitPython 6.0.0, the pin names have changed. Once 6.1.0 is released for real, I can remove those caveats.

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

Successfully merging this pull request may close these issues.

3 participants