Added MecanumDriveOdometry Python wrapper #549

ahcorde · 2023-08-23T10:55:32Z

🎉 New feature

Closes #548

Summary

Added MecanumDriveOdometry

Checklist

Signed all commits for DCO
Added tests
Added example and/or tutorial
Updated documentation (as needed)
Updated migration guide (as needed)
Consider updating Python bindings (if the library has them)
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: Alejandro Hernández Cordero <[email protected]>

codecov · 2023-08-23T11:25:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c5c3fc4) 99.38% compared to head (811947f) 99.38%.

Additional details and impacted files

@@            Coverage Diff             @@
##           ign-math6     #549   +/-   ##
==========================================
  Coverage      99.38%   99.38%           
==========================================
  Files             75       75           
  Lines           7029     7029           
==========================================
  Hits            6986     6986           
  Misses            43       43

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

azeey · 2023-08-23T14:28:14Z

Is it okay if we wait till after Harmonic for this?

ahcorde · 2023-08-23T14:35:31Z

this is targeting Fortress, but we can wait no problem

azeey · 2023-08-23T15:49:57Z

this is targeting Fortress, but we can wait no problem

I understand, but I think we should focus our review efforts on beta labeled PRs.

ahcorde · 2023-10-05T16:47:23Z

friendly ping @azeey @scpeters or @adityapande-1995

azeey · 2023-10-06T19:04:14Z

src/python_pybind11/src/MecanumDriveOdometry.cc

+  std::string pyclass_name = typestr;
+  py::class_<Class>(m,
+                    pyclass_name.c_str(),
+                    py::buffer_protocol(),


Do we need py::buffer_protocol here? If so, doesn't it require implementing def_buffer (https://pybind11.readthedocs.io/en/stable/advanced/pycpp/numpy.html)

I don't think this has been addressed

src/python_pybind11/src/MecanumDriveOdometry.cc

azeey · 2023-10-06T19:10:25Z

src/python_pybind11/test/MecanumDriveOdometry_TEST.py

+        # Setup the wheel parameters, and initialize
+        odom.set_wheel_params(wheelSeparation, wheelRadius, wheelRadius,wheelRadius)
+        startTime = datetime.datetime.now()
+        odom.init(datetime.timedelta())


Should this use startTime?

azeey · 2023-10-06T19:11:10Z

src/python_pybind11/test/MecanumDriveOdometry_TEST.py

+                    Angle(1.0 * math.pi / 180),
+                    Angle(1.0 * math.pi / 180),
+                    Angle(1.0 * math.pi / 180),
+                    time1 - startTime)


The C++ version uses just time1 here.

azeey · 2023-10-06T19:15:59Z

src/python_pybind11/test/MecanumDriveOdometry_TEST.py

+                    Angle(2.0 * math.pi / 180),
+                    Angle(2.0 * math.pi / 180),
+                    Angle(2.0 * math.pi / 180),
+                    time2 - startTime)


This is also different from the C++ test

src/python_pybind11/test/MecanumDriveOdometry_TEST.py

azeey

Looks good overall. Just a few issues with pybind11 options and differences with the C++ test

azeey

Looks good overall. Just a few issues with pybind11 options and differences with the C++ test

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

…mDriveOdometry_pytohn

ahcorde · 2023-10-16T13:02:30Z

@azeey Time is a little bit different in Python, you can take a look the DiffDriveOdometry_TEST.py test,

ahcorde · 2023-11-14T08:49:11Z

friendly ping @azeey

ahcorde · 2023-11-27T15:06:20Z

friendly ping @azeey

azeey · 2023-11-27T20:47:43Z

@azeey Time is a little bit different in Python, you can take a look the DiffDriveOdometry_TEST.py test,

So from the pybind11 docs, it looks like std::chrono::steady_clock::time_point is converted to datetime.timedelta. This makes it really inconvenient to use since datetime.timedelta() always returns a "0" time delta as opposed to the current monotonic (in C++ steady_clock) time. One option is to use datetime.timedelta(time.monotonic()) to initialize startTime instead of datetime.datetime.now().

j-rivero · 2023-12-07T11:59:14Z

@osrf-jenkins run tests

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

ahcorde · 2023-12-14T12:44:33Z

@azeey I added your feedback

azeey · 2023-12-14T15:27:47Z

@azeey I added your feedback

Thanks. I think now the test can be updated to match the C++ version. Specifically, odom.init(startTime) instead of odom.init(datetime.timedelta()) and remove startTime from odom.update calls.

…mDriveOdometry_pytohn

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

ahcorde · 2024-02-13T10:19:31Z

Thanks. I think now the test can be updated to match the C++ version. Specifically, odom.init(startTime) instead of odom.init(datetime.timedelta()) and remove startTime from odom.update calls.

