laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35466?usp=email )
Change subject: core: osmo_tdef_fsm_inst_state_chg(): allow millisecond precision ...................................................................... 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" --- M TODO-RELEASE M include/osmocom/core/tdef.h M src/core/tdef.c M tests/tdef/tdef_test.err M tests/tdef/tdef_test.ok 5 files changed, 57 insertions(+), 19 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved pespin: Looks good to me, but someone else must approve diff --git a/TODO-RELEASE b/TODO-RELEASE index 9f5240f..41bd5d8 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -11,6 +11,7 @@ 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 d9d2675..402d010 100644 --- a/include/osmocom/core/tdef.h +++ b/include/osmocom/core/tdef.h @@ -121,11 +121,13 @@ 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 @@ * \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 abbe581..f0c0f2e 100644 --- a/src/core/tdef.c +++ b/src/core/tdef.c @@ -337,26 +337,37 @@ 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 1a39b89..8e5860d 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 d934292..827b9ab 100644 --- a/tests/tdef/tdef_test.ok +++ b/tests/tdef/tdef_test.ok @@ -165,11 +165,11 @@ 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 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35466?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I4c4ee89e7e32e86f74cd215f5cbfa44ace5426c1 Gerrit-Change-Number: 35466 Gerrit-PatchSet: 8 Gerrit-Owner: fixeria <vyanits...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Reviewer: neels <nhofm...@sysmocom.de> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-MessageType: merged