MPU6050_tockn icon indicating copy to clipboard operation
MPU6050_tockn copied to clipboard

Unreliable endianness of retrieved data

Open edgar-bonet opened this issue 4 years ago • 1 comments

The method void MPU6050::calcGyroOffsets() contains the following line:

rx = wire->read() << 8 | wire->read();

and similar lines for ry and rz. The same idiom is used in MPU6050::update().

This kind of expression should be avoided, as it does not guarantee the endianness of the retrieved data. This is because, in C++, the order of evaluation of the arguments of the | operator is unspecified.

Presumably, the current version of gcc, when invoked with the options passed by the current version of the Arduino IDE, generates assembly that orders the bytes in the expected way. But this cannot be relied upon, and can very well depend on the compiler, compiler version, compiler options, or even surrounding code. This is the kind of software error that cannot be unveiled by testing, as the code can “just work” even if it is incorrect.

The simplest solution is to issue the calls to wire->read() within separate statements. Unlike the arguments of the | operator, separate statements have a well define order of evaluation:

rx = wire->read() << 8;
rx |= wire->read();

See also this post on the Arduino forum.

edgar-bonet avatar Mar 06 '20 16:03 edgar-bonet

Thanks for your comment @edgar-bonet , however it seems that @tockn didn't update his library for several months :( I forked the Tockn's library and took your comment into account in my own repository. Your comments are welcome.

rfetick avatar Mar 08 '20 22:03 rfetick