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

Only send motor commands if pid is enabled #222

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

garyservin
Copy link
Contributor

🦟 Bug fix

Fixes #184

Summary

Because

  1. the code updates the motor speed on every PID cycle here
  2. the code sets the speeds to 0 as per this change
  3. the code disables the pid when we use the o command as per here,

when we send an o command, the next time the PID loop runs, we set the speed to 0, which doesn't give the motors time to actually spin.

This PR fixes this by actually checking if the PID controllers are enabled before setting the motor speeds. It also changes the PID enable(bool) to enable() and disable() instead.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if it affects the public API)

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@garyservin garyservin force-pushed the fix-pid-enabled branch 2 times, most recently from 5c4145a to 7952ecf Compare February 12, 2024 00:58
Copy link
Member

@jballoffet jballoffet left a comment

Choose a reason for hiding this comment

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

Thanks @garyservin for contributing to Andino! Changes LGTM.

@jballoffet jballoffet merged commit 63320e0 into Ekumen-OS:humble Feb 12, 2024
4 checks passed
@garyservin garyservin deleted the fix-pid-enabled branch February 12, 2024 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Arduino Firmware - Command "o" not working
2 participants