IIO: Support oversampling_ratio configuration in IIO node #2354

guchaojie · 2017-01-22T06:52:15Z

Signed-off-by: Gu Chaojie [email protected]

guchaojie · 2017-01-22T08:17:31Z

@ceolin @edersondisouza @fulong82 @laykuanloon This PR is for issue #2330 please have a review. Thanks

laykuanloon · 2017-01-23T01:31:10Z

src/modules/flow/iio/iio.json

@@ -218,6 +218,12 @@
             "name": "offset"
         },
         {
+             "data_type": "direction-vector",


This "data_type" should be "sol_str_table" ?

In iio.json, data_type cannot be sol_str_table here, please check https://github.com/solettaproject/soletta/blob/master/src/lib/common/include/sol-types.h

laykuanloon · 2017-01-23T01:34:10Z

src/lib/io/sol-iio.c

+
+        if (craft_filename_path(path, sizeof(path), CHANNEL_OVERSAMPLING_RATIO_DEVICE_PATH,
+            device->device_id, iter->key)) {
+            if (sol_util_write_file(path, "%d", iter->val) > 0)


magnetometer have 3 items in sol_str_table, this only write the 1st item and return ?

It is OK for us to test BMP280. Yes, this will result into magnet bugs. I will fix it.

laykuanloon

Please check

cabelitos · 2017-01-23T13:55:06Z

src/modules/flow/iio/nodes.c

+        { }
+    };
+
+    mdata->iio_base.config.oversampling_ratio_table = table;


This is wrong and will lead to memory issues!

The table variable will no longer exists when magnet_open() exit, therefore mdata->iio_base.config.oversampling_ratio_table will point to an invalid area!

cabelitos · 2017-01-23T13:55:15Z

src/modules/flow/iio/nodes.c

+        { }
+    };
+
+    mdata->iio_base.config.oversampling_ratio_table = table;


same problem

cabelitos · 2017-01-23T13:55:23Z

src/modules/flow/iio/nodes.c

+        { }
+    };
+
+    mdata->iio_base.config.oversampling_ratio_table = table;


same problem

cabelitos · 2017-01-23T13:56:19Z

src/lib/io/sol-iio.c

+    char path[PATH_MAX];
+    struct sol_str_table *iter;
+
+    SOL_NULL_CHECK(device, false);


This is not a public API. Do you really need to make this check here?

YES, check the input parameter pointer in function "iio_set_sampling_frequency" too.

as @cabelitos said this does not make sense. There is only one place where this function is called and you have already checked this parameter there. It's looking redundant.

guchaojie · 2017-01-24T09:11:08Z

@ceolin @edersondisouza @fulong82 @laykuanloon This PR is for issue #2330 According to your first round comments, update the PR and have a review, please. Thanks

ceolin · 2017-01-26T17:50:56Z

src/lib/io/sol-iio.c

+            device->device_id, iter->key)) {
+            if (sol_util_write_file(path, "%d", iter->val) <= 0)
+                return false;
+        }


craft_filename_path fails is not an error ? you are only returning false when sol_util_write_file fails.

modified it.

ceolin · 2017-01-26T18:00:47Z

src/modules/flow/iio/nodes.c

+    SOL_NULL_CHECK(mdata->iio_base.config.oversampling_ratio_table, -ENOMEM);
+
+    table = mdata->iio_base.config.oversampling_ratio_table;
+    table->key = strdup("in_magn_x_");


you have to check the return of strdup, it can lead to a segv because you doing in the next line a strlen with it.

Since you'll need to do this with the others elements, you can create a macro here to avoid repeat code.

modified it.

ceolin · 2017-01-26T18:02:23Z

src/modules/flow/iio/nodes.c

+    ret = snprintf(mdata->iio_base.config.sampling_frequency_name,
+        sizeof(mdata->iio_base.config.sampling_frequency_name), "%s", "in_temp_");
+    SOL_INT_CHECK_GOTO(ret, >= (int)sizeof(mdata->iio_base.config.sampling_frequency_name), err);
+    SOL_INT_CHECK_GOTO(ret, < 0, err);


