mirror of https://gerrit.osmocom.org/libosmocore
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
This commit is contained in:
parent
4ff41d94ce
commit
bd5a1dc84f
21
src/fsm.c
21
src/fsm.c
|
@ -458,9 +458,10 @@ static int state_chg(struct osmo_fsm_inst *fi, uint32_t new_state,
|
|||
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 @@ static int state_chg(struct osmo_fsm_inst *fi, uint32_t new_state,
|
|||
* 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
|
||||
|
|
|
@ -349,6 +349,43 @@ static void test_state_chg_keep_timer()
|
|||
fprintf(stderr, "--- %s() done\n", __func__);
|
||||
}
|
||||
|
||||
static void test_state_chg_T()
|
||||
{
|
||||
struct osmo_fsm_inst *fi;
|
||||
|
||||
fprintf(stderr, "\n--- %s()\n", __func__);
|
||||
|
||||
fsm.timer_cb = NULL;
|
||||
|
||||
/* Test setting to timeout_secs = 0, T = 0 */
|
||||
fi = osmo_fsm_inst_alloc(&fsm, g_ctx, NULL, LOGL_DEBUG, NULL);
|
||||
OSMO_ASSERT(fi);
|
||||
|
||||
osmo_fsm_inst_state_chg(fi, ST_ONE, 23, 42);
|
||||
printf("T = %d\n", fi->T);
|
||||
OSMO_ASSERT(fi->T == 42);
|
||||
osmo_fsm_inst_state_chg(fi, ST_TWO, 0, 0);
|
||||
printf("T = %d\n", fi->T);
|
||||
OSMO_ASSERT(fi->T == 0);
|
||||
|
||||
osmo_fsm_inst_term(fi, OSMO_FSM_TERM_REQUEST, NULL);
|
||||
|
||||
/* Test setting to timeout_secs = 0, T != 0 */
|
||||
fi = osmo_fsm_inst_alloc(&fsm, g_ctx, NULL, LOGL_DEBUG, NULL);
|
||||
OSMO_ASSERT(fi);
|
||||
|
||||
osmo_fsm_inst_state_chg(fi, ST_ONE, 23, 42);
|
||||
printf("T = %d\n", fi->T);
|
||||
OSMO_ASSERT(fi->T == 42);
|
||||
osmo_fsm_inst_state_chg(fi, ST_TWO, 0, 11);
|
||||
printf("T = %d\n", fi->T);
|
||||
OSMO_ASSERT(fi->T == 11);
|
||||
|
||||
osmo_fsm_inst_term(fi, OSMO_FSM_TERM_REQUEST, NULL);
|
||||
|
||||
fprintf(stderr, "--- %s() done\n", __func__);
|
||||
}
|
||||
|
||||
static const struct log_info_cat default_categories[] = {
|
||||
[DMAIN] = {
|
||||
.name = "DMAIN",
|
||||
|
@ -390,6 +427,7 @@ int main(int argc, char **argv)
|
|||
|
||||
test_id_api();
|
||||
test_state_chg_keep_timer();
|
||||
test_state_chg_T();
|
||||
|
||||
osmo_fsm_unregister(&fsm);
|
||||
exit(0);
|
||||
|
|
|
@ -101,3 +101,18 @@ Test_FSM{TWO}: Timeout of T10
|
|||
[0;mTest_FSM{TWO}: Freeing instance
|
||||
[0;mTest_FSM{TWO}: Deallocated
|
||||
[0;m--- test_state_chg_keep_timer() done
|
||||
|
||||
--- test_state_chg_T()
|
||||
Test_FSM{NULL}: Allocated
|
||||
[0;mTest_FSM{NULL}: state_chg to ONE
|
||||
[0;mTest_FSM{ONE}: state_chg to TWO
|
||||
[0;mTest_FSM{TWO}: Terminating (cause = OSMO_FSM_TERM_REQUEST)
|
||||
[0;mTest_FSM{TWO}: Freeing instance
|
||||
[0;mTest_FSM{TWO}: Deallocated
|
||||
[0;mTest_FSM{NULL}: Allocated
|
||||
[0;mTest_FSM{NULL}: state_chg to ONE
|
||||
[0;mTest_FSM{ONE}: state_chg to TWO
|
||||
[0;mTest_FSM{TWO}: Terminating (cause = OSMO_FSM_TERM_REQUEST)
|
||||
[0;mTest_FSM{TWO}: Freeing instance
|
||||
[0;mTest_FSM{TWO}: Deallocated
|
||||
[0;m--- test_state_chg_T() done
|
||||
|
|
|
@ -0,0 +1,4 @@
|
|||
T = 42
|
||||
T = 0
|
||||
T = 42
|
||||
T = 11
|
Loading…
Reference in New Issue