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

Only call generator if motion model history empty #181

Conversation

efernandez
Copy link
Collaborator

@efernandez efernandez commented Apr 22, 2020

Changelog

This simplifies the changes from #154 by doing:

  • Only call generator if motion model history empty.
  • Assert beginning stamp < ending stamp. Or beginning stamp == ending stamp for the special case when the state history is empty.
  • Handle the dt == 0 special case in motion model, so no variables or constraints are generator, and no prediction is performed, or the same state is inserted twice into the history.
  • Revert the changes to the test_timestamp_manager.cpp unit test
  • Add a new test for the case when the transaction has a single involved stamp, since that was precisely the case not handled. Indeed, that test fails w/o Fix Unicycle2DIgnition set_pose #154 or this PR.
  • [minor] Declare state2 closer to where it is used.

The special case mentioned above is always the same.

From my tests locally, with a bunch of prints, I confirmed the state history already have the state for the given stamp, so IIRC that's all that was required for #154 . The assert added in this PR enforces the generator function isn't called for those cases it did before, so I think that constitutes a test for this change.

@efernandez
Copy link
Collaborator Author

@svwilliams @ayrton04 for review

git co 862a2d3 test_timestamp_manager.cpp

In order to revert "Fix Unicycle2DIgnition set_pose (locusrobotics#154)" only on
test_timestamp_manager.cpp
Copy link
Contributor

@svwilliams svwilliams left a comment

Choose a reason for hiding this comment

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

I like this better than always adding a dummy entry at the end. Thanks.

fuse_core/test/test_timestamp_manager.cpp Outdated Show resolved Hide resolved
@efernandez
Copy link
Collaborator Author

I like this better than always adding a dummy entry at the end. Thanks.

And the unit test changes I did in #154 aren't needed any more. Indeed, I reverted all those changes and added a new test. 😃

@efernandez
Copy link
Collaborator Author

FYI Note that it's important to get this PR in now that #180 has been merged, because you'll see ROS messages complaining of zero process noise covariance matrices. They'll be spamming at 0.1Hz, because they're throttled, but that could still be a lot of spam.

That's precisely the case of dt == 0 that is handled as special case in this PR, that also makes dt == 0 happen once per set pose or startup, as opposed to almost constantly.

@svwilliams svwilliams merged commit ec5e20e into locusrobotics:devel Apr 24, 2020
@efernandez efernandez deleted the only_call_generator_if_motion_model_history_empty branch April 24, 2020 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants