search

Yaskawa-Global / motoros2

ROS 2 (rcl, rclc & micro-ROS) node for MotoPlus-compatible Yaskawa Motoman robot controllers
89 stars 14 forks source link

More accurate/descriptive error messages for start_traj_mode and start_point_queue_mode services #265

Open jimmy-mcelwain opened 2 weeks ago

jimmy-mcelwain commented 2 weeks ago

In reference to #37 and #192. Now the error message and error codes match the reasons that the motion start failed.

jimmy-mcelwain commented 2 weeks ago

Dependent upon https://github.com/Yaskawa-Global/motoros2_interfaces/pull/17

gavanderhoorn commented 1 week ago

High-level comment (about the proposed approach): the current implementation seems to use a different approach: error reporting uses the result of Ros_Controller_GetNotReadySubcode() (based on the result of Ros_Controller_IsMotionReady()) if Ros_MotionControl_StartMotionMode(..) (or anything else) failed.

The current approach is slightly racy (as in: noticing something was wrong and reporting about what was wrong are done at different times -- but so far that hasn't really been problematic), but the proposed approach starts mixing responsibilities. It also duplicates quite a bit of code.

Perhaps we should try and find a middle-ground?

gavanderhoorn commented 1 week ago

And another (but only slightly related to this PR): seems the new error / pattern matcher in combination with the updated mpBuilder doesn't yet capture compiler errors correctly, resulting in the many incomplete annotations.

I'll look into this and fix it.

jimmy-mcelwain commented 1 week ago

@gavanderhoorn This is sort of an awkward responsibility-mixing design. I was trying to solve the issue without any sort of big redesign of how Ros_Controller_GetNotReadySubcode() is used.

I do think that the current way that errors are reported, by noticing that there is an issue, and then trying to identify that issue later based on the state of the controller, seems likely to cause to problems/makes it difficult to get useful error messages. I think that a more direct error-reporting approach may be more reliable/less 'racy', but yeah there would be more code duplication.

More significant changes would need to be made to get rid of the responsibility-mixing aspect, or I could move closer to the original approach. but I don't know what the best option is. I pushed some minor changes that make it a bit cleaner, but didn't change the approach.