Not a big deal, but we have a macro called SOL_EXP_CHECK_GOTO that an be used here to avoid two checks.

ceolin · 2017-01-26T18:03:07Z

src/modules/flow/iio/nodes.c

+
+    table = mdata->iio_base.config.oversampling_ratio_table;
+    table->key = strdup("in_temp_");
+    table->len = strlen(table->key);


ceolin · 2017-01-26T18:03:20Z

src/modules/flow/iio/nodes.c

+
+    table = mdata->iio_base.config.oversampling_ratio_table;
+    table->key = strdup("in_pressure_");
+    table->len = strlen(table->key);


Signed-off-by: Gu Chaojie <[email protected]>

guchaojie · 2017-02-03T08:01:05Z

@ceolin @edersondisouza @fulong82 @laykuanloon This PR is for issue #2330 According to your comments, update the PR and have a review, please. Thanks

peijiajames · 2017-02-13T03:48:18Z

@ceolin @cabelitos @laykuanloon any more comments for the PR? We want to have this for BMP/BME 280 support in Soletta.

cabelitos · 2017-02-13T15:37:57Z

I guess we're fine.
@ceolin what do you think?

ceolin · 2017-02-16T21:23:23Z

yep, looks good.

guchaojie force-pushed the master branch 2 times, most recently from b620098 to 89de6e9 Compare January 22, 2017 07:45

laykuanloon reviewed Jan 23, 2017

View reviewed changes

guchaojie force-pushed the master branch from 89de6e9 to 3d858d1 Compare January 23, 2017 07:44

cabelitos suggested changes Jan 23, 2017

View reviewed changes

guchaojie force-pushed the master branch from 3d858d1 to 11f9cbc Compare January 24, 2017 09:06

ceolin reviewed Jan 26, 2017

View reviewed changes

IIO: Support oversampling_ratio configuration in IIO node

fc1ceb1

Signed-off-by: Gu Chaojie <[email protected]>

guchaojie force-pushed the master branch from 11f9cbc to fc1ceb1 Compare February 3, 2017 07:37

ceolin merged commit fc1ceb1 into solettaproject:master Feb 16, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IIO: Support oversampling_ratio configuration in IIO node #2354

IIO: Support oversampling_ratio configuration in IIO node #2354

guchaojie commented Jan 22, 2017

guchaojie commented Jan 22, 2017

laykuanloon Jan 23, 2017 •

edited

Loading

guchaojie Jan 23, 2017

laykuanloon Jan 23, 2017

guchaojie Jan 23, 2017

laykuanloon left a comment

cabelitos Jan 23, 2017

cabelitos Jan 23, 2017

cabelitos Jan 23, 2017

cabelitos Jan 23, 2017

guchaojie Jan 24, 2017 •

edited

Loading

ceolin Jan 26, 2017

guchaojie commented Jan 24, 2017

ceolin Jan 26, 2017

guchaojie Feb 3, 2017

ceolin Jan 26, 2017

guchaojie Feb 3, 2017

ceolin Jan 26, 2017 •

edited

Loading

ceolin Jan 26, 2017

ceolin Jan 26, 2017

guchaojie commented Feb 3, 2017

peijiajames commented Feb 13, 2017

cabelitos commented Feb 13, 2017

ceolin commented Feb 16, 2017

IIO: Support oversampling_ratio configuration in IIO node #2354

IIO: Support oversampling_ratio configuration in IIO node #2354

Conversation

guchaojie commented Jan 22, 2017

guchaojie commented Jan 22, 2017

laykuanloon Jan 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laykuanloon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guchaojie Jan 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guchaojie commented Jan 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ceolin Jan 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guchaojie commented Feb 3, 2017

peijiajames commented Feb 13, 2017

cabelitos commented Feb 13, 2017

ceolin commented Feb 16, 2017

laykuanloon Jan 23, 2017 •

edited

Loading

guchaojie Jan 24, 2017 •

edited

Loading

ceolin Jan 26, 2017 •

edited

Loading