Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zio: initial channel definitions #14245

Closed

Conversation

microbuilder
Copy link
Member

This PR is a WIP and an attempt to define an initial list of valid sensor 'channels' (or types), along with the SI units and scale associated with them.

Further discussion is needed to refine, expand or restrict the current list.

@microbuilder microbuilder added In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on area: Sensors Sensors RFC Request For Comments: want input from the community area: API Changes to public APIs labels Mar 11, 2019
@microbuilder
Copy link
Member Author

microbuilder commented Mar 11, 2019

There is (clearly) room for discussion here, but an attempt has been made at providing a more scientifically precise representation of physical concepts like light, and to allow some level of precision with concepts like distance.

You can use a generic 'straight line' type distance measurement via:

	/**
	 * Distance in a straight line (regardless of obstacles) between the
	 * source of the measurement and a specific reference point.
	 *
	 * Expressed in meters (m).
	 */
	DEV_CHANNEL_POS_DIST,

Or opt for a more nuanced, but potentially clearer distance along the surface of an object (think Haversine, etc.), which is likely more accurate in certain real-world uses cases:

	/**
	 * Distance in a straight line along the surface of an object (such as
	 * a sphere) between the source of the measurement and a specific
	 * reference point.
	 *
	 * Expressed in meters (m).
	 */
	DEV_CHANNEL_POS_DIST_SURF,

Light Measurements

The list of light measurement channels are more complex on the surface (compared to the previous red, green, blue type values), but I believe they are scientifically more accurate and ultimately more flexible. Generic information like relative RGB weights can easily be inferred from multiple instances of DEV_CHAN_LIGHT_IRRAD_COMP, without being locked into an overly narrow RGB tuple:

	/**
	 * Irradiance limited to a specific spectral component. The exact
	 * wavelength or wavelength range of the component should be
	 * indicated with an appropriate channel attribute.
	 *
	 * Expressed as watts per square meter (W/m^2).
	 */
	DEV_CHAN_LIGHT_IRRAD_COMP,

A clear distinction has also been made between radiometric and photometric units.

Lux has it's own dedicated channel:

	/**
	 * An approximation of human vision, which is the result of a
	 * conversion from irradiance (a radiometric unit expressed in
	 * W/m^2) to illuminance (a photometric unit expressed in lumen/m^2)
	 * by means of a version of the CIE luminous efficiency function.
	 *
	 * ?PTE: More technically, this channel should probably be named
	 *       `ILLUMINANCE`, but `LUX` is used as present since it is
	 *       a significantly more familiar concept to most people.
	 *
	 * Expressed as 'lux', or more precisely lumens per square meter.
	 */
	DEV_CHAN_LIGHT_LUX,

And there are a number of radiometric units for specific wavelength ranges, for example:

	/**
	 * Irradiance in the ultaviolet range (nominally <380 nm).
	 *
	 * Expressed as watts per square meter (W/m^2).
	 */
	DEV_CHAN_LIGHT_IRRAD_UV,

Position Data

An attempt has been made at providing a way to encapsulate basic positional data, though some discussion is likely required here. I believe most key concepts around geo-localisation can be encapsulated in the following channel types, though, with both generic and specialized versions defined where appropriate:

	/**
	 * Latitude in a geo-localisation system.
	 *
	 * Expressed as a degree/minute/second notation string.
	 */
	DEV_CHAN_POS_LATITUDE,

	/**
	 * Longitude in a geo-localisation system.
	 *
	 * Expressed as a degree/minute/second notation string.
	 */
	DEV_CHAN_POS_LONGITUDE,

	/**
	 * The altitude or 'height' of an object or point in relation to an
	 * unspecified reference level (ground, sea-level, another object,
	 * etc.)
	 *
	 * Expressed in meters.
	 */
	DEV_CHAN_POS_ALT,

	/**
	 * The altitude or 'height' of an object or point in reference to sea
	 * level.
	 *
	 * Expressed in meters.
	 */
	DEV_CHAN_POS_ALT_SEALEVEL,

	/**
	 * The altitude or 'height' of an object or point in reference to the
	 * ground immediately below the point of measurement.
	 *
	 * Expressed in meters.
	 */
	DEV_CHAN_POS_ALT_GND,

	/**
	 * Distance in a straight line (regardless of obstacles) between the
	 * source of the measurement and a specific reference point.
	 *
	 * Expressed in meters (m).
	 */
	DEV_CHANNEL_POS_DIST,

	/**
	 * Distance in a straight line along the surface of an object (such as
	 * a sphere) between the source of the measurement and a specific
	 * reference point.
	 *
	 * Expressed in meters (m).
	 */
	DEV_CHANNEL_POS_DIST_SURF,

	/**
	 * Proximity to an external plane or object.
	 *
	 * Expressed in millimeters (mm).
	 */
	DEV_CHANNEL_POS_PROX,

	/**
	 * Encapsulates a raw NMEA sentence from a GPS-type device
	 *
	 * Expressed as a null-terminated string, with the exact string
	 * contents determined by NMEA standard. NMEA sentences are generally
	 * limited to 79 characters plus the null-termination character.
	 */
	DEV_CHAN_POS_NMEA,

I'm not sure if DEV_CHAN_POS_NMEA is appropriate here, but I've added it for the sake of discussion.

Any comments, criticism or suggestions are of course welcome!

@microbuilder
Copy link
Member Author

I'm 100% certain I'm missing several classes of sensors here, as well, and this should only be considered a starting point to a larger discussion. I'll dig through the box of sensors I have here to see what device classes I've missed that can't obviously be covered by the above enumeration.

@teburd
Copy link
Collaborator

teburd commented Mar 12, 2019

I think overall this is a great start, it makes me wonder if we should split units vs what it is.
I can easily place a voltage scaler in front of an A/D and change the scale to the point where my scale should be in mV or kV rather than V.

As an example of where this might be especially useful is in the case of several more recent TI digitzer chips which measure down to the femtofarads: http://www.ti.com/lit/ds/symlink/fdc1004.pdf

In the case of the raw channel data, in its 16bit form, describing the units as femtofarads seperated from capacitance I think makes a great deal of sense in this case but also many others. Let me know what you think.

include/zio/zio_defs.h Outdated Show resolved Hide resolved
@microbuilder
Copy link
Member Author

I think overall this is a great start, it makes me wonder if we should split units vs what it is.
I can easily place a voltage scaler in front of an A/D and change the scale to the point where my scale should be in mV or kV rather than V.

As an example of where this might be especially useful is in the case of several more recent TI digitzer chips which measure down to the femtofarads: http://www.ti.com/lit/ds/symlink/fdc1004.pdf

In the case of the raw channel data, in its 16bit form, describing the units as femtofarads seperated from capacitance I think makes a great deal of sense in this case but also many others. Let me know what you think.

I think this makes sense, but what about keeping the current default scales, and having an optional attribute to override the defaults with some other scalar multiplier when you want a larger or smaller range (0.001x, 1000x, etc.)? This avoids the need for an extra attribute in 90% of the cases, but we still have the necessary flexibility to move things up or down several orders of magnitude if useful?

@MaureenHelm
Copy link
Member

I think overall this is a great start, it makes me wonder if we should split units vs what it is.
I can easily place a voltage scaler in front of an A/D and change the scale to the point where my scale should be in mV or kV rather than V.

Along those lines, digital accelerometers come in high-g and low-g varieties, and many low-g versions have voltage scaling options (2g, 4g, 8g) before the internal A/D. Instead of drivers scaling sampled data to a common format and potentially losing precision or range, I think each driver should have an attribute that declares its fixed point representation (e.g., a Q format attribute).

include/zio/zio_defs.h Outdated Show resolved Hide resolved
include/zio/zio_defs.h Outdated Show resolved Hide resolved
*
* Expressed as degrees celsius (C).
*/
DEV_CHAN_TEMP,
Copy link
Collaborator

@avisconti avisconti Mar 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to have 3 types of temperature?
Aren't DIE and AMBIENT enough?
I'm not against it. Just want to understand the reason.
Maybe we could have only two types: AMBIENT and GENERAL. The DIE case would fall into the GENERAL case. Or maybe we can keep it this way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many sensors out there include temperature measurement, but it's often only of the die so of less use to the outside world, and I guess I just wanted to clearly distinguish die temp (which could me many degrees different than ambient temp or the same measurement on another IC), from ambient which is device nuetral and closer to what people want, without ignore the need to sometimes track die temp for either generating appropriate offets for the sensor data, or for safety reasons.

The third (first?) generic temp entry -- DEV_CHAN_TEMP -- is basically for anything that isn't the two most common cases of DIE or AMBIENT, and you could then define exactly what that is via attributes (water temperature, heating-element temperature, whatever).

I'm open to change this, just trying to explain the reasoning in my own head!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many sensors out there include temperature measurement, but it's often only of the die so of less use to the outside world, and I guess I just wanted to clearly distinguish die temp (which could me many degrees different than ambient temp or the same measurement on another IC), from ambient which is device nuetral and closer to what people want, without ignore the need to sometimes track die temp for either generating appropriate offets for the sensor data, or for safety reasons.

Yes, I agree.
Actually if you see the history of commits of the current sensor.h you'll see that I'm the one that separated the two temperature types. So, yes, I agree with this part.

The third (first?) generic temp entry -- DEV_CHAN_TEMP -- is basically for anything that isn't the two most common cases of DIE or AMBIENT, and you could then define exactly what that is via attributes (water temperature, heating-element temperature, whatever).

I'm open to change this, just trying to explain the reasoning in my own head!

I was in fact more questioning about this third type.
We can keep it, I'm not against it. Maybe it is more clear like that, but we cannot have as many
temperatures definition as are out there. So, I was thinking to have only two types: AMBIENT and TEMP which means other non ambient. But, again, maybe this way it is clearer.

* Expressed as meters per second squared (m/s^2).
*/
DEV_CHAN_ACCEL_VECTOR,

Copy link
Collaborator

@avisconti avisconti Mar 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the number of axis should be specified in a different parameter.
The channel type should be always ACCEL (or GYRO or MAGN).
So, no need for a different type. Does it make sense to you?

Copy link
Member Author

@microbuilder microbuilder Mar 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does and I'm not in love with the concept of vector, my concern is that with sensor fusion you often want to be sure that the three values come from the same sample or conversion process, and if you have three different channels for X+Y+Z, you need to add a means to associate those values with each other, which becomes more code to validate.

The need to group multiple channels will come up elsewhere, of course, such as in light sensors with multiple photo diodes, each with their own channel, though that takes the approach of having a single channel per photodiode and the attributes fill the details in.

How would you see the problem of needing to keep X/Y/Z related to each other so that you don't get X+Y from one conversion, but Z from the next?

Copy link
Collaborator

@avisconti avisconti Mar 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you see the problem of needing to keep X/Y/Z related to each other so that you don't get X+Y from one conversion, but Z from the next?

I need to see more carefully the example of Google CHRE, where they define the sensor type as ACCEL or GYRO or MAGN only, but also define in the sensor property the number of axis supported by the sensor (One of SINGLE, TRIPLE, EMBEDDED, with latter one used for temp/baro/humidity/stepc and so on).

Then a sensor when generates samples enqueue above them as events, and a general task would consume them. This is different from how zephyr works now where samples are pulled from above and not pushed from below as events.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have some suggestions here, it will never be easier than now to make the right decisions and tradeoffs!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sure.
I'll try to get back to this asap.

* Expressed in hectopascal (hPa)
*/
DEV_CHANNEL_PRESS_BAROM,

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I would keep hPa as unit (which equals mbar btw), just to be consistent with the usual unit.
Would DEV_CHANNEL_PRESS_AMBIENT be a better name?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ambient might actually be a clearer name, yes.

Copy link
Collaborator

@pabigot pabigot Jul 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer DEV_CHANNEL_PRESS_ABSOLUTE and DEV_CHANNEL_PRESS_RELATIVE, and perhaps DEV_CHANNEL_PRESS_GAUGE.

