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

remove 'all' commands #5

Open
danielasun opened this issue Feb 11, 2018 · 2 comments
Open

remove 'all' commands #5

danielasun opened this issue Feb 11, 2018 · 2 comments
Labels
enhancement New feature or request

Comments

@danielasun
Copy link
Collaborator

Just realized that we could potentially cut down on the function length by changing the id parameter to be an optional keyword argument and have the function figure things out automatically.

It's an option, not necessarily a good one.

@danielasun danielasun added the enhancement New feature or request label Apr 3, 2018
@TMengi
Copy link
Contributor

TMengi commented Apr 17, 2018

Do you want me to go through with this one?

Here's how I would try to handle it:

  1. Find and replace every call to an 'all' function with a call to the regular function
  2. Make ids into an optional argument. Default value is None, and triggers an assert that length of commands equals length of motor ids.
  3. Write a regular expression to fix driver code (e.g. replace "set_command_position([1,2,3,4], [pi,pi,pi,pi])" with "set_command_position(angles=[pi,pi,pi,pi], ids=[1,2,3,4])").
  4. Test, delete 'all' functions, test some more.

@danielasun
Copy link
Collaborator Author

let's hold off on this one for now. I want to make sure that we successfully finish some of the other pull requests first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants