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

Reply via email to