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

Fix types for methods in BufferedOrientationReference #3644

Merged
merged 4 commits into from
Dec 12, 2023

Conversation

RTnhN
Copy link
Contributor

@RTnhN RTnhN commented Dec 11, 2023

Fixes issue #3415

Brief summary of changes

The interface to BufferedOrientationReferene is defined here:

%include <OpenSim/Simulation/BufferedOrientationsReference.h>

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

  • Updated

This change is Reviewable

@aymanhab
Copy link
Member

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

@aymanhab
Copy link
Member

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
Copy link
Member

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.

@aymanhab aymanhab merged commit e565abf into opensim-org:main Dec 12, 2023
6 checks passed
@RTnhN
Copy link
Contributor Author

RTnhN commented Dec 12, 2023

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.

aymanhab pushed a commit that referenced this pull request Jan 10, 2024
* Add missing types

* Add tests to check BufferedOrientationReference

* Update CHANGELOG

* Add test case for appending data to oRefs
aymanhab added a commit that referenced this pull request Jan 10, 2024
* Add missing types

* Add tests to check BufferedOrientationReference

* Update CHANGELOG

* Add test case for appending data to oRefs

Co-authored-by: Zach Strout <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants