-
Notifications
You must be signed in to change notification settings - Fork 36
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
[FSM/Executor] Make resetting posture tasks optional #389
[FSM/Executor] Make resetting posture tasks optional #389
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @arntanguy
However, I think the default should be reconsidered:
- True for the main executor
- False for Meta executors
Note: this closes #353
doc/_data/schemas/State/Meta.json
Outdated
@@ -8,7 +8,8 @@ | |||
"properties": | |||
{ | |||
"Managed": { "type": "boolean", "default": false, "description": "If true, this state does not handle transitions by itself"}, | |||
"StepByStep": { "type": "boolean", "description": "Affects the StepByStep transition behaviour<br/>If not set, inherits the setting from its parent FSM" }, | |||
"StepByStep": { "type": "boolean", "default": true, "description": "Affects the StepByStep transition behaviour<br/>If not set, inherits the setting from its parent FSM" }, | |||
"ResetPostures": { "type": "boolean", "default": true, "description": "When true rest the posture tasks to the current posture before transitioning to the next state" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"ResetPostures": { "type": "boolean", "default": true, "description": "When true rest the posture tasks to the current posture before transitioning to the next state" }, | |
"ResetPostures": { "type": "boolean", "default": true, "description": "When true reset the posture tasks to the current posture before transitioning to the next state" }, |
doc/_data/schemas/State/Meta.json
Outdated
@@ -8,7 +8,8 @@ | |||
"properties": | |||
{ | |||
"Managed": { "type": "boolean", "default": false, "description": "If true, this state does not handle transitions by itself"}, | |||
"StepByStep": { "type": "boolean", "description": "Affects the StepByStep transition behaviour<br/>If not set, inherits the setting from its parent FSM" }, | |||
"StepByStep": { "type": "boolean", "default": true, "description": "Affects the StepByStep transition behaviour<br/>If not set, inherits the setting from its parent FSM" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unrelated to the PR but this documentation is actually wrong:
StepByStep
defaults to true for the main executor and false otherwiseStepByStep
does not inherits the setting from its parent FSM
Warning this may cause subtle behaviour changes if the controller relied on having the FSM automatically reset the posture between each states in a Meta state
0c5ac5f
to
fbb2270
Compare
Done, I agree this is a saner default. This might cause subtle issues in existing controllers as this changes the default reset behavior of all Meta states. I don't think this is a serious concern though, and we're better off with a good default for the future. |
Thanks for the update. Could you elaborate a bit more on how the behavior could change? |
Before this PR the posture task would be reset at every transition in a After this PR, this only happens (by default) for the main FSM and not for nested FSM. The old behavior can be re-enabled by setting |
Thank you, I understood that the behavior does not change unless nested FSM is used. |
Added --- - [mc_control] Added motor status to joint sensor (#395) - [mc_control/FSM] Posture tasks' reset can be disabled (#389) - [mc_rtc/Configuration] Added support for `std::variant` (#393) - [mc_rtc/Configuration] Added `Configuration::find` (#393) - [mc_rtc/GUI] Added form elements to provide more complex forms (#394) Changes --- - [mc_control/FSM] FSM embedded in a Meta state no longer reset the posture at transition by default (#389) Fixes --- - [mc_observers] KinematicInertial uses the correct function from state-observation (#391) - [mc_solver] Fix a crash in monitor activation - [mc_tvm] Correctly include refAccel in Orientation|PositionFunction
This PR allows to skip the reset of the posture tasks before transitioning to the next state. This has been requested multiple times, and proves useful in some cases where the user does not want to keep the current posture. Example:
This will set the desired posture to HalfSitting instead of keeping it to the current posture. Not that with
ResetPostures: true
in this example, the behaviour would be counterintuitive: the effet of the HalfSitting state is immediately canceled by the resetting of the posture tasks done by the Exectutor (as we are not awaiting for completion of the HalfSitting state).