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

Add StateSaver class. #184

Merged
merged 9 commits into from
Apr 23, 2017
Merged

Add StateSaver class. #184

merged 9 commits into from
Apr 23, 2017

Conversation

brianhou
Copy link
Contributor

RAII class suggested by @mkoval in personalrobotics/libherb#26.

@brianhou brianhou requested a review from mkoval April 21, 2017 21:29
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 80.35% when pulling 2068061 on enhancement/brianhou/state-saver into 2af5c55 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 80.35% when pulling 2068061 on enhancement/brianhou/state-saver into 2af5c55 on master.

@jslee02 jslee02 added this to the Aikido 0.1.0 milestone Apr 22, 2017
Copy link
Member

@mkoval mkoval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. Nice work @gilwoolee.

I filed #185 as a follow-up issue to revisit the implementation once we understand what other properties need to be preserved.

We also need to decide how to handle topological changes in a MetaSkeleton (e.g. change in the number of DegreeOfFreedoms) that make this class semantically meaningless. That is a broader issue in DART, so let's ignore it for now.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 79.153% when pulling c6d21d7 on enhancement/brianhou/state-saver into 5d19317 on master.


MetaSkeletonStateSpaceSaver::~MetaSkeletonStateSpaceSaver()
{
mSpace->getMetaSkeleton()->setPositions(mPositions);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: It would be nice if we have a simple validity check whether the size of mPositions is still equal to the dofs of the MetaSkeleton (if not, we print a warning). I understand it wouldn't be able to catch other structural changes such as changing the joint type with the same dofs, but at least we could catch dimension changing cases.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.813% when pulling e5e1b8d on enhancement/brianhou/state-saver into 4c08ac5 on master.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 412feea on enhancement/brianhou/state-saver into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 412feea on enhancement/brianhou/state-saver into ** on master**.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.813% when pulling 412feea on enhancement/brianhou/state-saver into a806042 on master.

@mkoval mkoval merged commit 966de76 into master Apr 23, 2017
@mkoval mkoval deleted the enhancement/brianhou/state-saver branch April 23, 2017 02:20
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.

4 participants