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

If exactly one port matches, use it #374

Merged
merged 2 commits into from
Apr 4, 2023
Merged

Conversation

jneem
Copy link
Contributor

@jneem jneem commented Mar 25, 2023

Fixes #367.

Since I added the matches closure, it felt like some clean-up was appropriate. But if you'd prefer a more minimal PR I can do that too.

This also slightly changes the behavior of non-usb serial ports, for consistency. Previously, you could make them match by manually editing the config file and adding "vid=0, pid=0", but they wouldn't be sorted and highlighted as though they matched. After this change, non-usb serial ports won't auto-match at all.

@jessebraham
Copy link
Member

Thanks for this PR, glad to see you've cleaned things up a bit as well!

These changes look good to me, no issues testing on my local system. Unfortunately I've merged another PR updating some documentation and there's a small merge conflict now, sorry about that. If you don't mind giving this a rebase then I'm happy to merge it!

@jneem
Copy link
Contributor Author

jneem commented Mar 31, 2023

Hm, I rebased but now when I test it I get an error when parsing my ~/.config/espflash/espflash.toml:

Error:
  × TOML parse error at line 4, column 7
  │   |
  │ 4 | vid = "303a"
  │   |       ^^^^^^
  │ invalid type: string "303a", expected a borrowed byte array
  │

Was this an intentional change?

@SergioGasquez
Copy link
Member

Was this an intentional change?

No, probably an error introduced when bumping the toml version in: #378, more specifically Id say this changes: https://github.com/esp-rs/espflash/pull/378/files#diff-16aedab0b163d4e6c5350f585959afd77901030799b31de2a9c3c47e64b9c253

Co-authored-by: Sergio Gasquez Arcos <sergio.gasquez@gmail.com>
Copy link
Member

@SergioGasquez SergioGasquez 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 contribution! This is a really cool thing to have!

@SergioGasquez SergioGasquez merged commit cdddf53 into esp-rs:main Apr 4, 2023
@kyrias
Copy link
Contributor

kyrias commented Jul 31, 2023

It would be nice if we could disable this behavior and always be prompted to select one.

Right now I commonly have both a ESP-C3-DevKitM-1 and an ESP-Prog connected and while the devkit is one of the "known" devices the ESP-Prog is not, which means that while previously I was always asked which one I wanted to flash to, now it always tries to flash the ESP-C3.

@jneem
Copy link
Contributor Author

jneem commented Jul 31, 2023

I'm not opposed to adding a flag, but is there some reason you can't just add the not-yet-known device to ~/.config/espflash/espflash.toml so that it becomes known?

@kyrias
Copy link
Contributor

kyrias commented Aug 1, 2023

For this specific case I don't have a problem with adding the ESP-Prog myself, but I personally don't ever want it to automatically pick the serial port to use as I don't trust it to necessarily pick the correct one, and I don't trust the other side of various serial ports to handle the wrong one picked well.

@kyrias
Copy link
Contributor

kyrias commented Aug 4, 2023

As an example of why I don't want it to ever decide on its own, earlier today espflash ended up flashing an ELF file for an ESP32-S3 board onto an ESP32-C3 because I forgot to enable one of the ports on my USB hub.

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.

option to auto-choose serial port if only one matches a known USB device
4 participants