From d4b79c877291e58bf7fafcfd3c771c9e66fa3f5b Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Wed, 6 Mar 2019 05:43:23 +0100 Subject: fsm: add osmo_fsm_inst_state_chg_keep_or_start_timer() During FSM design for osmo-msc, I noticed that the current behavior that keep_timer=true doesn't guarantee a running timer can make FSM design a bit complex, especially when using osmo_tdef for timeout definitions. A desirable keep_timer=true behavior is one that keeps the previous timer running, but starts a timer if no timer is running yet. The simplest example is: a given state repeatedly transitions back to itself, but wants to set a timeout only on first entering, avoiding to restart the timeout on re-entering. Another example is a repeated transition between two or more states, where the first time we enter this group a timeout should start, but it should not restart from scratch on every transition. When using osmo_tdef timeout definitions for this, so far separate meaningless states have to be introduced that merely set a fixed timeout. To simplify, add osmo_fsm_inst_state_chg_keep_or_start_timer(), and use this in osmo_tdef_fsm_inst_state_chg() when both keep_timer == true *and* T != 0. In tdef_test.ok, the changes show that on first entering state L, the previous T=1 is now kept with a large remaining timeout. When entering state L from O, where no timer was running, this time L's T123 is started. Change-Id: Id647511a4b18e0c4de0e66fb1f35dc9adb9177db --- src/fsm.c | 51 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 6 deletions(-) (limited to 'src/fsm.c') diff --git a/src/fsm.c b/src/fsm.c index 4876c04f..5723bab4 100644 --- a/src/fsm.c +++ b/src/fsm.c @@ -481,11 +481,20 @@ static int state_chg(struct osmo_fsm_inst *fi, uint32_t new_state, st->onleave(fi, new_state); if (fsm_log_timeouts) { - if (keep_timer && fi->timer.active && (osmo_timer_remaining(&fi->timer, NULL, &remaining) == 0)) - LOGPFSMSRC(fi, file, line, "State change to %s (keeping " OSMO_T_FMT ", %ld.%03lds remaining)\n", - osmo_fsm_state_name(fsm, new_state), - OSMO_T_FMT_ARGS(fi->T), remaining.tv_sec, remaining.tv_usec / 1000); - else if (timeout_secs && !keep_timer) + if (keep_timer && fi->timer.active) { + /* This should always give us a timeout, but just in case the return value indicates error, omit + * logging the remaining time. */ + if (osmo_timer_remaining(&fi->timer, NULL, &remaining)) + LOGPFSMSRC(fi, file, line, + "State change to %s (keeping " OSMO_T_FMT ")\n", + osmo_fsm_state_name(fsm, new_state), + OSMO_T_FMT_ARGS(fi->T)); + else + LOGPFSMSRC(fi, file, line, + "State change to %s (keeping " OSMO_T_FMT ", %ld.%03lds remaining)\n", + osmo_fsm_state_name(fsm, new_state), + OSMO_T_FMT_ARGS(fi->T), remaining.tv_sec, remaining.tv_usec / 1000); + } else if (timeout_secs) LOGPFSMSRC(fi, file, line, "State change to %s (" OSMO_T_FMT ", %lus)\n", osmo_fsm_state_name(fsm, new_state), OSMO_T_FMT_ARGS(T), timeout_secs); @@ -500,7 +509,8 @@ 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) { + if (!keep_timer + || (keep_timer && !osmo_timer_pending(&fi->timer))) { fi->T = T; if (timeout_secs) osmo_timer_schedule(&fi->timer, timeout_secs, 0); @@ -592,6 +602,35 @@ int _osmo_fsm_inst_state_chg_keep_timer(struct osmo_fsm_inst *fi, uint32_t new_s return state_chg(fi, new_state, true, 0, 0, file, line); } +/*! perform a state change while keeping the current timer if running, or starting a timer otherwise. + * + * This is useful to keep a timeout across several states, but to make sure that some timeout is actually running. + * + * Best invoke via the osmo_fsm_inst_state_chg_keep_or_start_timer() macro which logs the source file where the state + * change was effected. Alternatively, you may pass file as NULL to use the normal file/line indication instead. + * + * All changes to the FSM instance state must be made via an osmo_fsm_inst_state_chg_* + * function. It verifies that the existing state actually permits a + * transition to new_state. + * + * \param[in] fi FSM instance whose state is to change + * \param[in] new_state The new state into which we should change + * \param[in] timeout_secs If no timer is running yet, set this timeout in seconds (if !=0), maximum-clamped to + * 2147483647 seconds. + * \param[in] T Timer number, where positive numbers are considered to be 3GPP spec compliant timer numbers and are + * logged as "T1234", while negative numbers are considered Osmocom specific timer numbers logged as + * "X1234". + * \param[in] file Calling source file (from osmo_fsm_inst_state_chg macro) + * \param[in] line Calling source line (from osmo_fsm_inst_state_chg macro) + * \returns 0 on success; negative on error + */ +int _osmo_fsm_inst_state_chg_keep_or_start_timer(struct osmo_fsm_inst *fi, uint32_t new_state, + unsigned long timeout_secs, int T, + const char *file, int line) +{ + return state_chg(fi, new_state, true, timeout_secs, T, file, line); +} + /*! dispatch an event to an osmocom finite state machine instance * * Best invoke via the osmo_fsm_inst_dispatch() macro which logs the source -- cgit v1.2.3