-
Notifications
You must be signed in to change notification settings - Fork 70
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
Added PID pybind11 interface #323
Conversation
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-math6 #323 +/- ##
==========================================
Coverage 99.65% 99.65%
==========================================
Files 67 67
Lines 6359 6380 +21
==========================================
+ Hits 6337 6358 +21
Misses 22 22
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question about tests
self.update_test(pid, 0, -1, -1, -1, 0, 0) | ||
self.update_test(pid, 0, 1, -1, 1, 0, -2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these 2 test cases being removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't dig a lot on this, but could be related with the python module datetime timedelta
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some TODOs in 980b23b, we should look into this after the pybind11 migration is over. It's strange that the results are different from SWIG and C++
Signed-off-by: Louise Poubel <[email protected]>
self.update_test(pid, 0, -1, -1, -1, 0, 0) | ||
self.update_test(pid, 0, 1, -1, 1, 0, -2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some TODOs in 980b23b, we should look into this after the pybind11 migration is over. It's strange that the results are different from SWIG and C++
This pull request has been mentioned on Gazebo Community. There might be relevant details there: https://community.gazebosim.org/t/new-ignition-releases-2022-03-01-citadel-edifice-fortress/1313/1 |
Signed-off-by: ahcorde [email protected]
🎉 New feature
Summary
This PR creates a PID Pybind11 interface
Checklist
codecheck
passed (See contributing)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.