Incorrect bit masking (e.g. data &= ~0xfc ) in various functions

[pmichaud]

There appear to be several places where the wrong bits are being masked/reset to zero in the LIS331.cpp library.

For example, at

SparkFun_LIS331_Arduino_Library/src/SparkFun_LIS331.cpp

Line 240 in fe80a5d

data &= ~0xfc; // clear the low two bits of the register

there's a line of code that reads:

data &= ~0xfc; // clear the low two bits of the register

Actually, this is clearing all bits except the low two bits of the register. The correct statement would be either data &= ~0x03; or data &= 0xfc; (note the presence/absence of tildes in each of these versions).

Similarly this occurs in

SparkFun_LIS331_Arduino_Library/src/SparkFun_LIS331.cpp

Line 245 in fe80a5d

data &= ~0xe7; // clear bits 4:3 of the register

data &= ~0xe7; // clear bits 4:3 of the register

which should be data &= ~0x18;.

I found these examples/bugs when looking at the code for LIS331::setFullScale(), which should be clearing bits 5:4 but actually clears everything except those bits:

void LIS331::setFullScale(fs_range range)
{
  uint8_t data; 
  LIS331_read(CTRL_REG4, &data, 1);
  data &= ~0xcf;
  data |= range<<4;
  LIS331_write(CTRL_REG4, &data, 1);
}

The third statement should read data &= ~0x30;.

Thanks,

Pm

[jl-lewis]

I agree. I noticed this in the CTRL_REG4 as well. It is perhaps going unnoticed because the default is 00, and changing to non-zero values would work...once. If you set the range to non-zero bits and then back it would be incorrect. That's how it looks to me anyway, so this seems like confirmation. I have not done anything to test that idea yet, or try changing the method, but just in code review. Seems fairly easy to fix but needs these multiple places reviewed as it seems to be a systematic mistake.

[santaimpersonator]

To piggy back on this, another customer also found a similar issue with the library here: https://github.com/sparkfun/SparkFun_LIS331_Arduino_Library/blob/master/src/SparkFun_LIS331.cpp#L232-L246

and proposed the following changes:

void LIS331::intSrcConfig(int_sig_src src, uint8_t pin)
{

  uint8_t data;
  LIS331_read(CTRL_REG3, &data, 1);
  // Enable latching by setting the appropriate bit.
  if (pin == 1)
  {
    data &= ~0xfc; // clear the low two bits of the register BUG!!! remove ~
    data |= src;
  }
  if (pin == 2)
  {
    data &= ~0xe7; // clear bits 4:3 of the register BUG!!!! remove ~
    data |= src

[cirp-usf]

I can confirm that clearing the wrong bits in CTRL_REG4 (and not clearing the right ones) was preventing the sensor to switch the full scale range to lower ranges once HIGH_RANGE was selected.

Changing line 255 of SparkFun_LIS331.cpp to data &= 0xcf; (as one of the possible fixes) solved the problem for me.

Moreover, looking for further register bit-clearing operations, I've found that lines 116, 240, and 245 also seem wrong, with the bitwise negation incorrectly used (or wrong constants). I did not test these, though.