Feature/joint wrench msgs #221

my-rice · 2025-06-26T08:21:24Z

This PR introduces joint trajectory messages with task space wrench as described in issue #284. In particular, the following message definitions are added:

JointWrenchTrajectory.msg
JointWrenchTrajectoryPoint.msg

In addition to these messages, the corresponding action definition, FollowJointWrenchTrajectory.action, has also been created.

To avoid repeating code, I reused trajectory_msgs/JointTrajectoryPoint where possible. I am not completely sure if this was the best approach, so I am open to feedback on that.

- [REF] Modified old organization of the message to introduce only two message definitions: JointWrenchTrajectory and JointWrenchTrajectoryPoint - [REF] Adjust CMakeLists.txt to adapt to the new changes

…oryPoint messages

…ontrol

…to feature/joint_wrench_msgs

…intWrenchTrajectory.action

christophfroehlich

Thanks the addition. I have some minor comments

control_msgs/action/FollowJointWrenchTrajectory.action

christophfroehlich · 2025-06-26T08:59:53Z

control_msgs/action/FollowJointWrenchTrajectory.action

+int32 OLD_HEADER_TIMESTAMP = -3
+int32 PATH_TOLERANCE_VIOLATED = -4
+int32 GOAL_TOLERANCE_VIOLATED = -5
+int32 TRAJECTORY_ABORTED = -6


please add a description to TRAJECTORY_ABORTED (this is new compared to FollowTrajectory action)

Should we also add PREEMPTED or it doesn't make sense?

I added TRAJECTORY_ABORTED as a general message to cover most (if not all) possible reasons a trajectory might be aborted. The idea is that the specific reason can go in the error_string field.
Does that sound reasonable to you?

Aborted is when the goal is aborted by the controller itself right?
Is preempted somehow handled in ROS 2 actions?. I'm not sure about it. If it is handled. All good

Aborted is when the goal is aborted by the controller itself right?

Yes, TRAJECTORY_ABORTED indicates that the trajectory was aborted by the controller.

Regarding preemption in ROS 2 actions: from what I’ve seen, existing action messages don’t explicitly handle preemption using an error code. This is the first message to introduce an error code like TRAJECTORY_ABORTED as a general indicator of an aborted trajectory. That said, new controllers can choose to use this error code if other codes don’t suit their case.

control_msgs/msg/JointWrenchTrajectoryPoint.msg

Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>

…sistent formatting. Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>

…ectory.action

my-rice · 2025-06-26T10:13:40Z

@christophfroehlich I implemented all the requested changes.

christophfroehlich

LGTM

control_msgs/msg/JointWrenchTrajectory.msg

saikishor · 2025-06-27T10:37:34Z

control_msgs/msg/JointWrenchTrajectoryPoint.msg

+string wrench_frame
+geometry_msgs/Wrench wrench


Can we use the WrenchStamped here instead?

Suggested change

string wrench_frame

geometry_msgs/Wrench wrench

geometry_msgs/WrenchStamped wrench

If we use WrenchStamped here instead of Wrench + wrench_frame, the message would end up with two headers, each containing its own timestamp (one inside the JointTrajectoryPoint and another inside the WrenchStamped field). Generally, I’d expect users of this message to assume that the wrench and the JointTrajectoryPoint are synchronized, meaning they share the same timestamp.

I agree with you, but I'm not sure about separating it as a wrench_frame_id. It should belong within the same context in my opinion. I understand your point on using stamped, but I'm not convinced with the current variable naming and usage.

Any other proposals? Or thoughts? @my-rice @christophfroehlich

I understand your point, but would be happy with the current approach as well.

To avoid duplicate header, we could add another layer with a WrenchFramed message?

Introducing a new message like WrenchFramed could be a valid solution to keep the information together and avoid duplicating headers.
Let me know if this will be on the agenda for the Wednesday PMC meeting.

Yes!
Let's discuss in the PMC Meeting

Ok, I wrote an item in the 'New Business' section of the agenda. Feel free to revise it or move it to a different section.

WrenchFramed

@saikishor as we discussed in yesterday’s PMC meeting, the proposed solution is to introduce the WrenchFramed message. This would allow us to encapsulate both the wrench_frame_id and the wrench itself in a structured layer.
I have two questions about this:

