main: Rework sub states in STATE_TRIGGERING

The sub states in STATE_TRIGGERING have been updated to allow
for a dedicated wait state, and to make it easier to have
accurate intervals between samples.

Tests are updated, and we now also test that the correct interval
is maintained over time.

Signed-off-by: Jan Tore Guggedal <jantore.guggedal@nordicsemi.no>

Author: jtguggedal

app/src/cbor/cbor_helper.c

@@ -8,7 +8,7 @@
 
 int get_update_interval_from_cbor_response(const uint8_t *cbor,
 					   size_t len,
-					   uint64_t *interval_sec)
+					   uint32_t *interval_sec)
 {
 	struct config_object object = { 0 };
 	size_t not_used;

app/src/cbor/cbor_helper.h

@@ -20,4 +20,4 @@
  */
 int get_update_interval_from_cbor_response(const uint8_t *cbor,
 					   size_t len,
-					   uint64_t *interval_sec);
+					   uint32_t *interval_sec);

app/src/cbor/device_shadow.cddl

@@ -1,5 +1,5 @@
 config-object = {
     "config": {
-        "update_interval": uint .size 8
+        "update_interval": uint .size 4
     }
 }

app/src/main.c

@@ -77,12 +77,12 @@ static void running_run(void *o);
 static void triggering_entry(void *o);
 static void triggering_run(void *o);
 
-static void requesting_location_entry(void *o);
-static void requesting_location_run(void *o);
+static void sample_data_entry(void *o);
+static void sample_data_run(void *o);
 
-static void requesting_sensors_and_polling_entry(void *o);
-static void requesting_sensors_and_polling_run(void *o);
-static void requesting_sensors_and_polling_exit(void *o);
+static void wait_for_trigger_entry(void *o);
+static void wait_for_trigger_run(void *o);
+static void wait_for_trigger_exit(void *o);
 
 static void idle_entry(void *o);
 static void idle_run(void *o);
@@ -110,10 +110,13 @@ enum state {
 		STATE_IDLE,
 		/* Triggers are periodically sent at a configured interval */
 		STATE_TRIGGERING,
-			/* Requesting location from the location module */
-			STATE_REQUESTING_LOCATION,
-			/* Requesting sensor values and polling for downlink data */
-			STATE_REQUESTING_SENSORS_AND_POLLING,
+			/* Requesting data samples from relevant modules.
+			 * Location data is requested first, upon state entry.
+			 * After location data is received, the other modules are polled.
+			 */
+			STATE_SAMPLE_DATA,
+			/* Wait for timer or button press to trigger the next sample */
+			STATE_WAIT_FOR_TRIGGER,
 	/* Ongoing FOTA process, triggers are blocked */
 	STATE_FOTA,
 		/* FOTA image is being downloaded */
@@ -144,10 +147,15 @@ struct main_state {
 	uint8_t msg_buf[MAX_MSG_SIZE];
 
 	/* Trigger interval */
-	uint64_t interval_sec;
+	uint32_t interval_sec;
 
 	/* Cloud connection status */
 	bool connected;
+
+	/* Start time of the most recent sampling. This is used to calculate the correct
+	 * time when scheduling the next sampling trigger.
+	 */
+	uint32_t sample_start_time;
 };
 
 /* Construct state table */
@@ -171,19 +179,19 @@ static const struct smf_state states[] = {
 		triggering_run,
 		NULL,
 		&states[STATE_RUNNING],
-		&states[STATE_REQUESTING_LOCATION]
+		&states[STATE_SAMPLE_DATA]
 	),
-	[STATE_REQUESTING_LOCATION] = SMF_CREATE_STATE(
-		requesting_location_entry,
-		requesting_location_run,
+	[STATE_SAMPLE_DATA] = SMF_CREATE_STATE(
+		sample_data_entry,
+		sample_data_run,
 		NULL,
 		&states[STATE_TRIGGERING],
 		NULL
 	),
-	[STATE_REQUESTING_SENSORS_AND_POLLING] = SMF_CREATE_STATE(
-		requesting_sensors_and_polling_entry,
-		requesting_sensors_and_polling_run,
-		requesting_sensors_and_polling_exit,
+	[STATE_WAIT_FOR_TRIGGER] = SMF_CREATE_STATE(
+		wait_for_trigger_entry,
+		wait_for_trigger_run,
+		wait_for_trigger_exit,
 		&states[STATE_TRIGGERING],
 		NULL
 	),
@@ -293,18 +301,6 @@ static void sensor_and_poll_triggers_send(void)
 		SEND_FATAL_ERROR();
 		return;
 	}
-
-	/* Send trigger for shadow polling */
-	struct cloud_msg cloud_msg = {
-		.type = CLOUD_POLL_SHADOW
-	};
-
-	err = zbus_chan_pub(&CLOUD_CHAN, &cloud_msg, K_SECONDS(1));
-	if (err) {
-		LOG_ERR("zbus_chan_pub shadow trigger, error: %d", err);
-		SEND_FATAL_ERROR();
-		return;
-	}
 }
 
 /* Delayable work used to send messages on the TIMER_CHAN */
@@ -464,7 +460,7 @@ static void triggering_run(void *o)
 				return;
 			}
 
