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

Zivid nodelet can't attach to a manager in the global namespace #84

Closed
danielcranston opened this issue Apr 5, 2023 · 2 comments
Closed

Comments

@danielcranston
Copy link
Contributor

danielcranston commented Apr 5, 2023

Summary

Hi,

I think it's great that the driver itself enforces REP-135, but I noticed a situation where this enforcement is too strict.

It has to do with the nodelet version of the driver. The method the driver uses to determine if it was started in the global namespace (the thing that REP-135 prohibits) is ros::this_node::getNamespace().

For nodelets, this resolves to the namespace of the nodelet manager, not the Zivid driver nodelet (at least in ROS Melodic). This means that, a Zivid driver nodelet, brought up in its own namespace but attached to a manager in the global namespace, will fail to start (claiming it doesn't satisfy REP-135, but in fact it does).

I made a commit with a proposed solution here: danielcranston@473bf4e

How to reproduce

To test, you can use this minimal launchfile

<launch>
    <arg name="ns" default="/" />

    <node pkg="nodelet" type="nodelet" name="nodelet_manager" args="manager" output="screen" />
    <node ns="$(arg ns)" pkg="nodelet" type="nodelet" name="latch" args="load zivid_camera/nodelet /nodelet_manager" output="screen" />
</launch>

Notice how the manager is in the global namespace and the Zivid driver nodelet is put in $(arg ns).

roslaunch test.launch ns:=zivid fails on master, but succeeds on danielcranston@473bf4e. Starting the nodelet (or node) in the global namespace still has the same (intended) behavior as before.

I guess one could make the argument that nodelet managers shouldn't be put in the global namespace, but I don't interpret anything in REP-135 that would discourage it, and I think it's a pretty common practice to do so.

Thanks for reading.

@apartridge
Copy link
Collaborator

apartridge commented Jun 12, 2024

Hi @danielcranston , thanks for your report and fix. I have added a PR for it here #99 and we will aim to merge it in soon.

@apartridge
Copy link
Collaborator

Solved with #99

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

No branches or pull requests

2 participants