[GitHub] [mynewt-core] kasjer commented on issue #1930: Fix driver for BMP388 sensor

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

kasjer commented on issue #1930: Fix driver for BMP388 sensor
URL: https://github.com/apache/mynewt-core/pull/1930#issuecomment-516990103
 
 
   > I am a bit concerned about you removing the lock from the int_status structure. Also, the driver calls should be using the sensor_itf and not the os_dev or the driver as the primary argument. Reason for that is to make the calls abstract-able. If you look at the other drivers, we have done that in the past and it works well for the app.
   > Rest looks good.
   
   Don't be concern about lock, the way it was used was wrong across all sensors. Please take a look at non-sensor usage of critical sections. The way it was used did not have any negative impact simply because it was not used recursively as critical section could be.
   
   I totally disagree with second opinion, I don't see any reason to use sensor_itf for anything apart calls to non bus driver version of i2c and spi functions. Other functions are strictly connected to bmp388 struct and I don't see a point of passing mostly useless pointer that has to be somehow up-cast to get access to fields connected to pressure sensor stuff.
   

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


With regards,
Apache Git Services