What sort of error / result code would clients see?

A single error line in the controller's terminal is probably not sufficient to prevent surprises / confused users.

What sort of error / result code would clients see?

A single error line in the controller's terminal is probably not sufficient to prevent surprises / confused users.

The message will be rejected, which means that nothing moves and the user will have a look in the command prompt.
Using the topic interface, there is no chance for returning error codes. Using action interface, rclcpp_action::GoalResponse::REJECT is the result for the action client.

What else do you suggest?

The message will be rejected, which means that nothing moves and the user will have a look in the command prompt.

I guess I would phrase this as:

The message will be rejected, which means that nothing moves and the user will have to look in the command prompt.

that's what I meant when I wrote:

A single error line in the controller's terminal is probably not sufficient to prevent surprises / confused users.

I've been such a confused user in the past, hence my comment.

Using action interface, rclcpp_action::GoalResponse::REJECT is the result for the action client.

Last time I checked, there was no support for setting the error_code or the error_string as part of the rejection, as you could in ROS 1 (ros2/rclc#271).

That complicates things, but a work-around I've seen is to accept the goal, then 'abort' by never processing it and immediately sending the final result, in which you set the error_code and error_string to a meaningful value.

Clearly a work-around, and I still consider the limitation discussed in the linked issue a serious design flaw, but it seems without some work, the best you can do.

The advantage of doing something like this is that clients would have machine readable information about what happened to their goal.

"the human needs to look at the command line" is not a viable error handling / recovery strategy in many cases, so should be avoided as much as possible. The error_code field could be used by clients to implement automated recovery, or at the very least promote the error returned by the JTC in a way easily visible to users (using some form of UI, or perhaps still an ERROR in a log somewhere, but at least not hidden in the rest of the output).

The check introduced with this PR just follows the way the trajectory message gets rejected with any other errors. So I suggest discussing your point in a separate issue, because we then would have to change the overall behavior of the validate_trajectory_msg method, which is not linked to the particular issue with the effort fields.

Thanks @gavanderhoorn for the insightful comments, it's worth tracking that on a separate issue.