adafruit / Adafruit_BMP280_Library

Arduino Library for BMP280 sensors
250 stars 187 forks source link

ESP32 Core panic when using constructor Adafruit_BMP280(&Wire) and using Adafruit_Sensor #67

Closed hpsaturn closed 2 years ago

hpsaturn commented 2 years ago

Summary

The library works fine when you use the empty constructor, but when you pass the alternative Wire reference, and get the Adafruit sensors objects, it produce a PANIC:

Code

The next code fails when it try to use the second Wire (Wire1):


void Sensors::init() {
#ifdef M5STICKCPLUS
    Wire.begin(32,33); // M5CoreInk Ext port (default for all sensors)
    Wire1.begin(0,26);   // M5CoreInk hat pines (header on top)
#else
    Wire.begin(34,33); // M5CoreInk Ext port (default for all sensors)
    Wire1.begin(21,22);
#endif
}

void Sensors::bmp280Init() {
    DEBUG("-->[SLIB] try to enable sensor \t: BMP280..");

    if (!bmp280.begin() && !bmp280.begin(BMP280_ADDRESS_ALT)) {
        bmp280 = Adafruit_BMP280(&Wire1);
        if (!bmp280.begin() && !bmp280.begin(BMP280_ADDRESS_ALT)) return;
    }
    Serial.println("-->[SLIB] I2C sensor detected\t: BMP280");
    bmp280.setSampling(Adafruit_BMP280::MODE_NORMAL,  // Operating Mode.
                    Adafruit_BMP280::SAMPLING_X2,     // Temp. oversampling
                    Adafruit_BMP280::SAMPLING_X16,    // Pressure oversampling
                    Adafruit_BMP280::FILTER_X16,      // Filtering.
                    Adafruit_BMP280::STANDBY_MS_500); // Standby time.
    Adafruit_Sensor *bmp_temp = bmp280.getTemperatureSensor();
    Adafruit_Sensor *bmp_pressure = bmp280.getPressureSensor();
    bmp_temp->printSensorDetails();
    bmp_pressure->printSensorDetails();
}

Also the next code:

    bmp280 = Adafruit_BMP280(&Wire);
    if (!bmp280.begin() && !bmp280.begin(BMP280_ADDRESS_ALT)) return;
    Serial.println("-->[SLIB] I2C sensor detected\t: BMP280");
    bmp280.setSampling(Adafruit_BMP280::MODE_NORMAL,  // Operating Mode.
                    Adafruit_BMP280::SAMPLING_X2,     // Temp. oversampling
                    Adafruit_BMP280::SAMPLING_X16,    // Pressure oversampling
                    Adafruit_BMP280::FILTER_X16,      // Filtering.
                    Adafruit_BMP280::STANDBY_MS_500); // Standby time.
    Adafruit_Sensor *bmp_temp = bmp280.getTemperatureSensor();
    bmp_temp->printSensorDetails();

The next code works fine (when the second constructor it is empty)

   void Sensors::init() {
       Wire1.begin(21,22);
    }

    if (!bmp280.begin() && !bmp280.begin(BMP280_ADDRESS_ALT)) {
        bmp280 = Adafruit_BMP280(&Wire1);
        if (!bmp280.begin() && !bmp280.begin(BMP280_ADDRESS_ALT)) return;
    }
    Serial.println("-->[SLIB] I2C sensor detected\t: BMP280");
    bmp280.setSampling(Adafruit_BMP280::MODE_NORMAL,  // Operating Mode.
                    Adafruit_BMP280::SAMPLING_X2,     // Temp. oversampling
                    Adafruit_BMP280::SAMPLING_X16,    // Pressure oversampling
                    Adafruit_BMP280::FILTER_X16,      // Filtering.
                    Adafruit_BMP280::STANDBY_MS_500); // Standby time.
    Adafruit_Sensor *bmp_temp = bmp280.getTemperatureSensor();
    bmp_temp->printSensorDetails();

Maybe the issue is because when you pass the Wire parameter the library delete the objects:

Adafruit_BMP280::~Adafruit_BMP280(void) {
  if (spi_dev)
    delete spi_dev;
  if (i2c_dev)
    delete i2c_dev;
  delete temp_sensor;
  delete pressure_sensor;
}

Environment

PlatformIO Core version: 5.2.4 framework: espressif32, Arduino boards: ESP32 dev board, M5StickC

ladyada commented 2 years ago

OK! please use the debugger or stack trace to figure out where its crashing - maybe you have to init Wire1 specially, also there was a lot of refactoring in ESP BSP -- it may not be a crash in this library

hpsaturn commented 2 years ago

Nop, like I mentioned, the issue is in the library, in de Adafruit_sensor object. This is the backtrace:

esp32decode -e firmware.elf 0x0000400e:0x3ffcf630 0x400e3959:0x3ffcf680 0x400e4936:0x3ffcf6e0 0x400d5f8b:0x3ffcf720 0x400d6362:0x3ffcf760 0x400ecb6a:0x3ffcf7c0 0x400915f2:0x3ffcf7e0
??:0
/home/avp/pio/canairio_firmware/lib/sensorlib/src/Sensors.cpp:1085 (discriminator 1)
/home/avp/pio/canairio_firmware/lib/sensorlib/src/Sensors.cpp:102
/home/avp/pio/canairio_firmware/src/main.cpp:154 (discriminator 2)
/home/avp/pio/canairio_firmware/src/main.cpp:209 (discriminator 6)
/home/avp/.platformio/packages/framework-arduinoespressif32/cores/esp32/main.cpp:18
/home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/freertos/port.c:143

And this line is a call in the Adafruit library:

bmp_temp->printSensorDetails();

The crash is in this method, like I mentioned in the first message in this thread. If I comment this line, everything works fine.

Hot fix

I'm going to send a pull request to you that fix the issue..

hpsaturn commented 2 years ago

I already have two tests more in three boards, and I did replicate the issue. Also I updated the first message with more details of the initialization of the Wire objects, because the issue happen when you have two Wire objects only.

photo_2022-01-13_00-57-39

photo_2022-01-13_00-57-14

adams13x13 commented 2 years ago

I did not get it. When you write (lines 36-39)

  if (spi_dev)
    delete spi_dev;
  if (i2c_dev)
    delete i2c_dev;

this means, if spi_dev points to something is not nullptr, delete it. But when you write (lines 40-43 in the version 2.6.1)

  if (temp_sensor == nullptr)
    delete temp_sensor;
  if (pressure_sensor == nullptr)
    delete pressure_sensor;

this means, if temp_sensor does not point to an object, it is nullptr, try to delete it and probably crash or whatever. When it is nullptr, you should not do it! Correct would be

  if (temp_sensor)
    delete temp_sensor;
  if (pressure_sensor)
    delete pressure_sensor;

Or did I miss something? May be destructor is not called that often in the embedded world and that is why nobody complains? After all, how could the destructor resolve the issue with bmp_temp->printSensorDetails();? As I understand, you do not call the destructor.

Anyway, I think "== nullptr" should be removed.

ladyada commented 2 years ago

@adams13x13 can you submit a PR? i think they made a typo

caternuson commented 2 years ago

Closing. I think this is fixed now. Can reopen if not.