Skip to content

Added PID pybind11 interface#323

Merged
chapulina merged 7 commits intoign-math6from
ahcorde/pybind11/pid
Dec 29, 2021
Merged

Added PID pybind11 interface#323
chapulina merged 7 commits intoign-math6from
ahcorde/pybind11/pid

Conversation

@ahcorde
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde commented Dec 21, 2021

Signed-off-by: ahcorde ahcorde@gmail.com

🎉 New feature

Summary

This PR creates a PID Pybind11 interface

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

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.

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from scpeters December 21, 2021 12:35
@ahcorde ahcorde self-assigned this Dec 21, 2021
@github-actions github-actions bot added Gazebo 1️1️ Dependency of Gazebo classic version 11 🌱 garden Ignition Garden 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome labels Dec 21, 2021
Signed-off-by: ahcorde <ahcorde@gmail.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 21, 2021

Codecov Report

Merging #323 (3d8c170) into ign-math6 (b399035) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           ign-math6     #323   +/-   ##
==========================================
  Coverage      99.65%   99.65%           
==========================================
  Files             67       67           
  Lines           6359     6380   +21     
==========================================
+ Hits            6337     6358   +21     
  Misses            22       22           
Impacted Files Coverage Δ
include/ignition/math/AxisAlignedBox.hh 100.00% <ø> (ø)
include/ignition/math/Temperature.hh 100.00% <ø> (ø)
src/AxisAlignedBox.cc 100.00% <100.00%> (ø)
src/Color.cc 98.58% <100.00%> (+<0.01%) ⬆️
src/Temperature.cc 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85224e3...3d8c170. Read the comment docs.

@chapulina chapulina added the scripting Scripting interfaces to Ignition label Dec 21, 2021
Copy link
Copy Markdown
Contributor

@chapulina chapulina left a 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)
Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor

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++

self.update_test(pid, 0, -1, -1, -1, 0, 0)
self.update_test(pid, 0, 1, -1, 1, 0, -2)
Copy link
Copy Markdown
Contributor

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++

@chapulina chapulina merged commit 3d1d076 into ign-math6 Dec 29, 2021
@chapulina chapulina deleted the ahcorde/pybind11/pid branch December 29, 2021 00:47
@osrf-triage
Copy link
Copy Markdown

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏰 citadel Ignition Citadel 🔮 dome Ignition Dome 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress 🌱 garden Ignition Garden Gazebo 1️1️ Dependency of Gazebo classic version 11 scripting Scripting interfaces to Ignition

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants