opensim-org / opensim-core

SimTK OpenSim C++ libraries and command-line applications, and Java/Python wrapping.
https://opensim.stanford.edu
Apache License 2.0
783 stars 316 forks source link

Fix types for methods in BufferedOrientationReference #3644

Closed RTnhN closed 9 months ago

RTnhN commented 9 months ago

Fixes issue #3415

Brief summary of changes

The interface to BufferedOrientationReferene is defined here:

https://github.com/opensim-org/opensim-core/blob/2aa34e167990226142b3765f3e188c3f332f93c1/Bindings/simulation.i#L242-L242

In BufferedOrientationReference.h, there were missing _<double> type annotations for SimTK::RowVector_<SimTK::Rotation>. I think that when swig pulled in the header file for defining the interface, it was not sure how to handle some methods including the putValues since that did not have the _<double> type on it. Ref: 26e9f22

Also, I noticed that the python opensense testing file did not include any tests for the initialization of the BufferedOrienationReference, so I added some tests for that. To make sure that the ikSolver initializes well with the BufferedOrienationReference, I also added a test using it to initialize it.

Testing I've completed

I have built with these changes locally using the linux build script, and all tests have passed.

I have made minor changes to Patrick's code to incorporate the new BufferedOrientationReference, and it runs with no problems.

Looking for feedback on...

CHANGELOG.md


This change is Reviewable

aymanhab commented 9 months ago

Thanks for the nice contribution @RTnhN πŸ‘ I will review the change and test case and get back to you shortly.

aymanhab commented 9 months ago

Perfect, thank you very much, will merge once all tests pass. We'll need to update the repo by Patrick as well. I'd suggest you either open a PR there or open an issue and include your proposed code change so that future users of OpenSenseRT don't need to start from scratch. Thanks again πŸ’―

aymanhab commented 9 months ago

Will merge this PR, thanks again for the contribution, in the meantime please document the changes you made for successful use of OpenSenseRT on an issue/PR in https://github.com/pslade2/RealTimeKin to help future fellow users.

RTnhN commented 9 months ago

Will merge this PR, thanks again for the contribution, in the meantime please document the changes you made for successful use of OpenSenseRT on an issue/PR in https://github.com/pslade2/RealTimeKin to help future fellow users.

Awesome, thanks! I'll document my changes to that OpenSenseRT repo.