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

Add "legacy" commands for serial & DFU mode management w/ baud-switcher #16

Merged

Conversation

justicefreed
Copy link
Contributor

Adds appropriate wrapper fns for serial open/close and dfu open/close
as subcommands for the "legacy" command. Also adds docs / help strings.

tested and working on linux

only thing that may need to still be added is an update to "completion.py"
to account for the new fns but it was unclear if the overall format would
need to be changed to support doing so, so this was left off this commit

@justicefreed
Copy link
Contributor Author

I'll note that the behavior I implemented will show an "Invalid Command" error on unsupported systems. Not sure if you have a specific preference on how you want to handle people trying to run the command on a non-linux system where baud-switcher isn't installed. If you'd rather have the commands show as available but show the more-specific error message I can easily make that change.

Also, the way I did the checks, if someone is able to install baud-switcher successfully on a non-linux system it should still allow you to access and run the legacy commands using it.

@nrobinson2000
Copy link
Owner

nrobinson2000 commented Mar 7, 2022

Wow. This is excellent. Thanks for updating the man page and help info as well.

As far as running this on non-Linux systems goes, macOS supports arbitrary baud rates with stty -f last I checked, so that could be a drop-in for BAUD_TOOL.

As you can probably tell, I don't care too much about Windows support, so an error message is fine.

I'll look into adding a completion function for finding /dev/ttyACM* or /dev/tty.usbmodem* on macOS since particle serial list is a little too slow for my liking.

EDIT:
I'll probably just reuse the original getModems function:
https://github.com/nrobinson2000/po/blob/master/completion/po#L16-L30

@justicefreed
Copy link
Contributor Author

Interesting - stty -f /dev/tty... 14400 / etc worked perfectly on M1 mac on Monterrey. Not sure why I thought it didn't. That's fantastic. Instead of trying to do a platform based check I'm thinking it would probably be best to:

  1. if system has stty use it (which would probably include many Linux systems it seems?)
  2. else, if system has baud-switcher use it
  3. else, give a detailed error message when you try to run one of those functions.

This way we can avoid edge cases where the install doesn't work or some future OS update breaks support for one of them. I'm wondering though - do we even need baud-switcher then? Is there a specific reason not to just exclusively use stty? Seems like it might remove the need for a dependancy.

@nrobinson2000
Copy link
Owner

nrobinson2000 commented Mar 8, 2022

Linux has stty but it cannot use 14400 or 28800 as baud rates, hence the reason baud-switcher exists.

I think pyserial can do arbitrary baud rates, but I'm hesitant to use it because I tried to use only the standard library for this implementation of po-util/neopo. In a future refactor I'll probably use pyserial along with other helpful libraries like requests.

@justicefreed
Copy link
Contributor Author

Ahh, gotcha. Makes sense. Is baud-switcher installed during the standard install or during neopo install? It's the normal install right? If so, then I'd say let's just purely select based on platform and not check if it's installed or not.

@nrobinson2000
Copy link
Owner

nrobinson2000 commented Mar 8, 2022

In setup.py, baud-switcher is built. This isn't ideal since this really should be done by a package manager instead of the pip install process.
https://github.com/nrobinson2000/neopo/blob/serial_commands/setup.py#L43

@nrobinson2000
Copy link
Owner

I managed to copy the code from pyserial for switching to the baud rate on Linux and it works great. I think I'll use this instead of baud-switcher moving forward.

If you want to resolve the merge conflicts I'll merge this PR.

Adds appropriate wrapper fns for serial open/close and dfu open/close
as subcommands for the "legacy" command.  Also adds docs / help strings.

tested and working on linux

only thing that may need to still be added is an update to "completion.py"
to account for the new fns but it was unclear if the overall format would
need to be changed to support doing so, so this was left off this commit
Merge replacement of baud-switcher
Add faster serial port search than `particle serial list` but with fallback for non-Unix systems to the original particle command
general cleanup, and pushed error throwing into serial.py
@justicefreed
Copy link
Contributor Author

justicefreed commented Mar 9, 2022

Git stuff got a little out of order with rebasing, but I resolved the merge conflicts as well as implemented the faster serial port lookup. Some general cleanup as well. Tested on osx and linux.

EDIT: also - not sure how you are using the version.py file btw - it updated automatically to a number but not sure if that is intended here or not.

@nrobinson2000 nrobinson2000 merged commit ab58611 into nrobinson2000:serial_commands Mar 10, 2022
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.

None yet

2 participants