LSM6DS33 - Reinitialization memory leak

adamgarbo commented 2 years ago

Hi @ladyada,

I have been investigating the cause of the most recent version of my Adafruit-powered Cryologger iceberg tracking beacons resetting themselves after 355 hourly samples. After some troubleshooting, I discovered there appears to be a memory leak caused by reinitializations of the LSM6DS33 sensor. Each time the initialization occurs, it consumes 72 bytes of free memory. With the Adafruit Feather M0 Basic Proto - ATSAMD21 it takes approximately 355 initializations for the SRAM to be exhausted and the "KABOOM" to occur. I should also note that reinitializations of the LIS3MDL are not affected by this issue.

I have tested and confirmed this issue on both SAMD21 and SAMD51 microcontrollers using the simplified code example below. I haven't been able to find the 72-byte memory monster yet and would appreciate any insight you can offer on how to fix the library.

Best regards, Adam

Arduino board: Adafruit LSM6DS33 + LIS3MDL - 9 DoF IMU with Accel / Gyro / Mag - STEMMA QT Qwiic
Arduino IDE version: 1.18.13
Steps to reproduce the problem: Please see below.

Example Code:

#include <Wire.h>
#include <Adafruit_Sensor.h>
#include <Adafruit_LSM6DS33.h>

Adafruit_LSM6DS33 lsm6ds33;

void setup() {

  Serial.begin(115200);
  while (!Serial);

  pinMode(A4, OUTPUT);
  digitalWrite(A4, LOW);
}

void loop() {
  Wire.begin(); // Initialize I2C

  digitalWrite(A4, HIGH);
  delay(500);

  configureImu();

  digitalWrite(A4, LOW);
  delay(500);

  Serial.print(" freeRam(): "); Serial.println(freeRam());
  delay(5000);
}

void configureImu() {

  if (!lsm6ds33.begin_I2C()) {
    Serial.println("Failed to find LSM6DS33 chip");
  }
  else {
    Serial.print("Initialized LSM6DS33");
  }
}

// Function to print available 32K SRAM memory
extern "C" char *sbrk(int i);
int freeRam()
{
  char stack_dummy = 0;
  return &stack_dummy - sbrk(0);
}

Feather M0 Proto + LSM6DS33 + LIS3MDL

Pin powered by A4.

Output:

15:48:27.447 -> Initialized LSM6DS33 freeRam(): 27615
15:48:33.484 -> Initialized LSM6DS33 freeRam(): 27543
15:48:39.545 -> Initialized LSM6DS33 freeRam(): 27471
15:48:45.614 -> Initialized LSM6DS33 freeRam(): 27399
15:48:51.685 -> Initialized LSM6DS33 freeRam(): 27327
15:48:57.736 -> Initialized LSM6DS33 freeRam(): 27255
15:49:03.781 -> Initialized LSM6DS33 freeRam(): 27183
15:49:09.825 -> Initialized LSM6DS33 freeRam(): 27111
15:49:15.878 -> Initialized LSM6DS33 freeRam(): 27039
15:49:21.942 -> Initialized LSM6DS33 freeRam(): 26967
15:49:27.982 -> Initialized LSM6DS33 freeRam(): 26895
15:49:34.043 -> Initialized LSM6DS33 freeRam(): 26823
15:49:40.110 -> Initialized LSM6DS33 freeRam(): 26751
15:49:46.161 -> Initialized LSM6DS33 freeRam(): 26679
15:49:52.199 -> Initialized LSM6DS33 freeRam(): 26607
15:49:58.278 -> Initialized LSM6DS33 freeRam(): 26535
15:50:04.321 -> Initialized LSM6DS33 freeRam(): 26463

SparkFun MicroMod Data Logging Carrier Board + SAMD51 Processor

Power controlled by AP2112 regulator.

Output:

