sensors: Add channel specifier

Use a structured channel specifier rather than a single enum when
specifying channels to read in the new read/decoder API.

Signed-off-by: Tom Burdick <[email protected]>

drivers/sensor/bosch/bma4xx/bma4xx.c

@@ -343,7 +343,7 @@ static int bma4xx_submit_one_shot(const struct device *dev, struct rtio_iodev_sq
 	struct bma4xx_data *bma4xx = dev->data;
 
 	const struct sensor_read_config *cfg = iodev_sqe->sqe.iodev->data;
-	const enum sensor_channel *const channels = cfg->channels;
+	const struct sensor_chan_spec *const channels = cfg->channels;
 	const size_t num_channels = cfg->count;
 
 	uint32_t min_buf_len = sizeof(struct bma4xx_encoded_data);
@@ -370,7 +370,11 @@ static int bma4xx_submit_one_shot(const struct device *dev, struct rtio_iodev_sq
 
 	/* Determine what channels we need to fetch */
 	for (int i = 0; i < num_channels; i++) {
-		switch (channels[i]) {
+		if (channels[i].chan_idx != 0) {
+			LOG_ERR("Only channel index 0 supported");
+			return -ENOTSUP;
+		}
+		switch (channels[i].chan_type) {
 		case SENSOR_CHAN_ALL:
 			edata->has_accel = 1;
 #ifdef CONFIG_BMA4XX_TEMPERATURE

drivers/sensor/default_rtio_sensor.c

@@ -46,12 +46,13 @@ const struct rtio_iodev_api __sensor_iodev_api = {
  * @param[in] num_channels Number of channels on the @p channels array
  * @return The number of samples required to read the given channels
  */
-static inline int compute_num_samples(const enum sensor_channel *channels, size_t num_channels)
+static inline int compute_num_samples(const struct sensor_chan_spec *const channels,
+				      size_t num_channels)
 {
 	int num_samples = 0;
 
 	for (size_t i = 0; i < num_channels; ++i) {
-		num_samples += SENSOR_CHANNEL_3_AXIS(channels[i]) ? 3 : 1;
+		num_samples += SENSOR_CHANNEL_3_AXIS(channels[i].chan_type) ? 3 : 1;
 	}
 
 	return num_samples;
@@ -113,7 +114,7 @@ static inline int check_header_contains_channel(const struct sensor_data_generic
 static void sensor_submit_fallback(const struct device *dev, struct rtio_iodev_sqe *iodev_sqe)
 {
 	const struct sensor_read_config *cfg = iodev_sqe->sqe.iodev->data;
-	const enum sensor_channel *const channels = cfg->channels;
+	const struct sensor_chan_spec *const channels = cfg->channels;
 	const int num_output_samples = compute_num_samples(channels, cfg->count);
 	uint32_t min_buf_len = compute_min_buf_len(num_output_samples);
 	uint64_t timestamp_ns = k_ticks_to_ns_floor64(k_uptime_ticks());
@@ -148,24 +149,26 @@ static void sensor_submit_fallback(const struct device *dev, struct rtio_iodev_s
 	/* Populate values, update shift, and set channels */
 	for (size_t i = 0, sample_idx = 0; i < cfg->count; ++i) {
 		struct sensor_value value[3];
-		const int num_samples = SENSOR_CHANNEL_3_AXIS(channels[i]) ? 3 : 1;
+		const int num_samples = SENSOR_CHANNEL_3_AXIS(channels[i].chan_type) ? 3 : 1;
 
 		/* Get the current channel requested by the user */
-		rc = sensor_channel_get(dev, channels[i], value);
+		rc = sensor_channel_get(dev, channels[i].chan_type, value);
 
 		if (num_samples == 3) {
 			header->channels[sample_idx++] =
-				rc == 0 ? channels[i] - 3 : SENSOR_CHAN_MAX;
+				rc == 0 ? channels[i].chan_type - 3 : SENSOR_CHAN_MAX;
 			header->channels[sample_idx++] =
-				rc == 0 ? channels[i] - 2 : SENSOR_CHAN_MAX;
+				rc == 0 ? channels[i].chan_type - 2 : SENSOR_CHAN_MAX;
 			header->channels[sample_idx++] =
-				rc == 0 ? channels[i] - 1 : SENSOR_CHAN_MAX;
+				rc == 0 ? channels[i].chan_type - 1 : SENSOR_CHAN_MAX;
 		} else {
-			header->channels[sample_idx++] = rc == 0 ? channels[i] : SENSOR_CHAN_MAX;
+			header->channels[sample_idx++] =
+				rc == 0 ? channels[i].chan_type : SENSOR_CHAN_MAX;
 		}
 
 		if (rc != 0) {
-			LOG_DBG("Failed to get channel %d, skipping", channels[i]);
+			LOG_DBG("Failed to get channel (type: %d, index %d), skipping",
+				channels[i].chan_type, channels[i].chan_idx);
 			continue;
 		}
 

drivers/sensor/sensor_shell.c

@@ -217,7 +217,7 @@ static enum dynamic_command_context current_cmd_ctx = NONE;
 K_MUTEX_DEFINE(cmd_get_mutex);
 
 /* Crate a single common config for one-shot reading */
-static enum sensor_channel iodev_sensor_shell_channels[SENSOR_CHAN_ALL];
+static struct sensor_chan_spec iodev_sensor_shell_channels[SENSOR_CHAN_ALL];
 static struct sensor_read_config iodev_sensor_shell_read_config = {
 	.sensor = NULL,
 	.is_streaming = false,
@@ -521,12 +521,12 @@ static int cmd_get_sensor(const struct shell *sh, size_t argc, char *argv[])
 	}
 
 	if (argc == 2) {
-		/* read all channels */
+		/* read all channel types */
 		for (int i = 0; i < ARRAY_SIZE(iodev_sensor_shell_channels); ++i) {
 			if (SENSOR_CHANNEL_3_AXIS(i)) {
 				continue;
 			}
-			iodev_sensor_shell_channels[count++] = i;
+			iodev_sensor_shell_channels[count++] = (struct sensor_chan_spec){i, 0};
 		}
 	} else {
 		/* read specific channels */
@@ -538,7 +538,8 @@ static int cmd_get_sensor(const struct shell *sh, size_t argc, char *argv[])
 				shell_error(sh, "Failed to read channel (%s)", argv[i]);
 				continue;
 			}
-			iodev_sensor_shell_channels[count++] = chan;
+			iodev_sensor_shell_channels[count++] =
+				(struct sensor_chan_spec){chan, 0};
 		}
 	}
 
@@ -570,7 +571,6 @@ static int cmd_get_sensor(const struct shell *sh, size_t argc, char *argv[])
 	return 0;
 }
 
-
 static int cmd_sensor_attr_set(const struct shell *shell_ptr, size_t argc, char *argv[])
 {
 	const struct device *dev;

drivers/sensor/tdk/icm42688/icm42688_decoder.c

@@ -178,7 +178,7 @@ static uint8_t icm42688_encode_channel(enum sensor_channel chan)
 	return encode_bmask;
 }
 
-int icm42688_encode(const struct device *dev, const enum sensor_channel *const channels,
+int icm42688_encode(const struct device *dev, const struct sensor_chan_spec *const channels,
 		    const size_t num_channels, uint8_t *buf)
 {
 	struct icm42688_dev_data *data = dev->data;
@@ -187,7 +187,7 @@ int icm42688_encode(const struct device *dev, const enum sensor_channel *const c
 	edata->channels = 0;
 
 	for (int i = 0; i < num_channels; i++) {
-		edata->channels |= icm42688_encode_channel(channels[i]);
+		edata->channels |= icm42688_encode_channel(channels[i].chan_type);
 	}
 
 	edata->header.is_fifo = false;

drivers/sensor/tdk/icm42688/icm42688_decoder.h

@@ -36,7 +36,7 @@ struct icm42688_encoded_data {
 	int16_t readings[7];
 };
 
-int icm42688_encode(const struct device *dev, const enum sensor_channel *const channels,
+int icm42688_encode(const struct device *dev, const struct sensor_chan_spec *const channels,
 		    const size_t num_channels, uint8_t *buf);
 
 int icm42688_get_decoder(const struct device *dev, const struct sensor_decoder_api **decoder);

drivers/sensor/tdk/icm42688/icm42688_rtio.c

@@ -46,7 +46,7 @@ static int icm42688_rtio_sample_fetch(const struct device *dev, int16_t readings
 static int icm42688_submit_one_shot(const struct device *dev, struct rtio_iodev_sqe *iodev_sqe)
 {
 	const struct sensor_read_config *cfg = iodev_sqe->sqe.iodev->data;
-	const enum sensor_channel *const channels = cfg->channels;
+	const struct sensor_chan_spec *const channels = cfg->channels;
 	const size_t num_channels = cfg->count;
 	uint32_t min_buf_len = sizeof(struct icm42688_encoded_data);
 	int rc;

include/zephyr/drivers/sensor.h

@@ -418,6 +418,17 @@ typedef int (*sensor_channel_get_t)(const struct device *dev,
 				    enum sensor_channel chan,
 				    struct sensor_value *val);
 
+/**
+ * @brief Sensor Channel Specification
+ *
+ * A sensor channel specification is a unique identifier per sensor device describing
+ * a measurement channel.
+ */
+struct sensor_chan_spec {
+	uint16_t chan_type; /**< A sensor channel type */
+	uint16_t chan_idx;  /**< A sensor channel index */
+};
+
 /**
  * @brief Decodes a single raw data buffer
  *
@@ -435,7 +446,7 @@ struct sensor_decoder_api {
 	 * @return 0 on success
 	 * @return -ENOTSUP if the channel/channel_idx aren't found
 	 */
-	int (*get_frame_count)(const uint8_t *buffer, enum sensor_channel channel,
+	int (*get_frame_count)(const uint8_t *buffer, struct sensor_chan_spec channel,
 			       size_t channel_idx, uint16_t *frame_count);
 
 	/**
@@ -450,7 +461,8 @@ struct sensor_decoder_api {
 	 * @return 0 on success
 	 * @return -ENOTSUP if the channel is not supported
 	 */
-	int (*get_size_info)(enum sensor_channel channel, size_t *base_size, size_t *frame_size);
+	int (*get_size_info)(struct sensor_chan_spec channel, size_t *base_size,
+			     size_t *frame_size);
 
 	/**
 	 * @brief Decode up to @p max_count samples from the buffer
@@ -478,7 +490,7 @@ struct sensor_decoder_api {
 	 * @return >0 the number of decoded frames
 	 * @return <0 on error
 	 */
-	int (*decode)(const uint8_t *buffer, enum sensor_channel channel, size_t channel_idx,
+	int (*decode)(const uint8_t *buffer, struct sensor_chan_spec channel, size_t channel_idx,
 		      uint32_t *fit, uint16_t max_count, void *data_out);
 
 	/**
@@ -518,7 +530,7 @@ struct sensor_decoder_api {
 struct sensor_decode_context {
 	const struct sensor_decoder_api *decoder;
 	const uint8_t *buffer;
-	enum sensor_channel channel;
+	struct sensor_chan_spec channel;
 	size_t channel_idx;
 	uint32_t fit;
 };
@@ -549,7 +561,7 @@ static inline int sensor_decode(struct sensor_decode_context *ctx, void *out, ui
 				    max_count, out);
 }
 
-int sensor_natively_supported_channel_size_info(enum sensor_channel channel, size_t *base_size,
+int sensor_natively_supported_channel_size_info(struct sensor_chan_spec channel, size_t *base_size,
 						size_t *frame_size);
 
 /**
@@ -582,6 +594,7 @@ struct sensor_stream_trigger {
 	{                                                                                          \
 		.trigger = (_trigger), .opt = (_opt),                                              \
 	}
+
 /*
  * Internal data structure used to store information about the IODevice for async reading and
  * streaming sensor data.
@@ -590,7 +603,7 @@ struct sensor_read_config {
 	const struct device *sensor;
 	const bool is_streaming;
 	union {
-		enum sensor_channel *const channels;
+		struct sensor_chan_spec *const channels;
 		struct sensor_stream_trigger *const triggers;
 	};
 	size_t count;
@@ -604,15 +617,16 @@ struct sensor_read_config {
  *
  * @code(.c)
  * SENSOR_DT_READ_IODEV(icm42688_accelgyro, DT_NODELABEL(icm42688),
- *     SENSOR_CHAN_ACCEL_XYZ, SENSOR_CHAN_GYRO_XYZ);
+ *     { SENSOR_CHAN_ACCEL_XYZ, 0 },
+ *     { SENSOR_CHAN_GYRO_XYZ, 0 });
  *
  * int main(void) {
  *   sensor_read(&icm42688_accelgyro, &rtio);
  * }
  * @endcode
  */
 #define SENSOR_DT_READ_IODEV(name, dt_node, ...)                                                   \
-	static enum sensor_channel _CONCAT(__channel_array_, name)[] = {__VA_ARGS__};              \
+	static struct sensor_chan_spec _CONCAT(__channel_array_, name)[] = {__VA_ARGS__};          \
 	static struct sensor_read_config _CONCAT(__sensor_read_config_, name) = {                  \
 		.sensor = DEVICE_DT_GET(dt_node),                                                  \
 		.is_streaming = false,                                                             \
@@ -886,10 +900,10 @@ struct __attribute__((__packed__)) sensor_data_generic_header {
 	int8_t shift;
 
 	/* This padding is needed to make sure that the 'channels' field is aligned */
-	int8_t _padding[sizeof(enum sensor_channel) - 1];
+	int8_t _padding[sizeof(struct sensor_chan_spec) - 1];
 
 	/* Channels present in the frame */
-	enum sensor_channel channels[0];
+	struct sensor_chan_spec channels[0];
 };
 
 /**

tests/drivers/build_all/sensor/src/generic_test.c

@@ -30,7 +30,7 @@ union sensor_data_union {
  * Set up an RTIO context that can be shared for all sensors
  */
 
-static enum sensor_channel iodev_all_channels[SENSOR_CHAN_ALL];
+static struct sensor_chan_spec iodev_all_channels[SENSOR_CHAN_ALL];
 static struct sensor_read_config iodev_read_config = {
 	.channels = iodev_all_channels,
 	.max = SENSOR_CHAN_ALL,
@@ -120,7 +120,7 @@ static void run_generic_test(const struct device *dev)
 				channel_table[ch].epsilon, shift);
 
 			/* Add to the list of channels to read */
-			iodev_all_channels[iodev_read_config.count++] = ch;
+			iodev_all_channels[iodev_read_config.count++].chan_type = ch;
 
 			/* Generate a set of CONFIG_GENERIC_SENSOR_TEST_NUM_EXPECTED_VALS test
 			 * values.
@@ -155,16 +155,17 @@ static void run_generic_test(const struct device *dev)
 
 		/* Set this iteration's expected values in emul for every supported channel */
 		for (size_t i = 0; i < iodev_read_config.count; i++) {
-			enum sensor_channel ch = iodev_all_channels[i];
+			struct sensor_chan_spec ch = iodev_all_channels[i];
 
 			rv = emul_sensor_backend_set_channel(
-				emul, ch, &channel_table[ch].expected_values[iteration],
-				channel_table[ch].expected_value_shift);
-			zassert_ok(
-				rv,
-				"Cannot set value 0x%08x on channel %d (error %d, iteration %d/%d)",
-				channel_table[i].expected_values[iteration], ch, rv, iteration + 1,
-				CONFIG_GENERIC_SENSOR_TEST_NUM_EXPECTED_VALS);
+				emul, ch, &channel_table[ch.chan_type].expected_values[iteration],
+				channel_table[ch.chan_type].expected_value_shift);
+			zassert_ok(rv,
+				   "Cannot set value 0x%08x on channel (type: %d, index: %d) "
+				   "(error %d, iteration %d/%d)",
+				   channel_table[i].expected_values[iteration], ch.chan_type,
+				   ch.chan_idx, rv, iteration + 1,
+				   CONFIG_GENERIC_SENSOR_TEST_NUM_EXPECTED_VALS);
 		}
 
 		/* Perform the actual sensor read */