Skip to content
This repository has been archived by the owner on Jun 27, 2019. It is now read-only.

New IIO C-API to read configuration attributes #2117

Closed
lblim opened this issue Jun 2, 2016 · 20 comments
Closed

New IIO C-API to read configuration attributes #2117

lblim opened this issue Jun 2, 2016 · 20 comments
Labels
Projects
Milestone

Comments

@lblim
Copy link

lblim commented Jun 2, 2016

Task Description

The existing IIO C API does not allow the users to read out the existing configuration. The API only support reading out the sensor raw data or processed data.

Need new API be introduced to allow users to read and modify the attributes of IIO devices.

@barbieri barbieri added the task label Jun 9, 2016
@barbieri barbieri added this to the v2 milestone Jun 9, 2016
@lblim
Copy link
Author

lblim commented Jun 24, 2016

May I know when is the target date for v2?

@barbieri
Copy link

no dates set in stone, but we're looking into 2-3 months maximum.

@yongli3
Copy link
Contributor

yongli3 commented Sep 21, 2016

@laykuanloon @fulong82 could you share more information about which configuration items are needed for the reading/modifying?

@fulong82
Copy link
Contributor

Hi @yongli3,

Sorry for the late response. Here are the list that we would like to have a read API that can be used in sensor application:

  • sampling frequency
  • hysteresis
  • scale
  • offset

@laykuanloon do you have any interfaces want to add on?

Thanks & Regards,
Wilson

@yongli3
Copy link
Contributor

yongli3 commented Sep 28, 2016

@fulong82 which sensor has the "hysteresis" ?

@yongli3
Copy link
Contributor

yongli3 commented Oct 10, 2016

Just like the sol_iio_mount_calibration API, we can add a new one as below:
Int sol_iio_read_device_config(struct sol_iio_device *device, struct iio_device_config *config);

The API will query the device configuration, and save them into config

struct iio_device_config {
double hysteresis;
double offset;
double sampling_frequency;
double scale;
}

@fulong82 @laykuanloon @edersondisouza @ceolin any comment/suggestion ?

@guchaojie
Copy link
Contributor

guchaojie commented Oct 10, 2016

I suppose we should have 2 different APIs to get parameter:
eg:
In fbp level: The principle is that we should let user get config parameter easily and clearly. So write like this to get config: tick OUT -> OFFSET gyro CONFIG-> IN _(console:prefix="offset: ")

In nodes.c level: we should check two key parameter from fbp include what config parameter(offset, scale, ...) and what kind of sensor type (different type has different config data type)

In sol-iio API level:

  1. double sol_iio_read_device__double_config(struct sol_iio_device *device, const char * config_name);
  2. sol_direction_vector sol_iio_read_device_vector_config(struct sol_iio_device *device, const char * config_name);

@fulong82 @laykuanloon @edersondisouza @ceolin @yongli3 any comment/suggestion ?

@guchaojie
Copy link
Contributor

For considering the return of error code , the improvement for API design is :

  1. int sol_iio_read_device__double_config(struct sol_iio_device *device, const char * config_name, double *value);
  2. int sol_iio_read_device_vector_config(struct sol_iio_device *device, const char * config_name, sol_direction_vector *value);
    @fulong82 @laykuanloon @edersondisouza @ceolin @yongli3 any more comment/suggestion ?

@laykuanloon
Copy link
Contributor

@guchaojie
Do you consider to add API to read a list of supported value?
For e.g:
sampling_frequency_available
0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800 1600

in_accel_scale_available
0.009582 0.019163 0.038326

@guchaojie
Copy link
Contributor

For get sensor config available config , I suggest two APIs:
To get the count of available value for user to malloc double array to store

  1. int sol_iio_read_device_available_config_count(struct sol_iio_device *device, const char * config_name, uint32_t *count)
    To get the available config value into user malloc memory
  2. int sol_iio_read_device_available_config(struct sol_iio_device *device, const char * config_name, uint32_t *count, double *values)

@fulong82 @laykuanloon @edersondisouza @ceolin @yongli3 any comment/suggestion ?

@laykuanloon
Copy link
Contributor

@guchaojie
Can just combine into 1 API ? malloc in this function also
2. int sol_iio_read_device_available_config(struct sol_iio_device *device, const char * config_name, uint32_t *count, double *values)

@guchaojie
Copy link
Contributor

@laykuanloon If do so, when to release the memory in API internal function?
2 APIs can let user malloc and free the memory

@laykuanloon
Copy link
Contributor

@guchaojie
I think you can use pointer to pointer if want to combine the API. I am fine with what your propose.

@peijiajames
Copy link

I think memory allocation/free is not the key issue, though it's better to be released by assigner itself.

Actually the problem is, usually user does not know how may values can be supported for that configuration, so it's difficult to predict the size of the list. A query-response mechanism could be helpful to get the actual size here.

@guchaojie
Copy link
Contributor

guchaojie commented Oct 24, 2016

For Summary, we finally define 4 APIs for two categories:

  • To read specific current configuration attribute:
    int sol_iio_read_device__double_config(struct sol_iio_device *device, const char * config_name, double *value);
    int sol_iio_read_device_vector_config(struct sol_iio_device *device, const char * config_name, sol_direction_vector *value);
  • To read specific configuration available attributes
    //To get the count of available value for user to malloc double array to store
    int sol_iio_read_device_available_config_count(struct sol_iio_device *device, const char * config_name, uint32_t *count)
    //To get the available config value into user malloc memory
    int sol_iio_read_device_available_config(struct sol_iio_device *device, const char * config_name, uint32_t *count, double *values)

@fulong82 @laykuanloon @edersondisouza @ceolin @yongli3 @peijiajames
If no more comments, we are going on new C-API work.

@laykuanloon
Copy link
Contributor

@guchaojie
I am ok. Why sol_iio_read_device__double_config have 2 underscore?

@edersondisouza
Copy link
Contributor

Hi everyone,

I'd say that int sol_iio_read_device_double_config(struct sol_iio_device *device, const char * config_name, double *value); and int sol_iio_read_device_vector_config(struct sol_iio_device *device, const char * config_name, sol_direction_vector *value); are fine. They blend well with Soletta current APIs.
However, I'm a bit concerned about the one to get a list of available configurations. Soletta has sol_vector - couldn't it come to help?
What about something like struct sol_vector *sol_iio_list_available_config(struct sol_iio_device *device, const char *config_name)?
It would return NULL on error. It would be caller responsibility to clear the sol_vector, and sol_vector use is quite common throughout Soletta.

@guchaojie
Copy link
Contributor

guchaojie commented Oct 26, 2016

Hi everyone,
According to the deep implementation, Some changes have to be modified, Let me classify two categories to make statement clear.

  • For reading current configuration attributes, we found two issues:

1 parameter "config_name" generally would be prefix name such as "in_anglvel_x" or "in_anglvel_y"...
So for this condition, we input only one config_name which cannnot get sol_direction_vector data type. It would be better to let user format double data from C-API into sol_direction_vector for output if necessary such as in fbp

2 In existing Soletta sol-iio design, we found configuration has been classified to two structure: sol_iio_config and sol_iio_channel_config.
we can design this style API:
int sol_iio_read_device_iio_config(struct sol_iio_device *device, const char * config_name, sol_iio_config *value);
int sol_iio_read_device_iio_channel_config(struct sol_iio_device *device, const char * config_name, sol_iio_channel_config *value);
However I think this style is not good because two reasons:
1st is that it will make user confused and get all the config attribute for one time which let user has no choice.
2nd is that if these two structure happen changed, this style will impact exist implementation by using old C-API

So I think new C-API should be like this, 1st version would be include 3 APIs , and would be easy to add more configuration reading API in future.
int sol_iio_device_get_scale(struct sol_iio_device *device, const char * config_name, double *scale);
int sol_iio_device_get_offset(struct sol_iio_device *device, const char * config_name, double *offset);
int sol_iio_device_get_sampling_frequency(struct sol_iio_device *device, const char * config_name, double *sampling_frequency);

And we have to pay attention to sampling_frequency and offset data types should be "double", however, in sol-iio library these two configuration attribute data type is "int". This inconformity will result into imprecise attribute reading.

  • For reading a list of available configuration attributes,
    @edersondisouza your proposal is good idea, more details would be better to discuss in another new issue thread, this implementation would not be include 1st version of PR which is totally different from getting current configuration attribute in fact.

@fulong82 @laykuanloon @edersondisouza @ceolin @yongli3 @peijiajames
Sorry for updating the C-API design because of actual issue found in implementation, Thanks!

@ceolin
Copy link
Contributor

ceolin commented Nov 4, 2016

It was merged. Closing it.

@ceolin ceolin closed this as completed Nov 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
V2
Review
Development

No branches or pull requests

9 participants