ZIO channel and attribute rework WIP #16456
Conversation
Add a new sensor device API called zio_dev along with supporting interface and implmentations needed to better encapsulate a large range of functionality typical sampled input/output (sensor and signal generator) type devices commonly provide. Every device has a set of channels and attributes associated with it. Channels may be attached to a fifo like interface (zio_buf) which provides a pollable mechanism for obtaining the raw channel data at a designated watermark level. Signed-off-by: Tom Burdick <thomas.burdick@gmail.com>
Provide a software only example driver and sample application which generates a left and right sine wave at CD quality bit width and rate. Signed-off-by: Tom Burdick <thomas.burdick@gmail.com>
Rather than providing the attribute values directly in get_attrs for zio devices and channels, provide descriptions of each attribute. This should allow a driver to map many attributes directly to registers or internalized struct members and avoid storing these values in a zio_variant which is confusing. Signed-off-by: Tom Burdick <thomas.burdick@gmail.com>
|
Found the following issues, please fix and resubmit: Identity/Emails issues5c34ec6785b8512c2148a81a204be7b63b43ce52: author email (Michael polli104@mail.chapman.edu) needs to match one of the signed-off-by entries. 4178b64146430615e5bf854566429bc974bb81f9: author email (Michael polli104@mail.chapman.edu) needs to match one of the signed-off-by entries. 375621015c1313bf8ad03942ab9091d718498b92: author email (Michael polli104@mail.chapman.edu) needs to match one of the signed-off-by entries. checkpatch issuesGitlint issuesCommit 5c34ec6785: Commit 4178b64146: Commit 375621015c: Documentation issues |
|
This is just an early wip. Its a significant change from where zio was going at the moment. I kind want to see what people think about this change first before putting in significant effort to develop something more complete. |
|
OK. I also see a PR for Documentation for zio #16384 |
|
@dbkinder Yeah, I made a boilerplate PR just to get started, but haven't invested time into documenting the details until the dust settles on this a bit. |
| struct zio_attr_desc { | ||
| /* Attribute data type describing the expected data type. */ | ||
| enum zio_variant_type data_type; | ||
| zio_set_attr_t set_attr; |
There was a problem hiding this comment.
These two function pointers are I think the most significant part of this change set and I definitely agree that they make the device driver API clearer, avoids unnecessary switch/case function wrappers, and generally makes for a nicer API
| * | ||
| * @return -EINVAL if an invalid attribute id was given, 0 otherwise | ||
| */ | ||
| typedef int (*zio_dev_set_attr_t)(struct device *dev, const u16_t attr_idx, const struct zio_variant var); |
There was a problem hiding this comment.
This seems like it'd be unnecessary given each attribute description has a get/set function pointer
| */ | ||
| typedef int (*zio_chan_set_attr_t)(struct device *dev, const u32_t chan_idx, | ||
| const u32_t attr_idx, | ||
| const struct zio_variant var); | ||
| /** | ||
| * @brief Function to get a channel attribute for a device | ||
| * | ||
| * @return -EINVAL if an invalid attribute id was given, 0 otherwise | ||
| */ | ||
| typedef int (*zio_chan_get_attr_t)(struct device *dev, const u32_t chan_idx, |
There was a problem hiding this comment.
I don't think this, or any of the other get/set attr function pointer defs are needed anymore with your change, where each attribute has its own getter/setter
There was a problem hiding this comment.
I think the general idea of putting the setter/getter function pointers into the attribute description makes a lot of sense. What to pass into those function pointers is another issue, the callback might just need a parameter to give it the context it needs. I don't think CONTAINER_OF would solve the problem, you would need something like it that could understand pointer math for array members where the ptr is pointing to one thing in that array. Maybe that already exists somewhere.
Another thing I would keep in mind is that the current API has some faults I think, we don't support the idea of related sets of channels where for example the x,y,z accelerometer channels of a 9 axis sensor like the one I believe your writing a driver for share a lot of attributes such as value ranges (+/- 2g, 8g, 16g, etc).
This changeset seems like its on the right track to creating the building block to help solve that problem as well.
|
@BFrog: Some feedback re: 4fe8819 :
I find "zio" naming to be confusing. Which letter exactly in "zio" stands for "sensor"? And why someone would need to decipher letters, why not call it with "sensor" in name for clarity? We already have one cryptic, confusing, conflicting name like that: #16114 . |
I don't disagree, I needed a name, and I wanted something short, zephyr io abbreviated to zio seemed easy enough. The general problem with naming it sensors is that this API is meant to encompass more of what IIO does in Linux, provide an input/output stream with attributes and triggers. I'm open to suggestions. |
I'm still +1 on rtio for real time input/output. I believe that this I/O framework is meant to cover actuators as well, so I disagree that "sensor" should be in the name, as that would be misleading. If I'm wrong about that, then never mind. Edit: or maybe "zesa" (zephyr sensors/actuators)? |
MaureenHelm
force-pushed
the
topic-sensors
branch
from
e4bdd9c
to
14b8922
Compare
|
Hi All, I'm not sure if anyone has reviewed the BT documentation to consider potential relevance? I think it would valuable to have a look. Billy.. |
|
The initial ZIO work has since been replaced by RTIO; see the branch by the same name on @BFrog's zephyr fork. |
Thanks, I'll have a look. |
|
I think since zio has become a dead end, it might be worth considering closing this. |
I've been looking at the zio api and I have a rework of the api that defines all the attributes and channels through anonymous nested structs. you use ZIO_ATTR(id) and ZIO_CHANNEL(id) to retrieve the accessor type. attributes can either be nested within the definition of a channel or at the root of the api struct for device attribute configurations. At the moment this does not quite work and I was looking for some suggestions. I realize this is a pretty large departure from the original api.
I think this could address some of my problems with the current zio sensor api. The definitions and for attributes and channels are more defined. this removes the need for the setter functions in the base api. I was also looking into storing the attributes and channels into nested arrays but I think this is more defined. I'm not sure about using the preprocessor in this way, but I think these could be tweaked and reworked.
The majority of the interesting changes are in synth.c and synth.h file.