search

eclipse / upm

UPM is a high level repository that provides software drivers for a wide variety of commonly used sensors and actuators. These software drivers interact with the underlying hardware platform through calls to MRAA APIs.
MIT License
659 stars 411 forks source link

lis3dh: add sensor support based on lis2ds12 module #623

Closed alext-mkrs closed 6 years ago

alext-mkrs commented 6 years ago

Adding STMicro LIS3DH sensor support. This module is based on the one for LIS2DS12 (thanks, @jontrulson !), but as sensors are noticeably different, the contents underwent major rework. I've also run clang-format on all C/CXX/H/HPP module files and C/C++ examples.

Examples and basic API are left the same.

Tested on Intel Edison with Arduino board using both I2C and SPI.

One particular difference to note is that I removed the accFactor coefficient, as it is effectively cancelled out right away in lis2ds12_get_accelerometer() calculations: https://github.com/intel-iot-devkit/upm/blob/master/src/lis2ds12/lis2ds12.c#L455-L458, so seems to be of no use and I couldn't find any reason for preserving it. Line 455 adds it to the numerator, line 458 - to the denominator, cancelling it out. In addition, the logic of the necessary conversion itself doesn't call for such an additional coefficient IMHO, which is additionally confirmed by the calculation example in LIS2DS12 appnote, see section 3.5.1 "Example of output data", page 16.

If you think those are actually necessary, I'd be grateful for a clarification. If these are indeed surplus - I'd be glad to submit a separate PR for removing them from lis2ds12 module as well.

In addition, as we anyway return values in "g" and not "milli g", I also removed the divison by 1000 in lis3dh_get_accelerometer() and simply set proper scaling factor in "g" right away in lis3dh_set_full_scale().

jtrulson commented 6 years ago

If you think those are actually necessary, I'd be grateful for a clarification. If these are indeed surplus - I'd be glad to submit a separate PR for removing them from lis2ds12 module as well.

Nope, I think you are correct there. Of course, please make sure the changes work -- they should, but I've been unpleasantly surprised before. :)

alext-mkrs commented 6 years ago

Thanks! Yeah, they do work, I've surely tested that :)

EDIT: Just to make it clear - I meant I tested that on lis3dh, I don't have lis2ds12 at hand. But as their axes data registers format is identical + taking the lis2ds12 appnote into account, I think that doesn't leave any space for such surprises. I'll submit that PR and we can discuss there if real sensor testing is necessary though, np with that.

alext-mkrs commented 6 years ago

So, any comments or do you think we can merge this, @propanu, @pylbert?

pylbert commented 6 years ago

Great add! Merged.

alext-mkrs commented 6 years ago

Thanks!