RobotWebTools / rclnodejs

Node.js version of ROS 2.0 client
https://docs.ros.org/en/jazzy/Concepts/Basic/About-Client-Libraries.html#community-maintained
Apache License 2.0
335 stars 72 forks source link

Use null-terminated string in the tests when sending a raw topic #772

Closed minggangw closed 3 years ago

minggangw commented 3 years ago

Fix #771

wayneparrott commented 3 years ago

@minggangw I think it would be helpful to extend the comment on Publisher#publish() to note the null terminated string requirement. If you update the doc please update the publisher.d.ts api accordingly. https://github.com/RobotWebTools/rclnodejs/blob/31efa7c28e8d20bcd6fe1311fb10e9afa7bd63a7/lib/publisher.js#L41

Also I notice the raw publisher example does not use null terminated string. https://github.com/RobotWebTools/rclnodejs/blob/31efa7c28e8d20bcd6fe1311fb10e9afa7bd63a7/example/publisher-raw-message.js#L35

minggangw commented 3 years ago

The description of #771 may mislead the readers. I'd like to clarify it:

I think we should update the comments for the subscription instead, what do you think?

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.002%) to 91.789% when pulling e6c5d6e443a92c1fc0ec8f0113aa88d000ef1a60 on minggangw:fix-issue-771 into 31efa7c28e8d20bcd6fe1311fb10e9afa7bd63a7 on RobotWebTools:develop.