Adds support for shutting down action clients

[manzato]

I know ActionClients normally are long-lived objects, but I needed to create & destroy them with certain frequency, and that lead to a warning message from EventEmitter:

W20170919-11:11:57.897(-3)? (STDERR) Trace
W20170919-11:11:57.897(-3)? (STDERR)     at SubscriberImpl.addListener (events.js:239:17)
W20170919-11:11:57.897(-3)? (STDERR)     at Ultron.on (/app/node_modules/ultron/index.js:42:11)
W20170919-11:11:57.897(-3)? (STDERR)     at rebroadcast (/app/node_modules/rosnodejs/dist/utils/event_utils.js:5:13)
W20170919-11:11:57.898(-3)? (STDERR)     at new Subscriber (/app/node_modules/rosnodejs/dist/lib/Subscriber.js:60:5)
W20170919-11:11:57.898(-3)? (STDERR)     at RosNode.subscribe (/app/node_modules/rosnodejs/dist/lib/RosNode.js:137:17)
W20170919-11:11:57.898(-3)? (STDERR)     at NodeHandle.subscribe (/app/node_modules/rosnodejs/dist/lib/NodeHandle.js:131:27)
W20170919-11:11:57.898(-3)? (STDERR)     at new ActionClient (/app/node_modules/rosnodejs/dist/lib/ActionClient.js:73:27)
W20170919-11:11:57.898(-3)? (STDERR)     at NodeHandle.actionClient (/app/node_modules/rosnodejs/dist/lib/NodeHandle.js:226:14)
W20170919-11:11:57.898(-3)? (STDERR)     at MyActionClient.run (imports/ros/my-action-client.js:170:30)
W20170919-11:11:57.899(-3)? (STDERR) (node) warning: possible EventEmitter memory leak detected. 11 error listeners added. Use emitter.setMaxListeners() to increase limit.
W20170919-11:11:57.899(-3)? (STDERR) Trace

This PR adds a shutdown method to the action client that cancels subscribers & publishers as well as removing attached listeners.

[chris-smith]

Awesome! I should have included this initially - glad it's getting in. Just a request to add a return value

[chris-smith]

could you return all these shutdowns in a Promise.all() ? Publishers and subscribers return promises on shutdown so that callers can know when they are fully destroyed (unregistered from the ROS master, clients disconnected, etc)

[chfritz]

@chris-smith : just as a means of introducing him here, @manzato is on my team (which may not be obvious from his profile). We have been using rosnodejs for a while now and it's gotten a lot of mileage that way -- literally, actually :-)

Looking forward to your feedback on this PR. In particular, I'm wondering what you think the lifecycle of an action client should be: long-lived, i.e., created once and reused when needed, or created on demand and destroyed when done. I think the main concern about the long-lived approach is that there will be a lot of subscribers that receive messages while they really don't need to.

Understanding that will allow us to better design the upper layers of our architecture.

[chris-smith]

Hey @chfritz - really happy to hear you're still using rosnodejs!

I definitely think action clients should be able to be shutdown. I know at Rethink we have lots of instances of needing to destroy and create pubs/subs/services at run time. That seems atypical if you look at a lot of the examples in ROS but we've found it necessary and useful. It does become more important to pay attention to the initialization state of everything once you're creating and destroying them on the fly but depending on what you're doing it may not be a problem.

I'd recommend adding the return to the action client's shutdown method but otherwise it looks good!

[manzato]

@chris-smith comments addressed. Thanks!

[chris-smith]

Nice!