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.

Sign up for GitHub

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

Jump to bottom

add daemon command with verbs status, start, stop #38

Merged
merged 1 commit into from
Jun 30, 2017
Merged

add daemon command with verbs status, start, stop #38

merged 1 commit into from
Jun 30, 2017

Conversation

dirk-thomas
Copy link
Member

Addresses #34.

@dirk-thomas dirk-thomas added the in review Waiting for review (Kanban column) label Jun 30, 2017
@dirk-thomas dirk-thomas self-assigned this Jun 30, 2017
@Kukanani
Copy link

LGTM, confirmed working on Win 10.

Would it be useful to add some context to the help output, explaining what the daemon is/does? Maybe just a pointer to documentation, if such documentation exists.

@clalancette
Copy link
Contributor

This also works for me on Linux. There's a couple of overall things that would make this nicer:

  1. In all of the sub-commands, it would be nice to tell the user which ros-domain-id we are starting/stopping/statusing the daemon.
  2. Starting and stopping the daemon for a particular domain id is controlled through the environment variable ROS_DOMAIN_ID. How do we control the rmw_implementation? It would be nice to document how to control both of these somewhere.

@dirk-thomas
Copy link
Member Author

Thanks for testing. More documentation is always nice. I am not sure I will add it right now though. If you have specific ideas what to add please consider making a PR.

The rmw impl. is selected by the environment variable RMW_IMPLEMENTATION. If not set a default is being selected. So "normal" users will only use one rmw impl at a time I don't think it needs exposure in the tool atm.

@mikaelarguedas
Copy link
Member

Starting and stopping the daemon for a particular domain id is controlled through the environment variable ROS_DOMAIN_ID. How do we control the rmw_implementation? It would be nice to document how to control both of these somewhere.

I think both of them (ROS_DOMAIN_ID and RMW_IMPLEMENTATION) should be documented globally because they apply to every ROS2 executable and not only this tool. For now the rmw information is documented in the Working-with-multiple-RMW-implementations wiki page

@dirk-thomas
Copy link
Member Author

Merging this for now as is. With potential improvements coming in the future.

@dirk-thomas dirk-thomas merged commit 48bd83b into master Jun 30, 2017
@dirk-thomas dirk-thomas deleted the add_daemon_command branch June 30, 2017 21:01
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Jun 30, 2017
@dirk-thomas
Copy link
Member Author

Follow up fix in 45bbfab

esteve pushed a commit to esteve/ros2cli that referenced this pull request Dec 16, 2022
@mikaelarguedas
Pass environment to launched processes (ros2#38)
aa66d2b 
* pass env parameter to subprocess

* add unit test

* restore original file path

* adding back main

* use deepcopy to be sure to use disctinct dictionaries

* add copyright

* move import to module level, remove unused brackets assert

* add env to kwargs

* change key existence checking logic

* its 2016 already
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@dirk-thomas dirk-thomas

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@dirk-thomas @Kukanani @clalancette @mikaelarguedas