Reset the error when rcl_take_response failed

[minggangw]

In the latest implementation of rcl of rcl_take_response, the function will set the error message when returning RCL_RET_CLIENT_TAKE_FAILED and we have to reset the error so long as the return value doesn't equal to RCL_RET_OK. PR #396 has been submitted.

[dhood]

Thanks for the patch @minggangw

Regarding RCL_RET_CLIENT_TAKE_FAILED, this is not seen as an error state. The equivalent situation for services (rmw_take_request returning false) has been discussed in ros2/rcl@534a49c#commitcomment-19763950: this resulted in ros2/rcl#88

Look like the parallel of PR ros2/rcl#88 is just missing for clients, and then there will be no error message to be cleared.
If you are interested in making the change yourself (and confirming that it also addresses the original issue in your particular use case) we can consider a PR that makes that change in place of #396

[minggangw]

Hi @dhood, we are implementing a rcl Node.js client named rclnodejs, which is under the Web Robot Tools umbrella. I found this problem when running the unit test by npm test and we created an issue to track. It seems that although the RCL_RET_CLIENT_TAKE_FAILED is not considered as an error state, there is still an error message to be written, please see our log recorded in our issue:

>>> [rcutils|error_handling.c:148] rcutils_set_error_state()
This error state is being overwritten:

  'error not set, at /home/qiuzhong/Workspace/Work/ros2/src/ros2/rcl/rcl/src/rcl/client.c:279, at /home/qiuzhong/Workspace/Work/ros2/src/ros2/rcl/rcl/src/rcl/client.c:279, at /home/qiuzhong/Workspace/Work/ros2/src/ros2/rcl/rcl/src/rcl/client.c:279, at /home/qiuzhong/Workspace/Work/ros2/src/ros2/rcl/rcl/src/rcl/client.c:279, at /home/qiuzhong/Workspace/Work/ros2/src/ros2/rcl/rcl/src/rcl/client.c:279, at /home/qiuzhong/Workspace/Work/ros2/src/ros2/rcl/rcl/src/rcl/client.c:279, at /home/qiuzhong/Workspace/Work/ros2/src/ros2/rcl/rcl/src/rcl/client.c:279, at /home/qiuzhong/Workspace/Work/ros2/src/ros2/rcl/rcl/src/rcl/client.c:279, at /home/qiuzhong/Workspace/Work/ros2/src/ros2/rcl/rcl/src/rcl/client.c:279'

with this new error message:

  'rcl_init() has not been called, at /home/qiuzhong/Workspace/Work/ros2/src/ros2/rcl/rcl/src/rcl/node.c:121'

rcutils_reset_error() should be called after error handling to avoid this.
<<<
    ✓ rclnodejs create node without init

So I think we should do the same as ros2/rcl#88 does for clients.

P.S. I want to have a brief introduction about us. We are from Open Source Technology, Intel and our mission is to promote web technology in every possible field. From middle of this year, we collaborated with RWT guys to develop the Node.js client of ROS2.0, @jihoonl gives us great support. The first alpha release of rclnodejs were published on npmjs two weeks ago. We have a thorough team who focus on robotics/IoT, including developers and QAs, and want to introduce the Node.js ecosystem to ROS. I know the first non-beta release of ROS 2.0 will happen next month, we are planning to release our first stable client around that timing to be synchronized. I hope we can keep a strong tie to make programming for ROS much easier!

[dhood]

Yes, I agree that the parallel of PR ros2/rcl#88 is just missing for clients, and then there will be no error message to be cleared. This is proposed in ros2/rcl#182 so I'll close this ticket as it should address your initial issue.

Great to hear the context around the contributions.. I was wondering what client library you were using, but you are developing your own 😄 It's nice to see the work being done on rclnodejs; we certainly appreciate you taking the time to propose improvements to the other client libraries as you come across things while developing rclnodejs. Please let us know if anything else comes up!