@azeey Included feedback

src/python_pybind11/test/DiffDriveOdometry_TEST.py

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

Added MecanumDriveOdometry

0eb2d28

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

ahcorde self-assigned this Aug 23, 2023

ahcorde requested review from scpeters and adityapande-1995 as code owners August 23, 2023 10:55

ahcorde mentioned this pull request Aug 23, 2023

Add MecanumDriveOdometry Python wrapper #548

Open

github-actions bot added 🏯 fortress Ignition Fortress 🏰 citadel Ignition Citadel labels Aug 23, 2023

Merge branch 'ign-math6' into ahcorde/6/mecanumDriveOdometry_pytohn

9e8a054

azeey requested changes Oct 6, 2023

View reviewed changes

ahcorde added 2 commits October 16, 2023 15:01

Feedback

d1c70f2

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

Merge remote-tracking branch 'origin/ign-math6' into ahcorde/6/mecanu…

f23425e

…mDriveOdometry_pytohn

ahcorde requested a review from azeey October 16, 2023 13:02

Merge branch 'ign-math6' into ahcorde/6/mecanumDriveOdometry_pytohn

2e8d571

ahcorde changed the title ~~Added MecanumDriveOdometry~~ Added MecanumDriveOdometry Python wrapper Nov 14, 2023

ahcorde added 2 commits December 14, 2023 13:43

Added feedback

fc1b7d2

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

Added feedback

88ab851

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

ahcorde added 2 commits February 13, 2024 10:57

Merge remote-tracking branch 'origin/ign-math6' into ahcorde/6/mecanu…

baa2611

…mDriveOdometry_pytohn

Added feedback

e0aaab1

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

azeey reviewed Feb 13, 2024

View reviewed changes

src/python_pybind11/test/DiffDriveOdometry_TEST.py Outdated Show resolved Hide resolved

src/python_pybind11/test/DiffDriveOdometry_TEST.py Outdated Show resolved Hide resolved

src/python_pybind11/test/DiffDriveOdometry_TEST.py Show resolved Hide resolved

Diffdrive test

811947f

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

ahcorde requested a review from azeey February 13, 2024 21:46

azeey approved these changes Feb 13, 2024

View reviewed changes

ahcorde enabled auto-merge (squash) February 13, 2024 21:57

ahcorde merged commit 5b2522a into ign-math6 Feb 13, 2024
11 checks passed

ahcorde deleted the ahcorde/6/mecanumDriveOdometry_pytohn branch February 13, 2024 22:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added MecanumDriveOdometry Python wrapper #549

Added MecanumDriveOdometry Python wrapper #549

ahcorde commented Aug 23, 2023

codecov bot commented Aug 23, 2023 •

edited

Loading

azeey commented Aug 23, 2023

ahcorde commented Aug 23, 2023

azeey commented Aug 23, 2023

ahcorde commented Oct 5, 2023

azeey Oct 6, 2023

azeey Nov 27, 2023

azeey Oct 6, 2023

azeey Oct 6, 2023

azeey Oct 6, 2023

azeey left a comment

azeey left a comment

ahcorde commented Oct 16, 2023

ahcorde commented Nov 14, 2023

ahcorde commented Nov 27, 2023

azeey commented Nov 27, 2023

j-rivero commented Dec 7, 2023

ahcorde commented Dec 14, 2023

azeey commented Dec 14, 2023

ahcorde commented Feb 13, 2024

Added MecanumDriveOdometry Python wrapper #549

Added MecanumDriveOdometry Python wrapper #549

Conversation

ahcorde commented Aug 23, 2023

🎉 New feature

Summary

Checklist

codecov bot commented Aug 23, 2023 • edited Loading

Codecov Report

azeey commented Aug 23, 2023

ahcorde commented Aug 23, 2023

azeey commented Aug 23, 2023

ahcorde commented Oct 5, 2023

azeey Oct 6, 2023

Choose a reason for hiding this comment

azeey Nov 27, 2023

Choose a reason for hiding this comment

azeey Oct 6, 2023

Choose a reason for hiding this comment

azeey Oct 6, 2023

Choose a reason for hiding this comment

azeey Oct 6, 2023

Choose a reason for hiding this comment

azeey left a comment

Choose a reason for hiding this comment

azeey left a comment

Choose a reason for hiding this comment

ahcorde commented Oct 16, 2023

ahcorde commented Nov 14, 2023

ahcorde commented Nov 27, 2023

azeey commented Nov 27, 2023

j-rivero commented Dec 7, 2023

ahcorde commented Dec 14, 2023

azeey commented Dec 14, 2023

ahcorde commented Feb 13, 2024

codecov bot commented Aug 23, 2023 •

edited

Loading