You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2020/09/15 23:57:08 UTC

[GitHub] [mynewt-core] supervillain101 opened a new pull request #2376: FWS-665 pressure sensor optimization

supervillain101 opened a new pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376


   Added new driver code for BMP388 to limit the amount of i2c traffic needed to get sensor data 


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

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



[GitHub] [mynewt-core] apache-mynewt-bot commented on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-693778497


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3379,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3406,13 +3440,13 @@
    
        /* If the read isn't looking for pressure or temperature data, don't do anything. */
        if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
   -            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
   +        (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
            BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
            return SYS_EINVAL;
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3428,20 +3462,21 @@
                BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
                goto error;
            }
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   +
        if (time_ms != 0) {
   -        if (time_ms > BMP388_MAX_STREAM_MS)
   +        if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   +        }
            rc = os_time_ms_to_ticks(time_ms, &time_ticks);
            if (rc) {
                goto error;
   @@ -3459,7 +3494,7 @@
            delay_msec(2);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        } while (((bmp388->bmp3_dev.status.intr.fifo_wm == 0) &&
   -                  (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
   +              (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
    #else
        } while ((--try_count > 0) && (rc != BMP3_OK));
    #endif
   @@ -3475,11 +3510,11 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK) {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -    
   +
        if (fifo.settings.time_en) {
            fifo.no_need_sensortime = false;
        } else {
   @@ -3487,7 +3522,7 @@
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   +
        if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
   @@ -3498,10 +3533,10 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++) {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc) {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
   @@ -3516,21 +3551,21 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
   -            if(bmp388->bmp3_dev.status.intr.drdy) {
   +            if (bmp388->bmp3_dev.status.intr.drdy) {
                    break;
                }
    #else
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -#endif       
   +#endif
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3556,7 +3591,7 @@
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r491719696



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3367,226 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[MYNEWT_VAL(BMP388_FIFO_CONVERTED_DATA_SIZE)];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
+            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false) {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc) {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 

Review comment:
       The return code is not accounted for. Please do this.
   ```
   3516         if (rc) {
   3517             BMP388_LOG_ERROR("fifo flush failed, err=0x%02x\n", rc);
   3518             goto err;
   3519         }
   ```




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489094121



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {

Review comment:
       { should be on the same line as the if

##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {

Review comment:
       { should be on the same line as the if

##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {

Review comment:
       { should be on the same line as the if




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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489113308



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {

Review comment:
       Done

##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {

Review comment:
       Done

##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {

Review comment:
       Done

##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {

Review comment:
       Done




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

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



[GitHub] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-693778497


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3379,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3406,13 +3440,13 @@
    
        /* If the read isn't looking for pressure or temperature data, don't do anything. */
        if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
   -            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
   +        (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
            BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
            return SYS_EINVAL;
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3428,20 +3462,21 @@
                BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
                goto error;
            }
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   +
        if (time_ms != 0) {
   -        if (time_ms > BMP388_MAX_STREAM_MS)
   +        if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   +        }
            rc = os_time_ms_to_ticks(time_ms, &time_ticks);
            if (rc) {
                goto error;
   @@ -3459,7 +3494,7 @@
            delay_msec(2);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        } while (((bmp388->bmp3_dev.status.intr.fifo_wm == 0) &&
   -                  (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
   +              (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
    #else
        } while ((--try_count > 0) && (rc != BMP3_OK));
    #endif
   @@ -3475,11 +3510,11 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK) {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -    
   +
        if (fifo.settings.time_en) {
            fifo.no_need_sensortime = false;
        } else {
   @@ -3487,7 +3522,7 @@
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   +
        if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
   @@ -3498,10 +3533,10 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++) {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc) {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
   @@ -3516,21 +3551,21 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
   -            if(bmp388->bmp3_dev.status.intr.drdy) {
   +            if (bmp388->bmp3_dev.status.intr.drdy) {
                    break;
                }
    #else
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -#endif       
   +#endif
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3556,7 +3591,7 @@
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] vrahane merged pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane merged pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376


   


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

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



[GitHub] [mynewt-core] apache-mynewt-bot commented on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-695137233


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3380,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3407,13 +3440,13 @@
    
        /* If the read isn't looking for pressure or temperature data, don't do anything. */
        if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
   -            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
   +        (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
            BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
            return SYS_EINVAL;
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3430,7 +3463,7 @@
                goto error;
            }
    
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388_set_fifo_cfg(bmp388, cfg->fifo_mode, cfg->fifo_threshold);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
            bmp388_set_int_enable(bmp388, 1, bmp388->cfg.read_mode.int_type);
   @@ -3438,14 +3471,14 @@
    #endif
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   +
        if (time_ms != 0) {
            if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   @@ -3467,7 +3500,7 @@
            delay_msec(2);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        } while (((bmp388->bmp3_dev.status.intr.fifo_wm == 0) &&
   -                  (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
   +              (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
    
    #else
        } while ((--try_count > 0) && (rc != BMP3_OK));
   @@ -3484,11 +3517,11 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK) {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -   
   +
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        rc = bmp388_clear_int(bmp388);
        if (rc) {
   @@ -3504,7 +3537,7 @@
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   +
        if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
   @@ -3515,10 +3548,10 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++) {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc) {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
   @@ -3533,21 +3566,21 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
   -            if(bmp388->bmp3_dev.status.intr.drdy) {
   +            if (bmp388->bmp3_dev.status.intr.drdy) {
                    break;
                }
    #else
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -#endif       
   +#endif
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3573,7 +3606,7 @@
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489109109



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3416,8 +3625,10 @@ bmp388_sensor_read(struct sensor *sensor, sensor_type_t type,
 
     if (cfg->read_mode.mode == BMP388_READ_M_POLL) {
         rc = bmp388_poll_read(sensor, type, data_func, data_arg, timeout);
-    } else {
+    } else if (cfg->read_mode.mode == BMP388_READ_M_STREAM) {
         rc = bmp388_stream_read(sensor, type, data_func, data_arg, timeout);
+    } else { // hybrid mode

Review comment:
       Done




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

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



[GitHub] [mynewt-core] vrahane commented on pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-693099230


   @supervillain101 Can you please explain what this code exactly does ? The description does not really mention any information about the code.


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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489093804



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {
+        fifo.no_need_sensortime = false;
+    } else {
+        fifo.no_need_sensortime = true;
+    }
+
+    rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
+    
+    if (fifo.data.frame_not_available)
+    {
+        /* No valid frame read */
+        BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
+        goto error;
+    } else {
+#if BMP388_DEBUG
+        BMP388_LOG_ERROR("*****parsed_frames is %d\n", fifo.data.parsed_frames);
+#endif
+        frame_length = fifo.data.parsed_frames;
+
+        for(i = 0; i < frame_length; i++)
+        {
+            rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
+            
+            if(rc)
+            {
+                BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
+                goto error;
+            }
+        }
+
+        if (fifo.sensortime_updated)
+        {

Review comment:
       `{` should be on the same line as the `if`




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

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



[GitHub] [mynewt-core] vrahane merged pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane merged pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376


   


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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489885691



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];

Review comment:
       sensor_time is not part of the FIFO frames, as per the BMP388 Datasheet "The data for the sensor-time frame consists of register sensor_time content at the time the sensortime frame transmission has started. A sensor-time frame is not stored in the FIFO, but append to every FIFO burst read operation after all data has been transmitted if fifo_time_en=‘1’." So, the max number of samples is 512/7 ~= 74 but it is not necessary that both temperature and pressure are enabled at the same time, hence is only one of them is enabled, you will see more samples. Example: If only pressure is enabled you will see max 512/5 ~= 103 samples, whereas if only pressure is enabled you will see max 512/3 ~= 108 samples. So, thats the reason the buffer management needs to be properly handled. 




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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r490448172



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {
+        fifo.no_need_sensortime = false;
+    } else {
+        fifo.no_need_sensortime = true;
+    }
+
+    rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
+    
+    if (fifo.data.frame_not_available)
+    {
+        /* No valid frame read */
+        BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
+        goto error;
+    } else {
+#if BMP388_DEBUG
+        BMP388_LOG_ERROR("*****parsed_frames is %d\n", fifo.data.parsed_frames);
+#endif
+        frame_length = fifo.data.parsed_frames;
+
+        for(i = 0; i < frame_length; i++)
+        {
+            rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
+            
+            if(rc)
+            {

Review comment:
       Done




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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489113544



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {

Review comment:
       done




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

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



[GitHub] [mynewt-core] vrahane removed a comment on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane removed a comment on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-693769673


   @supervillain101 We also spoke about using `fwm_int` out of band which I do not see in your newer changes.


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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489113596



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {

Review comment:
       Done




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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r490448404



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {
+        fifo.no_need_sensortime = false;
+    } else {
+        fifo.no_need_sensortime = true;
+    }
+
+    rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
+    
+    if (fifo.data.frame_not_available)
+    {
+        /* No valid frame read */
+        BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
+        goto error;
+    } else {
+#if BMP388_DEBUG
+        BMP388_LOG_ERROR("*****parsed_frames is %d\n", fifo.data.parsed_frames);
+#endif
+        frame_length = fifo.data.parsed_frames;
+
+        for(i = 0; i < frame_length; i++)
+        {

Review comment:
       Done




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489093868



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {
+        fifo.no_need_sensortime = false;
+    } else {
+        fifo.no_need_sensortime = true;
+    }
+
+    rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
+    
+    if (fifo.data.frame_not_available)
+    {
+        /* No valid frame read */
+        BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
+        goto error;
+    } else {
+#if BMP388_DEBUG
+        BMP388_LOG_ERROR("*****parsed_frames is %d\n", fifo.data.parsed_frames);
+#endif
+        frame_length = fifo.data.parsed_frames;
+
+        for(i = 0; i < frame_length; i++)
+        {
+            rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
+            
+            if(rc)
+            {

Review comment:
       `{` should be on the same line as the `if`




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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r490440129



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,210 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[MYNEWT_VAL(BMP388_FIFO_CONVERTED_DATA_SIZE)];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
+            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false) {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc) {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0) {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;

Review comment:
       Done - fixed the same block in the pre-existing stream_read() as well




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

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



[GitHub] [mynewt-core] vrahane edited a comment on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane edited a comment on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-693769673


   @supervillain101 We also spoke about using `fwm_int` out of band which is I do not see in your newer changes.


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

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



[GitHub] [mynewt-core] apache-mynewt-bot commented on pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-693091375


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3379,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3411,7 +3445,7 @@
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3419,31 +3453,29 @@
            return SYS_EINVAL;
        }
    
   -    if (bmp388->bmp388_cfg_complete == false)
   -    {
   +    if (bmp388->bmp388_cfg_complete == false) {
            /* no interrupt feature */
            /* enable normal mode for fifo feature */
            rc = bmp388_set_normal_mode(bmp388);
   -        if (rc)
   -        {
   +        if (rc) {
                BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
                goto error;
            }
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   -    if (time_ms != 0)
   -    {
   -        if (time_ms > BMP388_MAX_STREAM_MS)
   +
   +    if (time_ms != 0) {
   +        if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   +        }
            rc = os_time_ms_to_ticks(time_ms, &time_ticks);
            if (rc) {
                goto error;
   @@ -3465,8 +3497,7 @@
        } while ((--try_count > 0) && (rc != BMP3_OK));
    
    
   -    if ((rc != BMP3_OK) || (try_count == 0))
   -    {
   +    if ((rc != BMP3_OK) || (try_count == 0)) {
    #if FIFOPARSE_DEBUG
            BMP388_LOG_ERROR("*****status %d\n", rc);
            BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
   @@ -3477,23 +3508,20 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK)
   -    {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -    
   -    if (fifo.settings.time_en)
   -    {
   +
   +    if (fifo.settings.time_en) {
            fifo.no_need_sensortime = false;
        } else {
            fifo.no_need_sensortime = true;
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   -    if (fifo.data.frame_not_available)
   -    {
   +
   +    if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
            goto error;
   @@ -3503,19 +3531,16 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++)
   -        {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc)
   -            {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
            }
    
   -        if (fifo.sensortime_updated)
   -        {
   +        if (fifo.sensortime_updated) {
                BMP388_LOG_ERROR("*****BMP388 SENSOR TIME %d\n", fifo.data.sensor_time);
                fifo.sensortime_updated = false;
            }
   @@ -3524,18 +3549,17 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
   -            
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp))
   -            {
   +
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -            
   +
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3555,13 +3579,13 @@
            }
    
        }
   -#endif // #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
   +#endif /* #if MYNEWT_VAL(BMP388_FIFO_ENABLE) */
    
        if (time_ms != 0 && OS_TIME_TICK_GT(os_time_get(), stop_ticks)) {
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   @@ -3627,7 +3651,7 @@
            rc = bmp388_poll_read(sensor, type, data_func, data_arg, timeout);
        } else if (cfg->read_mode.mode == BMP388_READ_M_STREAM) {
            rc = bmp388_stream_read(sensor, type, data_func, data_arg, timeout);
   -    } else { // hybrid mode
   +    } else { /* hybrid mode */
            rc = bmp388_hybrid_read(sensor, type, data_func, data_arg, timeout);
        }
    err:
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-695136251


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3380,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3407,13 +3440,13 @@
    
        /* If the read isn't looking for pressure or temperature data, don't do anything. */
        if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
   -            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
   +        (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
            BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
            return SYS_EINVAL;
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3430,20 +3463,20 @@
                goto error;
            }
    
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388_set_fifo_cfg(bmp388, cfg->fifo_mode, cfg->fifo_threshold);
            rc = bmp388_set_int_enable(bmp388, 1, bmp388->cfg.read_mode.int_type);
            bmp388_clear_int(bmp388);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   +
        if (time_ms != 0) {
            if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   @@ -3465,7 +3498,7 @@
            delay_msec(2);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        } while (((bmp388->bmp3_dev.status.intr.fifo_wm == 0) &&
   -                  (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
   +              (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
    
    #else
        } while ((--try_count > 0) && (rc != BMP3_OK));
   @@ -3482,11 +3515,11 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK) {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -   
   +
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        rc = bmp388_clear_int(bmp388);
        if (rc) {
   @@ -3502,7 +3535,7 @@
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   +
        if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
   @@ -3513,10 +3546,10 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++) {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc) {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
   @@ -3531,21 +3564,21 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
   -            if(bmp388->bmp3_dev.status.intr.drdy) {
   +            if (bmp388->bmp3_dev.status.intr.drdy) {
                    break;
                }
    #else
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -#endif       
   +#endif
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3571,7 +3604,7 @@
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-695136505


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3380,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3407,13 +3440,13 @@
    
        /* If the read isn't looking for pressure or temperature data, don't do anything. */
        if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
   -            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
   +        (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
            BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
            return SYS_EINVAL;
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3430,20 +3463,20 @@
                goto error;
            }
    
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388_set_fifo_cfg(bmp388, cfg->fifo_mode, cfg->fifo_threshold);
            bmp388_set_int_enable(bmp388, 1, bmp388->cfg.read_mode.int_type);
            bmp388_clear_int(bmp388);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   +
        if (time_ms != 0) {
            if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   @@ -3465,7 +3498,7 @@
            delay_msec(2);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        } while (((bmp388->bmp3_dev.status.intr.fifo_wm == 0) &&
   -                  (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
   +              (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
    
    #else
        } while ((--try_count > 0) && (rc != BMP3_OK));
   @@ -3482,11 +3515,11 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK) {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -   
   +
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        rc = bmp388_clear_int(bmp388);
        if (rc) {
   @@ -3502,7 +3535,7 @@
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   +
        if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
   @@ -3513,10 +3546,10 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++) {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc) {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
   @@ -3531,21 +3564,21 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
   -            if(bmp388->bmp3_dev.status.intr.drdy) {
   +            if (bmp388->bmp3_dev.status.intr.drdy) {
                    break;
                }
    #else
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -#endif       
   +#endif
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3571,7 +3604,7 @@
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r491719800



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3367,226 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[MYNEWT_VAL(BMP388_FIFO_CONVERTED_DATA_SIZE)];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
+            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false) {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc) {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388_set_fifo_cfg(bmp388, cfg->fifo_mode, cfg->fifo_threshold);
+#if MYNEWT_VAL(BMP388_INT_ENABLE)
+        bmp388_set_int_enable(bmp388, 1, bmp388->cfg.read_mode.int_type);
+        bmp388_clear_int(bmp388);

Review comment:
       For all the function calls above return code handling is missing.




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489093930



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {
+        fifo.no_need_sensortime = false;
+    } else {
+        fifo.no_need_sensortime = true;
+    }
+
+    rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
+    
+    if (fifo.data.frame_not_available)
+    {
+        /* No valid frame read */
+        BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
+        goto error;
+    } else {
+#if BMP388_DEBUG
+        BMP388_LOG_ERROR("*****parsed_frames is %d\n", fifo.data.parsed_frames);
+#endif
+        frame_length = fifo.data.parsed_frames;
+
+        for(i = 0; i < frame_length; i++)
+        {

Review comment:
       `{` should be on the same line as the `for`




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

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



[GitHub] [mynewt-core] apache-mynewt-bot commented on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-693737211


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3379,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3406,13 +3440,13 @@
    
        /* If the read isn't looking for pressure or temperature data, don't do anything. */
        if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
   -            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
   +        (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
            BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
            return SYS_EINVAL;
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3428,20 +3462,21 @@
                BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
                goto error;
            }
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   +
        if (time_ms != 0) {
   -        if (time_ms > BMP388_MAX_STREAM_MS)
   +        if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   +        }
            rc = os_time_ms_to_ticks(time_ms, &time_ticks);
            if (rc) {
                goto error;
   @@ -3459,7 +3494,7 @@
            delay_msec(2);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        } while (((bmp388->bmp3_dev.status.intr.fifo_wm == 0) &&
   -                  (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
   +              (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
    #else
        } while ((--try_count > 0) && (rc != BMP3_OK));
    #endif
   @@ -3475,11 +3510,11 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK) {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -    
   +
        if (fifo.settings.time_en) {
            fifo.no_need_sensortime = false;
        } else {
   @@ -3487,7 +3522,7 @@
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   +
        if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
   @@ -3498,10 +3533,10 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++) {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc) {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
   @@ -3516,21 +3551,21 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
   -            if(bmp388->bmp3_dev.status.intr.drdy) {
   +            if (bmp388->bmp3_dev.status.intr.drdy) {
                    break;
                }
    #else
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -#endif       
   +#endif
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3556,7 +3591,7 @@
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r490447998



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {
+        fifo.no_need_sensortime = false;
+    } else {
+        fifo.no_need_sensortime = true;
+    }
+
+    rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
+    
+    if (fifo.data.frame_not_available)
+    {
+        /* No valid frame read */
+        BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
+        goto error;
+    } else {
+#if BMP388_DEBUG
+        BMP388_LOG_ERROR("*****parsed_frames is %d\n", fifo.data.parsed_frames);
+#endif
+        frame_length = fifo.data.parsed_frames;
+
+        for(i = 0; i < frame_length; i++)
+        {
+            rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
+            
+            if(rc)
+            {
+                BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
+                goto error;
+            }
+        }
+
+        if (fifo.sensortime_updated)
+        {

Review comment:
       Done




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489094041



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {

Review comment:
       `{` should be on the same line as the `if`




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

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



[GitHub] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-693737211


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3379,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3406,13 +3440,13 @@
    
        /* If the read isn't looking for pressure or temperature data, don't do anything. */
        if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
   -            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
   +        (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
            BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
            return SYS_EINVAL;
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3428,20 +3462,21 @@
                BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
                goto error;
            }
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   +
        if (time_ms != 0) {
   -        if (time_ms > BMP388_MAX_STREAM_MS)
   +        if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   +        }
            rc = os_time_ms_to_ticks(time_ms, &time_ticks);
            if (rc) {
                goto error;
   @@ -3459,7 +3494,7 @@
            delay_msec(2);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        } while (((bmp388->bmp3_dev.status.intr.fifo_wm == 0) &&
   -                  (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
   +              (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
    #else
        } while ((--try_count > 0) && (rc != BMP3_OK));
    #endif
   @@ -3475,11 +3510,11 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK) {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -    
   +
        if (fifo.settings.time_en) {
            fifo.no_need_sensortime = false;
        } else {
   @@ -3487,7 +3522,7 @@
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   +
        if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
   @@ -3498,10 +3533,10 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++) {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc) {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
   @@ -3516,21 +3551,21 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
   -            if(bmp388->bmp3_dev.status.intr.drdy) {
   +            if (bmp388->bmp3_dev.status.intr.drdy) {
                    break;
                }
    #else
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -#endif       
   +#endif
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3556,7 +3591,7 @@
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489108940



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {
+        fifo.no_need_sensortime = false;
+    } else {
+        fifo.no_need_sensortime = true;
+    }
+
+    rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
+    
+    if (fifo.data.frame_not_available)
+    {
+        /* No valid frame read */
+        BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
+        goto error;
+    } else {
+#if BMP388_DEBUG
+        BMP388_LOG_ERROR("*****parsed_frames is %d\n", fifo.data.parsed_frames);
+#endif
+        frame_length = fifo.data.parsed_frames;
+
+        for(i = 0; i < frame_length; i++)
+        {
+            rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
+            
+            if(rc)
+            {
+                BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
+                goto error;
+            }
+        }
+
+        if (fifo.sensortime_updated)
+        {
+            BMP388_LOG_ERROR("*****BMP388 SENSOR TIME %d\n", fifo.data.sensor_time);
+            fifo.sensortime_updated = false;
+        }
+    }
+
+#else /* FIFO NOT ENABLED */
+    if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
+        /* make data is available */
+        try_count = 5; 
+
+
+        do {
+            rc = bmp3_get_status(&bmp388->bmp3_dev);
+            
+            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
+                    (bmp388->bmp3_dev.status.sensor.drdy_temp))
+            {
+                break;
+            }
+            
+            delay_msec(2);
+#if FIFOPARSE_DEBUG
+            BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+        } while (--try_count > 0);
+
+        rc = bmp388_get_sensor_data(bmp388, &sensor_data);
+        if (rc) {
+            BMP388_LOG_ERROR("bmp388_get_sensor_data failed %d\n", rc);
+            goto error;
+        }
+
+        rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data);
+        if (rc) {
+            BMP388_LOG_ERROR("bmp388_do_report failed %d\n", rc);
+            goto error;
+        }
+
+    }
+#endif // #if MYNEWT_VAL(BMP388_FIFO_ENABLE)

Review comment:
       Done




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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489832726



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];

Review comment:
       As discussed out of band, this is an array of bmp3_data structs.  I did move the magic number to a sysconfig, but I left it as 74.  The DS indicates the maximum frame length is 9 bytes. A data frame is 7 bytes (4 pressure, 2 temp, 1 header) and the sensor_time frame is 4 (3 time, 1 header).  I don't have insight into the historical reason for using 74 as a magic number, but if we did I suspect the correct number is 85 as that would be 510 bytes.




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

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



[GitHub] [mynewt-core] apache-mynewt-bot commented on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-695136505


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3380,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3407,13 +3440,13 @@
    
        /* If the read isn't looking for pressure or temperature data, don't do anything. */
        if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
   -            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
   +        (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
            BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
            return SYS_EINVAL;
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3430,20 +3463,20 @@
                goto error;
            }
    
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388_set_fifo_cfg(bmp388, cfg->fifo_mode, cfg->fifo_threshold);
            bmp388_set_int_enable(bmp388, 1, bmp388->cfg.read_mode.int_type);
            bmp388_clear_int(bmp388);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   +
        if (time_ms != 0) {
            if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   @@ -3465,7 +3498,7 @@
            delay_msec(2);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        } while (((bmp388->bmp3_dev.status.intr.fifo_wm == 0) &&
   -                  (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
   +              (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
    
    #else
        } while ((--try_count > 0) && (rc != BMP3_OK));
   @@ -3482,11 +3515,11 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK) {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -   
   +
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        rc = bmp388_clear_int(bmp388);
        if (rc) {
   @@ -3502,7 +3535,7 @@
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   +
        if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
   @@ -3513,10 +3546,10 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++) {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc) {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
   @@ -3531,21 +3564,21 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
   -            if(bmp388->bmp3_dev.status.intr.drdy) {
   +            if (bmp388->bmp3_dev.status.intr.drdy) {
                    break;
                }
    #else
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -#endif       
   +#endif
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3571,7 +3604,7 @@
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r490448976



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];

Review comment:
       Per out of band discussion this will stay as is to conform to the other code




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

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



[GitHub] [mynewt-core] apache-mynewt-bot commented on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-694417002


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3380,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3407,13 +3440,13 @@
    
        /* If the read isn't looking for pressure or temperature data, don't do anything. */
        if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
   -            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
   +        (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
            BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
            return SYS_EINVAL;
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3429,17 +3462,17 @@
                BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
                goto error;
            }
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   +
        if (time_ms != 0) {
            if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   @@ -3461,7 +3494,7 @@
            delay_msec(2);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        } while (((bmp388->bmp3_dev.status.intr.fifo_wm == 0) &&
   -                  (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
   +              (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
    #else
        } while ((--try_count > 0) && (rc != BMP3_OK));
    #endif
   @@ -3477,11 +3510,11 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK) {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -    
   +
        if (fifo.settings.time_en) {
            fifo.no_need_sensortime = false;
        } else {
   @@ -3489,7 +3522,7 @@
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   +
        if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
   @@ -3500,10 +3533,10 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++) {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc) {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
   @@ -3518,21 +3551,21 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
   -            if(bmp388->bmp3_dev.status.intr.drdy) {
   +            if (bmp388->bmp3_dev.status.intr.drdy) {
                    break;
                }
    #else
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -#endif       
   +#endif
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3558,7 +3591,7 @@
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489833630



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);

Review comment:
       Originally this was done this way to make sure the compiler didn't complain about ignoring return values, but I changed it as it is probably better




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489094301



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {

Review comment:
       I think this line exceeds the 79 character line limit, please wrap it




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

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



[GitHub] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-694417002


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3380,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3407,13 +3440,13 @@
    
        /* If the read isn't looking for pressure or temperature data, don't do anything. */
        if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
   -            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
   +        (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
            BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
            return SYS_EINVAL;
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3429,17 +3462,17 @@
                BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
                goto error;
            }
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   +
        if (time_ms != 0) {
            if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   @@ -3461,7 +3494,7 @@
            delay_msec(2);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        } while (((bmp388->bmp3_dev.status.intr.fifo_wm == 0) &&
   -                  (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
   +              (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
    #else
        } while ((--try_count > 0) && (rc != BMP3_OK));
    #endif
   @@ -3477,11 +3510,11 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK) {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -    
   +
        if (fifo.settings.time_en) {
            fifo.no_need_sensortime = false;
        } else {
   @@ -3489,7 +3522,7 @@
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   +
        if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
   @@ -3500,10 +3533,10 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++) {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc) {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
   @@ -3518,21 +3551,21 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
   -            if(bmp388->bmp3_dev.status.intr.drdy) {
   +            if (bmp388->bmp3_dev.status.intr.drdy) {
                    break;
                }
    #else
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -#endif       
   +#endif
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3558,7 +3591,7 @@
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489100645



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];

Review comment:
       Also, there is possibility of a buffer overrun in this case mainly because the FIFO is of 512 bytes. If you keep reading it and the entire FIFO is filled up, there is going to be a FIFO overrun. Please try to limit your reads based on the number of data samples in the buffer as well.




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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r490448583



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {
+        fifo.no_need_sensortime = false;
+    } else {
+        fifo.no_need_sensortime = true;
+    }
+
+    rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
+    
+    if (fifo.data.frame_not_available)
+    {

Review comment:
       Done




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

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



[GitHub] [mynewt-core] vrahane commented on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-693769673


   @supervillain101 We also spoke about using `fwm_init` out of band which is I do not see in your newer changes.


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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489094086



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {

Review comment:
       { should be on the same line as the if




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

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



[GitHub] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-695100380


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3380,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3407,13 +3440,13 @@
    
        /* If the read isn't looking for pressure or temperature data, don't do anything. */
        if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
   -            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
   +        (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
            BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
            return SYS_EINVAL;
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3429,18 +3462,18 @@
                BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
                goto error;
            }
   -        
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   +
        if (time_ms != 0) {
            if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   @@ -3462,7 +3495,7 @@
            delay_msec(2);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        } while (((bmp388->bmp3_dev.status.intr.fifo_wm == 0) &&
   -                  (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
   +              (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
    
    #else
        } while ((--try_count > 0) && (rc != BMP3_OK));
   @@ -3479,11 +3512,11 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK) {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -   
   +
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        rc = bmp388_clear_int(bmp388);
        if (rc) {
   @@ -3499,7 +3532,7 @@
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   +
        if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
   @@ -3510,10 +3543,10 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++) {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc) {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
   @@ -3528,21 +3561,21 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
   -            if(bmp388->bmp3_dev.status.intr.drdy) {
   +            if (bmp388->bmp3_dev.status.intr.drdy) {
                    break;
                }
    #else
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -#endif       
   +#endif
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3568,7 +3601,7 @@
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r490437646



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,210 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[MYNEWT_VAL(BMP388_FIFO_CONVERTED_DATA_SIZE)];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
+            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false) {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc) {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0) {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;

Review comment:
       As per the coding style this should be wrapped in `{` and `}`.




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489885691



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];

Review comment:
       sensor_time is not part of the FIFO frames, as per the BMP388 Datasheet "The data for the sensor-time frame consists of register sensor_time content at the time the sensortime frame transmission has started. A sensor-time frame is not stored in the FIFO, but append to every FIFO burst read operation after all data has been transmitted if fifo_time_en=‘1’."




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489885691



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];

Review comment:
       sensor_time is not part of the FIFO frames, as per the BMP388 Datasheet "The data for the sensor-time frame consists of register sensor_time content at the time the sensortime frame transmission has started. A sensor-time frame is not stored in the FIFO, but append to every FIFO burst read operation after all data has been transmitted if fifo_time_en=‘1’." So, the max number of samples is 512/7 ~= 74.




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489094801



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];

Review comment:
       This frame size should be a syscfg to make it configurable at build time.




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489093025



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3416,8 +3625,10 @@ bmp388_sensor_read(struct sensor *sensor, sensor_type_t type,
 
     if (cfg->read_mode.mode == BMP388_READ_M_POLL) {
         rc = bmp388_poll_read(sensor, type, data_func, data_arg, timeout);
-    } else {
+    } else if (cfg->read_mode.mode == BMP388_READ_M_STREAM) {
         rc = bmp388_stream_read(sensor, type, data_func, data_arg, timeout);
+    } else { // hybrid mode

Review comment:
       Please convert this C++ style comment to C style comment




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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r490446952



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {
+        fifo.no_need_sensortime = false;
+    } else {
+        fifo.no_need_sensortime = true;
+    }
+
+    rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
+    
+    if (fifo.data.frame_not_available)
+    {
+        /* No valid frame read */
+        BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
+        goto error;
+    } else {
+#if BMP388_DEBUG
+        BMP388_LOG_ERROR("*****parsed_frames is %d\n", fifo.data.parsed_frames);
+#endif
+        frame_length = fifo.data.parsed_frames;
+
+        for(i = 0; i < frame_length; i++)
+        {
+            rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
+            
+            if(rc)
+            {
+                BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
+                goto error;
+            }
+        }
+
+        if (fifo.sensortime_updated)
+        {
+            BMP388_LOG_ERROR("*****BMP388 SENSOR TIME %d\n", fifo.data.sensor_time);
+            fifo.sensortime_updated = false;
+        }
+    }
+
+#else /* FIFO NOT ENABLED */
+    if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
+        /* make data is available */
+        try_count = 5; 
+
+
+        do {
+            rc = bmp3_get_status(&bmp388->bmp3_dev);
+            
+            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
+                    (bmp388->bmp3_dev.status.sensor.drdy_temp))
+            {

Review comment:
       Done




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489096564



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);

Review comment:
       this should be `rc |= ` if you really don't want to check for errors twice.




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

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



[GitHub] [mynewt-core] apache-mynewt-bot commented on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-695136251


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3380,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3407,13 +3440,13 @@
    
        /* If the read isn't looking for pressure or temperature data, don't do anything. */
        if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
   -            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
   +        (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
            BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
            return SYS_EINVAL;
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3430,20 +3463,20 @@
                goto error;
            }
    
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388_set_fifo_cfg(bmp388, cfg->fifo_mode, cfg->fifo_threshold);
            rc = bmp388_set_int_enable(bmp388, 1, bmp388->cfg.read_mode.int_type);
            bmp388_clear_int(bmp388);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   +
        if (time_ms != 0) {
            if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   @@ -3465,7 +3498,7 @@
            delay_msec(2);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        } while (((bmp388->bmp3_dev.status.intr.fifo_wm == 0) &&
   -                  (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
   +              (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
    
    #else
        } while ((--try_count > 0) && (rc != BMP3_OK));
   @@ -3482,11 +3515,11 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK) {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -   
   +
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        rc = bmp388_clear_int(bmp388);
        if (rc) {
   @@ -3502,7 +3535,7 @@
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   +
        if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
   @@ -3513,10 +3546,10 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++) {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc) {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
   @@ -3531,21 +3564,21 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
   -            if(bmp388->bmp3_dev.status.intr.drdy) {
   +            if (bmp388->bmp3_dev.status.intr.drdy) {
                    break;
                }
    #else
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -#endif       
   +#endif
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3571,7 +3604,7 @@
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489092518



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {
+        fifo.no_need_sensortime = false;
+    } else {
+        fifo.no_need_sensortime = true;
+    }
+
+    rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
+    
+    if (fifo.data.frame_not_available)
+    {
+        /* No valid frame read */
+        BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
+        goto error;
+    } else {
+#if BMP388_DEBUG
+        BMP388_LOG_ERROR("*****parsed_frames is %d\n", fifo.data.parsed_frames);
+#endif
+        frame_length = fifo.data.parsed_frames;
+
+        for(i = 0; i < frame_length; i++)
+        {
+            rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
+            
+            if(rc)
+            {
+                BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
+                goto error;
+            }
+        }
+
+        if (fifo.sensortime_updated)
+        {
+            BMP388_LOG_ERROR("*****BMP388 SENSOR TIME %d\n", fifo.data.sensor_time);
+            fifo.sensortime_updated = false;
+        }
+    }
+
+#else /* FIFO NOT ENABLED */
+    if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
+        /* make data is available */
+        try_count = 5; 
+
+
+        do {
+            rc = bmp3_get_status(&bmp388->bmp3_dev);
+            
+            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
+                    (bmp388->bmp3_dev.status.sensor.drdy_temp))
+            {
+                break;
+            }
+            
+            delay_msec(2);
+#if FIFOPARSE_DEBUG
+            BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+        } while (--try_count > 0);
+
+        rc = bmp388_get_sensor_data(bmp388, &sensor_data);
+        if (rc) {
+            BMP388_LOG_ERROR("bmp388_get_sensor_data failed %d\n", rc);
+            goto error;
+        }
+
+        rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data);
+        if (rc) {
+            BMP388_LOG_ERROR("bmp388_do_report failed %d\n", rc);
+            goto error;
+        }
+
+    }
+#endif // #if MYNEWT_VAL(BMP388_FIFO_ENABLE)

Review comment:
       Please convert the C++ stye comment to C style comment




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r491719696



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3367,226 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[MYNEWT_VAL(BMP388_FIFO_CONVERTED_DATA_SIZE)];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
+            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false) {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc) {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 

Review comment:
       The return code is not accounted for. Please do this.
   ```
   3516         if (rc) {
   3517             BMP388_LOG_ERROR("fifo flush failed, err=0x%02x\n", rc);
   3518             goto err;
   3519         }
   ```

##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3367,226 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[MYNEWT_VAL(BMP388_FIFO_CONVERTED_DATA_SIZE)];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
+            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false) {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc) {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388_set_fifo_cfg(bmp388, cfg->fifo_mode, cfg->fifo_threshold);
+#if MYNEWT_VAL(BMP388_INT_ENABLE)
+        bmp388_set_int_enable(bmp388, 1, bmp388->cfg.read_mode.int_type);
+        bmp388_clear_int(bmp388);

Review comment:
       For all the function calls above return code handling is missing.




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489093983



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {
+        fifo.no_need_sensortime = false;
+    } else {
+        fifo.no_need_sensortime = true;
+    }
+
+    rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
+    
+    if (fifo.data.frame_not_available)
+    {

Review comment:
       `{` should be on the same line as the `if`




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

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



[GitHub] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-693091375


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3379,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3411,7 +3445,7 @@
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3419,31 +3453,29 @@
            return SYS_EINVAL;
        }
    
   -    if (bmp388->bmp388_cfg_complete == false)
   -    {
   +    if (bmp388->bmp388_cfg_complete == false) {
            /* no interrupt feature */
            /* enable normal mode for fifo feature */
            rc = bmp388_set_normal_mode(bmp388);
   -        if (rc)
   -        {
   +        if (rc) {
                BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
                goto error;
            }
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   -    if (time_ms != 0)
   -    {
   -        if (time_ms > BMP388_MAX_STREAM_MS)
   +
   +    if (time_ms != 0) {
   +        if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   +        }
            rc = os_time_ms_to_ticks(time_ms, &time_ticks);
            if (rc) {
                goto error;
   @@ -3465,8 +3497,7 @@
        } while ((--try_count > 0) && (rc != BMP3_OK));
    
    
   -    if ((rc != BMP3_OK) || (try_count == 0))
   -    {
   +    if ((rc != BMP3_OK) || (try_count == 0)) {
    #if FIFOPARSE_DEBUG
            BMP388_LOG_ERROR("*****status %d\n", rc);
            BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
   @@ -3477,23 +3508,20 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK)
   -    {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -    
   -    if (fifo.settings.time_en)
   -    {
   +
   +    if (fifo.settings.time_en) {
            fifo.no_need_sensortime = false;
        } else {
            fifo.no_need_sensortime = true;
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   -    if (fifo.data.frame_not_available)
   -    {
   +
   +    if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
            goto error;
   @@ -3503,19 +3531,16 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++)
   -        {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc)
   -            {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
            }
    
   -        if (fifo.sensortime_updated)
   -        {
   +        if (fifo.sensortime_updated) {
                BMP388_LOG_ERROR("*****BMP388 SENSOR TIME %d\n", fifo.data.sensor_time);
                fifo.sensortime_updated = false;
            }
   @@ -3524,18 +3549,17 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
   -            
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp))
   -            {
   +
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -            
   +
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3555,13 +3579,13 @@
            }
    
        }
   -#endif // #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
   +#endif /* #if MYNEWT_VAL(BMP388_FIFO_ENABLE) */
    
        if (time_ms != 0 && OS_TIME_TICK_GT(os_time_get(), stop_ticks)) {
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   @@ -3627,7 +3651,7 @@
            rc = bmp388_poll_read(sensor, type, data_func, data_arg, timeout);
        } else if (cfg->read_mode.mode == BMP388_READ_M_STREAM) {
            rc = bmp388_stream_read(sensor, type, data_func, data_arg, timeout);
   -    } else { // hybrid mode
   +    } else { /* hybrid mode */
            rc = bmp388_hybrid_read(sensor, type, data_func, data_arg, timeout);
        }
    err:
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489093703



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) && (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false)
+    {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc)
+        {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0)
+    {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;
+        rc = os_time_ms_to_ticks(time_ms, &time_ticks);
+        if (rc) {
+            goto error;
+        }
+        stop_ticks = os_time_get() + time_ticks;
+    }
+
+
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    try_count = 0xFFFF;
+
+    do {
+        rc = bmp3_get_status(&bmp388->bmp3_dev);
+        rc = bmp3_get_fifo_length(&current_fifo_len, &bmp388->bmp3_dev);
+        delay_msec(2);
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+#endif
+    } while ((--try_count > 0) && (rc != BMP3_OK));
+
+
+    if ((rc != BMP3_OK) || (try_count == 0))
+    {
+#if FIFOPARSE_DEBUG
+        BMP388_LOG_ERROR("*****status %d\n", rc);
+        BMP388_LOG_ERROR("*****try_count is %d\n", try_count);
+        BMP388_LOG_ERROR("*****fifo length is %d\n", current_fifo_len);
+#endif
+        BMP388_LOG_ERROR("*****BMP388 STATUS READ FAILED\n");
+        goto error;
+    }
+
+    rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
+    if(rc != BMP3_OK)
+    {
+        BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
+        goto error;
+    }
+    
+    if (fifo.settings.time_en)
+    {
+        fifo.no_need_sensortime = false;
+    } else {
+        fifo.no_need_sensortime = true;
+    }
+
+    rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
+    
+    if (fifo.data.frame_not_available)
+    {
+        /* No valid frame read */
+        BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
+        goto error;
+    } else {
+#if BMP388_DEBUG
+        BMP388_LOG_ERROR("*****parsed_frames is %d\n", fifo.data.parsed_frames);
+#endif
+        frame_length = fifo.data.parsed_frames;
+
+        for(i = 0; i < frame_length; i++)
+        {
+            rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
+            
+            if(rc)
+            {
+                BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
+                goto error;
+            }
+        }
+
+        if (fifo.sensortime_updated)
+        {
+            BMP388_LOG_ERROR("*****BMP388 SENSOR TIME %d\n", fifo.data.sensor_time);
+            fifo.sensortime_updated = false;
+        }
+    }
+
+#else /* FIFO NOT ENABLED */
+    if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
+        /* make data is available */
+        try_count = 5; 
+
+
+        do {
+            rc = bmp3_get_status(&bmp388->bmp3_dev);
+            
+            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
+                    (bmp388->bmp3_dev.status.sensor.drdy_temp))
+            {

Review comment:
       `{` should be on the same line as the `if`




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: FWS-665 pressure sensor optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489094801



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];

Review comment:
       This frame size should be a syscfg to make it configurable at build time, also it seems we would be better of putting this data buffer in the drivers structure rather than here, it can be 296 bytes.




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

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



[GitHub] [mynewt-core] apache-mynewt-bot commented on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-695100380


   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### hw/drivers/sensors/bmp388/src/bmp388.c
   <details>
   
   ```diff
   @@ -3380,7 +3413,7 @@
        os_time_t time_ticks;
        os_time_t stop_ticks = 0;
        uint16_t try_count;
   -    
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        /* FIFO object to be assigned to device structure */
        struct bmp3_fifo fifo;
   @@ -3407,13 +3440,13 @@
    
        /* If the read isn't looking for pressure or temperature data, don't do anything. */
        if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
   -            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
   +        (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
            BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
            return SYS_EINVAL;
        }
    
        bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
   -    
   +
        cfg = &bmp388->cfg;
    
        if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
   @@ -3429,18 +3462,18 @@
                BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
                goto error;
            }
   -        
   -        bmp3_fifo_flush(&bmp388->bmp3_dev); 
   +
   +        bmp3_fifo_flush(&bmp388->bmp3_dev);
            bmp388->bmp388_cfg_complete = true;
        }
   -     
   -    
   +
   +
    #if MYNEWT_VAL(BMP388_FIFO_ENABLE)
        bmp388->bmp3_dev.fifo = &fifo;
    
        fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
    #endif
   -    
   +
        if (time_ms != 0) {
            if (time_ms > BMP388_MAX_STREAM_MS) {
                time_ms = BMP388_MAX_STREAM_MS;
   @@ -3462,7 +3495,7 @@
            delay_msec(2);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        } while (((bmp388->bmp3_dev.status.intr.fifo_wm == 0) &&
   -                  (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
   +              (bmp388->bmp3_dev.status.intr.fifo_full == 0)) && (try_count > 0));
    
    #else
        } while ((--try_count > 0) && (rc != BMP3_OK));
   @@ -3479,11 +3512,11 @@
        }
    
        rc = bmp3_get_fifo_data(&bmp388->bmp3_dev);
   -    if(rc != BMP3_OK) {
   +    if (rc != BMP3_OK) {
            BMP388_LOG_ERROR("*****BMP388 FIFO READ FAILED\n");
            goto error;
        }
   -   
   +
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
        rc = bmp388_clear_int(bmp388);
        if (rc) {
   @@ -3499,7 +3532,7 @@
        }
    
        rc = bmp3_extract_fifo_data(sensor_data, &bmp388->bmp3_dev);
   -    
   +
        if (fifo.data.frame_not_available) {
            /* No valid frame read */
            BMP388_LOG_ERROR("*****No valid Fifo Frames %d\n", rc);
   @@ -3510,10 +3543,10 @@
    #endif
            frame_length = fifo.data.parsed_frames;
    
   -        for(i = 0; i < frame_length; i++) {
   +        for (i = 0; i < frame_length; i++) {
                rc = bmp388_do_report(sensor, sensor_type, read_func, read_arg, &sensor_data[i]);
   -            
   -            if(rc) {
   +
   +            if (rc) {
                    BMP388_LOG_ERROR("*****BMP388_DO_REPORT FAILED %d\n", rc);
                    goto error;
                }
   @@ -3528,21 +3561,21 @@
    #else /* FIFO NOT ENABLED */
        if (bmp388->cfg.fifo_mode == BMP388_FIFO_M_BYPASS) {
            /* make data is available */
   -        try_count = 5; 
   +        try_count = 5;
    
    
            do {
                rc = bmp3_get_status(&bmp388->bmp3_dev);
    #if MYNEWT_VAL(BMP388_INT_ENABLE)
   -            if(bmp388->bmp3_dev.status.intr.drdy) {
   +            if (bmp388->bmp3_dev.status.intr.drdy) {
                    break;
                }
    #else
   -            if((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   -                    (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
   +            if ((bmp388->bmp3_dev.status.sensor.drdy_press) &&
   +                (bmp388->bmp3_dev.status.sensor.drdy_temp)) {
                    break;
                }
   -#endif       
   +#endif
                delay_msec(2);
    #if FIFOPARSE_DEBUG
                BMP388_LOG_ERROR("*****status %d\n", rc);
   @@ -3568,7 +3601,7 @@
            BMP388_LOG_INFO("stream time expired\n");
            BMP388_LOG_INFO("you can make BMP388_MAX_STREAM_MS bigger to extend stream time duration\n");
            goto error;
   -    } 
   +    }
    
        return rc;
    
   ```
   
   </details>


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

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



[GitHub] [mynewt-core] vrahane edited a comment on pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane edited a comment on pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#issuecomment-693769673


   @supervillain101 We also spoke about using `fwm_int` out of band which I do not see in your newer changes.


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

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



[GitHub] [mynewt-core] supervillain101 commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
supervillain101 commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r490448976



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];

Review comment:
       Per out of band discussion this will stay as is.




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r489100645



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,215 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[74];

Review comment:
       Also, there is possibility of a buffer overrun in this case mainly because the FIFO is of 512 bytes. If you keep reading it and the entire FIFO is filled up, there is going to be a buffer overrun. Please try to limit your reads based on the number of data samples in the buffer as well.




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

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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2376: BMP388 Pressure/temperature sensor i2c communtication optimization

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2376:
URL: https://github.com/apache/mynewt-core/pull/2376#discussion_r490444557



##########
File path: hw/drivers/sensors/bmp388/src/bmp388.c
##########
@@ -3366,6 +3366,210 @@ bmp388_stream_read(struct sensor *sensor,
     return rc;
 }
 
+int
+bmp388_hybrid_read(struct sensor *sensor,
+                   sensor_type_t sensor_type,
+                   sensor_data_func_t read_func,
+                   void *read_arg,
+                   uint32_t time_ms)
+{
+    int rc;
+    struct bmp388 *bmp388;
+    struct bmp388_cfg *cfg;
+    os_time_t time_ticks;
+    os_time_t stop_ticks = 0;
+    uint16_t try_count;
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    /* FIFO object to be assigned to device structure */
+    struct bmp3_fifo fifo;
+    /* Pressure and temperature array of structures with maximum frame size */
+    struct bmp3_data sensor_data[MYNEWT_VAL(BMP388_FIFO_CONVERTED_DATA_SIZE)];
+    /* Loop Variable */
+    uint8_t i;
+    uint16_t frame_length;
+    /* Enable fifo */
+    fifo.settings.mode = BMP3_ENABLE;
+    /* Enable Pressure sensor for fifo */
+    fifo.settings.press_en = BMP3_ENABLE;
+    /* Enable temperature sensor for fifo */
+    fifo.settings.temp_en = BMP3_ENABLE;
+    /* Enable fifo time */
+    fifo.settings.time_en = BMP3_ENABLE;
+    /* No subsampling for FIFO */
+    fifo.settings.down_sampling = BMP3_FIFO_NO_SUBSAMPLING;
+    fifo.sensortime_updated = false;
+    uint16_t current_fifo_len;
+#else
+    struct bmp3_data sensor_data;
+#endif
+
+    /* If the read isn't looking for pressure or temperature data, don't do anything. */
+    if ((!(sensor_type & SENSOR_TYPE_PRESSURE)) &&
+            (!(sensor_type & SENSOR_TYPE_TEMPERATURE))) {
+        BMP388_LOG_ERROR("unsupported sensor type for bmp388\n");
+        return SYS_EINVAL;
+    }
+
+    bmp388 = (struct bmp388 *)SENSOR_GET_DEVICE(sensor);
+    
+    cfg = &bmp388->cfg;
+
+    if (cfg->read_mode.mode != BMP388_READ_M_HYBRID) {
+        BMP388_LOG_ERROR("*****bmp388_hybrid_read mode is not hybrid\n");
+        return SYS_EINVAL;
+    }
+
+    if (bmp388->bmp388_cfg_complete == false) {
+        /* no interrupt feature */
+        /* enable normal mode for fifo feature */
+        rc = bmp388_set_normal_mode(bmp388);
+        if (rc) {
+            BMP388_LOG_ERROR("******bmp388_set_normal_mode failed %d\n", rc);
+            goto error;
+        }
+        bmp3_fifo_flush(&bmp388->bmp3_dev); 
+        bmp388->bmp388_cfg_complete = true;
+    }
+     
+    
+#if MYNEWT_VAL(BMP388_FIFO_ENABLE)
+    bmp388->bmp3_dev.fifo = &fifo;
+
+    fifo.data.req_frames = bmp388->bmp3_dev.fifo_watermark_level;
+#endif
+    
+    if (time_ms != 0) {
+        if (time_ms > BMP388_MAX_STREAM_MS)
+            time_ms = BMP388_MAX_STREAM_MS;

Review comment:
       thanks 




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

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