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.

Sign up for GitHub

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

Add offset to iio_data_format, rework scale #1084

Merged
merged 5 commits into from
Nov 21, 2023
Merged

Add offset to iio_data_format, rework scale #1084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
82ab8b5
iio.h: Add offset to iio_data_format
pcercuei Nov 15, 2023
61ee13b
iio-private: Add padding after iio_data_format in iio_channel struct
pcercuei Nov 20, 2023
ea3b914
context: Update channels' scale/offset after creating context
pcercuei Nov 15, 2023
8108e0c
local: Remove custom code to set scale in the data format structure
pcercuei Nov 15, 2023
78bf1e3
python: Add offset to data format structure
pcercuei Nov 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bindings/python/iio.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ class DataFormat(Structure):
("with_scale", c_bool),
("scale", c_double),
("repeat", c_uint),
("offset", c_double),
]


Expand Down
53 changes: 53 additions & 0 deletions context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,50 @@ const struct iio_backend * const iio_backends[] = {
};
const unsigned int iio_backends_size = ARRAY_SIZE(iio_backends);

static int iio_context_update_scale_offset(struct iio_context *ctx)
{
const struct iio_attr *attr;
struct iio_channel *chn;
struct iio_device *dev;
unsigned int i, j;
int err;

if (!ctx->ops->read_attr)
return 0;

for (i = 0; i < ctx->nb_devices; i++) {
dev = ctx->devices[i];

for (j = 0; j < dev->nb_channels; j++) {
chn = dev->channels[j];

attr = iio_channel_find_attr(chn, "scale");
if (attr) {
err = iio_attr_read_double(attr,
&chn->format.scale);
if (err) {
chn_perror(chn, err, "Unable to read scale");
return err;
}

chn->format.with_scale = true;
}

attr = iio_channel_find_attr(chn, "offset");
if (attr) {
err = iio_attr_read_double(attr,
&chn->format.offset);
if (err) {
chn_perror(chn, err, "Unable to read offset");
return err;
Copy link
Contributor

Choose a reason for hiding this comment

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

didn't checked the code so just sanity checking... There's really no with_offset like in the scale case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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).

}
}
}
}

return 0;
}

struct iio_context * iio_create_context(const struct iio_context_params *params,
const char *uri)
{
Expand All @@ -406,6 +450,7 @@ struct iio_context * iio_create_context(const struct iio_context_params *params,
struct iio_context *ctx = NULL;
char *uri_dup = NULL;
unsigned int i;
int err;

if (params)
params2 = *params;
Expand Down Expand Up @@ -445,6 +490,14 @@ struct iio_context * iio_create_context(const struct iio_context_params *params,

free(uri_dup);

if (!iio_err(ctx)) {
err = iio_context_update_scale_offset(ctx);
if (err) {
iio_context_destroy(ctx);
ctx = iio_ptr(err);
}
}

return ctx;
}

Expand Down
1 change: 1 addition & 0 deletions iio-private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ struct iio_channel {
bool is_output;
bool is_scan_element;
struct iio_data_format format;
char __format_padding[128 - sizeof(struct iio_data_format)];
char *name, *id;
long index;
enum iio_modifier modifier;
Expand Down
4 changes: 4 additions & 0 deletions include/iio/iio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1328,6 +1328,10 @@ struct iio_data_format {

/** @brief Number of times length repeats (added in v0.8) */
unsigned int repeat;

/** @brief Contains a value to be added to the raw sample before
* applying the scale. */
double offset;
Copy link
Contributor

Choose a reason for hiding this comment

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

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...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, as a separate commit. Thanks for the review btw.

};


Expand Down
48 changes: 0 additions & 48 deletions local.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,16 +532,6 @@ static ssize_t local_write_dev_attr(const struct iio_device *dev,
return ret ? ret : -EIO;
}

static const char * get_filename(const struct iio_channel *chn,
const char *attr)
{
unsigned int i;
for (i = 0; i < chn->attrlist.num; i++)
if (!strcmp(attr, chn->attrlist.attrs[i].name))
return chn->attrlist.attrs[i].filename;
return attr;
}

static int channel_write_state(const struct iio_channel *chn,
unsigned int idx, bool en)
{
Expand Down Expand Up @@ -1572,42 +1562,6 @@ const struct iio_backend iio_local_backend = {
.default_timeout_ms = 1000,
};

static void init_data_scale(struct iio_channel *chn)
{
char *end, buf[1024];
const char *attr;
ssize_t ret;
float value;

chn->format.with_scale = false;
attr = get_filename(chn, "scale");

ret = local_read_dev_attr(chn->dev, 0, attr,
buf, sizeof(buf), IIO_ATTR_TYPE_DEVICE);
if (ret < 0)
return;

errno = 0;
value = strtof(buf, &end);
if (end == buf || errno == ERANGE)
return;

chn->format.with_scale = true;
chn->format.scale = value;
}

static void init_scan_elements(struct iio_context *ctx)
{
unsigned int i, j;

for (i = 0; i < iio_context_get_devices_count(ctx); i++) {
struct iio_device *dev = iio_context_get_device(ctx, i);

for (j = 0; j < dev->nb_channels; j++)
init_data_scale(dev->channels[j]);
}
}

static int populate_context_attrs(struct iio_context *ctx, const char *file)
{
struct INI *ini;
Expand Down Expand Up @@ -1704,8 +1658,6 @@ local_create_context(const struct iio_context_params *params, const char *args)

foreach_in_dir(ctx, ctx, "/sys/kernel/debug/iio", true, add_debug);

init_scan_elements(ctx);

if (WITH_LOCAL_CONFIG) {
ret = populate_context_attrs(ctx, "/etc/libiio.ini");
if (ret < 0)
Expand Down