Add offset to iio_data_format, rework scale #1084
Conversation
|
|
||
| /** @brief Contains a value to be added to the raw sample before | ||
| * applying the scale. */ | ||
| double offset; |
There was a problem hiding this comment.
Just a side note... structs in header files are always nice to get some ABI breakage down the road. I wonder if we shouldn't have some reserved bits in here? At least for when v1.0 is out...
There was a problem hiding this comment.
Yeah I was thinking that this is not a problem because old applications won't try to access the "offset" field. Apps built with Libiio > 1.0 may try to access new fields that are yet to be added, but we never really cared about forward compatibility.
With that said - I can add some padding, it's not that much more work.
There was a problem hiding this comment.
Yeah, if the new fields are always added in the end of the struct but we will always leave the door open for subtle things and like you said... it's not much work at all
There was a problem hiding this comment.
Done, as a separate commit. Thanks for the review btw.
context.c
Outdated
| int err; | ||
|
|
||
| if (!ctx->ops->read_attr) { | ||
| /* read_attr callback not implemented - nothing to do. */ |
There was a problem hiding this comment.
nit: don't think the comment adds much value. The code is very self explanatory...
| &chn->format.offset); | ||
| if (err) { | ||
| chn_perror(chn, err, "Unable to read offset"); | ||
| return err; |
There was a problem hiding this comment.
didn't checked the code so just sanity checking... There's really no with_offset like in the scale case?
There was a problem hiding this comment.
Well, there's no need, having no offset is the same as having offset == 0.0 (just like having no scale is the same as having scale == 1.0, but it's too late for that).
pcercuei
force-pushed
the
pcercuei/scale-offset
branch
from
7554f94 to
17fd469
Compare
Add an 'offset' field, which contains the offset that should be added to the raw sample values. Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Add padding after the iio_data_format, which will be cleared to zero during initialization. The total of (iio_data_format + padding) will be 128 bytes which should be plenty. The point of this, is to make sure that we can have forward compatibility with any apps built with a newer version of Libiio that may have more fields added to the iio_data_format structure. Signed-off-by: Paul Cercueil <paul@crapouillou.net>
The iio_data_format.scale field was previously only set for scan-element channels. This was wrong, as regular channels can have a scale as well. Both can also have an offset, which is added to the raw value before applying the scale; this offset was not present in the iio_data_format structure. Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Remove the code reading the scale into the format structure. This is now done better by iio_context_update_scale_offset(), which is called at the end of iio_create_context(), and has the added advantage of reading the scale for all channels (not just the scan-elements) and also read the offsets. Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Add an 'offset' field, which contains the offset that should be added to the raw sample values. Signed-off-by: Paul Cercueil <paul@crapouillou.net>
pcercuei
force-pushed
the
pcercuei/scale-offset
branch
from
17fd469 to
78bf1e3
Compare
Add an
offsetfield in theiio_data_formatstructure alongside the "scale". Make sure those two are initialized properly for all channels, not just the scan-element ones.