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

Name remapping specific to a fully-qualified node's name #296

Open
dhood opened this issue Sep 11, 2018 · 9 comments · May be fixed by #807
Open

Name remapping specific to a fully-qualified node's name #296

dhood opened this issue Sep 11, 2018 · 9 comments · May be fixed by #807
Assignees
Labels
enhancement New feature or request

Comments

@dhood
Copy link
Member

dhood commented Sep 11, 2018

Remapping can be passed to a specific node with e.g. talker:chatter:=my_chatter.
Suppose I have multiple nodes named talker in a process, but in different namespaces.
Is the ability to address the nodes individually implemented yet, so that I can do e.g. /ns1/talker:chatter:=my_chatter? I think not; am looking for confirmation. @sloretz

@dhood dhood added the question Further information is requested label Sep 11, 2018
@sloretz
Copy link
Contributor

sloretz commented Sep 11, 2018

The node-name prefix handling assumes node names are unique (edit: that is, not including the namespace) within a process before remapping.

@dhood
Copy link
Member Author

dhood commented Sep 11, 2018

Usually when we refer to node name uniqueness we are actually referring to "ns+node name" uniqueness. To clarify your statement: the remapping process is assuming there is only one talker in the process, in any namespace, and it is not possible to address them individually, is that right?

@sloretz
Copy link
Contributor

sloretz commented Sep 11, 2018

To clarify your statement: the remapping process is assuming there is only one talker in the process, in any namespace, and it is not possible to address them individually, is that right?

Correct

@wjwwood
Copy link
Member

wjwwood commented Sep 11, 2018

I think it has to consider the node namespace as well, because having two nodes with the same name but in different namespaces will be (has been in the past) a common case.

@mikaelarguedas
Copy link
Member

Usually when we refer to node name uniqueness we are actually referring to "ns+node name" uniqueness.

👍

I think it has to consider the node namespace as well, because having two nodes with the same name but in different namespaces will be (has been in the past) a common case.

👍

Agreed I think it should consider the namespace and assume uniqueness of "ns+node name".

I often use the example of a stereo image processing process that is a basic/common example of a process having 2 instances of a "camera" node under different namespaces (camera_left, camera_right).

@mikaelarguedas mikaelarguedas added enhancement New feature or request and removed question Further information is requested labels Sep 13, 2018
@iuhilnehc-ynos
Copy link
Collaborator

It seems that this feature is still not supported in ros2. I'd like to contribute to this issue.

$ ros2 run demo_nodes_cpp talker --ros-args --remap talker:chatter:=my_chatter
[INFO] [1600251961.967244593] [talker]: Publishing: 'Hello World: 1'
[INFO] [1600251962.967163181] [talker]: Publishing: 'Hello World: 2'

$ ros2 topic list
/my_chatter

but failed to run the following command line,

$ ros2 run demo_nodes_cpp talker --ros-args --remap /talker:chatter:=my_chatter
[ERROR] [1600251921.506736587] [rcl]: Failed to parse global arguments

>>> [rcutils|error_handling.c:108] rcutils_set_error_state()
This error state is being overwritten:

  'Couldn't parse remap rule: '--remap /talker:chatter:=my_chatter'. Error: Expected lexeme type 19, got 5 at index 7, at /home/sharedata/Linux/ROS2/docker_ros2_master/src/ros2/rcl/rcl/src/rcl/lexer_lookahead.c:238, at /home/sharedata/Linux/ROS2/docker_ros2_master/src/ros2/rcl/rcl/src/rcl/arguments.c:371'

with this new error message:

  'context is zero-initialized, at /home/sharedata/Linux/ROS2/docker_ros2_master/src/ros2/rcl/rcl/src/rcl/context.c:52'

rcutils_reset_error() should be called after error handling to avoid this.
<<<
[ERROR] [1600251921.506782928] [rclcpp]: failed to finalize context: context is zero-initialized, at /home/sharedata/Linux/ROS2/docker_ros2_master/src/ros2/rcl/rcl/src/rcl/context.c:52
terminate called after throwing an instance of 'rclcpp::exceptions::RCLInvalidROSArgsError'
  what():  failed to initialize rcl: error not set

@iuhilnehc-ynos
Copy link
Collaborator

Actually, I found --param can also support this feature. After this issue, I'll create a new PR for --param.

@iuhilnehc-ynos
Copy link
Collaborator

To remap __node and __ns with a fully-qualified node's name, such as -r /my_talker:__ns:=/ns1 -r /talker:__node:=my_talker (current remapping order) or -r /talker:__ns:=/ns1 -r /ns1/talker:__node:=my_talker(if remmaping namespace before node name, #806), I need to update two functions by adding namespace.

rcl/rcl/include/rcl/remap.h

Lines 199 to 207 in 8bcada7

RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
rcl_remap_node_name(
const rcl_arguments_t * local_arguments,
const rcl_arguments_t * global_arguments,
const char * node_name,
rcl_allocator_t allocator,
char ** output_name);

rcl/rcl/include/rcl/remap.h

Lines 240 to 248 in 8bcada7

RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
rcl_remap_node_namespace(
const rcl_arguments_t * local_arguments,
const rcl_arguments_t * global_arguments,
const char * node_name,
rcl_allocator_t allocator,
char ** output_namespace);

@iuhilnehc-ynos
Copy link
Collaborator

@wjwwood

I have created a new PR for this issue at ros2/design#299 that also including #806, could you please help review it?

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

Successfully merging a pull request may close this issue.

5 participants