Digital pressure sensor Driver

[RoccoMarco]

Theo non problem in change the API from getAxes to getChannels. What I wanted to understand is if the problem is in the API name or in the in/out parameters. I hope I have explained better what I meant. No problem if we need to change current APIs. Improvements are always welcome!
Ciao,
RM

[tfAteba]

Hi,
Sorry to reply just now, I did not have the time since yesterday.

Marco, for me the problem was in API name, and even Marco had the same confusion. We can keep it like that but I'm afraid others people will have the same problem maybe.

Has I said before with your explanations, I finally see what you meant about Axes and I started to write thermometer and barometer classes with the actual API.

So if we can change the name has you suggest it will be better.

Thanks,

Theo

[tfAteba]

Hi,

Here is my first version of the hal_thermometer and hal_barometer classes base on Giovanni's hal_compass class.

Can you give me advice and feedback please?

For now, i'm going to work on the driver of the bmp085 it self.

Thanks in advance.

regards,

Theo.

[RoccoMarco]

Hi Theo,
please call me Rocco Marco or Rocco or RM (they are both my names) this way I can clearly understand if you are talking with me or with Marco.
I have noticed some things that IMHO should be edited.

1. The copyright. Since this class will be added under hal it must be relased using the hal copyright statement and attributed to Giovanni
   

   Code: Select all

   /*
       ChibiOS - Copyright (C) 2006..2016 Giovanni Di Sirio
   
       Licensed under the Apache License, Version 2.0 (the "License");
       you may not use this file except in compliance with the License.
       You may obtain a copy of the License at
   
           http://www.apache.org/licenses/LICENSE-2.0
   
       Unless required by applicable law or agreed to in writing, software
       distributed under the License is distributed on an "AS IS" BASIS,
       WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
       See the License for the specific language governing permissions and
       limitations under the License.
   */

   
   which is automatically updated using a tool. No problem to apply you own GPL copyright statement on the device driver.
   
2. I have notice that you have added a method to the table which is get_calib_data. It is not enough clear to me what should do this API but I don't think that it is enough abstract to fit every barometer. I made something similar with mems than I have decided to add 4 methods (set_sensitivity, set_bias, reset_sensitivity and reset_bias). Through this methods it is possible to provide calibration data to the driver. This data is actively used when driver is cooking data, when not provided or reset this data is fixed to values ​​of convenience (bias usually to 0 and sensitivity to the typical value provided by the datasheet). The calibration method is something which is part of the application and not part of the driver.
   
3. There are many indent problems.

Ciao,
RM

[steved]

I have notice that you have added a method to the table which is get_calib_data. It is not enough clear to me what should do this API but I don't think that it is enough abstract to fit every barometer. I made something similar with mems than I have decided to add 4 methods (set_sensitivity, set_bias, reset_sensitivity and reset_bias). Through this methods it is possible to provide calibration data to the driver. This data is actively used when driver is cooking data, when not provided or reset this data is fixed to values ​​of convenience (bias usually to 0 and sensitivity to the typical value provided by the datasheet). The calibration method is something which is part of the application and not part of the driver.

Rather than add device-specific methods to the various drivers, why not have a single "command" interface which accepts command codes as a parameter (among other relevant parameters). So, for example, you have four command codes (set_sensitivity, set_bias, reset_sensitivity and reset_bias in this case) which actually do something device-specific; there will be others which (if called for the specific device) return an "unsupported command" result.
This way the method table stays the same; its just the supported commands which differ between devices.

[RoccoMarco]

I have to think about this. By the way the solution intrigues me. But I would like to hear what Giovanni thinks about that.
Ciao,
RM

[tfAteba]

Hi RM,

Thank you for your feedback.

1.The copyright. Since this class will be added under hal it must be relased using the hal copyright statement and attributed to Giovanni

Sorry I'm not very aware of copyright, but no problem, I will correct that. I apologizes.

2.I have notice that you have added a method to the table which is get_calib_data. It is not enough clear to me what should do this API but I don't think that it is enough abstract to fit every barometer.

For me, this method help to read the calibration data from sensor and (set_sensitivity, set_bias, reset_sensitivity and reset_bias) all those methods were used to write to the sensor, that is what I was thinking. But you seem to tell me that they are there to read from the sensor. So get_calib_data is not realy necessary.

There are many indent problems.

Do you have a tools to verify idents problems? Otherwise wich rules do I have to follow to respect the project ident.

regards,

Theo

[RoccoMarco]

Unfortunately there is no tool to check the indent. Anyway, this is the last problem. I will help you with this once we are ready to commit your contribution.
Ciao,
RM

[tfAteba]

Hi RM,

Ok, I will then correct the two other problem and then resend it to be sure that every things look ok before to go on and work on the driver side.

Theo

[Giovanni]

Rather than add device-specific methods to the various drivers, why not have a single "command" interface which accepts command codes as a parameter (among other relevant parameters). So, for example, you have four command codes (set_sensitivity, set_bias, reset_sensitivity and reset_bias in this case) which actually do something device-specific; there will be others which (if called for the specific device) return an "unsupported command" result.
This way the method table stays the same; its just the supported commands which differ between devices.

It is interesting and it is what some other OSes do, see ioctl() from Posix however I don't see clear advantages in it and there are some disadvantages.

It would be a non-API, using it you would make sure that your SW would not run on other devices, another problems are parameters and error codes. You would have to define command codes and error cases on wrong codes. Function parameters would have to be the same for all codes too.

I prefer the current scenario where each device is a class of its own with common ancestors and own methods for specific features.

Open for discussion of course.

Giovanni