search

akupila / Arduino-MMA8452

MIT License
7 stars 7 forks source link

convertTo2sComplement() needs to be removed #6

Open torriem opened 4 years ago

torriem commented 4 years ago

I'm a baffled as to why there's a member function called, convertTo2sComplement(). Integers of any size in C are by definition two's complement already. There's nothing to convert.

Furthermore, the result of convertTo2sComplement() for any negative value is always 2, which is clearly incorrect.

The solution is to remove the function entirely. It's not needed and no other MMA8252 library I've looked at has one. They all just write the uint8_t offset values directly to the appropriate I2C registers.

akupila commented 4 years ago

The code is 6 years old so it's hard to say why anything is the way it is. Looks like this is a typo that never surfaced as it only affects setOffsets. The typo is the double &&, which should be &: https://github.com/akupila/Arduino-MMA8452/blob/2041b42f2660062d21ae9f6403910e228664ae29/MMA8452.cpp#L517 I believe you're correct in the method being unnecessary but i'm inclined to not touch this code too much as i have no way of testing it. If you put together a PR, i'll trust you to verify it works and accept it.