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 port selection to set_variable and read_variable #259

Merged
merged 1 commit into from
Jan 24, 2023

Conversation

ayushuk
Copy link
Member

@ayushuk ayushuk commented Jan 18, 2023

Summary:

Added optional port argument to the set_variable and read_variable functions.

New usage:
pros v5 set_variable [OPTIONS] VARIABLE VALUE [PORT]
pros v5 read_variable [OPTIONS] VARIABLE [PORT]

Motivation:

As described in #258 this is a quality of life improvement to allow the user to specify a destination port when using set_variable and read_variable. It allows users with multiple brains plugged in to be able to specify the target brain.

References (optional):

Closes #258

Test Plan:

  • run pros v5 set_variable and specify a port with no brain plugged in (the command should error out)
  • run pros v5 set_variable and specify no port with no brain plugged in ("No v5 ports were found!" message should show)
  • run pros v5 set_variable and specify no port with a brain plugged in (command should run normally on the default port)
  • run pros v5 set_variable and specify a port with a brain plugged in (command should run normally on the specified port)
  • run pros v5 read_variable and specify a port with no brain plugged in (the command should error out)
  • run pros v5 read_variable and specify no port with no brain plugged in ("No v5 ports were found!" message should show)
  • run pros v5 read_variable and specify no port with a brain plugged in (command should run normally on the default port)
  • run pros v5 read_variable and specify a port with a brain plugged in (command should run normally on the specified port)

Testing screenshots:
Screenshot 2023-01-18 at 2 34 05 PM
Screenshot 2023-01-18 at 2 34 32 PM
Screenshot 2023-01-18 at 2 36 41 PM
Screenshot 2023-01-18 at 2 36 58 PM

Screenshot 2023-01-19 at 7 32 11 PM

Screenshot 2023-01-19 at 7 32 19 PM

Screenshot 2023-01-19 at 7 31 03 PM

Screenshot 2023-01-19 at 7 31 26 PM

@ayushuk
Copy link
Member Author

ayushuk commented Jan 18, 2023

Needs hardware testing

@ayushuk
Copy link
Member Author

ayushuk commented Jan 18, 2023

Also was thinking we should probably clean up the error message for specifying a port that doesn't exist. Its the same error when specifying a port for other v5 functions such as pros v5 stop or pros v5 status. Thoughts? @omegaStag @kunwarsahni01

@omegaStag
Copy link
Contributor

Also was thinking we should probably clean up the error message for specifying a port that doesn't exist. Its the same error when specifying a port for other v5 functions such as pros v5 stop or pros v5 status. Thoughts? @omegaStag @kunwarsahni01

yeah that would be best

@ayushuk ayushuk changed the title Add port selection to set_variable and read_variable ✨ Add port selection to set_variable and read_variable Jan 24, 2023
@ayushuk ayushuk merged commit 0caacd1 into develop Jan 24, 2023
@ayushuk ayushuk deleted the bugfix/port-selection-set-read-variable branch January 24, 2023 23:12
@kunwarsahni01 kunwarsahni01 mentioned this pull request Apr 18, 2023
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.

🐛set_variable and read_variable don't have the "port" argument
2 participants