Closed mmurooka closed 2 years ago
If this understanding is correct, it would be helpful if it were clearly stated somewhere in the following lines or tutorials.
It is correct. I am going to elaborate a little here on why this is like this and how it could be changed.
This documentation states that:
configure
is used to configure the state; one important feature to consider and understand with the FSM implementation is that new states can be created by specifying a different configuration of an existing state. This function will thus be called multiple times;
And the doxygen does say:
This might be called multiple times.
But clearly, the doc should be clarified and maybe the default behavior should be changed.
The original intention behind this behavior could be used to circumvent the limitations/rules of mc_rtc::Configuration::load(const mc_rtc::Configuration &)
. For example this could allow to extend a list rather than overwriting it.
However in practice:
My personal go to implementation is:
void MyState::configure(const mc_rtc::Configuration & config)
{
config_.load(config);
}
void MyState::start(mc_control::fsm::Controller & ctl)
{
// Use config_ from here
}
So I can see two ways to evolve it in the future:
configure
; we lose no flexibility but the vast majority of users won't need to bother with implementing configure
anymoreconfigure
function all together (we keep the function around but it is not called inside mc_rtc) and just load the successive configuration in the state loader then provide this final configuration to the state; we lose the flexibility (that's not super useful) but state loading will be greatly simplifiedI have one more question. When transitioning to the same state, is a new instance of the state created? That is, is the destructor of the old state instance called, and are the members of the new state instance initialized?
Yes. The FSM executor always does this regardless of the previous state. When a state ::start
is called it's always a clean slate.
@gergondet Thank you for the answer.
My personal go to implementation is:
Thanks. After I understand this behavior, I've just done completely same thing just before seeing your comments ;) (You can also see previous hotfix in the removed lines.) https://github.com/mmurooka/Motion6DoF/commit/34fa4c196d55e381b26e814769ba0a4317b65ac1
This documentation states that:
So I just missed it, but it was already in the Tutorial. But it is a little difficult to understand the current behavior from this description alone.
However in practice: I don't think anybody is actually doing that (and actually doing it might be misleading because people have some expectation of what the YAML composition will do) this leads to badly written configure function (e.g. always expecting all the data to be available) or this leads to a lot of superfluous work in the configure method (setting some parameters multiple times)
I agree :+1:
The behavior of
configure
method for derived state (i.e., the state defined in yaml file with other state as base) was not well documented and unclear (at least to me)By adding a print statement and trying it, I understood as follows. The configure method is called repeatedly with passing the configuration of each state to the parameters of the configure method in order from the basic state to the derived state. If this understanding is correct, it would be helpful if it were clearly stated somewhere in the following lines or tutorials. https://github.com/jrl-umi3218/mc_rtc/blob/4609530431e5177e503ae7ea7d5d011d0dd06ebe/include/mc_control/fsm/State.h#L126-L131
I have one more question. When transitioning to the same state, is a new instance of the state created? That is, is the destructor of the old state instance called, and are the members of the new state instance initialized?