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

Suggestion: add jerk to CartesianTrajectoryPoint #3

Closed
gavanderhoorn opened this issue Apr 23, 2020 · 9 comments
Closed

Suggestion: add jerk to CartesianTrajectoryPoint #3

gavanderhoorn opened this issue Apr 23, 2020 · 9 comments
Labels
discussion This issue contains something where a consensus needs to be found.

Comments

@gavanderhoorn
Copy link
Contributor

In cases where this message would be used to communicate Cartesian trajectories to (interpolating) trajectory generators, being able to specify a max or desired jerk per trajectory point would be advantageous.

The current proposal doesn't include it, so the suggestion would be to add it -- similar to how the reviewed johnmichaloski example has it.

As there is no geometry_msgs/Jerk, it would probably require a custom message. Although it would essentially be a copy of geometry_msgs/Accel and geometry_msgs/Twist but with different semantics.

@fmauch
Copy link
Contributor

fmauch commented Apr 27, 2020

Not necessarily a counter-argument, but doesn't a jerk also make sense in the joint-based case? As the JointTrajectoryPoint does not contain one, my first attempt was to omit it here, as well.

However, I absolutely understadn that this is a point worth discussing.

@fmauch fmauch added the discussion This issue contains something where a consensus needs to be found. label Apr 27, 2020
@gavanderhoorn
Copy link
Contributor Author

I'm sort of assuming JointTrajectoryPoint doesn't have it as the hw / electronics which were part of the PR2 did not need it. Perhaps @codebot could confirm this.

I'd also say that JointTrajectoryPoint (and JointTrajectory) are not the end-all-be-all of generic messages, so we shouldn't necessarily take them as perfect examples.

@codebot
Copy link

codebot commented Apr 28, 2020

Interesting. I'm not familiar with those details of PR2, suggest asking @tfoote ?

I'm a big fan of iteration. Things can always be improved!

@gavanderhoorn
Copy link
Contributor Author

@codebot: apologies for dragging you into this discussion then. I was somehow sure you were involved in the hw/electronics design of PR2 as well.

@codebot
Copy link

codebot commented Apr 28, 2020

haha all good! I wish I was involved 😄 the PR2 electronics and low-level controls are seriously awesome!

@BrettHemes
Copy link

The current proposal doesn't include it, so the suggestion would be to add it

I second this.

@fmauch
Copy link
Contributor

fmauch commented May 26, 2020

So, if we add a jerk, what should be the datatype?

It feels like having a jerk on a per-dimension level would make sense as we have this for all other fields, as well.

Additionally, this would only make sense if there would be an underlying controller taking this into account, as well.

@gavanderhoorn
Copy link
Contributor Author

So, if we add a jerk, what should be the datatype?

It feels like having a jerk on a per-dimension level would make sense as we have this for all other fields, as well.

My suggestion (from #3 (comment)):

As there is no geometry_msgs/Jerk, it would probably require a custom message. Although it would essentially be a copy of geometry_msgs/Accel and geometry_msgs/Twist but with different semantics.

We could host that message wherever the new Cartesian msgs end up, for now. Ideally we'd revive ros/common_msgs#137 and get it merged there, but that could take a while.

Additionally, this would only make sense if there would be an underlying controller taking this into account, as well.

Well, I'd say that would result in the best execution of the trajectory. JointTrajectoryPoint includes acceleration limits, but a sufficient number of the consumers of that message don't take those into account either.

That would be OK, as long as the components responsible for execution are clear about this.

@fmauch
Copy link
Contributor

fmauch commented Sep 16, 2020

Defined this in #11 the datatype is noted as to be determined.

@fmauch fmauch closed this as completed Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This issue contains something where a consensus needs to be found.
Projects
None yet
Development

No branches or pull requests

4 participants