From e69ce68d409417b0519734056e10de528c9eeded Mon Sep 17 00:00:00 2001 From: Tom Burdick Date: Fri, 1 Dec 2023 13:40:25 -0600 Subject: [PATCH] sensing: Fix initializer compile-time constant Initializers must be compile time constants (not expressions!) for clang to be happy. Clang rightfully pointed out that the callback_list member of sensing_connection was being initialized using an expression that was not compile-time constant. The macros passed along a pointer created by taking the address of a global constant and then at initialization time attempting to dereference it using the * operator. So in the end the compiler has identifier its trying to first take the address of and then later dereference in an initializer. Clang didn't appreciate this, though gcc seemed to be just fine. Actually clang 17 might work just fine with this as well, but clang 16 (the clang I tried) didn't like it at all. Instead store the pointer to the callback list rather than copying the struct. This could be done the other way and not passing a pointer through the macros perhaps. Signed-off-by: Tom Burdick --- include/zephyr/sensing/sensing.h | 10 ++++++---- include/zephyr/sensing/sensing_sensor.h | 4 ++-- samples/subsys/sensing/simple/src/main.c | 14 ++++++++------ subsys/sensing/dispatch.c | 6 +++--- subsys/sensing/sensing.c | 4 ++-- subsys/sensing/sensor/hinge_angle/hinge_angle.c | 2 +- subsys/sensing/sensor_mgmt.c | 4 ++-- subsys/sensing/sensor_mgmt.h | 2 +- 8 files changed, 25 insertions(+), 21 deletions(-) diff --git a/include/zephyr/sensing/sensing.h b/include/zephyr/sensing/sensing.h index 62e48873f56..4491874fc84 100644 --- a/include/zephyr/sensing/sensing.h +++ b/include/zephyr/sensing/sensing.h @@ -196,7 +196,8 @@ int sensing_get_sensors(int *num_sensors, const struct sensing_sensor_info **inf * * @param info The sensor info got from \ref sensing_get_sensors * - * @param cb_list callback list to be registered to sensing. + * @param cb_list callback list to be registered to sensing, must have a static + * lifetime. * * @param handle The opened instance handle, if failed will be set to NULL. * @@ -204,7 +205,7 @@ int sensing_get_sensors(int *num_sensors, const struct sensing_sensor_info **inf */ int sensing_open_sensor( const struct sensing_sensor_info *info, - const struct sensing_callback_list *cb_list, + struct sensing_callback_list *cb_list, sensing_sensor_handle_t *handle); /** @@ -217,14 +218,15 @@ int sensing_open_sensor( * * @param dev pointer device get from device tree. * - * @param cb_list callback list to be registered to sensing. + * @param cb_list callback list to be registered to sensing, must have a static + * lifetime. * * @param handle The opened instance handle, if failed will be set to NULL. * * @return 0 on success or negative error value on failure. */ int sensing_open_sensor_by_dt( - const struct device *dev, const struct sensing_callback_list *cb_list, + const struct device *dev, struct sensing_callback_list *cb_list, sensing_sensor_handle_t *handle); /** diff --git a/include/zephyr/sensing/sensing_sensor.h b/include/zephyr/sensing/sensing_sensor.h index a4e87d8ed9e..55f00a5c29d 100644 --- a/include/zephyr/sensing/sensing_sensor.h +++ b/include/zephyr/sensing/sensing_sensor.h @@ -80,7 +80,7 @@ struct sensing_connection { /* client(sink) next consume time */ uint64_t next_consume_time; /* post data to application */ - struct sensing_callback_list callback_list; + struct sensing_callback_list *callback_list; }; /** @@ -136,7 +136,7 @@ extern struct sensing_sensor SENSING_SENSOR_SOURCE_NAME(idx, node); \ #define SENSING_CONNECTION_INITIALIZER(source_name, cb_list_ptr) \ { \ - .callback_list = *cb_list_ptr, \ + .callback_list = cb_list_ptr, \ .source = &source_name, \ } diff --git a/samples/subsys/sensing/simple/src/main.c b/samples/subsys/sensing/simple/src/main.c index b57c65c718c..d0749d3b52c 100644 --- a/samples/subsys/sensing/simple/src/main.c +++ b/samples/subsys/sensing/simple/src/main.c @@ -38,14 +38,16 @@ static void hinge_angle_data_event_callback(sensing_sensor_handle_t handle, cons LOG_INF("handle:%p, Sensor:%s data:(v:%d)", handle, info->name, sample->readings[0].v); } +static struct sensing_callback_list base_acc_cb_list = { + .on_data_event = &acc_data_event_callback, +}; + +static struct sensing_callback_list hinge_angle_cb_list = { + .on_data_event = &hinge_angle_data_event_callback, +}; + int main(void) { - const struct sensing_callback_list base_acc_cb_list = { - .on_data_event = &acc_data_event_callback, - }; - const struct sensing_callback_list hinge_angle_cb_list = { - .on_data_event = &hinge_angle_data_event_callback, - }; const struct sensing_sensor_info *info; sensing_sensor_handle_t base_acc; sensing_sensor_handle_t hinge_angle; diff --git a/subsys/sensing/dispatch.c b/subsys/sensing/dispatch.c index 5a85bd1d218..34cd45cf3c8 100644 --- a/subsys/sensing/dispatch.c +++ b/subsys/sensing/dispatch.c @@ -59,13 +59,13 @@ static int send_data_to_clients(struct sensing_sensor *sensor, update_client_consume_time(sensor, conn); - if (!conn->callback_list.on_data_event) { + if (!conn->callback_list->on_data_event) { LOG_WRN("sensor:%s event callback not registered", conn->source->dev->name); continue; } - conn->callback_list.on_data_event(conn, data, - conn->callback_list.context); + conn->callback_list->on_data_event(conn, data, + conn->callback_list->context); } return 0; diff --git a/subsys/sensing/sensing.c b/subsys/sensing/sensing.c index 9b5469b5c7e..1f3f05f6ab1 100644 --- a/subsys/sensing/sensing.c +++ b/subsys/sensing/sensing.c @@ -14,7 +14,7 @@ LOG_MODULE_DECLARE(sensing, CONFIG_SENSING_LOG_LEVEL); /* sensing_open_sensor is normally called by applications: hid, chre, zephyr main, etc */ int sensing_open_sensor(const struct sensing_sensor_info *sensor_info, - const struct sensing_callback_list *cb_list, + struct sensing_callback_list *cb_list, sensing_sensor_handle_t *handle) { int ret = 0; @@ -37,7 +37,7 @@ int sensing_open_sensor(const struct sensing_sensor_info *sensor_info, } int sensing_open_sensor_by_dt(const struct device *dev, - const struct sensing_callback_list *cb_list, + struct sensing_callback_list *cb_list, sensing_sensor_handle_t *handle) { int ret = 0; diff --git a/subsys/sensing/sensor/hinge_angle/hinge_angle.c b/subsys/sensing/sensor/hinge_angle/hinge_angle.c index 9d9dc70cef7..077f7ca3774 100644 --- a/subsys/sensing/sensor/hinge_angle/hinge_angle.c +++ b/subsys/sensing/sensor/hinge_angle/hinge_angle.c @@ -153,7 +153,7 @@ static void hinge_reporter_on_data_event(sensing_sensor_handle_t handle, #define DT_DRV_COMPAT zephyr_sensing_hinge_angle #define SENSING_HINGE_ANGLE_DT_DEFINE(_inst) \ static struct hinge_angle_context _CONCAT(hinge_ctx, _inst); \ - static const struct sensing_callback_list _CONCAT(hinge_cb, _inst) = { \ + static struct sensing_callback_list _CONCAT(hinge_cb, _inst) = { \ .on_data_event = hinge_reporter_on_data_event, \ .context = &_CONCAT(hinge_ctx, _inst), \ }; \ diff --git a/subsys/sensing/sensor_mgmt.c b/subsys/sensing/sensor_mgmt.c index 4270844ec00..08c5ea0462d 100644 --- a/subsys/sensing/sensor_mgmt.c +++ b/subsys/sensing/sensor_mgmt.c @@ -370,7 +370,7 @@ int close_sensor(struct sensing_connection **conn) } int sensing_register_callback(struct sensing_connection *conn, - const struct sensing_callback_list *cb_list) + struct sensing_callback_list *cb_list) { if (conn == NULL) { LOG_ERR("register sensing callback list, connection not be NULL"); @@ -383,7 +383,7 @@ int sensing_register_callback(struct sensing_connection *conn, LOG_ERR("callback should not be NULL"); return -ENODEV; } - conn->callback_list = *cb_list; + conn->callback_list = cb_list; return 0; } diff --git a/subsys/sensing/sensor_mgmt.h b/subsys/sensing/sensor_mgmt.h index 7988958af29..b29bd184b23 100644 --- a/subsys/sensing/sensor_mgmt.h +++ b/subsys/sensing/sensor_mgmt.h @@ -52,7 +52,7 @@ struct sensing_context { int open_sensor(struct sensing_sensor *sensor, struct sensing_connection **conn); int close_sensor(struct sensing_connection **conn); int sensing_register_callback(struct sensing_connection *conn, - const struct sensing_callback_list *cb_list); + struct sensing_callback_list *cb_list); int set_interval(struct sensing_connection *conn, uint32_t interval); int get_interval(struct sensing_connection *con, uint32_t *sensitivity); int set_sensitivity(struct sensing_connection *conn, int8_t index, uint32_t interval);