adafruit / Adafruit_LSM6DS

Arduino library for LSM6DS
Other
50 stars 41 forks source link

Fix repeated memory allocations upon sensor reinitialization #29

Closed adamgarbo closed 2 years ago

adamgarbo commented 2 years ago

Hi folks,

This minor pull request fixes the issue identified in #28, where repeated calls to 'lsm6ds33.begin_I2C();' would result in new memory allocations and eventually cause the stack to crash.

A simple fix was suggested by @caternuson to check for and delete the temp_sensor, accel_sensor and gyro_sensor objects prior to allocating new memory. The library was tested with this change and it is confirmed that the amount of free ram on a SAMD21-based Adafruit microcontroller remained constant throughout several dozen reinitializations.

Happy to know if there's a more eloquent way to achieve the same result!

Cheers, Adam

caternuson commented 2 years ago

Looks like the CI is failing on clang formatting: https://learn.adafruit.com/the-well-automated-arduino-library/formatting-with-clang-format

Happy to know if there's a more eloquent way to achieve the same result!

Me too. malloc/free and new/delete are the mechanisms. But would be interesting to see how other libraries have actually incorporated those with the low power scenario in mind, i.e. - the need to re-initialize after coming out of sleep.

ladyada commented 2 years ago

ya - most low power products would put the sensor into deep sleep mode with an i2c command (check the datasheet for how to get in and out of that mode) rather than re-init

adamgarbo commented 2 years ago

@caternuson Most I2C sensors have power-down or sleep modes that can be enabled to reduce the quiescent draw. However, supporting circuitry (e.g., LEDs, regulators, resistors) can often be unavoidable sources of power consumption that drive up the quiescent draw. In this case, it's preferable to cut power to the device and reinitialize the I2C bus when coming out of deep sleep. As @ladyada mentioned, most libraries don't use malloc/new during I2C initialization, so it's not something you typically need to worry about.

It looks like the check didn't like the formatting of the lines of code that were added. I'll update the PR.

adamgarbo commented 2 years ago

Unsure why the check is failing, as the code should now conform to Clang.

caternuson commented 2 years ago

just some residual whitespace. clang can be so picky! running clang-format -i should have taken care of that though.

ws