-			LOG_WRN("Received new interval: %lld seconds", state_object->interval_sec);
+			LOG_WRN("Received new interval: %d seconds", state_object->interval_sec);
 
 			err = k_work_reschedule(&trigger_work,
 						K_SECONDS(state_object->interval_sec));
@@ -476,17 +472,30 @@ static void triggering_run(void *o)
 	}
 }
 
-/* STATE_REQUESTING_LOCATION */
+/* STATE_SAMPLE_DATA */
 
-static void requesting_location_entry(void *o)
+static void sample_data_entry(void *o)
 {
 	int err;
 	enum location_msg_type location_msg = LOCATION_SEARCH_TRIGGER;
+	struct cloud_msg cloud_msg = {
+		.type = CLOUD_POLL_SHADOW
+	};
+	struct main_state *state_object = (struct main_state *)o;
 
-	ARG_UNUSED(o);
 
 	LOG_DBG("%s", __func__);
 
+	/* Record the start time of sampling */
+	state_object->sample_start_time = k_uptime_seconds();
+
+	err = zbus_chan_pub(&CLOUD_CHAN, &cloud_msg, K_SECONDS(1));
+	if (err) {
+		LOG_ERR("zbus_chan_pub shadow trigger, error: %d", err);
+		SEND_FATAL_ERROR();
+		return;
+	}
+
 	err = zbus_chan_pub(&LOCATION_CHAN, &location_msg, K_SECONDS(1));
 	if (err) {
 		LOG_ERR("zbus_chan_pub data sample trigger, error: %d", err);
@@ -495,62 +504,77 @@ static void requesting_location_entry(void *o)
 	}
 }
 
-static void requesting_location_run(void *o)
+static void sample_data_run(void *o)
 {
 	const struct main_state *state_object = (const struct main_state *)o;
 
 	if (state_object->chan == &LOCATION_CHAN) {
 		enum location_msg_type msg = MSG_TO_LOCATION_TYPE(state_object->msg_buf);
 
 		if (msg == LOCATION_SEARCH_DONE) {
-			smf_set_state(SMF_CTX(state_object),
-					      &states[STATE_REQUESTING_SENSORS_AND_POLLING]);
+			sensor_and_poll_triggers_send();
+			smf_set_state(SMF_CTX(state_object), &states[STATE_WAIT_FOR_TRIGGER]);
 			return;
 		}
 	}
 
+	/* We are already sampling, ignore any new triggers */
 	if (state_object->chan == &BUTTON_CHAN) {
 		smf_set_handled(SMF_CTX(state_object));
 		return;
 	}
+
+	if (state_object->chan == &TIMER_CHAN) {
+		smf_set_handled(SMF_CTX(state_object));
+		return;
+	}
 }
 
