rclnodejs icon indicating copy to clipboard operation
rclnodejs copied to clipboard

Features needed to implement - umbrella issue

Open minggangw opened this issue 5 years ago • 44 comments

As ROS 2 iterates rapidly, actually, there are two stable releases per year and a series of patch releases. I want to emphasize the target of this Node.js client is that it has the comparative feature set as the python client - rclpy.

But there is still a gap with rclpy, and we want to eliminate it finally. Due to limited resource, I'd like to invite any volunteer to contribute to this project, so the developers can have an additional choice besides C++ and Python languages. I believe that the requirement of web technology in ROS 2 ecosystem is strong, and JavaScript/Node.js enable more developers to work on ROS project quickly because of the low threshold.

I have tried my best to maintain rclnodejs module can work on the latest ROS 2 stable release, e.g. Dashing Diademata, as soon as possible. I hope there are more contributors to implement the absent features and I will offer any help to move this project forward. Your contributions are welcome!

I list the features needed to implement below, so we can record the status. If anyone wants to handle it, please just assign it to yourself to indicate the feature is being implemented. Thanks!

  • [ ] QoS - API and implementation for Liveliness and Deadline event callbacks #492 (Medial)
  • [x] Actions support in rclnodejs #469 (High)
  • [x] Support for parameters #416 (Medial)
  • [x] Parameters can be passed through CLI #421 (Low)
  • [x] Publish parameter events #415 (Medial)
  • [x] Implement node parameters and parameter services #414 (Medial)
  • [x] Add guard condition feature #315 (Medial)
  • [x] Expose rcl_get_node_names #555
  • [x] Add countPublishers and countSubscribers to Node #559
  • [x] Support spin once #562
  • [x] Rate support #564
  • [x] LifecycleNode support in rclnodejs #589
  • [x] Add getLogger to Node object #591
  • [x] Add getClock to Node object #592
  • [x] Access current time from Node #598
  • [x] Recognize the use_sim_time parameter #599
  • [x] Support to publish/subscribe serialized messages #646
  • [x] Enable rclnodejs to fully support rcl contexts #668

P.S. I add an indicator (Low/Medial/High) to represent the combination of workload and difficulty of each issue. You can reference it to decide which feature will be suitable for you to handle.

Some suggestions in my mind:

  • Please follow the Google C++ coding style to write your code, you can run npm run lint to verify it locally.
  • For JavaScript code, ES6 is preferred. Also, checked by eslint.
  • The current code on develop branch is based on the nightly build of ROS 2, and we will create tags for each stable release of ROS 2 when it's published.
  • The features above have counterparts in rclpy, so you can reference the C code there to wrap your JS.
  • Please add proper comments to illustrate your code if necessary, you can reference the existing ones (Currently the API doc is generated from them by jsdoc).
  • Please offer unit tests if features are added/changed, as we want to be TDD.
  • All PRs will be verified on CIs first, and the review work will start after all CIs pass.
  • Please read README to setup your own ROS2 env on your PC.
  • If you are the FIRST time to submit your code, please add yourself as a contributor.
  • If you meet any problems, please ping me and I will response ASAP. Also, I'd like to help part of the code (I cannot fully focus on it now, sorry).
  • Once your PR gets LGTM, I will merge it and you can get your credit :)

If I miss some feature or features you want to add, please append them to this bullet.

minggangw avatar Jun 26 '19 07:06 minggangw

I would like to contribute. Could you please elaborate a little

suab321321 avatar Jul 01 '19 20:07 suab321321

Thanks for your response @suab321321 which angle do you want to know more, e.g. workload, difficulty level or coding rule? I can write a summary or BKM to facilitate other people who are also interested in this project. Thus, we can move forward smoothly!

minggangw avatar Jul 02 '19 02:07 minggangw

I posted a topic on ROS discourse to let more people know it.

minggangw avatar Jul 02 '19 02:07 minggangw

I would like to know workload..

suab321321 avatar Jul 03 '19 12:07 suab321321

I would like to contribute to #492 first.. Thank you

suab321321 avatar Jul 03 '19 12:07 suab321321

@suab321321 I've marked it for each feature, it's just kind of rough evaluation, not exact. Also, I added some suggestions, please check it before starting your work as it may save your time.

Again, you are welcome. Now please go ahead!

minggangw avatar Jul 04 '19 05:07 minggangw

okay.. I understand the indicator tag now. Now I want to understand how will I contribute to #421 because no description is provided on what to do

Thank you

suab321321 avatar Jul 04 '19 05:07 suab321321

Actually, there is no specific requirement or design doc to describe the feature, all implementations should base on the rcl.

