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

Support using the built-in UART peripherials of a Raspberry Pi #234

Merged
merged 1 commit into from
Sep 26, 2022

Conversation

bugadani
Copy link
Contributor

@bugadani bugadani commented Sep 13, 2022

This PR is heavily in the draft state, especially because it requires a nightly compiler at its current state and it's currently untested.

Raspberry Pis don't have hardware support for DTR, so their internal UART peripherials can't direcly be used in espflash. This PR adds rppal to manage two user-configurable GPIOs as RTS/DTR. This requires some platform-specific code, for which I have introduced the Interface struct (name pending), and a cargo feature called raspberry.

Some follow up work:

  • I need to actually test if this works
  • espflash now reports an error if
    • GPIO isn't available (used or doesn't exist)
    • or if DTR is not set for Unknown serial ports (assuming internal peripherial is covered here)
  • README has not yet been updated with the RPi-specific input params.

Implementation depends on trait_upcasting because of https://github.com/esp-rs/espflash/pull/234/files#diff-9b8216d416248667f4612fb114829cb89c5754ca818d5784b3b0d879ec0d4ee6R266-R267 but the feature has been proposed to be stabilized in rust-lang/rust#101718 I'm open to suggestions if there is a way to avoid this feature requirement. See 5a25044 for my solution

@bugadani

This comment was marked as outdated.

@bugadani
Copy link
Contributor Author

bugadani commented Sep 16, 2022

Together with #238 I was able to read the serial output of my board, so this PR technically works. I've decided to allow USB serial adapters while overwriting RTS/DTR, because why not 😅

@bugadani
Copy link
Contributor Author

Example setup:

# /boot/config.txt excerpt

dtoverlay=uart0,txd_pin=14,rxd_pin=15

$ ./espflash serial-monitor --dtr 16 --rts 17 --monitor-speed 921600 /dev/ttyAMA0

@bugadani bugadani marked this pull request as ready for review September 20, 2022 14:18
@bugadani
Copy link
Contributor Author

@jessebraham barring CI failures, I think this PR is ready for review. I've been using this to flash and monitor a device and I'm happy with its stability.

@jessebraham
Copy link
Member

Awesome, thank you for implementing this! I don't have a Pi handy right now but I have asked @JurajSadel to test this out on some hardware. I will review the code shortly!

@JurajSadel
Copy link
Contributor

Hello @bugadani!
I'm trying to test this PR, but I'm not sure about the wiring setup.
The --dtr and --rst are the ESP DTR and RST pins, please?

@bugadani
Copy link
Contributor Author

bugadani commented Sep 21, 2022

@JurajSadel

--dtr and --rts correspond to high-active DTR and RTS pins on external USB - UART converters. They should be fed into a circuit like https://espressif-docs.readthedocs-hosted.com/projects/espressif-esp-iot-solution/en/latest/_images/Auto_program.png to produce EN and IO0 signals for an ESP32.

@bugadani
Copy link
Contributor Author

bugadani commented Sep 21, 2022

Also note that I believe the linked circuit is named using low-active RTS/DTR signals. This means that active-high signals need to be connected "backwards", i.e. the --dtr pin to the nRTS on the circuit and vice versa. I was a bit negligent here because espflash probably shouldn't keep all the possible hardware configurations/naming conventions in mind.

@bugadani
Copy link
Contributor Author

GitHub really should notify authors if their patches no longer merge.... I've squashed and rebased the PR.

Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

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

Thank you for your patience, finally got chance to look through this code today. Overall I think it looks really good, don't have anything noteworthy to say about it. So thanks for making my job easy 😁

I have not tested this on hardware yet, but you've been around these parts long enough that I trust you when you say its working. I'm going to try and wrangle up a Pi this weekend to verify it myself, but if I'm unable to then I will merge this on Monday regardless and we can deal with any issues if they arise.

@jessebraham
Copy link
Member

I ultimately was not able to find my Raspberry Pi this weekend (moved awhile back, it's gotta be in a box somewhere...), so let's just get this merged. I don't plan on releasing for awhile so there will be time to fix issues if they pop up.

Unfortunately just need one more rebase (sorry!) and it should be good to go.

@bugadani
Copy link
Contributor Author

Done - again, github really should ping me when my PR needs a rebase... :(

@jessebraham jessebraham merged commit e4b7be7 into esp-rs:master Sep 26, 2022
@jessebraham
Copy link
Member

Thanks for doing that. I agree, it's kind of annoying having to constantly keep an eye on your PRs like that 😅

@bugadani bugadani deleted the rpi branch September 26, 2022 15:06
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