ism330dhcx_from_lsb_to_nsec saturation

[GasparQ]

Hi, I noticed some strage behaviors with output of ism330dhcx_from_lsb_to_nsec function.

For example, with the uint32_t tick of 43656 it outputs 1091399936.

However the equation is simply timestamp_ns = tick * 25000, thus here it should returns 1091400000

Moreover, after a time, the returned timestamp simply saturates.

However, from what I understand in the application note of ISM330DHCX, it says:

When the maximum value 4294967295 LSB (equal to FFFFFFFFh) is reached corresponding to approximately 30 hours, the counter is automatically reset to 00000000h and continues to count. The timer count can be reset to zero at any time by writing the reset value AAh in the TIMESTAMP2 register.

Thus, I would suggest to convert the LSB into uint64_t nanoseconds with the following code:

uint64_t ism330dhcx_from_lsb_to_nsec(uint32_t lsb)
{
  return (uint64_t)lsb * 25000UL;
}

It my reasoning right or did I missed something ?

[TOUNSTM]

ST Internal Reference: 187672

[avisconti]

Hi, I noticed some strage behaviors with output of ism330dhcx_from_lsb_to_nsec function.

For example, with the uint32_t tick of 43656 it outputs 1091399936.

However the equation is simply timestamp_ns = tick * 25000, thus here it should returns 1091400000

Moreover, after a time, the returned timestamp simply saturates.

However, from what I understand in the application note of ISM330DHCX, it says:

When the maximum value 4294967295 LSB (equal to FFFFFFFFh) is reached corresponding to approximately 30 hours, the counter is automatically reset to 00000000h and continues to count. The timer count can be reset to zero at any time by writing the reset value AAh in the TIMESTAMP2 register.

Thus, I would suggest to convert the LSB into uint64_t nanoseconds with the following code:

uint64_t ism330dhcx_from_lsb_to_nsec(uint32_t lsb)
{
  return (uint64_t)lsb * 25000UL;
}

It my reasoning right or did I missed something ?

@GasparQ what you are saying is correct, thanks for reporting it. Other STdC drivers are already doing this, and others are not. I will fix it all this stuff asap.

[avisconti]

See https://github.com/STMicroelectronics/ism330dhcx-pid/issues/5

[GasparQ]

Thank you for the implementation, however I think there is a mistake.

In C, 25000.0f is of type float, and the result of an operation between an int and a float is always a float.

It won't change the overflow behavior than before using float cast.

I should recommand using the syntax I used in my issue (uint64_t)lsb * 25000ULL to have uint64_t on both sides creating no risks of overflow at all on this function.

[avisconti]

@GasparQ I will change the routine in this way:

diff --git a/ism330dhcx_reg.c b/ism330dhcx_reg.c
index 18384ce..32c64d8 100644
--- a/ism330dhcx_reg.c
+++ b/ism330dhcx_reg.c
@@ -155,9 +155,9 @@ float_t ism330dhcx_from_lsb_to_celsius(int16_t lsb)
   return (((float_t)lsb / 256.0f) + 25.0f);
 }

-uint64_t ism330dhcx_from_lsb_to_nsec(int32_t lsb)
+uint64_t ism330dhcx_from_lsb_to_nsec(uint32_t lsb)
 {
-  return ((uint64_t)lsb * 25000.0f);
+  return ((uint64_t)lsb * 25000ULL);
 }

As you can see, I think that the API argument should be uint32_t (and not signed). Does it make sense to you?