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

Fixes lifecycle_service_client namespace #369

Merged

Conversation

cevans87
Copy link
Contributor

@cevans87 cevans87 commented Jun 10, 2019

The lifecycle demo works as expected with the following commands.
terminal 1:

ros2 run lifecycle lifecycle_talker

terminal 2:

ros2 run lifecycle lifecycle_service_client

lifecycle_service_client creates clients for
/lc_talker/get_state
/lc_talker/change_state

By providing topic names starting with '/', the clients do not automatically
prepend the current namespace. So, the following did not work as expected.
terminal 1:

ros2 run lifecycle lifecycle_talker __ns:=/foo

terminal 2:

ros2 run lifecycle lifecycle_service_client __ns:=/foo

Results in the error:

[ERROR] [foo.lc_client]: Service /lc_talker/change_state is not available.

After removing the leading '/' from the lifecycle_service_client topic names,
the demo works as expected with and without providing a namespace.

Signed-off-by: cevans87 c.d.evans87@gmail.com

The lifecycle demo works as expected with the following commands.
terminal 1:
```
ros2 run lifecycle lifecycle_talker
```
terminal 2:
```
ros2 run lifecycle lifecycle_service_client
```

lifecycle_service_client creates clients for
/lc_talker/get_state
/lc_talker/change_state

By providing topic names starting with '/', the clients do not automatically
prepend the current namespace. So, the following did not work as expected.
terminal 1:
```
ros2 run lifecycle lifecycle_talker __ns:=/foo
```
terminal 2:
```
ros2 run lifecycle lifecycle_service_client __ns:=/foo
```
Results in the error:
```
[ERROR] [foo.lc_client]: Service /lc_talker/change_state is not available.
```

After removing the leading '/' from the lifecycle_service_client topic names,
the demo works as expected when providing a namespace.

Signed-off-by: cevans87 <c.d.evans87@gmail.com>
Copy link

@thomas-moulard thomas-moulard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thomas-moulard
Copy link

@tfoote Could someone PTAL at this?

Copy link
Contributor

@Karsten1987 Karsten1987 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

CI (sanity check for linters):

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@Karsten1987 Karsten1987 self-assigned this Jun 10, 2019
@Karsten1987 Karsten1987 merged commit df5ec3a into ros2:master Jun 10, 2019
@cevans87 cevans87 deleted the fix-lifecycle-service-client-namespace branch June 10, 2019 18:50
clalancette pushed a commit that referenced this pull request Jun 12, 2019
The lifecycle demo works as expected with the following commands.
terminal 1:
```
ros2 run lifecycle lifecycle_talker
```
terminal 2:
```
ros2 run lifecycle lifecycle_service_client
```

lifecycle_service_client creates clients for
/lc_talker/get_state
/lc_talker/change_state

By providing topic names starting with '/', the clients do not automatically
prepend the current namespace. So, the following did not work as expected.
terminal 1:
```
ros2 run lifecycle lifecycle_talker __ns:=/foo
```
terminal 2:
```
ros2 run lifecycle lifecycle_service_client __ns:=/foo
```
Results in the error:
```
[ERROR] [foo.lc_client]: Service /lc_talker/change_state is not available.
```

After removing the leading '/' from the lifecycle_service_client topic names,
the demo works as expected when providing a namespace.

Signed-off-by: cevans87 <c.d.evans87@gmail.com>
clalancette pushed a commit that referenced this pull request Jun 12, 2019
The lifecycle demo works as expected with the following commands.
terminal 1:
```
ros2 run lifecycle lifecycle_talker
```
terminal 2:
```
ros2 run lifecycle lifecycle_service_client
```

lifecycle_service_client creates clients for
/lc_talker/get_state
/lc_talker/change_state

By providing topic names starting with '/', the clients do not automatically
prepend the current namespace. So, the following did not work as expected.
terminal 1:
```
ros2 run lifecycle lifecycle_talker __ns:=/foo
```
terminal 2:
```
ros2 run lifecycle lifecycle_service_client __ns:=/foo
```
Results in the error:
```
[ERROR] [foo.lc_client]: Service /lc_talker/change_state is not available.
```

After removing the leading '/' from the lifecycle_service_client topic names,
the demo works as expected when providing a namespace.

Signed-off-by: cevans87 <c.d.evans87@gmail.com>
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.

3 participants