Report rcl/rclc errors

[jimmy-mcelwain]

Fix for #128. RCL(C) API errors get reported to the user now. It just notifies the user that an RCL(C) API error occurred and gives the integer value of the rcl_ret_t value associated with that error that the user has to look up themselves.

motoRos_RCLAssertOK_withMsg() checks that the rcl_ret_t value that is passed in is OK, and if it is not, it not only sets an alarm indicating that, but it also sets an alarm indicating where it went wrong, like motoRos_Assert_withMsg() does. It's a bit ugly, something like what I described in the last paragraph of this comment may be prettier/do a better job of separating functionality/not duplicating code, but that would increase complexity a bit. It would require either a global "problems" struct, or several local "problems" structs in different files that get passed around that the hypothetical RaiseAlarms() function could reference.

This is a draft for now because I would like advice on what to do with motoRos_RCLAssertOK_withMsg(). I can squash the commits afterwards and rebase on main.

[gavanderhoorn]

It just notifies the user that an RCL(C) API error occurred and gives the integer value of the rcl_ret_t value associated with that error that the user has to look up themselves.

I would phrase it slightly more positive: users would lookup alarms raised by MotoROS2 in the troubleshooting documentation already, so this is just one more to look up.

It's just that in this case, it's as-if there's an additional sub code (a sub-sub code?), which happens to correspond to the rcl_ret_t value.

[jimmy-mcelwain]

Yeah that's a good point

[jimmy-mcelwain]

I was updating the troubleshooting guide to add information for the new error code, but the suggestion here says to include information on user-serviceable errors. I am not sure which errors are user serviceable and which ones are not. For now I just have a link to the types.h file referenced in our most recent build but I know that isn't sufficient.

Also, while I agree that the previous approach with the "shadow codes" would require maintenance in case there are changes to types.h in the future, won't we need to make changes in the troubleshooting guide if types.h changes anyways?

Also, unrelated, but I was having trouble figuring out why the alarm linter was upset. Then I figured out that it didn't like me using rcl_ret_t as a parameter type in the function prototypes without it knowing about that typedef, so despite the fact that it was compiling and working correctly, the parser for the alarm linter was failing. I just changed it to int (rcl_ret_t's real type), but I don't know if that is the preferred way of resolving that problem. It makes it a little less clear what you're supposed to pass in to the function.

[gavanderhoorn]

Also, while I agree that the previous approach with the "shadow codes" would require maintenance in case there are changes to types.h in the future, won't we need to make changes in the troubleshooting guide if types.h changes anyways?

it's true we'd have to keep an eye on it in any case. The hope is we've reduced the amount of maintenance to a minimum by not also introducing a new enum on our side (whose sole purpose is to shadow an already existing enum in the RCL(C) code base).

Also, unrelated, but I was having trouble figuring out why the alarm linter was upset.

it looks like it's unhappy because the file it was parsing (ErrorHandling.h) referenced an undefined type before your edit in 59f9582 (this version): rcl_ret_t is not part of C proper, but a custom / derived type. But ErrorHandling.h does not #include anything, so that type is not defined.

despite the fact that it was compiling and working correctly

that's because all .c files include the top-level MotoROS.h, which does include the required other .hs which then define the rcl_ret_t type.

I've wanted/tried to switch to an IWYU style before, but the MotoPlus SDK VS integration isn't happy if we do that (see #44).

[ted-miller]

It's a bit ugly, something like what I described in the last paragraph of #128 (comment) may be prettier/do a better job of separating functionality/not duplicating code, but that would increase complexity a bit

I think this is ok for the sake of simplicity.