As for the rclnodejs, you can reference the rclpy client (It's a good way to get started). You may notice that the parameter feature contains several issues, I created these by https://github.com/ros2/rclpy/issues/202, so I think you can find the implementation then.

minggangw avatar Jul 04 '19 06:07 minggangw

Okay..I will start now

suab321321 avatar Jul 04 '19 07:07 suab321321

I m unable to install rclnodejs either from npm or from sractch...its showing some file import error

OS-ubuntu 18.04 Screenshot from 2019-07-05 01-23-23

suab321321 avatar Jul 04 '19 19:07 suab321321

Have you executed source <path/to/ros2>/install/local_setup.bash , if not please follow the steps here, thanks!

minggangw avatar Jul 05 '19 01:07 minggangw

Yes I have done that...

suab321321 avatar Jul 05 '19 05:07 suab321321

Is this issue common or is it just me?

suab321321 avatar Jul 05 '19 05:07 suab321321

It's not common, please double check with echo $COLCON_PREFIX_PATH. From the error log, it should be the problem of the wrong include path. I suggest that you clone this project and checkout the develop branch to start instead of adding it as a dependency.

minggangw avatar Jul 05 '19 07:07 minggangw

Nope..Its not happening

suab321321 avatar Jul 05 '19 07:07 suab321321

Hello, could you elaborate on the steps that need to be done to wrap rcl with Node.js? What are some resources for users who would like to help contribute to one of these features. I would like to help with Actions support #469 but I am not sure where to start. I found this article on Nodejs addons.

Maybe you could give an example of a rcl feature (such as publisher/subscriber) and show how the rcl feature is wrapped by the C++ addon (here?) and then where the C++ addon is implemented in the Node.js library (here?) Such that one could extrapolate the process to the requested features listed above.

I am a primarily a python developer and I have enough JS experience to build GUI’s (React) for my ROS projects (using ROS2-web-bridge). However, my experience with Node.js by itself is limited (never heard of C++ addons until now). Will I be getting myself in over my head with this?

mikelyndersOKCC avatar Jan 22 '20 15:01 mikelyndersOKCC

hi @mikelyndersOKCC, good question. Last week I started work on implementing parameter support for rclnodejs. My C coding skills are pretty rusty and basic at best - but why let that stop me :) It took me a couple of hours of exploring code and docs to develop a mental model and impl a hello-world example. From there, I systematically evolved my node-c++ rcl binding.

  • one of the project's objectives is to maximize compatibility across ros2 releases. The rcl is a C implementation for which other rcl language bindings including rclnodejs are based. So get familiar with the project and it's api as you'll want to bind to it if at all possible for your feature.
  • take a look at current rcl bindings in rcl_bindings.cpp and bindings.gyp

General resources (I took bits and pieces from these and others)

  • nan library - you can find examples via google
  • https://nodeaddons.com/

best wishes.

wayneparrott avatar Jan 22 '20 19:01 wayneparrott

Hi @mikelyndersOKCC thanks for your question, I think @wayneparrott has given some general steps to touch on the development of Node.js addon. I'd like to share some of my ideas

  • The development of addon for Node.js

    • We are using the "old" API (called nan) to bind the JS interface to C++ instead of using N-API which can decouple your code from runtime e.g. V8
    • As @wayneparrott has mentioned the nan library, here is the example repo. I think referencing the existing code in rclnodejs for binding is enough to write your own code :smile:
    • The toolchain used for addon development for Node.js is node-gyp . Ideally, you don't need to change it.
  • The development of rclnodejs

    • The general design of it, please see the 7th slide
    • rcl is the C interfaces we want to wrap and expose to JavaScript through the nan library.
    • There are usually two parts to implement a new feature for rclnodejs
      1. Define and write the JavaScript interfaces to be exposed to users
      2. Implement the C++ binding methods called on the JavaScript side (you need to add binding functions into rcl_bindings.cpp in most cases)
    • I strongly you could refence the implementation of the feature you are going to do for rclnodejs in rclpy, because they are very similar C++ <=> Python & C++ <=> JavaScript, it will save your time to do so.
  • For the test To ensure the quality and stability of rclnodejs, we are runing a series of unit tests on Linux, Windows10 and macOS platforms, you can see the three badges in README. Some static check will also run e.g. eslint & cpplint. So, if you implement a new feature, you'd better write the test cases for it. The framework for testing is mocha which is used widely for Node.js projects.

If you have any problems, please feel free to ask on Github. Thanks!

P.S. the original issue https://github.com/RobotWebTools/ros2-web-bridge/issues/130

minggangw avatar Jan 23 '20 02:01 minggangw

@minggangw @wayneparrott Thank you both very much for your insight! I really appreciate the guidance. I will study the materials that have been linked here and hopefully I can do something with it!

mikelyndersOKCC avatar Jan 23 '20 13:01 mikelyndersOKCC

@minggangw @mikelyndersOKCC I would like to contribute to #421 .Is anyone working on this issue?

suab321321 avatar Jan 28 '20 17:01 suab321321

@suab321321 I'm about 50% complete on a PR for #416. Hoping to submit it by EOW or some time next weekend.

wayneparrott avatar Jan 28 '20 20:01 wayneparrott

@wayneparrott is #414 free?Is this free?

suab321321 avatar Jan 29 '20 04:01 suab321321

