Change in libosmocore[master]: add osmo_fsm_inst_state_chg_keep_timer()

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Thu May 31 21:01:33 UTC 2018


Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/9389 )

Change subject: add osmo_fsm_inst_state_chg_keep_timer()
......................................................................

add osmo_fsm_inst_state_chg_keep_timer()

Change-Id: I3c0e53b846b2208bd201ace99777f2286ea39ae8
---
M include/osmocom/core/fsm.h
M src/fsm.c
M tests/fsm/fsm_test.c
M tests/fsm/fsm_test.err
4 files changed, 188 insertions(+), 35 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/osmocom/core/fsm.h b/include/osmocom/core/fsm.h
index 174396a..67e00ad 100644
--- a/include/osmocom/core/fsm.h
+++ b/include/osmocom/core/fsm.h
@@ -182,6 +182,21 @@
 			     unsigned long timeout_secs, int T,
 			     const char *file, int line);
 
+/*! perform a state change while keeping the current timer running.
+ *
+ *  This is useful to keep a timeout across several states (without having to round the
+ *  remaining time to seconds).
+ *
+ *  This is a macro that calls _osmo_fsm_inst_state_chg_keep_timer() with the given
+ *  parameters as well as the caller's source file and line number for logging
+ *  purposes. See there for documentation.
+ */
+#define osmo_fsm_inst_state_chg_keep_timer(fi, new_state) \
+	_osmo_fsm_inst_state_chg_keep_timer(fi, new_state, \
+				 __BASE_FILE__, __LINE__)
+int _osmo_fsm_inst_state_chg_keep_timer(struct osmo_fsm_inst *fi, uint32_t new_state,
+					const char *file, int line);
+
 /*! dispatch an event to an osmocom finite state machine instance
  *
  *  This is a macro that calls _osmo_fsm_inst_dispatch() with the given
diff --git a/src/fsm.c b/src/fsm.c
index 0370f65..b5af2e7 100644
--- a/src/fsm.c
+++ b/src/fsm.c
@@ -429,15 +429,56 @@
 		return fsm->states[state].name;
 }
 
+static int state_chg(struct osmo_fsm_inst *fi, uint32_t new_state,
+		     bool keep_timer, unsigned long timeout_secs, int T,
+		     const char *file, int line)
+{
+	struct osmo_fsm *fsm = fi->fsm;
+	uint32_t old_state = fi->state;
+	const struct osmo_fsm_state *st = &fsm->states[fi->state];
+
+	/* validate if new_state is a valid state */
+	if (!(st->out_state_mask & (1 << new_state))) {
+		LOGPFSMLSRC(fi, LOGL_ERROR, file, line,
+			    "transition to state %s not permitted!\n",
+			    osmo_fsm_state_name(fsm, new_state));
+		return -EPERM;
+	}
+
+	if (!keep_timer) {
+		/* delete the old timer */
+		osmo_timer_del(&fi->timer);
+	}
+
+	if (st->onleave)
+		st->onleave(fi, new_state);
+
+	LOGPFSMSRC(fi, file, line, "state_chg to %s\n",
+		   osmo_fsm_state_name(fsm, new_state));
+	fi->state = new_state;
+	st = &fsm->states[new_state];
+
+	if (!keep_timer && timeout_secs) {
+		fi->T = T;
+		osmo_timer_schedule(&fi->timer, timeout_secs, 0);
+	}
+
+	/* Call 'onenter' last, user might terminate FSM from there */
+	if (st->onenter)
+		st->onenter(fi, old_state);
+
+	return 0;
+}
+
 /*! perform a state change of the given FSM instance
  *
  *  Best invoke via the osmo_fsm_inst_state_chg() macro which logs the source
  *  file where the state change was effected. Alternatively, you may pass \a
  *  file as NULL to use the normal file/line indication instead.
  *
- *  All changes to the FSM instance state must be made via this
+ *  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
- *  transiiton to new_state.
+ *  transition to new_state.
  *
  *  timeout_secs and T are optional parameters, and only have any effect
  *  if timeout_secs is not 0.  If the timeout function is used, then the
@@ -457,39 +498,32 @@
 			     unsigned long timeout_secs, int T,
 			     const char *file, int line)
 {
-	struct osmo_fsm *fsm = fi->fsm;
-	uint32_t old_state = fi->state;
-	const struct osmo_fsm_state *st = &fsm->states[fi->state];
+	return state_chg(fi, new_state, false, timeout_secs, T, file, line);
+}
 
-	/* validate if new_state is a valid state */
-	if (!(st->out_state_mask & (1 << new_state))) {
-		LOGPFSMLSRC(fi, LOGL_ERROR, file, line,
-			    "transition to state %s not permitted!\n",
-			    osmo_fsm_state_name(fsm, new_state));
-		return -EPERM;
-	}
-
-	/* delete the old timer */
-	osmo_timer_del(&fi->timer);
-
-	if (st->onleave)
-		st->onleave(fi, new_state);
-
-	LOGPFSMSRC(fi, file, line, "state_chg to %s\n",
-		   osmo_fsm_state_name(fsm, new_state));
-	fi->state = new_state;
-	st = &fsm->states[new_state];
-
-	if (timeout_secs) {
-		fi->T = T;
-		osmo_timer_schedule(&fi->timer, timeout_secs, 0);
-	}
-
-	/* Call 'onenter' last, user might terminate FSM from there */
-	if (st->onenter)
-		st->onenter(fi, old_state);
-
-	return 0;
+/*! perform a state change while keeping the current timer running.
+ *
+ *  This is useful to keep a timeout across several states (without having to round the
+ *  remaining time to seconds).
+ *
+ *  Best invoke via the osmo_fsm_inst_state_chg_keep_timer() macro which logs the source
+ *  file where the state change was effected. Alternatively, you may pass \a
+ *  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] 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_timer(struct osmo_fsm_inst *fi, uint32_t new_state,
+					const char *file, int line)
+{
+	return state_chg(fi, new_state, true, 0, 0, file, line);
 }
 
 /*! dispatch an event to an osmocom finite state machine instance
diff --git a/tests/fsm/fsm_test.c b/tests/fsm/fsm_test.c
index e34164c..34a8399 100644
--- a/tests/fsm/fsm_test.c
+++ b/tests/fsm/fsm_test.c
@@ -266,6 +266,89 @@
 	osmo_fsm_inst_term(fi, OSMO_FSM_TERM_REQUEST, NULL);
 }
 
+const struct timeval fake_time_start_time = { 123, 456 };
+
+#define fake_time_passes(secs, usecs) do \
+{ \
+	struct timeval diff; \
+	osmo_gettimeofday_override_add(secs, usecs); \
+	osmo_clock_override_add(CLOCK_MONOTONIC, secs, usecs * 1000); \
+	timersub(&osmo_gettimeofday_override_time, &fake_time_start_time, &diff); \
+	fprintf(stderr, "Total time passed: %d.%06d s\n", \
+		(int)diff.tv_sec, (int)diff.tv_usec); \
+	osmo_timers_prepare(); \
+	osmo_timers_update(); \
+} while (0)
+
+void fake_time_start()
+{
+	struct timespec *clock_override;
+
+	osmo_gettimeofday_override_time = fake_time_start_time;
+	osmo_gettimeofday_override = true;
+	clock_override = osmo_clock_override_gettimespec(CLOCK_MONOTONIC);
+	OSMO_ASSERT(clock_override);
+	clock_override->tv_sec = fake_time_start_time.tv_sec;
+	clock_override->tv_nsec = fake_time_start_time.tv_usec * 1000;
+	osmo_clock_override_enable(CLOCK_MONOTONIC, true);
+	fake_time_passes(0, 0);
+}
+
+static int timeout_fired = 0;
+static int timer_cb(struct osmo_fsm_inst *fi)
+{
+	timeout_fired = fi->T;
+	return 0;
+}
+
+static void test_state_chg_keep_timer()
+{
+	struct osmo_fsm_inst *fi;
+
+	fprintf(stderr, "\n--- %s()\n", __func__);
+
+	fsm.timer_cb = timer_cb;
+
+	/* Test that no timer remains no timer */
+	fi = osmo_fsm_inst_alloc(&fsm, g_ctx, NULL, LOGL_DEBUG, NULL);
+	OSMO_ASSERT(fi);
+
+	osmo_fsm_inst_state_chg(fi, ST_ONE, 0, 0);
+	timeout_fired = -1;
+
+	osmo_fsm_inst_state_chg_keep_timer(fi, ST_TWO);
+
+	OSMO_ASSERT(timeout_fired == -1);
+	OSMO_ASSERT(fi->T == 0);
+
+	osmo_fsm_inst_term(fi, OSMO_FSM_TERM_REQUEST, NULL);
+
+	/* Test that a set time continues with exact precision */
+	fake_time_start();
+	fi = osmo_fsm_inst_alloc(&fsm, g_ctx, NULL, LOGL_DEBUG, NULL);
+	OSMO_ASSERT(fi);
+
+	osmo_fsm_inst_state_chg(fi, ST_ONE, 10, 10);
+
+	timeout_fired = -1;
+
+	fake_time_passes(2, 342);
+	osmo_fsm_inst_state_chg_keep_timer(fi, ST_TWO);
+
+	fake_time_passes(0, 0);
+	OSMO_ASSERT(timeout_fired == -1);
+
+	fake_time_passes(7, 1000000 - 342 - 1);
+	OSMO_ASSERT(timeout_fired == -1);
+
+	fake_time_passes(0, 1);
+	OSMO_ASSERT(timeout_fired == 10);
+
+	osmo_fsm_inst_term(fi, OSMO_FSM_TERM_REQUEST, NULL);
+
+	fprintf(stderr, "--- %s() done\n", __func__);
+}
+
 static const struct log_info_cat default_categories[] = {
 	[DMAIN] = {
 		.name = "DMAIN",
@@ -306,6 +389,7 @@
 	osmo_fsm_inst_free(finst);
 
 	test_id_api();
+	test_state_chg_keep_timer();
 
 	osmo_fsm_unregister(&fsm);
 	exit(0);
diff --git a/tests/fsm/fsm_test.err b/tests/fsm/fsm_test.err
index 3237def..85606e2 100644
--- a/tests/fsm/fsm_test.err
+++ b/tests/fsm/fsm_test.err
@@ -80,4 +80,24 @@
 Test_FSM(arbitrary_id){NULL}: Terminating (cause = OSMO_FSM_TERM_REQUEST)
 Test_FSM(arbitrary_id){NULL}: Freeing instance
 Test_FSM(arbitrary_id){NULL}: Deallocated
-
\ No newline at end of file
+
+--- test_state_chg_keep_timer()
+Test_FSM{NULL}: Allocated
+Test_FSM{NULL}: state_chg to ONE
+Test_FSM{ONE}: state_chg to TWO
+Test_FSM{TWO}: Terminating (cause = OSMO_FSM_TERM_REQUEST)
+Test_FSM{TWO}: Freeing instance
+Test_FSM{TWO}: Deallocated
+Total time passed: 0.000000 s
+Test_FSM{NULL}: Allocated
+Test_FSM{NULL}: state_chg to ONE
+Total time passed: 2.000342 s
+Test_FSM{ONE}: state_chg to TWO
+Total time passed: 2.000342 s
+Total time passed: 9.999999 s
+Total time passed: 10.000000 s
+Test_FSM{TWO}: Timeout of T10
+Test_FSM{TWO}: Terminating (cause = OSMO_FSM_TERM_REQUEST)
+Test_FSM{TWO}: Freeing instance
+Test_FSM{TWO}: Deallocated
+--- test_state_chg_keep_timer() done

-- 
To view, visit https://gerrit.osmocom.org/9389
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3c0e53b846b2208bd201ace99777f2286ea39ae8
Gerrit-Change-Number: 9389
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180531/a75f6da8/attachment.htm>


More information about the gerrit-log mailing list