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"

5 files changed, 34 insertions, 19 deletions

diff --git a/TODO-RELEASE b/TODO-RELEASE
index 9f5240f5..41bd5d83 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 d9d26751..402d0102 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 abbe581a..f0c0f2e8 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 1a39b896..8e5860d7 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 d934292f..827b9ab3 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