15:56:13.889 -> Initialized LSM6DS33 freeRam(): 192399
15:56:19.958 -> Initialized LSM6DS33 freeRam(): 192327
15:56:26.001 -> Initialized LSM6DS33 freeRam(): 192255
15:56:32.071 -> Initialized LSM6DS33 freeRam(): 192183
15:56:38.132 -> Initialized LSM6DS33 freeRam(): 192111
15:56:44.181 -> Initialized LSM6DS33 freeRam(): 192039
15:56:50.237 -> Initialized LSM6DS33 freeRam(): 191967
15:56:56.277 -> Initialized LSM6DS33 freeRam(): 191895
15:57:02.324 -> Initialized LSM6DS33 freeRam(): 191823
15:57:08.396 -> Initialized LSM6DS33 freeRam(): 191751
15:57:14.459 -> Initialized LSM6DS33 freeRam(): 191679
15:57:20.495 -> Initialized LSM6DS33 freeRam(): 191607
15:57:26.549 -> Initialized LSM6DS33 freeRam(): 191535
15:57:32.614 -> Initialized LSM6DS33 freeRam(): 191463
15:57:38.676 -> Initialized LSM6DS33 freeRam(): 191391
15:57:44.725 -> Initialized LSM6DS33 freeRam(): 191319
15:57:53.579 -> Initialized LSM6DS33 freeRam(): 191247
15:57:57.127 -> Initialized LSM6DS33 freeRam(): 191175
15:58:06.938 -> Initialized LSM6DS33 freeRam(): 191103

caternuson commented 2 years ago

In general, begin() is designed to only be called once.

I haven't been able to find the 72-byte memory monster yet and would appreciate any insight you can offer on how to fix the library.

Might be here: https://github.com/adafruit/Adafruit_LSM6DS/blob/1ea3bd2c77be022ea5c847029471f7bf57635ee6/Adafruit_LSM6DS.cpp#L77-L79

Gets called here: https://github.com/adafruit/Adafruit_LSM6DS/blob/1ea3bd2c77be022ea5c847029471f7bf57635ee6/Adafruit_LSM6DS33.cpp#L37

Try adding in these 3 lines to Adafruit_LSM6DS.cpp to check and delete if they exist:

  if (temp_sensor) delete temp_sensor;
  if (accel_sensor) delete accel_sensor;
  if (gyro_sensor) delete gyro_sensor;

  temp_sensor = new Adafruit_LSM6DS_Temp(this);
  accel_sensor = new Adafruit_LSM6DS_Accelerometer(this);
  gyro_sensor = new Adafruit_LSM6DS_Gyro(this);

See if that helps?

adamgarbo commented 2 years ago

Hi @caternuson,

Thanks for the quick reply. In low-power applications, it frequently necessary to power down and reinitialize I2C devices. This is actually the first memory leak I've encountered in over 5 years of doing so.

I've added the lines you suggested directly above line 77 of Adafruit_LSM6DS33.cpp file and it seemed to do the trick.

If you think this would be suitable as a "catch-all" fix, I'd be happy to submit a PR.

Cheers, Adam

Output:

17:41:57.890 -> Initialized LSM6DS33 freeRam(): 27615
17:42:03.964 -> Initialized LSM6DS33 freeRam(): 27615
17:42:09.993 -> Initialized LSM6DS33 freeRam(): 27615
17:47:18.707 -> Initialized LSM6DS33 freeRam(): 27615
17:47:24.737 -> Initialized LSM6DS33 freeRam(): 27615
17:47:30.780 -> Initialized LSM6DS33 freeRam(): 27615
17:47:36.843 -> Initialized LSM6DS33 freeRam(): 27615
17:47:42.893 -> Initialized LSM6DS33 freeRam(): 27615
17:47:48.954 -> Initialized LSM6DS33 freeRam(): 27615
17:47:55.019 -> Initialized LSM6DS33 freeRam(): 27615
17:48:01.045 -> Initialized LSM6DS33 freeRam(): 27615
17:48:07.108 -> Initialized LSM6DS33 freeRam(): 27615
17:48:13.165 -> Initialized LSM6DS33 freeRam(): 27615
17:48:19.225 -> Initialized LSM6DS33 freeRam(): 27615

caternuson commented 2 years ago

It'd be interesting to see how other Arduino libraries have handled this scenario. Can you link to a good representational library? That does dynamic memory allocation and works well with deep sleep?

ladyada commented 2 years ago

most dont malloc/new @adamgarbo please submit a PR, thanks! :)

caternuson commented 2 years ago

@adamgarbo Is this resolved?

adamgarbo commented 2 years ago

Thanks for the reminder. Appears to be resolved by https://github.com/adafruit/Adafruit_LSM6DS/pull/29

adafruit / Adafruit_LSM6DS

LSM6DS33 - Reinitialization memory leak #28