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

When stopping a capability, stop all dependent capabilities as well #37

Merged
merged 3 commits into from Dec 18, 2013
Merged

When stopping a capability, stop all dependent capabilities as well #37

merged 3 commits into from Dec 18, 2013

Conversation

esteve
Copy link
Contributor

@esteve esteve commented Dec 17, 2013

Doesn't look like this is happening right now.

@wjwwood
Copy link
Member

wjwwood commented Nov 10, 2013

This was working before, but it may have regressed.

Note that it will only stop them if they were not started by the user, i.e. if bar depends on foo and you start foo, and then bar, then stop bar, foo won't automatically stop.

@bit-pirate
Copy link
Contributor Author

It works only, if one stops the same capability, which he has started before. Then all the cap's dependencies, which have been started automatically, are also stopped automatically. Stopping any other cap will only stop that cap's dependencies.

IMO all caps, which are running and dependent on each other should be stopped, e.g. it doesn't make sense to me to keep the FakeLaser (depthimage_to_laserscan) running, when the RGBDSensor (Kinect driver) has been stopped. Am I missing a valid example to keep dependent caps running?

@wjwwood
Copy link
Member

wjwwood commented Nov 10, 2013

So you're saying that if fake_laser depends on kinect and I start fake_laser, which in turn starts kinect, and I then stop kinect, fake_laser continues to run?

@bit-pirate
Copy link
Contributor Author

So you're saying that if fake_laser depends on kinect and I start fake_laser, which in turn starts kinect, and I then stop kinect, fake_laser continues to run?

Correct.

@ghost ghost assigned esteve Dec 16, 2013
@esteve
Copy link
Contributor

esteve commented Dec 17, 2013

I fixed this issue, but it seems Travis is having trouble installing empy, I'll have a look at it. Anyway, I ran the testsuite locally and all the tests passed.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d31847b on esteve:stop-dependencies into 59d09f0 on osrf:master.

@bit-pirate
Copy link
Contributor Author

Tested. Very nice! 👍

Please merge!

esteve added a commit that referenced this pull request Dec 18, 2013
When stopping a capability, stop all dependent capabilities as well
@esteve esteve merged commit f065699 into osrf:master Dec 18, 2013
@esteve esteve deleted the stop-dependencies branch December 18, 2013 07:54
@wjwwood
Copy link
Member

wjwwood commented Dec 18, 2013

Agreed, +1

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

Successfully merging this pull request may close these issues.

None yet

4 participants