[jira] [Commented] (MYNEWT-881) SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
[ https://issues.apache.org/jira/browse/MYNEWT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16373453#comment-16373453 ] ASF subversion and git services commented on MYNEWT-881: Commit 5cd2eb6a8a0eaa18aca2b0959e557c39e9f9b4b8 in mynewt-core's branch refs/heads/master from [~vrahane] [ https://gitbox.apache.org/repos/asf?p=mynewt-core.git;h=5cd2eb6 ] MYNEWT-881 SensorAPI:Add sensor_clear_xx_thresh() SensorAPI: - Add sensor_clear_low_thresh() and sensor_clear_high_thresh() APIs LIS2DH12: - Combining interupt handlers for int1 and int2 - Adding lis2dh12_clear_low_thresh() and lis2dh12_clear_high_thresh() - Seperating out low_thresh and high_thresh functions for setting - Releasing IRQ on errors > SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix > -- > > Key: MYNEWT-881 > URL: https://issues.apache.org/jira/browse/MYNEWT-881 > Project: Mynewt > Issue Type: Improvement > Security Level: Public(Viewable by anyone) > Components: Drivers, Sensor Framework >Affects Versions: v1_3_0_rel > Environment: ruuvitag-revb2 >Reporter: Vipul Rahane >Assignee: Vipul Rahane >Priority: Major > Labels: core > Fix For: v1_4_0_rel > > > There are two issues for calling the lis2dh12_sensor_set_trigger_thresh() : > 1. gpio irq was not being released before doing a re-initialization > 2. srec was malloced and calling the function multiple times would have > malloced additional memory without freeing it. > Solution: > 1. Release the gpio irq > 2. Remove the malloc and add a ptr for the sensor in each type trait so that > a sensor read could be performed in the irq handler. > Also, sensor_unset_thresh() API was added to unset thresholds > triggers/interrupts. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (MYNEWT-881) SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
[
https://issues.apache.org/jira/browse/MYNEWT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16373451#comment-16373451
]
ASF GitHub Bot commented on MYNEWT-881:
---
vrahane closed pull request #718: MYNEWT-881: SensorAPI: Add
sensor_clear_high/low_thresh() API and LIS2DH12 set threshold cb fix
URL: https://github.com/apache/mynewt-core/pull/718
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/apps/sensors_test/src/main.c b/apps/sensors_test/src/main.c
index fe4a862d8..6b3af902f 100755
--- a/apps/sensors_test/src/main.c
+++ b/apps/sensors_test/src/main.c
@@ -36,6 +36,8 @@
#include
#include
#include
+#include
+#include
#if MYNEWT_VAL(BNO055_CLI)
#include
@@ -80,6 +82,10 @@ static const oc_handler_t sensor_oic_handler = {
/* Application-specified header. */
#include "bleprph.h"
+struct sensor_type_traits stt;
+struct sensor_accel_data sad_low;
+struct sensor_accel_data sad_high;
+
#if MYNEWT_VAL(SENSOR_OIC)
static int sensor_oic_gap_event(struct ble_gap_event *event, void *arg);
#endif
@@ -272,6 +278,9 @@ sensor_oic_gap_event(struct ble_gap_event *event, void *arg)
oc_ble_coap_conn_del(event->disconnect.conn.conn_handle);
+sensor_clear_low_thresh("lis2dh12_0", SENSOR_TYPE_ACCELEROMETER);
+sensor_clear_high_thresh("lis2dh12_0", SENSOR_TYPE_ACCELEROMETER);
+
/* Connection terminated; resume advertising. */
sensor_oic_advertise();
return 0;
@@ -525,6 +534,35 @@ main(int argc, char **argv)
/* log reboot */
reboot_start(hal_reset_cause());
+stt = (struct sensor_type_traits) {
+.stt_sensor_type = SENSOR_TYPE_ACCELEROMETER,
+.stt_low_thresh.sad = &sad_low,
+.stt_high_thresh.sad = &sad_high,
+.stt_algo= SENSOR_THRESH_ALGO_WATERMARK
+};
+
+/* The thresholds are specified in m/s^2 */
+
+sad_low = (struct sensor_accel_data) {
+.sad_x = 0.6712,
+.sad_x_is_valid = 1,
+.sad_y = 0.6712,
+.sad_y_is_valid = 1,
+.sad_z = 0.6712,
+.sad_z_is_valid = 1
+};
+
+sad_high = (struct sensor_accel_data) {
+.sad_x = 0.6712,
+.sad_x_is_valid = 1,
+.sad_y = 0.6712,
+.sad_y_is_valid = 1,
+.sad_z = 0.6712,
+.sad_z_is_valid = 1
+};
+
+sensor_set_thresh("lis2dh12_0", &stt);
+
/*
* As the last thing, process events from default event queue.
*/
diff --git a/hw/drivers/sensors/lis2dh12/include/lis2dh12/lis2dh12.h
b/hw/drivers/sensors/lis2dh12/include/lis2dh12/lis2dh12.h
index c853484b7..08a1c7f02 100644
--- a/hw/drivers/sensors/lis2dh12/include/lis2dh12/lis2dh12.h
+++ b/hw/drivers/sensors/lis2dh12/include/lis2dh12/lis2dh12.h
@@ -294,6 +294,24 @@ lis2dh12_set_int1_duration(struct sensor_itf *itf, uint8_t
dur);
int
lis2dh12_set_int2_duration(struct sensor_itf *itf, uint8_t dur);
+/**
+ * Disable interrupt 1
+ *
+ * @param the sensor interface
+ * @return 0 on success, non-zero on failure
+ */
+int
+lis2dh12_disable_int1(struct sensor_itf *itf);
+
+/**
+ * Disable interrupt 2
+ *
+ * @param the sensor interface
+ * @return 0 on success, non-zero on failure
+ */
+int
+lis2dh12_disable_int2(struct sensor_itf *itf);
+
/**
* Set high pass filter cfg
*
diff --git a/hw/drivers/sensors/lis2dh12/src/lis2dh12.c
b/hw/drivers/sensors/lis2dh12/src/lis2dh12.c
index e0849c838..bb2640c61 100644
--- a/hw/drivers/sensors/lis2dh12/src/lis2dh12.c
+++ b/hw/drivers/sensors/lis2dh12/src/lis2dh12.c
@@ -70,11 +70,19 @@ static int lis2dh12_sensor_get_config(struct sensor *,
sensor_type_t,
static int
lis2dh12_sensor_set_trigger_thresh(struct sensor *, sensor_type_t,
struct sensor_type_traits *);
+static int
+lis2dh12_sensor_clear_low_thresh(struct sensor *, sensor_type_t);
+
+static int
+lis2dh12_sensor_clear_high_thresh(struct sensor *, sensor_type_t);
+
static const struct sensor_driver g_lis2dh12_sensor_driver = {
.sd_read = lis2dh12_sensor_read,
.sd_get_config = lis2dh12_sensor_get_config,
/* Setting trigger threshold is optional */
-.sd_set_trigger_thresh = lis2dh12_sensor_set_trigger_thresh
+.sd_set_trigger_thresh = lis2dh12_sensor_set_trigger_thresh,
+.sd_clear_low_trigger_thresh = lis2dh12_sensor_clear_low_thresh,
+.sd_clear_high_trigger_thresh = lis2dh12_sensor_clear_high_thresh
};
/**
@@ -1242,6 +1250,60 @@ lis2dh12_set_int2_pin_cfg(struct sensor_itf *itf,
uint8_t cfg)
return lis2dh12_writelen(itf, LIS2DH12_REG_CTRL_REG6, &cfg, 1);
}
+/**
+ * Disable interrupt 1
+ *
+ * @param the sensor interface
+ * @return 0 on success, non-zero on failure
+ */
+int
+lis2dh12_disabl
[jira] [Commented] (MYNEWT-881) SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
[ https://issues.apache.org/jira/browse/MYNEWT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16373454#comment-16373454 ] ASF subversion and git services commented on MYNEWT-881: Commit 4e7537bbecf15d0b47adaf52807218ceb12dbedb in mynewt-core's branch refs/heads/master from [~vrahane] [ https://gitbox.apache.org/repos/asf?p=mynewt-core.git;h=4e7537b ] Merge pull request #718 from vrahane/lis2dh12_thresholds_fix MYNEWT-881: SensorAPI: Add sensor_clear_high/low_thresh() API and LIS2DH12 set threshold cb fix > SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix > -- > > Key: MYNEWT-881 > URL: https://issues.apache.org/jira/browse/MYNEWT-881 > Project: Mynewt > Issue Type: Improvement > Security Level: Public(Viewable by anyone) > Components: Drivers, Sensor Framework >Affects Versions: v1_3_0_rel > Environment: ruuvitag-revb2 >Reporter: Vipul Rahane >Assignee: Vipul Rahane >Priority: Major > Labels: core > Fix For: v1_4_0_rel > > > There are two issues for calling the lis2dh12_sensor_set_trigger_thresh() : > 1. gpio irq was not being released before doing a re-initialization > 2. srec was malloced and calling the function multiple times would have > malloced additional memory without freeing it. > Solution: > 1. Release the gpio irq > 2. Remove the malloc and add a ptr for the sensor in each type trait so that > a sensor read could be performed in the irq handler. > Also, sensor_unset_thresh() API was added to unset thresholds > triggers/interrupts. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (MYNEWT-881) SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
[ https://issues.apache.org/jira/browse/MYNEWT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16373452#comment-16373452 ] ASF subversion and git services commented on MYNEWT-881: Commit 615e3ce6e445ab9bdb7d101d14172ffe8135aee3 in mynewt-core's branch refs/heads/master from [~vrahane] [ https://gitbox.apache.org/repos/asf?p=mynewt-core.git;h=615e3ce ] MYNEWT-881 SensorAPI: LIS2DH12 set thresh cb fix There are two issues for calling the lis2dh12_sensor_set_trigger_thresh(): 1. gpio irq was not being released before doing a re-initialization 2. srec was malloced and calling the function multiple times would have malloced additional memory without freeing it. Solution: 1. Release the gpio irq 2. Remove the malloc and add a ptr for the sensor in each type trait so that a sensor read could be performed in the irq handler. > SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix > -- > > Key: MYNEWT-881 > URL: https://issues.apache.org/jira/browse/MYNEWT-881 > Project: Mynewt > Issue Type: Improvement > Security Level: Public(Viewable by anyone) > Components: Drivers, Sensor Framework >Affects Versions: v1_3_0_rel > Environment: ruuvitag-revb2 >Reporter: Vipul Rahane >Assignee: Vipul Rahane >Priority: Major > Labels: core > Fix For: v1_4_0_rel > > > There are two issues for calling the lis2dh12_sensor_set_trigger_thresh() : > 1. gpio irq was not being released before doing a re-initialization > 2. srec was malloced and calling the function multiple times would have > malloced additional memory without freeing it. > Solution: > 1. Release the gpio irq > 2. Remove the malloc and add a ptr for the sensor in each type trait so that > a sensor read could be performed in the irq handler. > Also, sensor_unset_thresh() API was added to unset thresholds > triggers/interrupts. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (MYNEWT-881) SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
[
https://issues.apache.org/jira/browse/MYNEWT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16365912#comment-16365912
]
ASF GitHub Bot commented on MYNEWT-881:
---
vrahane commented on a change in pull request #718: MYNEWT-881: SensorAPI: Add
sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
URL: https://github.com/apache/mynewt-core/pull/718#discussion_r168539007
##
File path: hw/drivers/sensors/lis2dh12/src/lis2dh12.c
##
@@ -1463,7 +1485,75 @@ lis2dh12_sensor_set_trigger_thresh(struct sensor
*sensor,
}
}
+err:
+return rc;
+}
+
+
+/**
+ * Set the trigger threshold values and enable interrupts
+ *
+ * @param ptr to sensor
+ * @param the Sensor type
+ * @param Sensor type traits
+ *
+ * @return 0 on success, non-zero on failure
+ */
+static int
+lis2dh12_sensor_set_trigger_thresh(struct sensor *sensor,
+ sensor_type_t type,
+ struct sensor_type_traits *stt)
+{
+int rc;
+uint8_t fs;
+struct sensor_itf *itf;
+sensor_data_t low_thresh;
+sensor_data_t high_thresh;
+
+itf = SENSOR_GET_ITF(sensor);
+
+if (type != SENSOR_TYPE_ACCELEROMETER) {
+rc = SYS_EINVAL;
+goto err;
+}
+
+memcpy(&low_thresh, &stt->stt_low_thresh, sizeof(low_thresh));
+memcpy(&high_thresh, &stt->stt_high_thresh, sizeof(high_thresh));
+
+rc = lis2dh12_get_full_scale(itf, &fs);
+if (rc) {
+goto err;
+}
+
+if (fs == LIS2DH12_FS_2G) {
+fs = 16;
+} else if (fs == LIS2DH12_FS_4G) {
+fs = 32;
+} else if (fs == LIS2DH12_FS_8G) {
+fs = 62;
+} else if (fs == LIS2DH12_FS_16G) {
+fs = 186;
+} else {
+rc = SYS_EINVAL;
+goto err;
+}
+
+/* Set low threshold and enable interrupt */
+rc = lis2dh12_set_low_thresh(itf, low_thresh, fs, stt);
+if (rc) {
+hal_gpio_irq_release(itf->si_low_pin);
+goto err;
+}
+
+/* Set high threshold and enable interrupt */
+rc = lis2dh12_set_high_thresh(itf, high_thresh, fs, stt);
+if (rc) {
+hal_gpio_irq_release(itf->si_high_pin);
Review comment:
Agreed. Will move it in.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
> --
>
> Key: MYNEWT-881
> URL: https://issues.apache.org/jira/browse/MYNEWT-881
> Project: Mynewt
> Issue Type: Improvement
> Security Level: Public(Viewable by anyone)
> Components: Drivers, Sensor Framework
>Affects Versions: v1_3_0_rel
> Environment: ruuvitag-revb2
>Reporter: Vipul Rahane
>Assignee: Vipul Rahane
>Priority: Major
> Labels: core
> Fix For: v1_4_0_rel
>
>
> There are two issues for calling the lis2dh12_sensor_set_trigger_thresh() :
> 1. gpio irq was not being released before doing a re-initialization
> 2. srec was malloced and calling the function multiple times would have
> malloced additional memory without freeing it.
> Solution:
> 1. Release the gpio irq
> 2. Remove the malloc and add a ptr for the sensor in each type trait so that
> a sensor read could be performed in the irq handler.
> Also, sensor_unset_thresh() API was added to unset thresholds
> triggers/interrupts.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (MYNEWT-881) SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
[
https://issues.apache.org/jira/browse/MYNEWT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16365909#comment-16365909
]
ASF GitHub Bot commented on MYNEWT-881:
---
vrahane commented on a change in pull request #718: MYNEWT-881: SensorAPI: Add
sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
URL: https://github.com/apache/mynewt-core/pull/718#discussion_r168538692
##
File path: hw/drivers/sensors/lis2dh12/src/lis2dh12.c
##
@@ -1463,7 +1485,75 @@ lis2dh12_sensor_set_trigger_thresh(struct sensor
*sensor,
}
}
+err:
+return rc;
+}
+
+
+/**
+ * Set the trigger threshold values and enable interrupts
+ *
+ * @param ptr to sensor
+ * @param the Sensor type
+ * @param Sensor type traits
+ *
+ * @return 0 on success, non-zero on failure
+ */
+static int
+lis2dh12_sensor_set_trigger_thresh(struct sensor *sensor,
+ sensor_type_t type,
+ struct sensor_type_traits *stt)
+{
+int rc;
+uint8_t fs;
+struct sensor_itf *itf;
+sensor_data_t low_thresh;
+sensor_data_t high_thresh;
+
+itf = SENSOR_GET_ITF(sensor);
+
+if (type != SENSOR_TYPE_ACCELEROMETER) {
+rc = SYS_EINVAL;
+goto err;
+}
+
+memcpy(&low_thresh, &stt->stt_low_thresh, sizeof(low_thresh));
+memcpy(&high_thresh, &stt->stt_high_thresh, sizeof(high_thresh));
+
+rc = lis2dh12_get_full_scale(itf, &fs);
+if (rc) {
+goto err;
+}
+
+if (fs == LIS2DH12_FS_2G) {
+fs = 16;
+} else if (fs == LIS2DH12_FS_4G) {
+fs = 32;
+} else if (fs == LIS2DH12_FS_8G) {
+fs = 62;
+} else if (fs == LIS2DH12_FS_16G) {
+fs = 186;
+} else {
+rc = SYS_EINVAL;
+goto err;
+}
+
+/* Set low threshold and enable interrupt */
+rc = lis2dh12_set_low_thresh(itf, low_thresh, fs, stt);
+if (rc) {
+hal_gpio_irq_release(itf->si_low_pin);
Review comment:
Agreed. Will move it in.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
> --
>
> Key: MYNEWT-881
> URL: https://issues.apache.org/jira/browse/MYNEWT-881
> Project: Mynewt
> Issue Type: Improvement
> Security Level: Public(Viewable by anyone)
> Components: Drivers, Sensor Framework
>Affects Versions: v1_3_0_rel
> Environment: ruuvitag-revb2
>Reporter: Vipul Rahane
>Assignee: Vipul Rahane
>Priority: Major
> Labels: core
> Fix For: v1_4_0_rel
>
>
> There are two issues for calling the lis2dh12_sensor_set_trigger_thresh() :
> 1. gpio irq was not being released before doing a re-initialization
> 2. srec was malloced and calling the function multiple times would have
> malloced additional memory without freeing it.
> Solution:
> 1. Release the gpio irq
> 2. Remove the malloc and add a ptr for the sensor in each type trait so that
> a sensor read could be performed in the irq handler.
> Also, sensor_unset_thresh() API was added to unset thresholds
> triggers/interrupts.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (MYNEWT-881) SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
[
https://issues.apache.org/jira/browse/MYNEWT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16365901#comment-16365901
]
ASF GitHub Bot commented on MYNEWT-881:
---
vrahane commented on a change in pull request #718: MYNEWT-881: SensorAPI: Add
sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
URL: https://github.com/apache/mynewt-core/pull/718#discussion_r168537739
##
File path: hw/drivers/sensors/lis2dh12/src/lis2dh12.c
##
@@ -1308,44 +1298,62 @@ lis2dh12_sensor_set_trigger_thresh(struct sensor
*sensor,
memcpy(&low_thresh, &stt->stt_low_thresh, sizeof(low_thresh));
memcpy(&high_thresh, &stt->stt_high_thresh, sizeof(high_thresh));
-rc = lis2dh12_get_full_scale(itf, &tmp);
-if (rc) {
-goto err;
+if (!low_thresh.sad->sad_x_is_valid &&
+!low_thresh.sad->sad_y_is_valid &&
+!low_thresh.sad->sad_z_is_valid) {
+/* if all thresholds are not valid, release gpio irq */
+hal_gpio_irq_release(itf->si_low_pin);
+} else {
+/* if one or more thresholds are valid, set trigger */
Review comment:
For lis2dh12, there are multiple interrupts that you can setup. Each axis
has its associated interrupt. Unsetting one of the interrupts does not mean the
IRQ is disabled. It means one of the interrupts from the sensor is disabled. By
doing this, user can unset specific interrupt or all interrupts. I know
unsetting an interrupt is actually just clearing registers of the particular
interrupt but it also means we need to keep other interrupts active. Almost all
the steps in the set thresh function would have to be repeated here which seems
more repetitive code. Hence, calling the set thresh function here.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
> --
>
> Key: MYNEWT-881
> URL: https://issues.apache.org/jira/browse/MYNEWT-881
> Project: Mynewt
> Issue Type: Improvement
> Security Level: Public(Viewable by anyone)
> Components: Drivers, Sensor Framework
>Affects Versions: v1_3_0_rel
> Environment: ruuvitag-revb2
>Reporter: Vipul Rahane
>Assignee: Vipul Rahane
>Priority: Major
> Labels: core
> Fix For: v1_4_0_rel
>
>
> There are two issues for calling the lis2dh12_sensor_set_trigger_thresh() :
> 1. gpio irq was not being released before doing a re-initialization
> 2. srec was malloced and calling the function multiple times would have
> malloced additional memory without freeing it.
> Solution:
> 1. Release the gpio irq
> 2. Remove the malloc and add a ptr for the sensor in each type trait so that
> a sensor read could be performed in the irq handler.
> Also, sensor_unset_thresh() API was added to unset thresholds
> triggers/interrupts.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (MYNEWT-881) SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
[
https://issues.apache.org/jira/browse/MYNEWT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16365670#comment-16365670
]
ASF GitHub Bot commented on MYNEWT-881:
---
rymanluk commented on a change in pull request #718: MYNEWT-881: SensorAPI: Add
sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
URL: https://github.com/apache/mynewt-core/pull/718#discussion_r168440930
##
File path: hw/drivers/sensors/lis2dh12/src/lis2dh12.c
##
@@ -1463,7 +1485,75 @@ lis2dh12_sensor_set_trigger_thresh(struct sensor
*sensor,
}
}
+err:
+return rc;
+}
+
+
+/**
+ * Set the trigger threshold values and enable interrupts
+ *
+ * @param ptr to sensor
+ * @param the Sensor type
+ * @param Sensor type traits
+ *
+ * @return 0 on success, non-zero on failure
+ */
+static int
+lis2dh12_sensor_set_trigger_thresh(struct sensor *sensor,
+ sensor_type_t type,
+ struct sensor_type_traits *stt)
+{
+int rc;
+uint8_t fs;
+struct sensor_itf *itf;
+sensor_data_t low_thresh;
+sensor_data_t high_thresh;
+
+itf = SENSOR_GET_ITF(sensor);
+
+if (type != SENSOR_TYPE_ACCELEROMETER) {
+rc = SYS_EINVAL;
+goto err;
+}
+
+memcpy(&low_thresh, &stt->stt_low_thresh, sizeof(low_thresh));
+memcpy(&high_thresh, &stt->stt_high_thresh, sizeof(high_thresh));
+
+rc = lis2dh12_get_full_scale(itf, &fs);
+if (rc) {
+goto err;
+}
+
+if (fs == LIS2DH12_FS_2G) {
+fs = 16;
+} else if (fs == LIS2DH12_FS_4G) {
+fs = 32;
+} else if (fs == LIS2DH12_FS_8G) {
+fs = 62;
+} else if (fs == LIS2DH12_FS_16G) {
+fs = 186;
+} else {
+rc = SYS_EINVAL;
+goto err;
+}
+
+/* Set low threshold and enable interrupt */
+rc = lis2dh12_set_low_thresh(itf, low_thresh, fs, stt);
+if (rc) {
+hal_gpio_irq_release(itf->si_low_pin);
+goto err;
+}
+
+/* Set high threshold and enable interrupt */
+rc = lis2dh12_set_high_thresh(itf, high_thresh, fs, stt);
+if (rc) {
+hal_gpio_irq_release(itf->si_high_pin);
Review comment:
Similar comment as above. hal_gpio_irq_release(itf->si_high_pin); could be
done in "err" label of lis2dh12_set_high_thresh() function
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
> --
>
> Key: MYNEWT-881
> URL: https://issues.apache.org/jira/browse/MYNEWT-881
> Project: Mynewt
> Issue Type: Improvement
> Security Level: Public(Viewable by anyone)
> Components: Drivers, Sensor Framework
>Affects Versions: v1_3_0_rel
> Environment: ruuvitag-revb2
>Reporter: Vipul Rahane
>Assignee: Vipul Rahane
>Priority: Major
> Labels: core
> Fix For: v1_4_0_rel
>
>
> There are two issues for calling the lis2dh12_sensor_set_trigger_thresh() :
> 1. gpio irq was not being released before doing a re-initialization
> 2. srec was malloced and calling the function multiple times would have
> malloced additional memory without freeing it.
> Solution:
> 1. Release the gpio irq
> 2. Remove the malloc and add a ptr for the sensor in each type trait so that
> a sensor read could be performed in the irq handler.
> Also, sensor_unset_thresh() API was added to unset thresholds
> triggers/interrupts.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (MYNEWT-881) SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
[
https://issues.apache.org/jira/browse/MYNEWT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16365396#comment-16365396
]
ASF GitHub Bot commented on MYNEWT-881:
---
rymanluk commented on a change in pull request #718: MYNEWT-881: SensorAPI: Add
sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
URL: https://github.com/apache/mynewt-core/pull/718#discussion_r168407716
##
File path: hw/drivers/sensors/lis2dh12/src/lis2dh12.c
##
@@ -1308,44 +1298,62 @@ lis2dh12_sensor_set_trigger_thresh(struct sensor
*sensor,
memcpy(&low_thresh, &stt->stt_low_thresh, sizeof(low_thresh));
memcpy(&high_thresh, &stt->stt_high_thresh, sizeof(high_thresh));
-rc = lis2dh12_get_full_scale(itf, &tmp);
-if (rc) {
-goto err;
+if (!low_thresh.sad->sad_x_is_valid &&
+!low_thresh.sad->sad_y_is_valid &&
+!low_thresh.sad->sad_z_is_valid) {
+/* if all thresholds are not valid, release gpio irq */
+hal_gpio_irq_release(itf->si_low_pin);
+} else {
+/* if one or more thresholds are valid, set trigger */
Review comment:
Could you elaborate why we would like to do it? I mean setting trigger in
unset_trigger function?
imho we should do it as simple as possible - whenever
lis2dh12_sensor_unset_trigger_thresh() is called we release IRQ and clear
releated HW registers.
If user what's to set new one, he calls lis2dh12_sensor_set_trigger_thresh.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
> --
>
> Key: MYNEWT-881
> URL: https://issues.apache.org/jira/browse/MYNEWT-881
> Project: Mynewt
> Issue Type: Improvement
> Security Level: Public(Viewable by anyone)
> Components: Drivers, Sensor Framework
>Affects Versions: v1_3_0_rel
> Environment: ruuvitag-revb2
>Reporter: Vipul Rahane
>Assignee: Vipul Rahane
>Priority: Major
> Labels: core
> Fix For: v1_4_0_rel
>
>
> There are two issues for calling the lis2dh12_sensor_set_trigger_thresh() :
> 1. gpio irq was not being released before doing a re-initialization
> 2. srec was malloced and calling the function multiple times would have
> malloced additional memory without freeing it.
> Solution:
> 1. Release the gpio irq
> 2. Remove the malloc and add a ptr for the sensor in each type trait so that
> a sensor read could be performed in the irq handler.
> Also, sensor_unset_thresh() API was added to unset thresholds
> triggers/interrupts.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (MYNEWT-881) SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
[
https://issues.apache.org/jira/browse/MYNEWT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16365395#comment-16365395
]
ASF GitHub Bot commented on MYNEWT-881:
---
rymanluk commented on a change in pull request #718: MYNEWT-881: SensorAPI: Add
sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
URL: https://github.com/apache/mynewt-core/pull/718#discussion_r168440754
##
File path: hw/drivers/sensors/lis2dh12/src/lis2dh12.c
##
@@ -1463,7 +1485,75 @@ lis2dh12_sensor_set_trigger_thresh(struct sensor
*sensor,
}
}
+err:
+return rc;
+}
+
+
+/**
+ * Set the trigger threshold values and enable interrupts
+ *
+ * @param ptr to sensor
+ * @param the Sensor type
+ * @param Sensor type traits
+ *
+ * @return 0 on success, non-zero on failure
+ */
+static int
+lis2dh12_sensor_set_trigger_thresh(struct sensor *sensor,
+ sensor_type_t type,
+ struct sensor_type_traits *stt)
+{
+int rc;
+uint8_t fs;
+struct sensor_itf *itf;
+sensor_data_t low_thresh;
+sensor_data_t high_thresh;
+
+itf = SENSOR_GET_ITF(sensor);
+
+if (type != SENSOR_TYPE_ACCELEROMETER) {
+rc = SYS_EINVAL;
+goto err;
+}
+
+memcpy(&low_thresh, &stt->stt_low_thresh, sizeof(low_thresh));
+memcpy(&high_thresh, &stt->stt_high_thresh, sizeof(high_thresh));
+
+rc = lis2dh12_get_full_scale(itf, &fs);
+if (rc) {
+goto err;
+}
+
+if (fs == LIS2DH12_FS_2G) {
+fs = 16;
+} else if (fs == LIS2DH12_FS_4G) {
+fs = 32;
+} else if (fs == LIS2DH12_FS_8G) {
+fs = 62;
+} else if (fs == LIS2DH12_FS_16G) {
+fs = 186;
+} else {
+rc = SYS_EINVAL;
+goto err;
+}
+
+/* Set low threshold and enable interrupt */
+rc = lis2dh12_set_low_thresh(itf, low_thresh, fs, stt);
+if (rc) {
+hal_gpio_irq_release(itf->si_low_pin);
Review comment:
can we have hal_gpio_irq_release(itf->si_low_pin); in case of error handler
inside lis2dh12_set_low_thresh ?
It can easly be put in "err" label there. And since mentioned function does
operation on this irq (init/release) it makes sense to handle release in case
of error.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
> --
>
> Key: MYNEWT-881
> URL: https://issues.apache.org/jira/browse/MYNEWT-881
> Project: Mynewt
> Issue Type: Improvement
> Security Level: Public(Viewable by anyone)
> Components: Drivers, Sensor Framework
>Affects Versions: v1_3_0_rel
> Environment: ruuvitag-revb2
>Reporter: Vipul Rahane
>Assignee: Vipul Rahane
>Priority: Major
> Labels: core
> Fix For: v1_4_0_rel
>
>
> There are two issues for calling the lis2dh12_sensor_set_trigger_thresh() :
> 1. gpio irq was not being released before doing a re-initialization
> 2. srec was malloced and calling the function multiple times would have
> malloced additional memory without freeing it.
> Solution:
> 1. Release the gpio irq
> 2. Remove the malloc and add a ptr for the sensor in each type trait so that
> a sensor read could be performed in the irq handler.
> Also, sensor_unset_thresh() API was added to unset thresholds
> triggers/interrupts.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (MYNEWT-881) SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
[
https://issues.apache.org/jira/browse/MYNEWT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16365394#comment-16365394
]
ASF GitHub Bot commented on MYNEWT-881:
---
rymanluk commented on a change in pull request #718: MYNEWT-881: SensorAPI: Add
sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
URL: https://github.com/apache/mynewt-core/pull/718#discussion_r168437966
##
File path: hw/sensor/src/sensor.c
##
@@ -1922,6 +1922,59 @@ sensor_set_thresh(char *devname, struct
sensor_type_traits *stt)
return rc;
}
+/**
+ * Unset the thresholds and comparison algo for a sensor
+ *
+ * @param name of the sensor
+ * @param Ptr to sensor threshold
+ *
+ * @return 0 on success, non-zero on failure
+ */
+int
+sensor_unset_thresh(char *devname, struct sensor_type_traits *stt)
+{
+struct sensor *sensor;
+struct sensor_type_traits *stt_tmp;
+int rc;
+
+if (!stt) {
+rc = SYS_EINVAL;
+goto err;
+}
+
+sensor = sensor_get_type_traits_byname(devname, &stt_tmp,
+ stt->stt_sensor_type);
+if (!sensor) {
Review comment:
maybe if (!sensor || !stt_tmp) {}
btw why we care about stt_tmp since this is not used?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
> --
>
> Key: MYNEWT-881
> URL: https://issues.apache.org/jira/browse/MYNEWT-881
> Project: Mynewt
> Issue Type: Improvement
> Security Level: Public(Viewable by anyone)
> Components: Drivers, Sensor Framework
>Affects Versions: v1_3_0_rel
> Environment: ruuvitag-revb2
>Reporter: Vipul Rahane
>Assignee: Vipul Rahane
>Priority: Major
> Labels: core
> Fix For: v1_4_0_rel
>
>
> There are two issues for calling the lis2dh12_sensor_set_trigger_thresh() :
> 1. gpio irq was not being released before doing a re-initialization
> 2. srec was malloced and calling the function multiple times would have
> malloced additional memory without freeing it.
> Solution:
> 1. Release the gpio irq
> 2. Remove the malloc and add a ptr for the sensor in each type trait so that
> a sensor read could be performed in the irq handler.
> Also, sensor_unset_thresh() API was added to unset thresholds
> triggers/interrupts.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (MYNEWT-881) SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
[
https://issues.apache.org/jira/browse/MYNEWT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16365397#comment-16365397
]
ASF GitHub Bot commented on MYNEWT-881:
---
rymanluk commented on a change in pull request #718: MYNEWT-881: SensorAPI: Add
sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
URL: https://github.com/apache/mynewt-core/pull/718#discussion_r168440930
##
File path: hw/drivers/sensors/lis2dh12/src/lis2dh12.c
##
@@ -1463,7 +1485,75 @@ lis2dh12_sensor_set_trigger_thresh(struct sensor
*sensor,
}
}
+err:
+return rc;
+}
+
+
+/**
+ * Set the trigger threshold values and enable interrupts
+ *
+ * @param ptr to sensor
+ * @param the Sensor type
+ * @param Sensor type traits
+ *
+ * @return 0 on success, non-zero on failure
+ */
+static int
+lis2dh12_sensor_set_trigger_thresh(struct sensor *sensor,
+ sensor_type_t type,
+ struct sensor_type_traits *stt)
+{
+int rc;
+uint8_t fs;
+struct sensor_itf *itf;
+sensor_data_t low_thresh;
+sensor_data_t high_thresh;
+
+itf = SENSOR_GET_ITF(sensor);
+
+if (type != SENSOR_TYPE_ACCELEROMETER) {
+rc = SYS_EINVAL;
+goto err;
+}
+
+memcpy(&low_thresh, &stt->stt_low_thresh, sizeof(low_thresh));
+memcpy(&high_thresh, &stt->stt_high_thresh, sizeof(high_thresh));
+
+rc = lis2dh12_get_full_scale(itf, &fs);
+if (rc) {
+goto err;
+}
+
+if (fs == LIS2DH12_FS_2G) {
+fs = 16;
+} else if (fs == LIS2DH12_FS_4G) {
+fs = 32;
+} else if (fs == LIS2DH12_FS_8G) {
+fs = 62;
+} else if (fs == LIS2DH12_FS_16G) {
+fs = 186;
+} else {
+rc = SYS_EINVAL;
+goto err;
+}
+
+/* Set low threshold and enable interrupt */
+rc = lis2dh12_set_low_thresh(itf, low_thresh, fs, stt);
+if (rc) {
+hal_gpio_irq_release(itf->si_low_pin);
+goto err;
+}
+
+/* Set high threshold and enable interrupt */
+rc = lis2dh12_set_high_thresh(itf, high_thresh, fs, stt);
+if (rc) {
+hal_gpio_irq_release(itf->si_high_pin);
Review comment:
Similar comment as above. hal_gpio_irq_release(itf->si_high_pin); could be
done in "err" label in of lis2dh12_set_high_thresh()
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
> --
>
> Key: MYNEWT-881
> URL: https://issues.apache.org/jira/browse/MYNEWT-881
> Project: Mynewt
> Issue Type: Improvement
> Security Level: Public(Viewable by anyone)
> Components: Drivers, Sensor Framework
>Affects Versions: v1_3_0_rel
> Environment: ruuvitag-revb2
>Reporter: Vipul Rahane
>Assignee: Vipul Rahane
>Priority: Major
> Labels: core
> Fix For: v1_4_0_rel
>
>
> There are two issues for calling the lis2dh12_sensor_set_trigger_thresh() :
> 1. gpio irq was not being released before doing a re-initialization
> 2. srec was malloced and calling the function multiple times would have
> malloced additional memory without freeing it.
> Solution:
> 1. Release the gpio irq
> 2. Remove the malloc and add a ptr for the sensor in each type trait so that
> a sensor read could be performed in the irq handler.
> Also, sensor_unset_thresh() API was added to unset thresholds
> triggers/interrupts.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (MYNEWT-881) SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix
[ https://issues.apache.org/jira/browse/MYNEWT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16363404#comment-16363404 ] ASF GitHub Bot commented on MYNEWT-881: --- vrahane commented on issue #718: MYNEWT-881 SensorAPI: LIS2DH12 set thresh cb fix URL: https://github.com/apache/mynewt-core/pull/718#issuecomment-365477855 @rymanluk I have addressed the comments and I do agree with your comments, I was a bit confused about the unset API since there are different ways of taking care of things but what I have done in the above commits should be good enough considering we have to enable/disable specific interrupts in the sensors registers if all the interrupts are not disabled. Hence, hal_gpio_irq_release() can only be called if all the thresholds are not valid. Hope I am making sense. Please go through the PR once again. Thank you. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > SensorAPI: Add sensor_unset_thresh() API and LIS2DH12 set threshold cb fix > -- > > Key: MYNEWT-881 > URL: https://issues.apache.org/jira/browse/MYNEWT-881 > Project: Mynewt > Issue Type: Improvement > Security Level: Public(Viewable by anyone) > Components: Drivers, Sensor Framework >Affects Versions: v1_3_0_rel > Environment: ruuvitag-revb2 >Reporter: Vipul Rahane >Assignee: Vipul Rahane >Priority: Major > Labels: core > Fix For: v1_4_0_rel > > > There are two issues for calling the lis2dh12_sensor_set_trigger_thresh() : > 1. gpio irq was not being released before doing a re-initialization > 2. srec was malloced and calling the function multiple times would have > malloced additional memory without freeing it. > Solution: > 1. Release the gpio irq > 2. Remove the malloc and add a ptr for the sensor in each type trait so that > a sensor read could be performed in the irq handler. > Also, sensor_unset_thresh() API was added to unset thresholds > triggers/interrupts. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
