Skip to content

Commit a1c3424

Browse files
nmplutanmpluta
committed
modules: sensor: remove timing, polling; make state machine event-driven
Previously, the sensor module used timing fields and periodic sleeps in the idle state to trigger automatic sensor readings after a timeout. This added unnecessary complexity and could cause redundant sensor reads. This change: - Removes last_read_time and read_timeout_ms fields from the state object - Eliminates all timeout and sleep logic from the idle state - Makes the state machine fully event-driven: sensor readings now occur only in response to explicit SENSOR_SAMPLE_REQUEST messages This simplifies the module, reduces unnecessary delays, and aligns the design with the event-driven approach used in the environmental module. Refs: #23 Signed-off-by: Natalia Pluta <pluta.natalia.m@gmail.com>
1 parent 448a336 commit a1c3424

1 file changed

Lines changed: 95 additions & 84 deletions

File tree

‎app/src/modules/sensor/sensor_module.c‎

Lines changed: 95 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <zephyr/drivers/sensor/sen0466.h>
99
#include <zephyr/logging/log.h>
1010
#include <zephyr/smf.h>
11+
#include <string.h>
1112

1213
#if defined(CONFIG_SENSOR_MODULE_TIMESTAMP)
1314
#include <date_time.h>
@@ -19,15 +20,21 @@ LOG_MODULE_REGISTER(sensor_module, CONFIG_SENSOR_MODULE_LOG_LEVEL);
1920
#define IS_VALID_SENSOR_TYPE(type) ((int)(type) >= 0 && (type) < SENSOR_TYPE_COUNT)
2021

2122
/* Macro for type-safe sensor state object initialization */
22-
#define SENSOR_STATE_OBJECT_INIT() \
23-
(struct sensor_state_object) \
24-
{ \
25-
.ctx = {0}, .current_state = SENSOR_MODULE_STATE_INIT, .current_data = {0}, \
26-
.error_count = 0, .max_retries = 0, .last_read_time = 0, .read_timeout_ms = 0, \
27-
IF_ENABLED(CONFIG_SENSOR_MODULE_WARMUP_ENABLE, \
28-
(.sensor_init_time = 0, \
29-
.sensor_warmup_complete = {false})) \
30-
}
23+
#define SENSOR_STATE_OBJECT_INIT() \
24+
(struct sensor_state_object) \
25+
{ \
26+
.ctx = {0}, .current_state = SENSOR_MODULE_STATE_INIT, .current_data = {0}, \
27+
.error_count = 0, .max_retries = 0, .recovery_attempts = 0, .chan = NULL, \
28+
.msg_buf = {0}, \
29+
IF_ENABLED(CONFIG_SENSOR_MODULE_WARMUP_ENABLE, \
30+
(.sensor_init_time = 0, \
31+
.sensor_warmup_complete = {false})) \
32+
}
33+
34+
/* Ticking period for SMF progression (recovery, time-based ops) */
35+
#ifndef CONFIG_SENSOR_MODULE_TICK_MS
36+
#define CONFIG_SENSOR_MODULE_TICK_MS 100
37+
#endif
3138

3239
/* ZBUS channel definition */
3340
ZBUS_CHAN_DEFINE(sensor_chan, struct sensor_msg, NULL, NULL, ZBUS_OBSERVERS_EMPTY,
@@ -42,7 +49,7 @@ ZBUS_CHAN_ADD_OBS(sensor_chan, sensor_subscriber, 0);
4249
#define MAX_MSG_SIZE sizeof(struct sensor_msg)
4350

4451
/* CCS811 environmental compensation configuration */
45-
#define SENSOR_VALUE_TO_MICRO(val) ((val)->val1 * 1000000UL + (val)->val2)
52+
#define SENSOR_VALUE_TO_MICRO(val) ((val)->val1 * 1000000LL + (val)->val2)
4653

4754
/* Sensor state machine states */
4855
enum sensor_module_state {
@@ -53,7 +60,7 @@ enum sensor_module_state {
5360
SENSOR_MODULE_STATE_RECOVERY
5461
};
5562

56-
/* Sensor state machine events */
63+
/* Sensor state machine events (reserved for future use) */
5764
enum sensor_module_event {
5865
SENSOR_MODULE_EVENT_INIT_COMPLETE,
5966
SENSOR_MODULE_EVENT_READ_REQUEST,
@@ -73,19 +80,14 @@ struct sensor_state_object {
7380
/* Current sensor data */
7481
struct sensor_msg current_data;
7582

76-
/* Last channel type that a message was received on */
83+
/* Last channel that delivered a message + raw message buffer */
7784
const struct zbus_channel *chan;
78-
79-
/* Buffer for last zbus message */
8085
uint8_t msg_buf[MAX_MSG_SIZE];
8186

82-
/* Error handling */
87+
/* Error & recovery handling */
8388
int error_count;
8489
int max_retries;
85-
86-
/* Timing */
87-
int64_t last_read_time;
88-
int64_t read_timeout_ms;
90+
int recovery_attempts;
8991

9092
#if defined(CONFIG_SENSOR_MODULE_WARMUP_ENABLE)
9193
/* Sensor warmup tracking */
@@ -112,12 +114,13 @@ static const struct smf_state sensor_states[] = {
112114
[SENSOR_MODULE_STATE_ERROR] =
113115
SMF_CREATE_STATE(NULL, sensor_state_error_run, NULL, NULL, NULL),
114116
[SENSOR_MODULE_STATE_RECOVERY] =
115-
SMF_CREATE_STATE(NULL, sensor_state_recovery_run, NULL, NULL, NULL)};
117+
SMF_CREATE_STATE(NULL, sensor_state_recovery_run, NULL, NULL, NULL),
118+
};
116119

117120
/* Global sensor state machine context */
118121
static struct sensor_state_object sensor_state_obj;
119122

120-
/* Thread synchronization */
123+
/* Thread synchronization: used by getters only */
121124
static K_MUTEX_DEFINE(sensor_sm_mutex);
122125

123126
/* Sensor device pointers */
@@ -131,8 +134,10 @@ static struct sensor_info sensors[SENSOR_TYPE_COUNT] = {
131134
[SENSOR_TYPE_HM3301] = {.device = DEVICE_DT_GET(DT_NODELABEL(hm3301)),
132135
.health = {0},
133136
.enabled = true},
134-
[SENSOR_TYPE_SEN0466] = {
135-
.device = DEVICE_DT_GET(DT_NODELABEL(sen0466)), .health = {0}, .enabled = true}};
137+
[SENSOR_TYPE_SEN0466] = {.device = DEVICE_DT_GET(DT_NODELABEL(sen0466)),
138+
.health = {0},
139+
.enabled = true},
140+
};
136141

137142
/* Forward declarations */
138143
static void sensor_thread(void *p1, void *p2, void *p3);
@@ -178,59 +183,41 @@ static void sensor_thread(void *p1, void *p2, void *p3)
178183
/* Initialize state machine context */
179184
sensor_state_obj = SENSOR_STATE_OBJECT_INIT();
180185
sensor_state_obj.max_retries = CONFIG_SENSOR_MODULE_MAX_RETRIES;
181-
sensor_state_obj.read_timeout_ms = CONFIG_SENSOR_MODULE_READ_TIMEOUT_MS;
182186

183187
/* Initialize state machine */
184188
smf_set_initial(SMF_CTX(&sensor_state_obj), &sensor_states[SENSOR_MODULE_STATE_INIT]);
185189
sensor_state_obj.current_state = SENSOR_MODULE_STATE_INIT;
186190

187-
/* Run initial state machine setup */
188-
k_mutex_lock(&sensor_sm_mutex, K_FOREVER);
189-
int ret = smf_run_state(SMF_CTX(&sensor_state_obj));
190-
k_mutex_unlock(&sensor_sm_mutex);
191-
if (ret < 0) {
192-
LOG_ERR("Failed to run sensor state machine (%d)", ret);
193-
}
194-
195191
LOG_INF("Sensor module thread started");
192+
196193
while (1) {
197-
/* Wait for messages directly from ZBUS subscriber */
194+
/* Wait for Zbus message with finite timeout so SMF can progress on time-based
195+
* states */
198196
int err = zbus_sub_wait_msg(&sensor_subscriber, &sensor_state_obj.chan,
199-
sensor_state_obj.msg_buf, K_FOREVER);
200-
if (err == 0 && sensor_state_obj.chan == &sensor_chan) {
201-
struct sensor_msg msg = MSG_TO_SENSOR_MSG(sensor_state_obj.msg_buf);
202-
if (msg.type == SENSOR_SAMPLE_REQUEST) {
203-
LOG_DBG("Processing sensor request directly");
204-
205-
/* Trigger state machine to read sensors with mutex protection */
206-
k_mutex_lock(&sensor_sm_mutex, K_FOREVER);
207-
if (sensor_state_obj.current_state == SENSOR_MODULE_STATE_IDLE) {
208-
sensor_set_state(&sensor_state_obj,
209-
SENSOR_MODULE_STATE_READING);
210-
}
211-
212-
/* Run state machine - this performs blocking sensor operations */
213-
int ret = smf_run_state(SMF_CTX(&sensor_state_obj));
214-
if (ret < 0) {
215-
LOG_ERR("State machine execution failed (%d)", ret);
216-
/* Error handling is done internally by the state machine */
217-
}
218-
k_mutex_unlock(&sensor_sm_mutex);
197+
sensor_state_obj.msg_buf,
198+
K_MSEC(CONFIG_SENSOR_MODULE_TICK_MS));
199+
if (err == 0) {
200+
/* We do NOT change state here. Idle will inspect the message and decide. */
201+
if (sensor_state_obj.chan != &sensor_chan) {
202+
/* Unrecognized channel: ignore but keep SMF ticking */
203+
sensor_state_obj.chan = NULL;
219204
}
220205
}
221206

222-
/* Also run state machine periodically for maintenance and recovery */
223-
k_mutex_lock(&sensor_sm_mutex, K_FOREVER);
224-
smf_run_state(SMF_CTX(&sensor_state_obj));
225-
k_mutex_unlock(&sensor_sm_mutex);
226-
k_sleep(K_MSEC(CONFIG_SENSOR_MODULE_THREAD_SLEEP_MS));
207+
/* Always progress the state machine once per loop tick (no mutex around sleeps) */
208+
int ret = smf_run_state(SMF_CTX(&sensor_state_obj));
209+
if (ret < 0) {
210+
LOG_ERR("State machine execution failed (%d)", ret);
211+
}
227212
}
228213
}
229214

230215
/* State machine implementation */
231216

232217
static void sensor_state_init_run(void *obj)
233218
{
219+
LOG_DBG("%s", __func__);
220+
234221
struct sensor_state_object *ctx = (struct sensor_state_object *)obj;
235222

236223
LOG_INF("Sensor SM: Initializing sensors");
@@ -253,9 +240,7 @@ static void sensor_state_init_run(void *obj)
253240

254241
/* Initialize context with configured values */
255242
ctx->error_count = 0;
256-
ctx->max_retries = sensor_state_obj.max_retries;
257-
ctx->read_timeout_ms = sensor_state_obj.read_timeout_ms;
258-
ctx->last_read_time = k_uptime_get();
243+
ctx->recovery_attempts = 0;
259244

260245
LOG_INF("Sensor SM: Initialization complete");
261246
sensor_set_state(ctx, SENSOR_MODULE_STATE_IDLE);
@@ -265,28 +250,49 @@ static void sensor_state_idle_run(void *obj)
265250
{
266251
struct sensor_state_object *ctx = (struct sensor_state_object *)obj;
267252

268-
LOG_DBG("Sensor SM: Idle state - waiting for requests");
253+
/* If we have a message on our channel, inspect it */
254+
if (ctx->chan == &sensor_chan) {
255+
struct sensor_msg msg;
256+
/* Avoid unaligned access by copying from the raw buffer */
257+
memcpy(&msg, ctx->msg_buf, MIN(sizeof(msg), MAX_MSG_SIZE));
269258

270-
/* In real implementation, this would wait for events */
271-
k_sleep(K_MSEC(CONFIG_SENSOR_MODULE_THREAD_SLEEP_MS));
259+
/* Consume the message exactly once */
260+
ctx->chan = NULL;
272261

273-
/* For demo purposes, automatically transition to reading after some time */
274-
if (k_uptime_get() - ctx->last_read_time > ctx->read_timeout_ms) {
275-
LOG_DBG("Sensor SM: Timeout - starting automatic read");
276-
sensor_set_state(ctx, SENSOR_MODULE_STATE_READING);
262+
if (msg.type == SENSOR_SAMPLE_REQUEST) {
263+
/* Optional: gate on warm-up completion of at least one enabled sensor */
264+
#if defined(CONFIG_SENSOR_MODULE_WARMUP_ENABLE)
265+
bool any_ready = false;
266+
for (int i = 0; i < SENSOR_TYPE_COUNT; i++) {
267+
if (sensors[i].enabled && is_sensor_warmup_complete(i)) {
268+
any_ready = true;
269+
break;
270+
}
271+
}
272+
if (!any_ready) {
273+
LOG_DBG("Sensor SM: request received but sensors still warming");
274+
return; /* stay IDLE, next ticks will re-check */
275+
}
276+
#endif
277+
sensor_set_state(ctx, SENSOR_MODULE_STATE_READING);
278+
return;
279+
}
277280
}
281+
282+
/* Otherwise remain idle */
278283
}
279284

280285
static void sensor_state_reading_run(void *obj)
281286
{
287+
LOG_DBG("%s", __func__);
288+
282289
struct sensor_state_object *ctx = (struct sensor_state_object *)obj;
283290
int ret;
284291

285292
LOG_DBG("Sensor SM: Reading sensor data");
286293

287294
/* Initialize response */
288295
ctx->current_data.type = SENSOR_SAMPLE_RESPONSE;
289-
ctx->last_read_time = k_uptime_get();
290296
#if defined(CONFIG_SENSOR_MODULE_TIMESTAMP)
291297
ret = date_time_now(&ctx->current_data.timestamp);
292298
if (ret < 0) {
@@ -313,7 +319,7 @@ static void sensor_state_reading_run(void *obj)
313319
}
314320
#endif
315321

316-
int ret = read_sensor_data(i, &ctx->current_data);
322+
ret = read_sensor_data(i, &ctx->current_data);
317323
if (ret == 0) {
318324
successful_reads++;
319325
update_sensor_health(&sensors[i].health, true);
@@ -378,11 +384,10 @@ static void sensor_state_reading_run(void *obj)
378384
sensor_set_state(ctx, SENSOR_MODULE_STATE_ERROR);
379385
#endif
380386
} else if (successful_reads < enabled_sensor_count) {
381-
LOG_WRN("Sensor SM: Partial sensor failure (%d/%d successful), but publishing "
387+
LOG_WRN("Sensor SM: Partial sensor failure (%d/%d successful), publishing "
382388
"available data",
383389
successful_reads, enabled_sensor_count);
384390

385-
/* Publish partial data */
386391
ret = zbus_chan_pub(&sensor_chan, &ctx->current_data,
387392
K_MSEC(CONFIG_SENSOR_MODULE_DATA_PUBLISH_TIMEOUT_MS));
388393
if (ret < 0) {
@@ -411,40 +416,46 @@ static void sensor_state_reading_run(void *obj)
411416

412417
static void sensor_state_error_run(void *obj)
413418
{
419+
LOG_DBG("%s", __func__);
414420
struct sensor_state_object *ctx = (struct sensor_state_object *)obj;
415421

416-
LOG_ERR("Sensor SM: Error state - attempting recovery");
417-
418-
/* Wait before attempting recovery using configurable delay */
422+
LOG_ERR("Sensor SM: Error state - attempting recovery after delay");
419423
k_sleep(K_MSEC(CONFIG_SENSOR_MODULE_RECOVERY_DELAY_MS));
420424

421425
sensor_set_state(ctx, SENSOR_MODULE_STATE_RECOVERY);
422426
}
423427

424428
static void sensor_state_recovery_run(void *obj)
425429
{
430+
LOG_DBG("%s", __func__);
426431
struct sensor_state_object *ctx = (struct sensor_state_object *)obj;
427432

428-
LOG_INF("Sensor SM: Attempting recovery");
429-
430-
/* Reset error count and try to re-initialize */
431-
ctx->error_count = 0;
433+
LOG_INF("Sensor SM: Attempting recovery (attempt %d)", ctx->recovery_attempts + 1);
432434

433435
int ret = init_sensors();
434436
if (ret < 0) {
435437
LOG_ERR("Sensor SM: Recovery failed (%d)", ret);
436-
ctx->error_count++;
438+
ctx->recovery_attempts++;
437439

438-
/* If recovery keeps failing, stay in error state */
439-
if (ctx->error_count > CONFIG_SENSOR_MODULE_MAX_RECOVERY_ATTEMPTS) {
440+
if (ctx->recovery_attempts > CONFIG_SENSOR_MODULE_MAX_RECOVERY_ATTEMPTS) {
440441
LOG_ERR("Sensor SM: Recovery attempts exhausted");
441442
sensor_set_state(ctx, SENSOR_MODULE_STATE_ERROR);
442443
} else {
443-
/* Try recovery again */
444444
k_sleep(K_MSEC(CONFIG_SENSOR_MODULE_RECOVERY_RETRY_DELAY_MS));
445+
/* Stay in RECOVERY; next tick will retry */
445446
}
446447
} else {
447448
LOG_INF("Sensor SM: Recovery successful");
449+
ctx->error_count = 0;
450+
ctx->recovery_attempts = 0;
451+
452+
#if defined(CONFIG_SENSOR_MODULE_WARMUP_ENABLE)
453+
/* Reset warm-up tracking after successful recovery */
454+
ctx->sensor_init_time = k_uptime_get();
455+
for (int i = 0; i < SENSOR_TYPE_COUNT; i++) {
456+
ctx->sensor_warmup_complete[i] = false;
457+
}
458+
#endif
448459
sensor_set_state(ctx, SENSOR_MODULE_STATE_IDLE);
449460
}
450461
}
@@ -459,7 +470,7 @@ static int init_sensors(void)
459470
continue;
460471
}
461472

462-
/* Devices are now initialized at declaration, just check if ready */
473+
/* Devices are initialized at declaration, just check if ready */
463474
if (!device_is_ready(sensors[i].device)) {
464475
LOG_ERR("%s sensor not ready", get_sensor_name(i));
465476
sensors[i].enabled = false; /* Disable if not ready */

0 commit comments

Comments
 (0)