Read Temperature fixes and version bump.#9
Conversation
There was a problem hiding this comment.
Pull request overview
This PR simplifies LIS3DH temperature reading by removing mode-specific temperature code paths and bumps the library version to reflect the change.
Changes:
- Remove implicit
LOW_POWERdefault selection and unifyreadTemperature()to a singleint8_tAPI. - Simplify temperature-reading implementation and update the Basic example accordingly.
- Bump package versions to
0.0.7for Arduino Library Manager and PlatformIO.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lis3dh-motion-detection.h | Removes implicit low-power defaulting and makes readTemperature() consistently return int8_t. |
| src/lis3dh-motion-detection.cpp | Removes alternate temperature path and updates comments/temperature read logic. |
| examples/Basic/Basic.ino | Removes mode-specific printing logic for temperature. |
| library.properties | Version bump to 0.0.7. |
| library.json | Version bump to 0.0.7. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #ifndef __LIS3DH_IMU_H__ | ||
| #define __LIS3DH_IMU_H__ | ||
|
|
||
| // Temperature works only if I define LOW_POWER here (!). | ||
| // Tested with arduino, not with platformio | ||
| #if !defined LOW_POWER & !defined NORMAL_MODE & !defined HIGH_RESOLUTION | ||
| #define LOW_POWER | ||
| #endif | ||
|
|
||
| #include "stdint.h" | ||
|
|
||
| #if defined(ARDUINO) && ARDUINO >= 100 |
There was a problem hiding this comment.
By removing the block that implicitly defined LOW_POWER when no mode macro is set, the library’s default operating mode changes for consumers who don’t define any of LOW_POWER, NORMAL_MODE, or HIGH_RESOLUTION. Previously they would compile in low-power (8-bit) mode; now they’ll compile with LPen=0 and HR=0 (normal mode). If this is intended, it should be called out in docs/release notes; otherwise consider restoring an explicit default to avoid a silent behavior/power/resolution change.
| temperatureEnable(1); // Although temperature is enabled, it must | ||
| // re-run to re-apply settings. Otherwise | ||
| // the temperature report is always the same. | ||
|
|
||
| // do we have data? | ||
| readRegister(&r, LIS3DH_STATUS_REG_AUX); | ||
| // check if we have data | ||
| if ( ( r & 0x04 ) == 0x04 ) { | ||
| readRegister(&r, LIS3DH_OUT_ADC3_H); // here are the data | ||
| return (int8_t) r; |
There was a problem hiding this comment.
readTemperature() enables BDU in temperatureEnable(true) but then only reads OUT_ADC3_H. With BDU enabled, the sensor’s output registers typically don’t update until both bytes are read, so reading only the high byte can cause the temperature value to appear “stuck” and forces this function to re-apply settings every time. Consider either (a) reading both OUT_ADC3_L and OUT_ADC3_H in one burst and using the high byte as the 8-bit value, or (b) not enabling BDU for this path.
| readRegister(&r, LIS3DH_STATUS_REG_AUX); | ||
| // check if we have data | ||
| if ( ( r & 0x04 ) == 0x04 ) { | ||
| readRegister(&r, LIS3DH_OUT_ADC3_H); // here are the data | ||
| return (int8_t) r; | ||
| } | ||
| // we don't have data. Return deep freezing for error. | ||
| return 0xFF; | ||
| } |
There was a problem hiding this comment.
readTemperature() uses 0xFF as an error sentinel, but as an int8_t this is -1, which can also be a legitimate relative temperature reading. Returning a value that can’t be distinguished from valid data makes error handling unreliable; consider returning a status (e.g., imu_status_t + out param) or using a separate “data ready” query so callers can distinguish error/no-data from a valid -1 reading.
| // Read temperature - need to calibrate. | ||
| Serial.print(" Temperature = "); | ||
| #ifdef LOW_POWER | ||
| Serial.println( tempCalibrate + (int8_t) myIMU.readTemperature()); |
There was a problem hiding this comment.
Since LIS3DH::readTemperature() now returns int8_t, the explicit cast to (int8_t) here is redundant. Dropping it makes the example a bit clearer and ensures the printed value reflects the API directly.
| Serial.println( tempCalibrate + (int8_t) myIMU.readTemperature()); | |
| Serial.println( tempCalibrate + myIMU.readTemperature()); |
Hello there,
now temperature works fine on all modes. I had the wrong impression that different code was needed for other modes. I removed the different code.