RFC: Sensor Channel Specifiers #63830
Comments
teburd
changed the title
|
If index here for the spectrometer examples is analogous to the wavelength in nm (500 nm here), a 16-bit integer should be sufficient for most use cases from UV to IR, but is there a mechanism to programmatically determine what index values are exposed by the driver? Ex: 285, 315, 360, 405, 452, 505, 580, 670, 745, 780 (random numbers) might be a sensible UV to visible to IR sensor range in nm, but how do I know what's available as a developer? Otherwise, would a linear index 0..9 make more sense with per channel/index attributes to indicate the center wavelength in nm, potentially with more precision since it's unlikely to be a clean integer? I could then just scan the index range until I hit an exception, and probably also set a precise value like 524.6 nm via an attribute? Sorry if the answer is obvious ... on my phone and I've been away from the sensor API for a long time. |
yeah I changed it in realizing this exact issue, the index is just the channel index into the sensor channel array now. An attribute of the channel could be the wavelength. I think some of them also let you maybe select which wavelength in some channels to use? I swore I saw one that did that. Which makes this even more dubious to try and use the index as a wavelength.
No, you were spot on, and I've corrected it. |
|
Hi @teburd, This is a very relevant use-case for one of the sensors i am working on. +1 from me and would be happy to collaborate on this. |
Same here. As mentioned in the Sensors WG meeting yesterday, my only concern is adding a new API and adding "legacy" wrappers instead of just modifying the old API. At minimum, I think we need to have a migration plan for all in-tree drivers to use the new sensor channel specifiers. One option could be to go with the legacy wrappers for the initial PR and create a meta issue for assigning/tracking driver conversions. |
|
Another candidate would be for particulate matter and particle count sensors, where the subtype could be used to specify the particle size, among the common values 1.0, 2.5, etc. |
The more I keep looking at this, the more I wonder if what we want is to put channel descriptors into DT and solely use an index into this channel descriptor array as the identifier, with of course some functions to find the correct channel index. uint32_t pm25_idx = DT_SENSOR_CHANNEL_IDX(mysensornode, SENSOR_CHAN_PM, SENSOR_CHAN_ST_2_5, 0);Kind of thing perhaps... This way we can at runtime also iterate over all potential channels a sensor has for things like the sensor shell, or maybe a sensor tracing app (dump the data to a visualizer on a host PC) I think there's some nice reasons to go this route and maybe worth revamping the RFC a little to account for the need to introspect channels at runtime. |
|
Since it was pointed as something relevant to this, I would like to explain my use case. I have a custom board with two DS2482-800, an 8 channels IC used to handle the communication with multiple DS18B20 and other one-wire devices. On my project I will have up to 32 sensors per channel on a long cable (one sensor each 1.5m), to the max of 16*32=512 sensors. Another board will periodically send a packet through Modbus or LoRaWAN asking for a temperature reading on all the active channels, one by one. This is how I have things set at this moment: Shell devices list ouput w1 seach on a short cable with 7 devices w1 search on a much longer cable with 32 devices Since I don't know how many channels will be in use or how many sensors will be connected to these channels (and their ROM ID) at the building time, I will need to save somewhere the ROM of each sensor alongside the channel and position on cable. It could be done inside the sensor own two register (1 byte each) or to an external eeprom. My problem relies on how to set this to make use of sensors API. 512 node entries for each sensor would be impractical for obvious reasons. I would like to hear your opinions and would be nice to know if my case could be take in account or if it has relevance to this discussion. |
|
We have channel specifiers in now with #71093 now with the new read+decode API and I'm considering this complete. |
|
A small addendum here, I think its still great if we could have a channel descriptor of some kind available, but doing it in DT turns out to be somewhat painful, either we have an array with modulo dependent data (e.g. position 0 is type, position 1 is index, etc) or better would be to have a sub node representing what really is a subset of sensor channels typically, e.g. accel x,y,z and gyro x,y,z with their own independent properties and configuration (this is often possible). So that's where #71235 comes in and hopefully resolves the issue of describing sensors in a way that could be converted to C code useful to zephyr. |
|
I still have a problem understanding how would a fetch an entire bus of sensors wired to the channels of a DS2482-800. I have many sensors per channel and need to send a skip rom command to convert all and after the conversion ends I would need to read one by one by rom ID. |
|
assuming you describe the device layout property with dt as you noted above you could setup a single reader that asks for all the temperature sensors hanging off the all of the i2c to 1 wire converters. It would require a sensor device driver representing the i2c to 1wire converter that understands your setup though (the w1, w2, etc in this case) E.g. something like... SENSOR_DT_READ_IODEV(w1, DT_NODELABEL(w1),
{ SENSOR_CHAN_TEMP, 0 },
{ SENSOR_CHAN_TEMP, 1 },
...
{ SENSOR_CHAN_TEMP, 8 },
);
SENSOR_DT_READ_IODEV(w2, DT_NODELABEL(w2),
{ SENSOR_CHAN_TEMP, 0 },
{ SENSOR_CHAN_TEMP, 1 },
...
{ SENSOR_CHAN_TEMP, 8 },
);
...
/* put all the iodev pointers in an array for ease of use */
struct rtio_iodev *temp_devices[TEMP_DEVS] = LISTIFY(...);
/* put a matching decoder for each sensor into an array for ease of use */
struct sensor_decoder *decoders[TEMP_DEVS] = LISTIFY(...);
/* create an async executor context with a mempool and an exactly matching number of inflight requests/completions as the number of sensors */
RTIO_MEMPOOL_DEFINE(r, TEMP_DEVS, TEMP_DEVS, 64, 64);
int main() {
struct rtio_sqe *sqe;
struct rtio_cqe *cqe;
uint8_t *buf;
size_t buf_len;
while(true) {
/* setup read requests */
for (int i = 0; i < TEMP_DEVS; i++) {
sqe = rtio_sqe_acquire(r);
rtio_prep_read_with_pool(sqe, temp_devs[i], temp_devs[i]);
}
/* wait for all temp sensors to read, may happen concurrently if multiple i2c buses are used */
rtio_submit(r, TEMP_DEVS);
for (int i = 0; i < TEMP_DEVS; i++) {
/* get the completion queue entry for each read request to the sensors */
cqe = rtio_cqe_consume(r);
/* contains the result of the read request, 0 success, -errno for error as usual */
rc = cqe->result;
/* is a pointer to one of the temp devs, letting you determine *which* sensor the completion came from */
userdata = cqe->userdata;
/* since we used the builtin memory pool to stash the readings, we need to get the pointer to the associated buffer */
rtio_cqe_get_mempool_buffer(ctx, cqe, &buf, &buf_len);
/* Release the CQE */
rtio_cqe_release(ctx, cqe);
/* Decode the buffer with decoder->decode() into a vector of fixed point values, optional! */
/* decoders[i]->decode(...);
/* Release the memory back to the mempool */
rtio_release_buffer(ctx, buf, buf_len);
}
/* Poll once a second */
k_msleep(1000);
}
} |
As someone who is beginning on my journey through learning Zephyr and RTOS I must say this is really nice. I didn't knew about RTIO API and it seemed to be exactly what I need. Thank you for providing this example. I have a lot more to learn now. May I ask some questions? What you mean by "a device driver that understands my setup"? I guess ds2482/w1 doesn't support RTIO API by default, is this the case? Would you please elaborate a little more the concept behind it, considering I'm still getting the general idea behind this? |
Absolutely, the RTIO API from a sensor driver needs to implement the submit and decoder APIs (decoder is specific to sensors, for interpreting the readings). The idea here is that the driver gets submit called whenever requests (read requests in this case) come in, the read request comes with a buffer you can directly read into. For sensors the idea would be to directly read the register readings into the buffer without manipulation, allowing for the raw data to be directly used by the application if desired. A small prefix or suffix can be used to stash some data for the decoder (the decoder is stateless!). so for example.. something like the following void ds2482_consume(const struct device *dev, bool completion);
void ds2482_read_complete(....)
{
/* get completions from our driver's internal async context (data->r); */
/* note that our current request is *done* /
rtio_iodev_sqe_ok(data->r, data->sqe);
/* continue working through the queue */
ds2482_consume(dev, true);
}
void ds2482_start(const struct device *dev)
{
struct ds2482_data *data = dev->data;
/* start async i2c request with rtio */
struct rtio_mpsc_node *node = rtio_mpsc_pop(&data->io_q);
struct rtio_iodev_sqe *sqe = CONTAINER_OF(struct rtio_iodev_sqe, q, node);
__ASSERT(sqe->op == RTIO_SQE_OP_RX, "ds2482 only accepts read requests");
/* the iodev associated with the sqe can be looked at to determine the channels to read here, not shown though */
uint8_t *buf;
size_t buf_len;
/* get a reference to the read buffer (or allocate if using pools) from the incoming request
* this can validate that the buffer is large enough for our needs
*/
int rc = rtio_sqe_rx_buf(sqe, min_buf_len, max_buf_len, &buf, &buf_len);
__ASSERT(rc == 0, "expected a valid read buffer");
/* we have our io task descriptor (sqe), our array of channels (from the iodev) request the appropriate reads from i2c peripheral asynchronously */
struct rtio_sqe *i2c_sqe;
i2c_sqe = rtio_sqe_acquire(data->r);
/* write the register value to read... */
rtio_sqe_prep_write(i2c_sqe, i2c_iodev, RTIO_PRIO_NORM, (uint8_t*)®_addr, sizeof(reg_addr));
/* mark the request with a flag signaling the next one is part of this transaction */
i2c_sqe->flags |= RTIO_SQE_TRANSACTION;
i2c_sqe = rtio_sqe_acquire(data->r);
/* note we read *directly* into the buffer provided by the application here */
rtio_sqe_prep_read(i2c_sqe, i2c_iodev, buf, buf_len);
/* we want the next sqe after this one to be chained to it (requires this one completes first). */
i2c_sqe->flags |= RTIO_SQE_CHAIN;
i2c_sqe = rtio_sqe_acquire(data->r);
/* finally we want a callback when the transaction completes */
rtio_sqe_prep-callback(i2c_sqe, ds2482_read_complete);
/* don't wait here, submit and return */
rtio_submit(data->r, 0);
}
void ds2482_consume(const struct device *dev, bool completion)
{
/* attempt to kick off async interrupt driven tasks if not already doing things
* since the tasks are asynchronous, this really amounts to a few register writes
* typically and should be just fine in a spin lock
*/
k_spinlock_key_t key = k_spin_lock(&data->lock);
if ((data->sqe == NULL && !completion) || (completion)) {
data->sqe = rtio_mpsc_pop(data->io_q);
}
if (data->sqe) {
ds2482_start(dev);
}
k_spin_unlock(&data->lock, key);
}
void ds2482_submit(const struct device *dev, struct rtio_iodev_sqe *sqe)
{
struct ds2482_data *data = dev->data;
/* put sqe into atomic queue, no lock needed here, the "multiple producer" side */
rtio_mpsc_push(&data->io_q, &sqe->q);
ds2482_consume(dev, false);
} |
|
Thank you very much for your time. This is a very nice explanation! Now I have plenty of things to work with. |
|
The comments in this thread sound like they could be fleshed out a bit more and become a good reference example for RTIO usage. It seems like there are important exemplary details here that aren't in the existing |
Fair! I have on my TODO list updating the docs... which I was working on and then got side tracked trying to make building docs not take so long 💻 🦥 |
Introduction
Sensor channels today are solely identified by the type of information they contain using a single enum. Many sensors provide multiple channels of the same measurement type with varying need for a sub type or index.
Example devices of varying kinds
Problem description
Selecting channels simply by their type of measurement isn't flexible enough for such array like sensor devices that exist and are used widely in the real world. Subtypes and indexes are encoded in enum suffixes.
Some examples...
In multi-channel array like devices custom channel defines are needed
FDC2X1X
Proposed change
Define sensor channels as a triple (type, optional subtype, optional idx)
Proposed change (Detailed)
Sensor channels become defined as a uint64_t unioned with a struct to define a structured channel identifier with a copy friendly size.
A pair of conversion functions are available, perhaps infallible and which assert on unknown legacy values or channel specs that cannot be converted back to a legacy enum.
Some samples of how channel specs might be defined...
All newer APIs, e.g. sensor_read and the sensor_stream API would be updated to only support the newer struct sensor_chan_spec type as arrays. It is up to the backwards compatibility layer of this new API then to translate the sensor_chan_spec to a legacy enum sensor_channel when used with existing drivers.
A new pair of attribute set/get functions to act on a chan spec rather than a channel type
would be available for drivers to implement. Some mappings from old to new or vice versa could be provided at the sensor API layer.
Introspection of available channels could optionally be provided with a Kconfig and relevant DT entries perhaps as a future improvement.
Related PRs/Issues/Drivers
The text was updated successfully, but these errors were encountered: