-
Notifications
You must be signed in to change notification settings - Fork 134
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
Add error message string to MoveItErrorCode msg #171
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Maybe consider removing the error_*
prefix to avoid code like this:
MoveItErrorCode error_code;
error.error_message = "Ahh";
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 <[email protected]>
Co-authored-by: Sebastian Jahr <[email protected]>
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.