Lifecycle design #335

alsora · 2023-08-17T17:47:16Z

No description provided.

clalancette · 2023-08-24T17:06:52Z

@SteveMacenski Since Nav2 is a heavy user of lifecycle, would you mind giving this a look? We'd be very interested in your feedback.

SteveMacenski · 2023-08-24T17:33:03Z

It'll be next week at this point for me, but yes I will! The only thing I see immediately that I would ask a question about is the throwing of exceptions. Who, if anyone, is able to catch that error in the state transition? It would be unacceptable if that was uncatchable by the user / rclcpp such that the server crashed.

alsora · 2023-08-24T18:02:58Z

@clalancette @SteveMacenski This wasn't supposed to be a public-facing PR, but rather to first go through a last round of internal reviews (I should have targeted our fork rather than the official repo)

I won't take it down but I converted it to draft.

SteveMacenski · 2023-08-29T23:46:10Z

articles/node_lifecycle.md


-To transition out of a primary state requires action from an external supervisory process, with the exception of an error being triggered in the `Active` state.


I think this remark has value to keep (unless you just moved it elsewhere)

SteveMacenski · 2023-08-29T23:48:21Z

articles/node_lifecycle.md


-### Primary State: Unconfigured
+### Primary State Definitions and Expected Behaviors
+Primary States allow for assumptions and expected behavior of the Lifecycle Node with respect to the  node’s [Managed Entities](#managed-entities). 


I have no idea what this sentences are trying to communicate. I also don't understand what the second sentence in this block is trying to say that shouldn't have already been made clear when introducing the idea of transition states.

SteveMacenski · 2023-08-29T23:50:02Z

articles/node_lifecycle.md

-This is the life cycle state the node is in immediately after being instantiated.
-This is also the state in which a node may be retuned to after an error has happened.
-In this state there is expected to be no stored state.
+Note "processing" refers to Managed Entities receiving/sending messages, those messages being propagated through the `ROS 2` stack, and user callbacks processing these messages with respect to the executor.


I understand fully what this is communicating, but I think its overly technocratic.

"Processing in managed entities means messages, services, actions, etc are being processed, whereas not processing indicates that messages are not being sent or received by the middleware."

SteveMacenski · 2023-08-29T23:53:07Z

articles/node_lifecycle.md


-The main purpose of this state is to allow a node to be (re-)configured (changing configuration parameters, adding and removing topic publications/subscriptions, etc) without altering its behavior while it is running.
+#### **Primary State: `Inactive`**
+ This state represents a node that is performing minimal processing with allocated Managed Entities.


What is "minimal processing" the original documents' seems more clear (maybe just update this with the entity language)

This state represents a node that is not currently performing any processing.

SteveMacenski · 2023-08-29T23:54:26Z

articles/node_lifecycle.md

-The main purpose of this state is to allow a node to be (re-)configured (changing configuration parameters, adding and removing topic publications/subscriptions, etc) without altering its behavior while it is running.
+#### **Primary State: `Inactive`**
+ This state represents a node that is performing minimal processing with allocated Managed Entities.
+ Thus, as defined in the [Managed Entities](#managed-entities) section, the Managed Entities will not be added to the `ROS` graph with data retention to topics following that of an unallocated Persistent Entity.


If pre-defined and within the definition, it needs not be restated repeatedly. Especially when the sentence prior basically just says "not processing"

SteveMacenski · 2023-08-30T00:08:28Z

articles/node_lifecycle.md

+If the user accepts and successfully handles the cancellation request, the state will follow the `CallbackReturn::FAILURE` path of the state machine. 
+This follows the "fallback" behavior of a transition.
+If the user accepts and unsuccesfully handles the cancellation request, the state will follow the `CallbackReturn::ERROR` path of the state machine. 
+An ignored cancellation request becomes inactive and failed the instant a transition is inactive.


Suggested change

An ignored cancellation request becomes inactive and failed the instant a transition is inactive.

An ignored cancellation request becomes inactive and failed the instant a transition is inactive.

I would think that we'd basically transition back down to what the cancel would have done for us if it were processed properly. E.g. If transitioning to Active and cancel is ignored, we just force the state machine back down to Configured.

Or, if cancel accepted + processed puts it always back in the unconfigured state, then an ignored cancel request would transition the system back down the unconfigured automatically so it seems as if it were handled regardless

SteveMacenski · 2023-08-30T00:09:09Z

articles/node_lifecycle.md


-### Destroy Transition
+#### **Operation `raise_error()`**
+This operation transitions the node from the primary states `{Unconfigured, Inactive, Active}` to `ErrorProcessing`.


What exceptions can I throw that will do this? I think that needs to be explicit (e.g. std::runtime_error)

SteveMacenski · 2023-08-30T00:11:14Z

articles/node_lifecycle.md


-### Create Transition
+### Return Values


I get that for transition states, but is that true for primary states as well? So no exception catching to cause us to go into error processing if an uncaught exception is thrown in a primary state? I think that's untrue given how the primary states work, so I think this needs clarification that we're only referring to transition states.

SteveMacenski · 2023-08-30T00:15:38Z

articles/node_lifecycle.md

-It will publish every time that a transition is triggered, whether successful or not.
+### Supported Interfaces
+
+This section outlines Persistent Entities created by the backend of Lifecycle nodes. These Persistent Entities make up the external facing management interface of a Lifecycle node. Each of these external interfaces must also include an internal equivalent (i.e., a function the user can call from within the node itself).


L373 - L383 I have no idea what you're trying to communicate or why its an important thing to distinctly discuss

Actually, this entire subsection "Supported Interfaces" I don't understand. Something is really unclear about what the intention is. There's some stuff about the ROS interfaces (e.g. topics/services) which I ack as standard practice, but no idea what anything before "The following interface behaviors will be supported" seems unrelated.

Also, all the interfaces are CamelCase and if these are supposed to be topics, then theey should be snake_case. If these are interface message types, that's unclear.

SteveMacenski · 2023-08-30T00:19:06Z

articles/node_lifecycle.md

+
+## Managed Entities
+
+Managed entities transition with the state machine, being automatically allocated and spun. 


Huh (on this whole subsection)

SteveMacenski · 2023-08-30T00:19:27Z

More general thoughts

I'm not sure why ROS 2 is in code-styling, its quite distracting.
I don't know if Persistent Entities is already part of this technology's lexicon. If its not, I might suggest changing it. I'm not sure it describes well what you want to differentiate. May I suggest Unmanaged Entities.
The T:Shutdown in the error processing marked half of the diagram is out of place and should be on the right hand side
Also, the diagram should be inverted, the left (first) half should be the main state machine. Its confusing to inspect it in reverse
I think some more of the concepts needs to be introduced up top before going into describing all the states. I don't think we should wait until introducing all the transition states to even tell someone what that means. Same with primary states, although its less an issue since its the first up. I think there needs to be a concepts section after Background to talk about the types of states like you do types of entities. Then you can lump all the states + expected behaviors in one section.

There are lots of things in here that are overly technocratic that even someone that uses lifecycle nodes every day it takes me 2-3 sometimes 4 times reading through to actually understand what you mean. I'd recommend looking over the original document where the same points were addressed previously that were sufficiently technical (might benefit from more detail) but written in language that I can grokk.

Thomas Groechel and others added 11 commits August 2, 2023 09:31

Fill in Lifecycle Node design gaps

b7eff30

include state machine draw.io

f4d27b9

update managed entities active state schemes

07733f4

mid-transition failure

e526f84

error

5174a15

captial

9d11292

more caps

1c89962

Fill in Lifecycle Node design gaps

a4c1af2

merge

4cea94a

Fixed typo in state machine diagram

d532593

Cleaned up state diagram image

6b5d5bf

clalancette assigned fujitatomoya Aug 24, 2023

alsora marked this pull request as draft August 24, 2023 18:01

SteveMacenski reviewed Aug 30, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lifecycle design #335

Lifecycle design #335

alsora commented Aug 17, 2023

clalancette commented Aug 24, 2023

SteveMacenski commented Aug 24, 2023

alsora commented Aug 24, 2023 •

edited

Loading

SteveMacenski Aug 29, 2023

SteveMacenski Aug 29, 2023

SteveMacenski Aug 29, 2023

SteveMacenski Aug 29, 2023

SteveMacenski Aug 29, 2023

SteveMacenski Aug 30, 2023

SteveMacenski Aug 30, 2023

SteveMacenski Aug 30, 2023

SteveMacenski Aug 30, 2023

SteveMacenski Aug 30, 2023

SteveMacenski Aug 30, 2023

SteveMacenski commented Aug 30, 2023 •

edited

Loading


		To transition out of a primary state requires action from an external supervisory process, with the exception of an error being triggered in the `Active` state.

	An ignored cancellation request becomes inactive and failed the instant a transition is inactive.
	An ignored cancellation request becomes inactive and failed the instant a transition is inactive.


		## Managed Entities

		Managed entities transition with the state machine, being automatically allocated and spun.

Lifecycle design #335

Are you sure you want to change the base?

Lifecycle design #335

Conversation

alsora commented Aug 17, 2023

clalancette commented Aug 24, 2023

SteveMacenski commented Aug 24, 2023

alsora commented Aug 24, 2023 • edited Loading

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

SteveMacenski commented Aug 30, 2023 • edited Loading

alsora commented Aug 24, 2023 •

edited

Loading

SteveMacenski commented Aug 30, 2023 •

edited

Loading