-/* STATE_REQUESTING_SENSORS_AND_POLLING */
+/* STATE_WAIT_FOR_TRIGGER */
 
-static void requesting_sensors_and_polling_entry(void *o)
+static void wait_for_trigger_entry(void *o)
 {
 	int err;
 	const struct main_state *state_object = (const struct main_state *)o;
+	uint32_t time_elapsed = k_uptime_seconds() - state_object->sample_start_time;
+	uint32_t time_remaining;
+
+	if (time_elapsed > state_object->interval_sec) {
+		LOG_WRN("Sampling took longer than the interval, skipping next trigger");
+		time_remaining = 0;
+	} else {
+		time_remaining = state_object->interval_sec - time_elapsed;
+	}
 
 	LOG_DBG("%s", __func__);
 
-	sensor_and_poll_triggers_send();
+	LOG_DBG("Next trigger in %d seconds", time_remaining);
 
-	LOG_DBG("Next trigger in %lld seconds", state_object->interval_sec);
-
-	err = k_work_reschedule(&trigger_work, K_SECONDS(state_object->interval_sec));
+	(void)k_work_cancel_delayable(&trigger_work);
+	err = k_work_reschedule(&trigger_work, K_SECONDS(time_remaining));
 	if (err < 0) {
 		LOG_ERR("k_work_reschedule, error: %d", err);
 		SEND_FATAL_ERROR();
 	}
 }
 
-static void requesting_sensors_and_polling_run(void *o)
+static void wait_for_trigger_run(void *o)
 {
 	const struct main_state *state_object = (const struct main_state *)o;
 
 	if (state_object->chan == &TIMER_CHAN) {
-		smf_set_state(SMF_CTX(state_object), &states[STATE_REQUESTING_LOCATION]);
+		LOG_DBG("Timer trigger received");
+		smf_set_state(SMF_CTX(state_object), &states[STATE_SAMPLE_DATA]);
 		return;
 	}
 
 	if (state_object->chan == &BUTTON_CHAN) {
-		smf_set_state(SMF_CTX(state_object), &states[STATE_REQUESTING_LOCATION]);
+		smf_set_state(SMF_CTX(state_object), &states[STATE_SAMPLE_DATA]);
 		return;
 	}
 }
 
