-
Notifications
You must be signed in to change notification settings - Fork 22
Allow the user to change the target file within the --stay-connected menu #123
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
Conversation
|
Both the limitations I mentioned should be fixed in b735de6. Any invalid file paths will now have an error message printed to the console, and the user will be re-prompted with the main menu. |
dlech
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a useful feature. I just have some comments on how we could make it even better.
|
I think I got most of the changes you suggested. I wasn't entirely sure if my enum implementation was what you meant for me to do, so let me know if you want me to change it. I also did the internal ordering that you wanted, but I feel like putting the "change target file" as the top option internally was a bit cleaner because I didn't have to call _get_script_path() in every single option. |
|
It looks like the linting workflow doesn't allow python features that were added after 3.8? Edit: I found the flag in pyproject.toml, but it looks like changing the version makes it think that a bunch of other stuff is incorrect? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks better now, thanks for updating. The match statement with the enum really helped make it easier to read.
I pushed some changes to the master branch to update black for Python 3.10, so if we rebase this branch (or merge the master branch), the linter should be happy now.
|
OK, I tested it now. It even has automatic completion on the file name, which is nice! One thing I found missing though was a way to start the program without sending it again. But I consider that a separate feature request. So I'll go ahead and merge this one. |
I didn't really see a point in doing this because you can just run the program directly from the hub in this case, but I can look at adding it if you make a feature request. |
Not if the hub is far away and you can't reach it. 😄 |
One of the main complaints I have gotten from the team I help coach is that they have to exit the menu, wait for the hub to disconnect, and then re-connect with the separate file.
I have added the ability to change the file within the menu, and I have also added the name of the current file at the top to avoid confusion.
known limitationsIf you change the path to '-', the program will error out.You must re-send the file to the robot after changing it.Fixed in b735de6