From 83c8c8002f377a8feb0aad3941fc373b0dbbd769 Mon Sep 17 00:00:00 2001 From: Vadim Yanitskiy Date: Sat, 30 Dec 2023 03:18:18 +0700 Subject: [PATCH] core: osmo_tdef_fsm_inst_state_chg(): allow millisecond precision This API predates commit 7b74551b9, which added support for millisecond granularity to osmo_fsm. Let's do the same for the tdef FSM wrapper API, allowing the millisecond precision without rounding-up to seconds. Of course, this patch changes behavior of the existing API, but having more precise state timeouts is not going to make the API user experience worse. The old behavior of using seconds is for kept for: * OSMO_TDEF_CUSTOM -- still treated as if it was OSMO_TDEF_S. * \param[in] default_timeout -- still expected to be in seconds. Change-Id: I4c4ee89e7e32e86f74cd215f5cbfa44ace5426c1 Related: 7b74551b9 "fsm: Allow millisecond granularity in osmo_fsm built-in timer" --- TODO-RELEASE | 1 + include/osmocom/core/tdef.h | 17 ++++++++++------- src/core/tdef.c | 23 +++++++++++++++++------ tests/tdef/tdef_test.err | 6 +++--- tests/tdef/tdef_test.ok | 6 +++--- 5 files changed, 34 insertions(+), 19 deletions(-) diff --git a/TODO-RELEASE b/TODO-RELEASE index 9f5240f50..41bd5d83e 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -11,6 +11,7 @@ core ADD osmo_sock_multiaddr_{add,del}_local_addr() core ADD osmo_sock_multiaddr_get_ip_and_port(), osmo_multiaddr_ip_and_port_snprintf(), osmo_sock_multiaddr_get_name_buf() core ADD osmo_sock_sctp_get_peer_addr_info() core ADD gsmtap_inst_fd2() core, DEPRECATE gsmtap_inst_fd() +core behavior change osmo_tdef_fsm_inst_state_chg(): allow millisecond precision isdn ABI change add states and flags for external T200 handling gsm ABI change add T200 timer states to lapdm_datalink gsm ABI change add UI queue to struct lapdm_datalink diff --git a/include/osmocom/core/tdef.h b/include/osmocom/core/tdef.h index d9d267511..402d01029 100644 --- a/include/osmocom/core/tdef.h +++ b/include/osmocom/core/tdef.h @@ -121,11 +121,13 @@ struct osmo_tdef_state_timeout { const struct osmo_tdef_state_timeout *osmo_tdef_get_state_timeout(uint32_t state, const struct osmo_tdef_state_timeout *timeouts_array); -/*! Call osmo_fsm_inst_state_chg() or osmo_fsm_inst_state_chg_keep_timer(), depending on the timeouts_array, tdefs and - * default_timeout. +/*! Call osmo_fsm_inst_state_chg[_ms]() or osmo_fsm_inst_state_chg_keep_timer[_ms](), + * depending on the timeouts_array, tdefs and default_timeout. * - * A T timer configured in sub-second precision is rounded up to the next full second. A timer in unit = - * OSMO_TDEF_CUSTOM is applied as if the unit is in seconds (i.e. this macro does not make sense for custom units!). + * A timer defined with sub-millisecond precision (e.g OSMO_TDEF_US) is rounded up to the next full millisecond. + * A timer value defined in units higher than millisecond (e.g. OSMO_TDEF_S, OSMO_TDEF_M) is converted to milliseconds. + * A timer in unit = OSMO_TDEF_CUSTOM is applied as if the unit is in seconds (i.e. this macro does not make sense + * for custom units!). * * See osmo_tdef_get_state_timeout() and osmo_tdef_get(). * @@ -153,9 +155,10 @@ const struct osmo_tdef_state_timeout *osmo_tdef_get_state_timeout(uint32_t state * \param[in] state State number to transition to. * \param[in] timeouts_array Array of struct osmo_tdef_state_timeout[32] to look up state in. * \param[in] tdefs Array of struct osmo_tdef (last entry zero initialized) to look up T in. - * \param[in] default_timeout If a T is set in timeouts_array, but no timeout value is configured for T, then use this - * default timeout value as fallback, or pass -1 to abort the program. - * \return Return value from osmo_fsm_inst_state_chg() or osmo_fsm_inst_state_chg_keep_timer(). + * \param[in] default_timeout If a T is set in timeouts_array, but no timeout value is configured for T, + * then use this default timeout value (in seconds) as fallback, + * or pass a negative number to abort the program. + * \return Return value from osmo_fsm_inst_state_chg[_ms]() or osmo_fsm_inst_state_chg_keep_timer[_ms](). */ #define osmo_tdef_fsm_inst_state_chg(fi, state, timeouts_array, tdefs, default_timeout) \ _osmo_tdef_fsm_inst_state_chg(fi, state, timeouts_array, tdefs, default_timeout, \ diff --git a/src/core/tdef.c b/src/core/tdef.c index abbe581aa..f0c0f2e80 100644 --- a/src/core/tdef.c +++ b/src/core/tdef.c @@ -337,26 +337,37 @@ int _osmo_tdef_fsm_inst_state_chg(struct osmo_fsm_inst *fi, uint32_t state, const char *file, int line) { const struct osmo_tdef_state_timeout *t = osmo_tdef_get_state_timeout(state, timeouts_array); - unsigned long val = 0; + unsigned long val_ms = 0; /* No timeout defined for this state? */ if (!t) return _osmo_fsm_inst_state_chg(fi, state, 0, 0, file, line); - if (t->T) - val = osmo_tdef_get(tdefs, t->T, OSMO_TDEF_S, default_timeout); + if (t->T) { + const struct osmo_tdef *tdef = osmo_tdef_get_entry((struct osmo_tdef *)tdefs, t->T); + if (tdef == NULL) { + /* emulate the old behavior: treat default_timeout as OSMO_TDEF_S */ + OSMO_ASSERT(default_timeout >= 0); + val_ms = default_timeout * 1000; + } else { + val_ms = osmo_tdef_round(tdef->val, tdef->unit, OSMO_TDEF_MS); + /* emulate the old behavior: treat OSMO_TDEF_CUSTOM as OSMO_TDEF_S */ + if (tdef->unit == OSMO_TDEF_CUSTOM) + val_ms *= 1000; + } + } if (t->keep_timer) { if (t->T) - return _osmo_fsm_inst_state_chg_keep_or_start_timer(fi, state, val, t->T, file, line); + return _osmo_fsm_inst_state_chg_keep_or_start_timer_ms(fi, state, val_ms, t->T, file, line); else return _osmo_fsm_inst_state_chg_keep_timer(fi, state, file, line); } - /* val is always initialized here, because if t->keep_timer is false, t->T must be != 0. + /* val_ms is always initialized here, because if t->keep_timer is false, t->T must be != 0. * Otherwise osmo_tdef_get_state_timeout() would have returned NULL. */ OSMO_ASSERT(t->T); - return _osmo_fsm_inst_state_chg(fi, state, val, t->T, file, line); + return _osmo_fsm_inst_state_chg_ms(fi, state, val_ms, t->T, file, line); } const struct value_string osmo_tdef_unit_names[] = { diff --git a/tests/tdef/tdef_test.err b/tests/tdef/tdef_test.err index 1a39b8961..8e5860d7c 100644 --- a/tests/tdef/tdef_test.err +++ b/tests/tdef/tdef_test.err @@ -1,10 +1,10 @@ DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){A}: Allocated DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){A}: State change to A (T1, 100s) -DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){A}: State change to B (T2, 1s) +DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){A}: State change to B (T2, 100ms) DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){B}: State change to C (T3, 3000s) DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){C}: State change to D (T4, 100s) -DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){D}: State change to E (X5, 1s) -DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){E}: State change to F (X6, 1s) +DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){D}: State change to E (X5, 100ms) +DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){E}: State change to F (X6, 1ms) DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){F}: State change to G (T7, 50s) DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){G}: State change to H (T8, 300s) DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){H}: State change to I (T9, 300s) diff --git a/tests/tdef/tdef_test.ok b/tests/tdef/tdef_test.ok index d934292fa..827b9ab35 100644 --- a/tests/tdef/tdef_test.ok +++ b/tests/tdef/tdef_test.ok @@ -165,11 +165,11 @@ osmo_tdef_get(7, s) = 50 test_tdef_state_timeout() state=A T=0, no timeout --> A (configured as T1 100 s) rc=0; state=A T=1, 100.000000 s remaining - --> B (configured as T2 100 ms) rc=0; state=B T=2, 1.000000 s remaining + --> B (configured as T2 100 ms) rc=0; state=B T=2, 0.100000 s remaining --> C (configured as T3 50 m) rc=0; state=C T=3, 3000.000000 s remaining --> D (configured as T4 100 custom-unit) rc=0; state=D T=4, 100.000000 s remaining - --> E (configured as T-5 100 ms) rc=0; state=E T=-5, 1.000000 s remaining - --> F (configured as T-6 100 us) rc=0; state=F T=-6, 1.000000 s remaining + --> E (configured as T-5 100 ms) rc=0; state=E T=-5, 0.100000 s remaining + --> F (configured as T-6 100 us) rc=0; state=F T=-6, 0.001000 s remaining --> G (configured as T7 50 s) rc=0; state=G T=7, 50.000000 s remaining --> H (configured as T8 300 s) rc=0; state=H T=8, 300.000000 s remaining --> I (configured as T9 5 m) rc=0; state=I T=9, 300.000000 s remaining