osmo_fsm_inst_state_chg(): set T also for zero timeout
Before this patch, if timeout_secs == 0 was passed to
osmo_fsm_inst_state_chg(), the previous T value remained set in the
osmo_fsm_inst->T.
For example:
osmo_fsm_inst_state_chg(fi, ST_X, 23, 42);
// timer == 23 seconds; fi->T == 42
osmo_fsm_inst_state_chg(fi, ST_Y, 0, 0);
// no timer; fi->T == 42!
Instead, always set to the T value passed to osmo_fsm_inst_state_chg().
Adjust osmo_fsm_inst_state_chg() API doc; need to rephrase to accurately
describe the otherwise unchanged behaviour independently from T.
Verify in fsm_test.c.
Rationale: it is confusing to have a T number remaining from some past state,
especially since the user explicitly passed a T number to
osmo_fsm_inst_state_chg(). (Usually we are passing timeout_secs=0, T=0).
I first thought this behavior was introduced with
osmo_fsm_inst_state_chg_keep_timer(), but in fact osmo_fsm_inst_state_chg()
behaved this way from the start.
This shows up in the C test for the upcoming tdef API, where the test result
printout was showing some past T value sticking around after FSM state
transitions. After this patch, there will be no such confusion.
Change-Id: I65c7c262674a1bc5f37faeca6aa0320ab0174f3c
diff --git a/src/fsm.c b/src/fsm.c
index 1f6141fa..ae7c0f5 100644
--- a/src/fsm.c
+++ b/src/fsm.c
@@ -458,9 +458,10 @@
fi->state = new_state;
st = &fsm->states[new_state];
- if (!keep_timer && timeout_secs) {
+ if (!keep_timer) {
fi->T = T;
- osmo_timer_schedule(&fi->timer, timeout_secs, 0);
+ if (timeout_secs)
+ osmo_timer_schedule(&fi->timer, timeout_secs, 0);
}
/* Call 'onenter' last, user might terminate FSM from there */
@@ -480,11 +481,17 @@
* function. It verifies that the existing state actually permits a
* transition to new_state.
*
- * timeout_secs and T are optional parameters, and only have any effect
- * if timeout_secs is not 0. If the timeout function is used, then the
- * new_state is entered, and the FSM instances timer is set to expire
- * in timeout_secs functions. At that time, the FSM's timer_cb
- * function will be called for handling of the timeout by the user.
+ * If timeout_secs is 0, stay in the new state indefinitely, without a timeout
+ * (stop the FSM instance's timer if it was runnning).
+ *
+ * If timeout_secs > 0, start or reset the FSM instance's timer with this
+ * timeout. On expiry, invoke the FSM instance's timer_cb -- if no timer_cb is
+ * set, an expired timer immediately terminates the FSM instance with
+ * OSMO_FSM_TERM_TIMEOUT.
+ *
+ * The value of T is stored in fi->T and is then available for query in
+ * timer_cb. If passing timeout_secs == 0, it is recommended to also pass T ==
+ * 0, so that fi->T is reset to 0 when no timeout is invoked.
*
* \param[in] fi FSM instance whose state is to change
* \param[in] new_state The new state into which we should change