Allow move_indefinitely to move in the 'negative' direction#446
Allow move_indefinitely to move in the 'negative' direction#446Cbantz wants to merge 6 commits intoinstrumentkit:mainfrom
Conversation
Negative velocities would throw a MAXIMUM VELOCITY EXCEEDED error, and move_indefinitely only sent the "MV" command. This meant there was no was to move_indefinitely in the 'negative' direction. Now, when setting velocity, a direction will be set as well, allowing for indefinite movement in either direction using the "MV+" or "MV-" commands. This is the only area that direction currently applies.
Used _direction when changing and returning value to fix recursion error.
Commented out previous 'direction' code and simply added an argument to "move_indefinitely()" to specify direction. This will allow velocity to be adjusted during a move without problems of changing the sign.
Corrected the syntax for checking if direction is '+' or '-'
for more information, see https://pre-commit.ci
|
Looks pretty good, might be nice to add put make the respective unit test a parameterized one to check for positive and negative direction (test here) A more general note and something that is surely outside of this PR but we should keep in mind: |
|
|
||
| @velocity.setter | ||
| def velocity(self, velocity): | ||
| velocity = abs(velocity) |
| def move_indefinitely(self, direction: str = "+"): | ||
| """ | ||
| Move until told to stop | ||
| Move until told to stop in the direction ("+" or "-") passed. |
There was a problem hiding this comment.
I'm not a fan of magic strings as arguments. I would rather see an enum, or just forward/reverse booleans.
This change allows the NewportESP301 to take a direction argument when calling the 'move_indefinitely' function. Currently, this function only allows movement in the 'positive' direction. By adding an optional 'direction' argument, the function can be used in either direction. This update should not affect any existing code, as the default if no argument is passed will still be positive movement.