Most pressure sensors, e.g. BME280 or MS5637, produce absolute atmospheric pressure, which would range from about 1068 hPa down to 314 hPa. Others like SDP810 produce relative pressure in a differential application, such as output and return in an HVAC system, which is often on the order of 10-25 Pa (with precision down to cPa) and can be negative.

Gauge pressure would be like a tire pressure sensor: relative to absolute atmospheric pressure.

Terms like "barometric" or "atmospheric" may be interpreted as referring to absolute pressure adjusted for altitude, like you get in weather reports. This can be very different from absolute pressure. Barometric pressure could be a derived measurement, but having a sensor that measures it directly seems pretty unlikely (maybe one built into a GPS). Avoiding those terms might reduce confusion.

As for units: I'd really prefer SI throughout, so Pa. Let the consumer deal with scale conversions.

* channel type.
*/
DEV_CHAN_USER_DEF = ZIO_CHAN_LIMIT,
} zio_dev_chan_type_t;
Copy link
Collaborator

@avisconti avisconti Mar 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

STEP_COUNT, STEP_DETECT, TAP, DOUBLE_TAP, WAKEUP, GESTURE?
HALL?

Maybe we should handle also this types.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback ... slow to reply because I've been thinking about this, sorry. There's a point where it becomes blurry what is a *'channel' or an 'attribute', and what is better served by something like an 'event', which is seems like STEP_COUNT, STEP_DETECT, TAP, etc., would be.

That also gets me thinking about the concept of attributes with state, though, which are probably better described as properties at the sensor API level, like STEPS_DETECTED which would be tracked over time with an option to reset.

I don't want to over complicate the sensor API with extra options just to satisfy my need to have a box for everything, but at what point does something like 'GESTUR' become specific to a single driver's API or user code, and when should we have generic solution at the sensor API level (gesture is hard for that if you want to define gesture types)?

The idea of state management is a good question to consider, though, and how to handle that with the current channel + attribute approach, and we may need or benefit from a concept like property as well?

Runtime contributed a stats module that I've used numerous times to track incrementing variables like this (it works with mcumgr from the command-line which is nice!), and that might be a viable solution using an existing API ... but since that subsystem is more intended for device diagnostics and debugging in the field, it's may be forcing it to do something it isn't designed for and we should have this directly in the sensor API.

Thanks for getting the wheels turning on the events and properties issues, though, and any thoughts are highly welcome?

@nashif nashif closed this Apr 4, 2019
@nashif nashif reopened this Apr 4, 2019
@ghost ghost removed the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Apr 4, 2019
@zephyrbot
Copy link
Collaborator

zephyrbot commented Apr 4, 2019

Found the following issues, please fix and resubmit:

Identity/Emails issues

19637d397d84b2ebf6cba6cfd51f788cb4693fb8: author email (Kevin Townsend contact@microbuilder.eu) needs to match one of the signed-off-by entries.

fd7ad5753b80a56b5c5a8e788306637fd53a88ae: author email (Kevin Townsend contact@microbuilder.eu) needs to match one of the signed-off-by entries.

a4f89dc2c7a0e4aeabcd2124880f02a1ad9ce2a9: author email (Kevin Townsend contact@microbuilder.eu) needs to match one of the signed-off-by entries.

714824226485fa31a9ca221011afcb196462f74e: author email (Kevin Townsend contact@microbuilder.eu) needs to match one of the signed-off-by entries.

7fe43a6b0df02b4afeb1cc67f7c6f3cf3fb0c104: author email (Kevin Townsend contact@microbuilder.eu) needs to match one of the signed-off-by entries.

a0c742d36082c38e33076a6635de864e33ec543b: author email (Kevin Townsend contact@microbuilder.eu) needs to match one of the signed-off-by entries.

b7585c5b866b8633a826986ebe462bd49f1d399f: author email (Kevin Townsend contact@microbuilder.eu) needs to match one of the signed-off-by entries.

f6d85e1ff6843b49e9881d40286c8d903dbbe43d: author email (Kevin Townsend contact@microbuilder.eu) needs to match one of the signed-off-by entries.

