[GitHub] [mynewt-core] kasjer commented on a change in pull request #2639: hw/bsp/dwm1001-dev Add LIS2DH12 accelerometer support

[GitBox <gi...@apache.org>]

kasjer commented on a change in pull request #2639:
URL: https://github.com/apache/mynewt-core/pull/2639#discussion_r676875853



##########
File path: hw/bsp/dwm1001-dev/src/hal_bsp.c
##########
@@ -93,6 +118,47 @@ hal_bsp_get_nvic_priority(int irq_num, uint32_t pri)
     return cfg_pri;
 }
 
+int
+config_lis2dh12_sensor(void)
+{
+#if MYNEWT_VAL(LIS2DH12_ONB)
+    int rc;
+    struct os_dev *dev;
+    struct lis2dh12_cfg cfg;
+
+    dev = (struct os_dev *) os_dev_open("lis2dh12_0", OS_TIMEOUT_NEVER, NULL);
+    assert(dev != NULL);
+

Review comment:
       extra empty line

##########
File path: hw/bsp/dwm1001-dev/src/hal_bsp.c
##########
@@ -93,6 +118,47 @@ hal_bsp_get_nvic_priority(int irq_num, uint32_t pri)
     return cfg_pri;
 }
 
+int
+config_lis2dh12_sensor(void)
+{
+#if MYNEWT_VAL(LIS2DH12_ONB)
+    int rc;
+    struct os_dev *dev;
+    struct lis2dh12_cfg cfg;
+
+    dev = (struct os_dev *) os_dev_open("lis2dh12_0", OS_TIMEOUT_NEVER, NULL);
+    assert(dev != NULL);
+
+
+    memset(&cfg, 0, sizeof(cfg));
+
+    cfg.lc_s_mask = SENSOR_TYPE_ACCELEROMETER;
+    cfg.lc_rate = LIS2DH12_DATA_RATE_HN_1344HZ_L_5376HZ;
+    cfg.lc_fs = LIS2DH12_FS_2G;
+    cfg.lc_pull_up_disc = 1;
+
+    rc = lis2dh12_config((struct lis2dh12 *)dev, &cfg);
+    SYSINIT_PANIC_ASSERT(rc == 0);
+
+    os_dev_close(dev);
+#endif
+    return 0;
+}
+
+static void
+sensor_dev_create(void)
+{
+    int rc;
+    (void)rc;
+
+#if MYNEWT_VAL(LIS2DH12_ONB)
+    rc = os_dev_create((struct os_dev *) &lis2dh12, "lis2dh12_0",

Review comment:
       extra space after cast expression

##########
File path: hw/bsp/dwm1001-dev/pkg.yml
##########
@@ -41,3 +41,9 @@ pkg.deps.SOFT_PWM:
 
 pkg.deps.UARTBB_0:
     - "@apache-mynewt-core/hw/drivers/uart/uart_bitbang"
+
+pkg.deps.LIS2DH12_ONB:
+    - "@apache-mynewt-core/hw/drivers/sensors/lis2dh12"
+    
+pkg.init:

Review comment:
       if it was changed to:
   ```yml
   pkg.init.LIS2DH12_ONB:
   ```
   whole config_lis2dh12_sensor()  function could be ifdef'ed-out not just its body

##########
File path: hw/bsp/dwm1001-dev/syscfg.yml
##########
@@ -62,3 +71,6 @@ syscfg.vals.BLE_CONTROLLER:
     OS_CPUTIME_FREQ: 32768
     OS_CPUTIME_TIMER_NUM: 5
     BLE_LL_RFMGMT_ENABLE_TIME: 1500
+
+syscfg.restrictions:
+    - "LIS2DH12_ONB && I2C_0_PIN_SCL == 28 && I2C_0_PIN_SDA == 29"

Review comment:
       if it was:
   ```yml
   syscfg.restrictions:
       - "!LIS2DH12_ONB || (I2C_0 == 1 && I2C_0_PIN_SCL == 28 && I2C_0_PIN_SDA == 29)"
   ```
   sensor could be turned on and off as syscfg value implies it could.
   notice extra `I2C_0 == 1` that is also needed I guess

##########
File path: hw/bsp/dwm1001-dev/src/hal_bsp.c
##########
@@ -93,6 +118,47 @@ hal_bsp_get_nvic_priority(int irq_num, uint32_t pri)
     return cfg_pri;
 }
 
+int
+config_lis2dh12_sensor(void)
+{
+#if MYNEWT_VAL(LIS2DH12_ONB)
+    int rc;
+    struct os_dev *dev;
+    struct lis2dh12_cfg cfg;
+
+    dev = (struct os_dev *) os_dev_open("lis2dh12_0", OS_TIMEOUT_NEVER, NULL);

Review comment:
       in mynewt code it is custom not to put space after finishing ) in casts like this
   ```c
   dev = (struct os_dev *)os_dev_open(....
   ```

##########
File path: hw/bsp/dwm1001-dev/src/hal_bsp.c
##########
@@ -33,6 +33,31 @@
 #include "mcu/nrf52_periph.h"
 #include "bsp/bsp.h"
 
+#if MYNEWT_VAL(LIS2DH12_ONB)
+#include "lis2dh12/lis2dh12.h"
+static struct lis2dh12 lis2dh12;
+#endif
+
+#if MYNEWT_VAL(LIS2DH12_ONB)
+static struct sensor_itf i2c_0_itf_lis = {
+    .si_type = SENSOR_ITF_I2C,
+    .si_num  = 0,
+    .si_addr = 0x19,
+    .si_ints = {
+        { /* TODO: determine correct values */
+            .host_pin = 25,
+            .device_pin = 0,
+            .active = true
+        },
+        { /* TODO: determine correct values */
+            .host_pin = 25,
+            .device_pin = 0,
+            .active = true
+        }

Review comment:
       I would remove this second table entry, first one seems to be correct




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org