STMicroelectronics / iis3dwb-pid

iis3dwb platform independent driver based on Standard C language and compliant with MISRA standard
BSD 3-Clause "New" or "Revised" License
13 stars 9 forks source link

Doubt on iis3dwb_xl_hp_path_on_out_set() #3

Closed escherstair closed 3 years ago

escherstair commented 3 years ago

Looking to source code for iis3dwb_xl_hp_path_on_out_set here https://github.com/STMicroelectronics/iis3dwb/blob/d825d34bb3310b13dbce0cb7c3a67cb6c5d82ebd/iis3dwb_reg.c#L1376-L1382 I see one difference from iis3dwb datasheet.

Paragraph 9.17 of the datasheet descriibes how to configure CTRL8_XL register. In particular HP_REF_MODE_XL fields states

Enables accelerometer high-pass filter reference mode (valid for high-pass path - FDS bit must be ‘1’ and HPCFXL[2:0] must be set to “111”).

But the following line https://github.com/STMicroelectronics/iis3dwb/blob/d825d34bb3310b13dbce0cb7c3a67cb6c5d82ebd/iis3dwb_reg.c#L1379 is responsible to set this field. Looking to the allowed values for iis3dwb_hp_slope_xl_en_t https://github.com/STMicroelectronics/iis3dwb/blob/d825d34bb3310b13dbce0cb7c3a67cb6c5d82ebd/iis3dwb_reg.h#L882-L901 you can see that the only value that sets ctrl8_xl.hp_ref_mode_xlto 1 is https://github.com/STMicroelectronics/iis3dwb/blob/d825d34bb3310b13dbce0cb7c3a67cb6c5d82ebd/iis3dwb_reg.h#L884

But with this value, ctrl8_xl.hpcf_xl is set to 0. This is not what is described in the datasheet.

Can you clarify, please? I started looking into the source code because it seems that there is some unexpected behavior with some of the HPF/LPF2 configurations. I can be wrong, but I need to investigate deeper.

Thanks in advance

albezanc commented 3 years ago

I am very sorry for the inconvenience, you are right. It also appears that the naming of functions can be misleading. Please consider the following fix:

- IIS3DWB_SLOPE_ODR_DIV_4           = 0x30, 
+ IIS3DWB_HP_REF_MODE                = 0x37,

also renaming function: iis3dwb_xl_hp_path_on_out_set \ get in iis3dwb_xl_filt_path_on_out_set \ get

escherstair commented 3 years ago

Thanks. Are you going to commit the above changes to this repository too?

albezanc commented 3 years ago

Yes sure

escherstair commented 3 years ago

@albezanc Looking to commit https://github.com/STMicroelectronics/iis3dwb/commit/e97f730f81edc59ee16d0ce4b856f8da9c40bc12 I see that IIS3DWB_SLOPE_ODR_DIV_4 has been removed at all. Looking to the datasheet, it seems that this configuration exists. Maybe we can leave it with the following definition IIS3DWB_SLOPE_ODR_DIV_4 = 0x10 ?