Gitlint issues

Commit fd7ad5753b:
1: UC2 Body does not contain a 'Signed-off-by:' line
1: UC3 Title does not follow [subsystem]: [subject] (and should not start with literal subsys:): "Added channel and device attributes"
1: UC6 Body has no content, should at least have 1 line.
3: B6 Body message is missing

Commit a4f89dc2c7:
1: UC2 Body does not contain a 'Signed-off-by:' line
1: UC3 Title does not follow [subsystem]: [subject] (and should not start with literal subsys:): "Fixed naming errors"
1: UC6 Body has no content, should at least have 1 line.
3: B6 Body message is missing

Commit 7148242264:
1: UC2 Body does not contain a 'Signed-off-by:' line
1: UC3 Title does not follow [subsystem]: [subject] (and should not start with literal subsys:): "Set a limit on channel IDs"
1: UC6 Body has no content, should at least have 1 line.
3: B6 Body message is missing

Commit 7fe43a6b0d:
1: UC2 Body does not contain a 'Signed-off-by:' line
1: UC3 Title does not follow [subsystem]: [subject] (and should not start with literal subsys:): "Renamed channel 'kind' to 'type'"
1: UC6 Body has no content, should at least have 1 line.
3: B6 Body message is missing

Commit a0c742d360:
1: UC2 Body does not contain a 'Signed-off-by:' line
1: UC3 Title does not follow [subsystem]: [subject] (and should not start with literal subsys:): "Added amplitude and user defined"
1: UC6 Body has no content, should at least have 1 line.
3: B6 Body message is missing

Commit b7585c5b86:
1: UC2 Body does not contain a 'Signed-off-by:' line
1: UC3 Title does not follow [subsystem]: [subject] (and should not start with literal subsys:): "Fixed several typographical errors."
1: UC6 Body has no content, should at least have 1 line.
3: B6 Body message is missing

Commit f6d85e1ff6:
1: UC2 Body does not contain a 'Signed-off-by:' line
1: UC3 Title does not follow [subsystem]: [subject] (and should not start with literal subsys:): "Initial sensor channel list"
1: UC6 Body has no content, should at least have 1 line.
3: B6 Body message is missing

Documentation issues

/var/lib/shippable/build/IN/main_repo/zephyr/doc/_build/rst/doc/samples/zio/synth/README.rst:4: WARNING: duplicate label lsm6dsl, other instance in /var/lib/shippable/build/IN/main_repo/zephyr/doc/_build/rst/doc/samples/sensor/lsm6dsl/README.rst
/var/lib/shippable/build/IN/main_repo/zephyr/doc/_build/rst/doc/reference/zio/index.rst: WARNING: document isn't included in any toctree


@avisconti
Copy link
Collaborator

@microbuilder
I'm assuming that this work will be sooner or later merged into #14008, correct?

@microbuilder
Copy link
Member Author

@microbuilder
I'm assuming that this work will be sooner or later merged into #14008, correct?

Yes

teburd and others added 4 commits May 23, 2019 07:59
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>
This commit adds a .rst file referencing the doxygen
groups in zio_dev.h and zio_attr.h so that they can be
pulled in during documentation builds.

There are also minor formatting issues address with
the table in zio_dev.h.

Signed-off-by: Kevin Townsend <kevin@ktownsend.com>
@microbuilder microbuilder changed the title WIP: Sensor API channel definitions zio: initial channel definitions Jun 18, 2019
Kevin Townsend and others added 2 commits June 18, 2019 17:39
This commit adds an initial attempt at defining generic
channels and attributes for the zio sensor API.

Signed-off-by: Kevin Townsend <kevin@ktownsend.com>
This commit adds a list of initial channel and attributes
defitions for the zio sensor API.

Signed-off-by: Kevin Townsend <kevin@ktownsend.com>
@microbuilder
Copy link
Member Author

Closing this pending a total rethink of the general purpose sensor API, and this PR and comments herein can be consulted at that point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Sensors Sensors RFC Request For Comments: want input from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants