Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving node subscription frequency #916

Open
gmp-capgemini opened this issue Apr 20, 2023 · 2 comments
Open

Improving node subscription frequency #916

gmp-capgemini opened this issue Apr 20, 2023 · 2 comments

Comments

@gmp-capgemini
Copy link

gmp-capgemini commented Apr 20, 2023

This might be more a question than a feature, but I am not getting help anywhere else. I apologize in advance.

I've got a subscription to a camera (sensor_msgs/msg/Image) as I am using rclnodejs to make a real-time camera display. Even though the topic publishing the camera image is approximately 30 fps, I receive approximately 8 fps. Both publisher and subscriber are on the same host (my laptop), no network involved / no latency involved. It would be nice being able to improve the frequency of received messages for a subscription. I'm just using a basic call to createSubscription with an fps_counter (value displayed every second and then resetted):

const node_sub = new rclnodejs.Node('subscription_example_node');
  node_sub.createSubscription('sensor_msgs/msg/Image', 'rgb_front/image', (msg) => {
    console.log(`Received message: ${typeof msg}`, msg);
    fps_counter1++;
  });
  node_sub.spin();

The link for the on-line documentation is not working. Also executing npm run docs . as suggested by the README of the repo did not work either. I had a view at the implementation of Node.createSubscription and as far as I understood there's an option about QoS, but I could not understand how it works.

Please, lend me a hand with this in case it can be improved.

@minggangw
Copy link
Member

Hi @gmp-capgemini thanks for your feedback, I don't think the bottleneck is the frequency, the current timeout is 10ms and you can try to improve it to see if any change happens:

rclnodejs/lib/node.js

Lines 400 to 408 in 83958f4

/**
* Trigger the event loop to continuously check for and route.
* incoming events.
* @param {Node} node - The node to be spun up.
* @param {number} [timeout=10] - Timeout to wait in milliseconds. Block forever if negative. Don't wait if 0.
* @throws {Error} If the node is already spinning.
* @return {undefined}
*/
spin(timeout = 10) {

The efficiency could be petty low for your case because passing the image data of a frame to the JS side needs to allocate additional memory, which costs lots of time and depends on the image format and size. I would suggest:

  1. You can compare with the C++ implementation, e.g., Using rclcpp to implement and see if any improvement there.
  2. You should use video streaming instead of sensor_msgs/msg/Image if you want the video.

@minggangw
Copy link
Member

@gmp-capgemini I suggest you can try the latest 0.22.2, which removes the deep copy and may improve the performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants