Define external life cycle management interface #99

gbiggs · 2016-11-09T16:51:41Z

The previous version of the document only gave an ambiguous description of the external interface used to manage a node's life cycle. This pull request describes the interface in detail, including topic names, and message and service definitions.

dirk-thomas · 2016-11-09T17:28:18Z

Some high level feedback for now:

Do we need a specific service for every requested transition? Having a single service which takes an argument would make the API more flexible (since it would work for any state machine defined).
Currently ROS 2 does neither have the concept of a namespace nor latching. While we hope to support something similar in the future it might be problematic to refer to those terms in the design doc for now.

Regarding the Travis error: every sentence in the markdown files should start on a new line. Hopefully if you update the patch the ugly error should go away.

gbiggs · 2016-11-09T19:00:55Z

Both good points. Thanks, Dirk. I've made changes for both.

Karsten1987 · 2016-11-10T05:37:19Z

I'd like to propose to rename create and destroy into init and deinit respectively. I am having trouble with the word create, since we don't have a factory which creates nodes and we encourage users to inherit from a generic base class. That being said, a create function is more like a constructor and a destroy is a destructor which never really gets called externally.

Concretely speaking, this would result in an non-op constructor, getting called when a user instantiates a new node. The respective init function, then, which is responsible for enabling all pub/sub etc, can get triggered externally.

gbiggs · 2016-11-10T08:03:14Z

I don't feel that renaming them to init and deinit is correct. These two functions are more intended to be provided by a container system that has a factory to create and destroy nodes on demand, rather than by the nodes themselves. This is why they are not included in the node's external interface specification. If a managed node is created by a user in their own main(), then there will be no create and destroy calls.

If I clarify the document to make this clear, would that be satisfactory?

jack-oquin · 2016-11-10T14:21:15Z

If I clarify the document to make this clear, would that be satisfactory?

+1; that would be a useful improvement.

jack-oquin · 2016-11-10T14:31:51Z

Overall, this document looks good to me.

Maybe I just missed it, but where and how does the management interface return the current state?

gbiggs · 2016-11-11T14:40:22Z

The current state is broadcast on a topic, and is also accessible via a service.

gbiggs · 2016-11-11T14:41:26Z

I clarified that create and destroy are functions of a container system, but I think the document could do with more detail on what a container is and what interface it should provide. However, that needs more discussion about what, if anything, we want to do with containers.

Karsten1987 · 2016-12-16T16:50:23Z

articles/node_lifecycle.md

-It is expected that a common pattern will be to have a container class which loads a managed node implementation from a library and through a plugin architecture automatically exposes the required management interface via methods and the container is not subject to the lifecycle management.
-However, it is fully valid to consider any implementation which provides this interface and follows the lifecycle policies a managed node.
-Conversely, any object that provides these services but does not behave in the way defined in the life cycle state machine is malformed.
+The interface shall be provided in a namespace named "infra/lifecycle" underneath the node's namespace.


As we don't have namespaces in ROS2.0 for the moment, I believe this paragraph can not be merged as is. Currently, the topics and services are advertised with *__get_state, *__change_state and *__transition_event

Which is still a kind of namespace - just not with a / but with __.

Now that namespace support is being implemented, is this paragraph acceptable?

It should probably make use of the leading _ in topic names to make it "hidden":

http://design.ros2.org/articles/topic_and_service_names.html#hidden-topic-or-service-names

Otherwise, it uses a namespace as currently written, so I'm not sure what else you might be referring to.

I was referring to @dirk-thomas 's comment that it doesn't use an actual namespace (separated with a /) but uses __ to mimic a namespace. The proposal from @Karsten1987 is to change the topics used by the lifecycle interface to not use namespaces.

That's what I was getting at.

And on going to add the underscore to make it a hidden interface, I found myself unsure where to add it. On infra or on lifecycle? Is infra going to be a widely-used thing for ROS infrastructure topics? (I hope it is.)

I don't know how I feel about infra over infrastructure or impl or ros, or even just letting individual things create their own "impl" namespaces. For example, maybe the common namespace for these things should be just ~/_lifecycle/. But in general, namespacing the plumbing is something I think we should be doing.

I'm in favour of having all infrastructure stuff under a single namespace because it makes things tidier, but I can see the argument of "who defines/controls what is 'infrastructure' enough to go in there?".

I can see that angle too. It seems to be a bit a bikeshed thing atm. I'm fine with leaving it as-is because we can always come back and change it later once we get more of these "infrastructure" topics and services in place. Perhaps then a better pattern with emerge.

OK, I make the infra namespace hidden. We can wait a while and see if anything else starts using this same namespace. If not, we can remove it.

Karsten1987 · 2016-12-16T16:53:04Z

articles/node_lifecycle.md

+    uint8 SHUTTING_DOWN=12
+    uint8 ACTIVATING=13
+    uint8 DEACTIVATING=14
+    uint8 ERROR_PROCESSING=15


we can link to the actual msg: https://github.com/ros2/rcl_interfaces/blob/master/lifecycle_msgs/msg/State.msg

Should the msg file follow the specification, or should the specification reference the msg? One needs to be considered the master definition, and the other needs to be kept in sync with it.

Although I prefer keeping the specification document as the master definition, when considering maintenance, having the spec link to the msg file would be easier.

I don't mind either way. Linking to the spec might have the problem of tracking changes over time. Then again it may be just as hard to represent that in this document as well. So I don't see that one way is strictly better than the other. At least with the interface file being the sole authority, you can avoid some duplication of information if you choose.

Having thought about it more, I prefer keeping the spec as the master definition. In theory there could be other implementations. I will add a link, though.

Karsten1987 · 2016-12-16T16:53:27Z

articles/node_lifecycle.md

+
+### Life cycle state changes topic
+
+When the node's life cycle changes, it shall broadcast the following message on the `infra/lifecycle/state_change` topic.


`<node_name>__transition_event'

See above about namespaces.

Karsten1987 · 2016-12-16T16:54:11Z

articles/node_lifecycle.md

+
+### Current life cycle state service
+
+The node's current life cycle state shall be available via the `infra/lifecycle/get_state` service.


<node_name>__get_state

See above about namespaces.

Karsten1987 · 2016-12-16T16:54:50Z

articles/node_lifecycle.md

+
+### Transition request service
+
+The service `infra/lifecycle/change_state` service shall be provided by the life cycle interface.


<node_name>__change_state

See above about namespaces.

dirk-thomas · 2017-04-10T17:50:08Z

@gbiggs Can you please address the pending comments.

This reverts commit 391764a.

This reverts commit 1ef7e71.

This reverts commit 7a7a72d.

Define external life cycle management interface

1052ecc

gbiggs mentioned this pull request Nov 9, 2016

Lifecyle node and C state_machine ros2/rclcpp#265

Merged

Combined transition control services into one

c0f964b

Clarify that create and destroy are container functionality

db9b3a8

Karsten1987 reviewed Dec 16, 2016

View reviewed changes

Geoffrey Biggs and others added 9 commits April 17, 2017 15:40

Add link to message definition file for state enum

6a96db1

Make the infra namespace hidden

f7edec9

Merge remote-tracking branch 'upstream/gh-pages' into gh-pages

47068fa

Start of article about actions

7a7a72d

Unwrap

1ef7e71

Shift over to upstream's version of node_lifecycle.md

391764a

Revert "Shift over to upstream's version of node_lifecycle.md"

c1afa2d

This reverts commit 391764a.

Revert "Unwrap"

a8cee35

This reverts commit 1ef7e71.

Revert "Start of article about actions"

f65f48d

This reverts commit 7a7a72d.

gbiggs mentioned this pull request Sep 6, 2017

Start article defining actions #143

Closed

clalancette added ready Work is about to start (Kanban column) requires-changes and removed ready Work is about to start (Kanban column) labels Apr 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Define external life cycle management interface #99

Define external life cycle management interface #99

gbiggs commented Nov 9, 2016

dirk-thomas commented Nov 9, 2016 •

edited

Loading

gbiggs commented Nov 9, 2016 •

edited

Loading

Karsten1987 commented Nov 10, 2016

gbiggs commented Nov 10, 2016

jack-oquin commented Nov 10, 2016

jack-oquin commented Nov 10, 2016

gbiggs commented Nov 11, 2016

gbiggs commented Nov 11, 2016

Karsten1987 Dec 16, 2016

dirk-thomas Dec 16, 2016

gbiggs Apr 11, 2017

wjwwood Apr 17, 2017

gbiggs Apr 17, 2017

gbiggs Apr 17, 2017

wjwwood Apr 17, 2017

gbiggs Apr 17, 2017

wjwwood Apr 17, 2017

gbiggs Apr 17, 2017

Karsten1987 Dec 16, 2016

gbiggs Apr 11, 2017

wjwwood Apr 17, 2017

gbiggs Apr 17, 2017

Karsten1987 Dec 16, 2016

gbiggs Apr 11, 2017

Karsten1987 Dec 16, 2016

gbiggs Apr 11, 2017

Karsten1987 Dec 16, 2016

gbiggs Apr 11, 2017

dirk-thomas commented Apr 10, 2017


		### Life cycle state changes topic

		When the node's life cycle changes, it shall broadcast the following message on the `infra/lifecycle/state_change` topic.


		### Current life cycle state service

		The node's current life cycle state shall be available via the `infra/lifecycle/get_state` service.


		### Transition request service

		The service `infra/lifecycle/change_state` service shall be provided by the life cycle interface.

Define external life cycle management interface #99

Are you sure you want to change the base?

Define external life cycle management interface #99

Conversation

gbiggs commented Nov 9, 2016

dirk-thomas commented Nov 9, 2016 • edited Loading

gbiggs commented Nov 9, 2016 • edited Loading

Karsten1987 commented Nov 10, 2016

gbiggs commented Nov 10, 2016

jack-oquin commented Nov 10, 2016

jack-oquin commented Nov 10, 2016

gbiggs commented Nov 11, 2016

gbiggs commented Nov 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dirk-thomas commented Apr 10, 2017

dirk-thomas commented Nov 9, 2016 •

edited

Loading

gbiggs commented Nov 9, 2016 •

edited

Loading