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

Make launch_hw param reconfigurable. #2

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

mbharatheesha
Copy link
Collaborator

The problem:

nebula_ros doesn't support dynamic enabling/disabling of hw driver at runtime because the launch_hw parameter is read-only.

This PR:

A WIP solution attempt to make the parameter dynamically reconfigurable and setup necessary hardware interfaces when the launch_hw parameter is set dynamically. I have tried to ensure the threading part is handled as carefully as possible, but it would be great if I can get some feedback on the same

What has been tested so far:

  • ros2 param set <nebula_driver_node_name> launch_hw true successfully returns: Set parameter successful.
  • The runtime behavior remains the same with these changes as compared to the behavior before these changes i.e.,
    • This warning is continuously printed Missed pointcloud output deadline when launch_hw parameter is false (statically or dynamically set). This is because in this configuration, the driver is expecting some other source to publish on /velodyne/packets topic. When the parameter was read-only, this was the same behavior too.
    • When launch_hw is set to true (statically or dynamically) without a physical lidar connected to the device, the lidar driver crashes with this exception Could not initialize HTTP client: Http Connection Error

To test:

  • Connect a real velodyne lidar and see if the node doesn't crash
  • Replicate similar changes for hesai and other wrappers
  • Create an upstream PR

@mbharatheesha mbharatheesha requested a review from Timple February 5, 2025 23:00
@mbharatheesha
Copy link
Collaborator Author

The fixes pushed today enable the dynamic update of launch_hw parameter for velodyne wrapper. That is:

  • Start the driver with launch_hw set to false. Then change the parameter to true. The driver successfully starts communicating with connected hardware and publishes pointclouds
  • Start the driver with launch_hw set to true. The driver publishes pointclouds as expected. Then change the parameter to false. The driver stops publishing pointclouds.
  • If the parameter was already true and it is set to true again, nothing happens. Same if the parameter was false and set to false again.

There are a few questions I have regarding the behavior of one of the threads. I will ask the same in the upstream PR. But I think we can incrementally generate binaries as and when the other drivers are adapted to support dynamic reconfiguration of launch_hw parameter.

@mbharatheesha
Copy link
Collaborator Author

Upstream PR: tier4#263

@mbharatheesha
Copy link
Collaborator Author

I made improvements to the decoder thread management. Now, the decoder thread is monitored and confirmed it exits gracefully before reconfiguration happens. I checked this by giving the decoder thread a custom name and monitor it on htop when launch_hw is false and launch_hw is true.

launch_hw is false

image

We can see that the VelDecThread is running and is sorted to be at the lowest when sorted on % %CPU use. In this configuration, the thread was basically doing nothing as I wasn't publishing anything on velodyne_packets.

Reconfiguring phase after setting launch_hw to true

image
During the reconfiguration process, the thread is exited and joined to the calling thread. And it can also be seen on htop that the VelDecThread disappears from the htop list.

After reconfiguration complete on setting launch_hw to true

image
Once the reconfiguration is complete, the VelDecThread re-appears on the htop list. And now, it is on the top of the pile when sorted on % CPU use. This is natural because it is actually doing the work of processing lidar packets and publishing ROS pointcloud messages.

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

Successfully merging this pull request may close these issues.

1 participant