search

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

Add node 13-17 support, bump to v0.21.0 #825

Closed wayneparrott closed 2 years ago

wayneparrott commented 2 years ago

Public API Changes none

Description

  1. package.json

    • Bumped to version 0.21.0
    • Support for node 13-16 required forking and customizing the following packages: @rclnodejs/ref-array-di @rclnodejs/ref-napi @rclnodejs/ref-struct-di Revised package dependencies to use the 3 packaged listed above
    • Added jsdoc as devDependency

  2. test-security-related.js

    • Fixed minor assertion error caused by change in Nodejs error message.

  3. Docs

    • Updated README.md, Build.md and npmjs-readme.md to expand valid versions of node 13-16.
    • Modified docs/Makefile to use npx to run jsdoc to generate api documentation

Fix #735 #717

coveralls commented 2 years ago

Coverage Status

Coverage decreased (-0.07%) to 90.471% when pulling 4ce69c63b3646a6c9703c5953eb55d897d4ecacf on wayneparrott:develop into 5f841743165e25188f01e2fd03ac91db768cf6bc on RobotWebTools:develop.

wayneparrott commented 2 years ago

On linux running install with node16 as root is a PIA with permissions issues. No such issues with node 12 & 14. Thus the zillion commits as I slowly convert from running scripts and test suite to non-root user. Grant me patience!

felixdivo commented 2 years ago

In the very beginning of README.md, the node tag still mentions the old version restriction.

wayneparrott commented 2 years ago

@felixdivo hi, the node tag is dynamically generated from https://img.shields.io/node/v/rclnodejs. When we publish the next rclnodejs release to npmjs.com the tag should be updated with the latest range of supported node versions.

felixdivo commented 2 years ago

Ah okay, sorry for the bump. Neat! 😃

minggangw commented 2 years ago

There is an update, saying that this crash issue (from node v13 to v16) has been fixed on node v17, but with performance regression. @wayneparrott

minggangw commented 2 years ago

I verify rclnodejs v0.20.1 with node v17.4.0 and the crash is gone.

wayneparrott commented 2 years ago

I verify rclnodejs v0.20.1 with node v17.4.0 and the crash is gone

Was this build using the original ref-napi and related packages or did you use the forked and udpated @rclnodejs/ref-napi and related packages?

Zaaler commented 2 years ago

I'm running rclnodejs v0.21.0 with node v17.8.0 on aarch64 and ros2 galactic and I'm experiencing the cannot reinterpret from nullptr pointer.

I found a work around earlier but, I've since flashed the computer and now, I'm unfortunately debugging something I already solved. I think I may have reverted node to 16 and used the previously mentioned solution.

Should I be experiencing this error still?