Add error message string to MoveItErrorCode msg#171
Conversation
sjahr
left a comment
There was a problem hiding this comment.
What do you think about adding an optional source string too?
E.g. like this:
// Name of the component that created the status
string source;
sjahr
left a comment
There was a problem hiding this comment.
Nit: Maybe consider removing the error_* prefix to avoid code like this:
MoveItErrorCode error_code;
error.error_message = "Ahh";
sea-bass
left a comment
There was a problem hiding this comment.
I think this is conflating a bunch of things.
It makes more sense to me to have a MoveItError message which contains the MoveItErrorCode code plus a string message?
Co-authored-by: Sebastian Jahr <sebastian.jahr@tuta.io>
Co-authored-by: Sebastian Jahr <sebastian.jahr@tuta.io>
We'd need to do a lot of integration work by introducing an additional message type for that. What I like about having it in this message is that the message + the source information is immediately everywhere available where this message is used and we don't need to break anything |
|
Please consider extending the 'enum' where necessary as well. Strings are hard to translate and error prone to handle in logic/error mitigation |
If this error message will be used as the "human-readable" description of the enum, that's very good and let's go ahead. |
The message shouldn't serve as a description of the enum - this can be created statically and we already have a method to do so. Rather, the message should be used (if at all) to provide some more specific reason of the failure, e.g. why planning failed. |
When planning with MoveIt and MTC, we often assign the FAILURE error code. This is in an effort to add more description on why planning/execution failed.