-
Notifications
You must be signed in to change notification settings - Fork 767
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
Wrap additional ISAM2 methods #941
Conversation
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.
Confused. Most additions are commented out. Make draft PR until finalized.
Okay removed out the commented stuff. That was my mistake, I added those in as part of another change not related to this PR. Thank you for catching them. 🙂 |
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.
LGTM, but is that the only method you want to add? Are there others public methods that could easily be added in the same PR?
No other methods so far that we need for MHS + State Estimator. If we find something, we can make a PR for then. |
My question was not about MHS, but expanding this PR to include other methods now that are easily wrapped. |
We can. It just depends on the requirements. My comment about MHS is that the current wrapped method is all I need so I didn't spend too much time looking at other methods. |
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.
OK, you're (choosing) to not hear me :-) For this GTSAM PR, please also wrap any other easy wrap-able functions before merging, so GTSAM users have this convenience in the future.
Gotcha |
Cool, thanks !!! |
Wrapping additional methods in ISAM2 needed for legged state estimation.