RobotWebTools / rclnodejs

Node.js version of ROS 2.0 client
https://docs.ros.org/en/humble/Concepts/Basic/About-Client-Libraries.html?highlight=rclnodejs#community-maintained
Apache License 2.0
320 stars 70 forks source link

Features needed to implement - umbrella issue #498

Open minggangw opened 5 years ago

minggangw commented 5 years ago

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!

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:

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

suab321321 commented 5 years ago

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

minggangw commented 5 years ago

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 commented 5 years ago

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

suab321321 commented 5 years ago

I would like to know workload..

suab321321 commented 5 years ago

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

minggangw commented 5 years ago

@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!

suab321321 commented 5 years ago

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

minggangw commented 5 years ago

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.

suab321321 commented 5 years ago

Okay..I will start now

suab321321 commented 5 years ago

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

minggangw commented 5 years ago

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

suab321321 commented 5 years ago

Yes I have done that...

suab321321 commented 5 years ago

Is this issue common or is it just me?

minggangw commented 5 years ago

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.

suab321321 commented 5 years ago

Nope..Its not happening

mikelyndersOKCC commented 4 years ago

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?

wayneparrott commented 4 years ago

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.

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

best wishes.

minggangw commented 4 years ago

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

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

mikelyndersOKCC commented 4 years ago

@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!

suab321321 commented 4 years ago

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

wayneparrott commented 4 years ago

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

suab321321 commented 4 years ago

@wayneparrott is #414 free?Is this free?

wayneparrott commented 4 years ago

@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.

suab321321 commented 4 years ago

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

minggangw commented 4 years ago

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

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!

suab321321 commented 4 years ago

@minggangw okay I will begin the work soon.

suab321321 commented 4 years ago

@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?

minggangw commented 4 years ago

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.

suab321321 commented 4 years ago

@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 commented 4 years ago

@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). :)

minggangw commented 4 years ago

1.Why is this?

In CreateTimer, we need to allocate the memory for the handle, while IsTimerReady, we get the handle passed from JavaScript, which was created by the first method. So they are different usages.

2.RclHandle* timer_handle = RclHandle::Unwrap(info[0]->ToObject());

Here we unwrap the C++ object from the parameter as you said, some detailed explanations please see https://nodejs.org/api/addons.html#addons_passing_wrapped_objects_around and https://github.com/nodejs/nan/blob/master/doc/object_wrappers.md

suab321321 commented 4 years ago

@minggangw sir I think I m on verge solving of this issue :) I have a questions.

  1. are callbacks functions called using event driven architecture of nodejs(Event emitter)?
suab321321 commented 4 years ago

I can see that there is execute() function in node.js which is checking for callbacks for subscription,timer,etc.but shouldnt this run infinetly to check for if a certain event has occured?

suab321321 commented 4 years ago

One more question.. When creating a new timer we are needed to create a timerHandle but not in case of Publisher or Subscriber.why is this?

minggangw commented 4 years ago

I can see that there is execute() function in node.js

executed() function is triggered by the sub-thread, so it doesn't need to run infinitely (as a loop).

When creating a new timer we are needed to create a timerHandle but not in case of Publisher or Subscriber.why is this?

the handle represents a "handle" of C resource, see https://github.com/RobotWebTools/rclnodejs/blob/e38e2a289e3c357ca9406728631d7b6150ff4e15/lib/entity.js#L23

suab321321 commented 4 years ago

@minggangw I need to add some parameters like publisher class constructor so shoud I pass it as an inside the options objects because see this

/**
   * Create a Publisher.
   * @param {function|string|object} typeClass - The ROS message class,
        OR a string representing the message class, e.g. 'std_msgs/msg/String',
        OR an object representing the message class, e.g. {package: 'std_msgs', type: 'msg', name: 'String'}
   * @param {string} topic - The name of the topic.
   * @param {object} options - The options argument used to parameterize the publisher.
   * @param {boolean} options.enableTypedArray - The topic will use TypedArray if necessary, default: true.
   * @param {QoS} options.qos - ROS Middleware "quality of service" settings for the publisher, default: QoS.profileDefault.
   * @return {Publisher} - An instance of Publisher.

or should I add one more parameter in method.

minggangw commented 4 years ago

@suab321321 do you have a specific requirement or an issue (please create a new one if there is none), I suggest we'd better discuss there as this is a general issue for features to be implemented, thanks!

suab321321 commented 4 years ago

@minggangw no no,see when we add the feature of liveliness and deadline callback, then this should be declared while creating Publisher/Subscriber, for this thing I need to add one more paramter in the methods which is used for creating Publisher/Subsciber, so I m asking if need to embed this new parameter inside options object or add just add a new paramter field. What guidelines is followed I dont know?

and one more thing :) what is the use of timerCallback(),I have seen that at the end this function calls rcl_timer.c but cant understand what is its role

this._timers.forEach((timer) => {
      if (timer.isReady()) {
        rclnodejs.callTimer(timer.handle);
        timer.callback();
      }
    });

Should we to continue this to #492 thread.

wayneparrott commented 4 years ago

A useful feature to consider is the message_filter. The current ros2 implementation is found here https://github.com/ros2/message_filters.

mattrichard commented 4 years ago

@wayneparrott It seems like message filters would be better implemented as a separate library, if possible, since it's not built into rclcpp. But that starts bringing up the question of how to distribute rclnodejs packages. Distributing on npm with rclnodejs as a peer-dep would probably be fine for now.

flynneva commented 4 years ago

I'd love to help out with this! it looks like most of the topics are already done (sorry for being late to the party).

I'll poke around and see where I can help

minggangw commented 4 years ago

Hi @flynneva welcome, the checklist in c#1 shows some main features which are not implemented yet, but I think there should be some others not found. If you have experience with rclpy, it's easy to discover the missing functions or you could enhance some existing ones. Some insights:

Also, you can add/fix any features, thanks!

FlorisE commented 4 years ago

roslibjs has some support for getting information about a running ROS system via http://robotwebtools.org/jsdoc/roslibjs/current/Ros.html, like getting the list of active nodes, mesage information, etc. Should rclnodejs offer such functionality, and if yes, is it already implemented somewhere? If no, is there another JS library that offers such functionality for ROS 2?

minggangw commented 4 years ago

rclnodejs is not proper to implement this sort of APIs, instead, rosbridge acting as back-end of roslibjs should offer it, some reference

If no, is there another JS library that offers such functionality for ROS 2?

I haven't heard some JS library offers that.