-
Notifications
You must be signed in to change notification settings - Fork 37
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 Trajectory class #164
Add Trajectory class #164
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.
Giving me work! Also check out that sweet, sweet cached docker build.
Took a quick skim and added some small thoughts. Nothing too important. I haven't checked anything locally... yet. This will encourage me to finally setup my new computer.
:type yaw_pts: :class:`numpy.array` | ||
""" | ||
num_pts = len(t_pts) | ||
if len(x_pts) != num_pts or len(y_pts) != num_pts or len(yaw_pts) != num_pts: |
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.
I think this is six of one, half dozen of the other, but if you stored a list of something more akin to StampedPose
(ugh) it would simplify the code slightly. You could just add a time value to Pose
and then only use it in trajectories. Then I think the only place that you would need to do conversions to/from np arrays is in interpolate, and you could delete a bunch of these kinds of statements and those on line 37. It's still the same amount of converting back and forth though...
Though idk how you feel about adding a time value to Pose
. Also this is just fine so might not be worth changing.
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.
this seems to work well, actually. I might have been overthinking how bad that was gonna be. Thanks!
As a huge bonus, storing as poses lets me do the interpolation for a generic SO(3) trajectory -- not that pyrobosim is using that right now, but if it ever does we're all set here 💪🏻
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.
Adds a new
Trajectory
class that replaces the(t_pts, x_pts, y_pts, yaw_pts)
tuple that was being used before.Closes #135.