utiasASRL / steam

The Simultaneous Trajectory Estimation and Mapping (STEAM) Engine.
BSD 3-Clause "New" or "Revised" License
94 stars 14 forks source link

Feature/velocity interpolation #46

Closed mikew99 closed 6 years ago

mikew99 commented 6 years ago

Added interpolation of velocity from trajectory

dyoon823 commented 6 years ago

If I'm understanding this correctly, because SteamTrajPoseInterpEval::MakeShared() creates a new instance of SteamTrajPoseInterpEval, calling getInterpTrajVal() once creates 2 instances of SteamTrajPoseInterpEval which we probably shouldn't do.

Do you need the interpolated velocity to be of a state type? Or is getting the values as a vector (ie Eigen::Matrix<double,6,1>) enough? I think it'll be simpler if getInterpTrajVal() is removed and we work off of getInterpPoseEval().

mikew99 commented 6 years ago

I swung both ways on that, trying to find the cleanest solution. The problem with removing getInterpTrajVal() and working off getInterpPoseEval() is that it only returns a TransformEvaluator::ConstPtr, so we could try upcasting the returned pointer to a SteamTrajPoseInterpEval to be able to call a getVelocity() function, but that seems pretty ugly for an end user. I could change the return type of getInterpPoseEval() but that really breaks the desire to return a more generic TransformEvaluator, and lots of downstream issues for vtr2 code that calls it.

Yeah, there is no real necessity to return a velocity as a state, I just thought it was a little cleaner as SteamTrajVar. I can change it to a vector.

dyoon823 commented 6 years ago

Yeah it's definitely not an ideal situation because SteamTrajPoseInterpEval inherits from TransformEvaluator. Don't think it was meant to do anything velocity related. Perhaps we should just add a function to SteamTrajInterface to return the interpolated velocity, without using SteamTrajPoseInterpEval. In the future, it may be useful to actually implement an evaluator-type class for velocity. I'd like to talk to you about this in person, maybe during or after the Thursday meeting.

mikew99 commented 6 years ago

Updated based on our conversation @dyoon823 . Much cleaner.