@suab321321 re: #414, it is part of #416. I've started working on #416 based on the rcl library. I've temporarily halted development in order to finish a PR for ros2cli to support pluggable build-types. My plan is to finish the ros2cli PR tomorrow and resume #416. But if you are interested in #416 I will be glad to yield it to you or anyone else that is ready to push it forward. While you asked about #414 my assessment is that it's design needs to account for #421 which will initialize the node's parameters from the commandline.

wayneparrott avatar Jan 29 '20 07:01 wayneparrott

@wayneparrott can you tell me which is most beginners friendly issue?

suab321321 avatar Jan 29 '20 07:01 suab321321

Hi @suab321321 I suggest @wayneparrott is going to handle all param related features, so there will be no blockers or dependencies throughout the implementation (I have updated the assignees for these issues).

Meanwhile, I check the existing issues and probably #492 is suitable for beginners (I have tagged it with good-first-issue). Again, I'd like to mention some necessary knowledge one may need if he/she wants to contribute, please see below:

Generally, the whole framework of rclnodejs is not complicated, as it's only a thin wrapper of rcl library and some platform related implementations. So you'd better have experiences

  • Language skills: JavaScript (ES6) and C++ (C+14/17)
  • Node.js C++ addon development
  • Have some basic concepts of ROS/ROS2, e.g. pub/sub, client/service

Steps and useful methods for ramping up, please see the comments (https://github.com/RobotWebTools/rclnodejs/issues/498#issuecomment-577342510 and https://github.com/RobotWebTools/rclnodejs/issues/498#issuecomment-577479575) in this thread, thanks!

minggangw avatar Jan 29 '20 13:01 minggangw

@minggangw okay I will begin the work soon.

suab321321 avatar Jan 30 '20 03:01 suab321321

@minggangw @wayneparrott I m finding it extremly difficult to understand rcl_binding.cpp Is is rcl_binding.cpp same to rclnodejs as rmw for rclpy and rclcpp? I found that inside rclpy _rclpy.c the implementations are similar to what is there rcl_bindings.cpp isnt it?

suab321321 avatar Jan 31 '20 20:01 suab321321

Hi @suab321321 I recommend you learn from some examples of Node.js addon and try to understand the mechanism of binding C++ to JavaScript. https://github.com/RobotWebTools/rclnodejs/pull/554 is a good demonstration.

Is is rcl_binding.cpp same to rclnodejs as rmw for rclpy and rclcpp?

rmw is the middleware layer of ROS2 and it's transparent to us. What we should concern is rcl (based on rmw) library. _rclpy.c is the counterpart in Python bindings compared with Node.js (rcl_bindings.cpp), so the behaviors between them are similar.

minggangw avatar Feb 01 '20 09:02 minggangw

@minggangw I have some doubt in rcl_binding.cpp

NAN_METHOD(CreateTimer) {
  int64_t period_ms = info[0]->IntegerValue();
  RclHandle* context_handle = RclHandle::Unwrap<RclHandle>(info[1]->ToObject());
  rcl_context_t* context =
      reinterpret_cast<rcl_context_t*>(context_handle->ptr());
  rcl_timer_t* timer =
      reinterpret_cast<rcl_timer_t*>(malloc(sizeof(rcl_timer_t)));
  *timer = rcl_get_zero_initialized_timer();
  rcl_clock_t* clock =
      reinterpret_cast<rcl_clock_t*>(malloc(sizeof(rcl_clock_t)));
  rcl_allocator_t allocator = rcl_get_default_allocator();
  THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK,
                           rcl_clock_init(RCL_STEADY_TIME, clock, &allocator),
                           rcl_get_error_string().str);
  THROW_ERROR_IF_NOT_EQUAL(
      RCL_RET_OK,
      rcl_timer_init(timer, clock, context, RCL_MS_TO_NS(period_ms), nullptr,
                     rcl_get_default_allocator()),
      rcl_get_error_string().str);

  auto js_obj = RclHandle::NewInstance(timer, nullptr, [timer, clock] {
    rcl_ret_t ret_clock = rcl_clock_fini(clock);
    free(clock);
    rcl_ret_t ret_timer = rcl_timer_fini(timer);
    return ret_clock || ret_timer;
  });
  info.GetReturnValue().Set(js_obj);
}

NAN_METHOD(IsTimerReady) {
  RclHandle* timer_handle = RclHandle::Unwrap<RclHandle>(info[0]->ToObject());
  rcl_timer_t* timer = reinterpret_cast<rcl_timer_t*>(timer_handle->ptr());
  bool is_ready = false;

  THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, rcl_timer_is_ready(timer, &is_ready),
                           rcl_get_error_string().str);

  info.GetReturnValue().Set(Nan::New(is_ready));
}

suab321321 avatar Feb 03 '20 16:02 suab321321

@minggangw in above code in function CreateTimer() rcl_timer_t* variable is by one method but in function IsTimerReady() rcl_timer_t* variable is created by another method. 1.Why is this? 2.RclHandle* timer_handle = RclHandle::Unwrap<RclHandle>(info[0]->ToObject()); can you give a brief explanation what is happening here? (I only know that info is used to access the paramter passed from javascript functions). :)

suab321321 avatar Feb 03 '20 16:02 suab321321