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

Motor #29

Merged
merged 6 commits into from
May 14, 2024
Merged

Motor #29

merged 6 commits into from
May 14, 2024

Conversation

njooma
Copy link
Member

@njooma njooma commented May 13, 2024

Add motor wrappers and tests

@njooma njooma marked this pull request as ready for review May 13, 2024 22:43
@njooma njooma requested a review from a team as a code owner May 13, 2024 22:43
@njooma njooma requested review from stuqdog and purplenicole730 and removed request for a team May 13, 2024 22:43
Copy link
Member

@stuqdog stuqdog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor question/clarification on a comment otherwise LGTM!

Comment on lines 84 to 86
* Regardless of the directionality of the [rpm] this function will move
* the motor towards the specified position.
* @param rpm speed at which the motor should move in rotations per minute (negative implies backwards)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I misunderstand what we mean by "directionality", but it seems to me that we're first saying that a negative RPM won't affect the direction, but then the param docstring states the opposite.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope you are correct I fixed the javadoc prior to merge

@njooma njooma merged commit 5fb1010 into viamrobotics:main May 14, 2024
2 checks passed
@njooma njooma deleted the motor branch May 14, 2024 15:52
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.

2 participants