-static void requesting_sensors_and_polling_exit(void *o)
+static void wait_for_trigger_exit(void *o)
 {
 	ARG_UNUSED(o);
 

tests/module/main/src/checks.c

@@ -20,7 +20,7 @@ ZBUS_CHAN_ADD_OBS(LOCATION_CHAN, location_subscriber, 0);
 ZBUS_CHAN_ADD_OBS(NETWORK_CHAN, network_subscriber, 0);
 ZBUS_CHAN_ADD_OBS(POWER_CHAN, power_subscriber, 0);
 
-LOG_MODULE_REGISTER(trigger_module_checks, 4);
+LOG_MODULE_REGISTER(main_module_checks, 4);
 
 void check_location_event(enum location_msg_type expected_location_type)
 {
@@ -31,7 +31,7 @@ void check_location_event(enum location_msg_type expected_location_type)
 	/* Allow the test thread to sleep so that the DUT's thread is allowed to run. */
 	k_sleep(K_MSEC(100));
 
-	err = zbus_sub_wait_msg(&location_subscriber, &chan, &location_msg_type, K_MSEC(1000));
+	err = zbus_sub_wait_msg(&location_subscriber, &chan, &location_msg_type, K_MSEC(10000));
 	if (err == -ENOMSG) {
 		LOG_ERR("No location event received");
 		TEST_FAIL();
@@ -60,7 +60,7 @@ void check_network_event(enum network_msg_type expected_network_type)
 	/* Allow the test thread to sleep so that the DUT's thread is allowed to run. */
 	k_sleep(K_MSEC(100));
 
-	err = zbus_sub_wait_msg(&network_subscriber, &chan, &network_msg, K_MSEC(1000));
+	err = zbus_sub_wait_msg(&network_subscriber, &chan, &network_msg, K_MSEC(10000));
 	if (err == -ENOMSG) {
 		LOG_ERR("No network event received");
 		TEST_FAIL();
@@ -89,7 +89,7 @@ void check_power_event(enum power_msg_type expected_power_type)
 	/* Allow the test thread to sleep so that the DUT's thread is allowed to run. */
 	k_sleep(K_MSEC(100));
 
-	err = zbus_sub_wait_msg(&power_subscriber, &chan, &power_msg, K_MSEC(1000));
+	err = zbus_sub_wait_msg(&power_subscriber, &chan, &power_msg, K_MSEC(10000));
 	if (err == -ENOMSG) {
 		LOG_ERR("No power event received");
 		TEST_FAIL();
@@ -116,7 +116,7 @@ static void check_no_location_events(void)
 	const struct zbus_channel *chan;
 	enum location_msg_type location_msg_type;
 
-	err = zbus_sub_wait_msg(&location_subscriber, &chan, &location_msg_type, K_MSEC(1000));
+	err = zbus_sub_wait_msg(&location_subscriber, &chan, &location_msg_type, K_MSEC(10000));
 	if (err == -ENOMSG) {
 		return;
 	} else if (err) {
@@ -136,7 +136,7 @@ static void check_no_network_events(void)
 	const struct zbus_channel *chan;
 	struct network_msg network_msg;
 
-	err = zbus_sub_wait_msg(&network_subscriber, &chan, &network_msg, K_MSEC(1000));
+	err = zbus_sub_wait_msg(&network_subscriber, &chan, &network_msg, K_MSEC(10000));
 	if (err == -ENOMSG) {
 		return;
 	} else if (err) {
@@ -156,7 +156,7 @@ static void check_no_power_events(void)
 	const struct zbus_channel *chan;
 	struct power_msg power_msg;
 
-	err = zbus_sub_wait_msg(&power_subscriber, &chan, &power_msg, K_MSEC(1000));
+	err = zbus_sub_wait_msg(&power_subscriber, &chan, &power_msg, K_MSEC(10000));
 	if (err == -ENOMSG) {
 		return;
 	} else if (err) {
@@ -243,3 +243,39 @@ void purge_all_events(void)
 	purge_network_events();
 	purge_power_events();
 }
+
+int wait_for_location_event(enum location_msg_type expected_type, uint32_t timeout_sec)
+{
+	int err;
+	const struct zbus_channel *chan;
+	enum location_msg_type location_msg_type;
+	uint32_t start_time = k_uptime_seconds();
+	uint32_t elapsed_time;
+
+	err = zbus_sub_wait_msg(&location_subscriber, &chan, &location_msg_type,
+				K_SECONDS(timeout_sec));
+	if (err == -ENOMSG) {
+		return -ENOMSG;
+	} else if (err) {
+		LOG_ERR("zbus_sub_wait, error: %d", err);
+
+		return err;
+	}
+
+	if (chan != &LOCATION_CHAN) {
+		LOG_ERR("Received message from wrong channel, expected %s, got %s",
+			zbus_chan_name(&LOCATION_CHAN), zbus_chan_name(chan));
+		return -EINVAL;
+	}
+
+	if (location_msg_type != expected_type) {
+		LOG_ERR("Received unexpected location event: %d", location_msg_type);
+		return -EINVAL;
+	}
+
+	elapsed_time = k_uptime_seconds() - start_time;
+
+	LOG_DBG("Received expected location event: %d, wait: %d", location_msg_type, elapsed_time);
+
+	return elapsed_time;
+}

tests/module/main/src/checks.h

@@ -23,3 +23,10 @@ void purge_network_events(void);
 void purge_power_events(void);
 
 void purge_all_events(void);
+
+/* Wait for a location event of the expected type.
+ *
+ * Returns the time in seconds it took to receive the event if it is received within the timeout.
+ * Otherwise, it returns a negative value.
+ */
+int wait_for_location_event(enum location_msg_type expected_type, uint32_t timeout_sec);