Should I introduce the WrenchFramed message as part of this PR, or submit it in a separate one?

Should WrenchFramed be added to the control_msgs package or another one?

saikishor · 2025-06-27T10:38:32Z

control_msgs/action/FollowJointWrenchTrajectory.action

+int32 OLD_HEADER_TIMESTAMP = -3
+int32 PATH_TOLERANCE_VIOLATED = -4
+int32 GOAL_TOLERANCE_VIOLATED = -5
+int32 TRAJECTORY_ABORTED = -6


Should we also add PREEMPTED or it doesn't make sense?

my-rice · 2025-07-14T09:21:23Z

Hey @saikishor, have you had a chance to review the updates on this PR?

saikishor · 2025-07-14T19:29:19Z

Hey @saikishor, have you had a chance to review the updates on this PR?

Hey @my-rice
I'm extremely sorry, this got out of my inbox last week. Thanks for the ping.

saikishor

I left some comments. Please take a look

my-rice · 2025-07-15T13:11:34Z

I'm extremely sorry, this got out of my inbox last week. Thanks for the ping.

@saikishor No worries at all. Thanks for getting back to me.

I left some comments. Please take a look

I believe I have responded to all of your comments on the PR.

my-rice added 7 commits June 19, 2025 18:58

[ADD] Add new message definitions for joint trajectory control

003f9b2

[REF] Change the defintions of Joint Wrench Point messages.

2ddf8a0

- [REF] Modified old organization of the message to introduce only two message definitions: JointWrenchTrajectory and JointWrenchTrajectoryPoint - [REF] Adjust CMakeLists.txt to adapt to the new changes

[DOC] Refine comments in JointWrenchTrajectory and JointWrenchTraject…

d826b89

…oryPoint messages

[ADD] Introduce FollowJointWrenchTrajectory action for joint wrench c…

f1ea3b4

…ontrol

[DOC] Update comments in JointWrenchTrajectoryPoint message for clarity

7b5efc6

Merge branch 'master' of https://github.com/unisa-acg/control_msgs in…

4ecc473

…to feature/joint_wrench_msgs

[FMT] Fix formatting issue by adding a newline at the end of FollowJo…

a365fdf

…intWrenchTrajectory.action

github-actions bot requested review from bmagyar, fmauch, homalozoa, mcbed, olivier-stasse, saikishor and VX792 June 26, 2025 08:21

christophfroehlich requested changes Jun 26, 2025

View reviewed changes

my-rice and others added 3 commits June 26, 2025 11:44

[ADD] Add index field in FollowJointWrenchTrajectory.action

8efa4e5

Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>

[DOC] Update comments in JointWrenchTrajectoryPoint.msg to ensure con…

d41b7b2

…sistent formatting. Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>

[DOC] Add description for TRAJECTORY_ABORTED in FollowJointWrenchTraj…

d374e54

…ectory.action

christophfroehlich approved these changes Jun 27, 2025

View reviewed changes

saikishor reviewed Jun 27, 2025

View reviewed changes

my-rice added 2 commits July 1, 2025 15:11

Merge branch 'master' into feature/joint_wrench_msgs

c80f747

Merge branch 'master' into feature/joint_wrench_msgs

635fc77

eferre89 mentioned this pull request Jul 2, 2025

Proposal: Modular Reference Handling in ROS 2 Controllers ros-controls/ros2_controllers#1757

Open

christophfroehlich requested a review from saikishor July 2, 2025 15:42

saikishor reviewed Jul 14, 2025

View reviewed changes

	string wrench_frame
	geometry_msgs/Wrench wrench
	geometry_msgs/WrenchStamped wrench

Feature/joint wrench msgs #221

Are you sure you want to change the base?

Feature/joint wrench msgs #221

Uh oh!

Conversation

my-rice commented Jun 26, 2025

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

my-rice commented Jun 26, 2025

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saikishor Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

my-rice Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

my-rice commented Jul 14, 2025

Uh oh!

saikishor commented Jul 14, 2025

Uh oh!

saikishor left a comment

Choose a reason for hiding this comment

Uh oh!

my-rice commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

saikishor Jul 14, 2025 •

edited

Loading

my-rice Jul 17, 2025 •

edited

Loading

my-rice commented Jul 15, 2025 •

edited

Loading