wollewald / ADXL345_WE

Arduino Library for the ADXL345 accelerometer. I2C and SPI are implemented.
https://wolles-elektronikkiste.de/en/adxl345-the-universal-accelerometer-part-1
MIT License
40 stars 9 forks source link

in init() shouldn't reset the ADXL345_DATA_FORMAT after setRange #1

Closed llinjupt closed 3 years ago

llinjupt commented 3 years ago

Well done and thanks for your well-organized code.

bool ADXL345_WE::init(){
    ......
    setRange(ADXL345_RANGE_2G); // in this func, will set the low two bits of ADXL345_DATA_FORMAT
    writeRegister(ADXL345_DATA_FORMAT, 0); // but here, ADXL345_DATA_FORMAT will be reset.
    setFullRes(true);
      ......

Because the ADXL345_RANGE_2G is defined as 0b00, so it doesn't matter here. But if try to set ADXL345_RANGE_16G, it won't work as expected.

wollewald commented 3 years ago

Good catch! Thank you. I will change the order.

wollewald commented 3 years ago

...or simply delete the setRange!

llinjupt commented 3 years ago

...or simply delete the setRange!

Or maybe just leave ADXL345_RANGE_2G as a default parameter of init().

wollewald commented 3 years ago

It's not the only setting to default values. I do these "manual" settings to have a defined status after resetting the Arduino (or other) microcontroller. The issue is that the ADXL345 only takes the default values if you disconnect it from current. Unfortunately it has no reset function. When I developed the library I spent quite some time searching for a bug which did not really exist. It was just the setting from the former program which was still in the ADXL345.

llinjupt commented 3 years ago

It's not the only setting to default values. I do these "manual" settings to have a defined status after resetting the Arduino (or other) microcontroller. The issue is that the ADXL345 only takes the default values if you disconnect it from current. Unfortunately it has no reset function. When I developed the library I spent quite some time searching for a bug which did not really exist. It was just the setting from the former program which was still in the ADXL345.

I see. Below's how I fix this minor issue, to leave range as a parameter of init() for convenience.

bool ADXL345_WE::init(adxl345_range range=ADXL345_RANGE_2G){    
    writeRegister(ADXL345_POWER_CTL, 0);
    if(!setMeasureMode(true)){
        return false;
    }
    corrFact.x = 1.0;
    corrFact.y = 1.0;
    corrFact.z = 1.0;
    offsetVal.x = 0.0;
    offsetVal.y = 0.0;
    offsetVal.z = 0.0;

    /* reset the data format register */
    writeRegister(ADXL345_DATA_FORMAT, 0);

    setRange(range);
    setFullRes(true);

Another small suggestion is that using spaces (2 or 4 spaces) to replace a tab will be better for others to review the code with different IDEs. Anyway your code style is pretty neat and I enjoy it.

wollewald commented 3 years ago

I have just deleted the setRange() command. Spaces vs. tabs is something I will consider longterm.

Many thanks for your input and your kind feedback.