Suspected crash when calling begin()

[djcaf]

Had a strange issue with code suddenly not running giving an USB device error in Windows and uploads no longer possible via USB. I have replicated (mostly) with the code below using the EEPROM library. Windows does not give an USB device error with this code (maybe due to delay() in my test code which delays the point at which the device fails). However uploads still are no longer possible when running this code so I guess the CPU is still crashing.

What should happen is after stat or upload the builtin LED is off for 3 seconds then comes on. What happens is the LED does not come on and USB uploads do not work. Pressing reset twice in quick succession allows new code to be uploaded.

It is begin() that fails and from what I could tell _wire is nullptr in the begin() code for some reason - does not look possile though.

It works (may just hide the issue) when I use the I2C_eeprom ee(0x50, I2C_DEVICESIZE_24LC256, &Wire) line instead. My first thought is undefined behaviour somewhere, but I have eliminated my code so that can't be the cause. I do not have a spare Arduino with me at the moment to test. The Arduino is only connected to a EEPROM chip but the same issue occurs when completely disconnected (i.e. the Arduino is removed from the breadboard).

Board in question is the Nano 33 IoT and the IDE is 2.0.2 The RaspberryPi PICO does the same thing. It flashes a code with its built in LED as well - Cannot find what my specific pattern means but essentially means it has crashed also which probably eliminates my hardware as a cause.

Thanks.

#include "Wire.h"
#include "I2C_eeprom.h"

I2C_eeprom ee(0x50, &Wire); //Does not work
//I2C_eeprom ee(0x50, I2C_DEVICESIZE_24LC256, &Wire); //Works

void setup() {
  pinMode(LED_BUILTIN, OUTPUT);
  digitalWrite(LED_BUILTIN, LOW);
  delay(3000);
  ee.begin();
  digitalWrite(LED_BUILTIN, HIGH);
}

void loop() {
  // put your main code here, to run repeatedly:

}

[RobTillaart]

Thanks for this issue, I'm quite busy and it might take some time before investigating.

[RobTillaart]

updated your post for syntax highlighting (is quite simple have a look)

[djcaf]

updated your post for syntax highlighting (is quite simple have a look)

Thanks. I don't log things in git hub often I will remember in the future :-)

[RobTillaart]

now the constructor calls the other one. If you place the (appropriate) code in the first constructor does it work?

I2C_eeprom::I2C_eeprom(const uint8_t deviceAddress, TwoWire * wire)
{
    _deviceAddress = deviceAddress;
    _deviceSize = I2C_PAGESIZE_24LC256;
    _pageSize = getPageSize(_deviceSize);
    _wire = wire;

    //  Chips 16Kbit (2048 Bytes) or smaller only have one-word addresses.
    this->_isAddressSizeTwoWords = _deviceSize > I2C_DEVICESIZE_24LC16;
}

Question could be when the constructor code is executed...

[djcaf]

Yes it does. I did do an experiement with somthing in Visual Studio and C++ which may shed some light.

Here is my test code from VS2022 / C++:

#pragma once
class TestClass
{
public:
    int _textX = 0;
    TestClass()
    {
        TestClass(10);
    }

    TestClass(int x)
    {
        _textX = x;
    }
};

This is called like so:

TestClass c1;
TestClass c2(100);
int x = c1._textX;
std::cout << x << std::endl;
x = c2._textX;
std::cout << x << std::endl;

The output is: 0 100

Not as expected

I think the TestClass(10); creates a new local copy of TestClass then discards it thus _testX is never set on the original instance just this new temporary instance.

If I replace the class with:

#pragma once
class TestClass
{
public:
    int _textX = 0;
    TestClass() : TestClass(10)
    {

    }

    TestClass(int x)
    {
        _textX = x;
    }
};

The output is now 10 100

as expected.

Back to the library;

I2C_eeprom::I2C_eeprom(const uint8_t deviceAddress, TwoWire * wire)
{
    I2C_eeprom(deviceAddress, I2C_PAGESIZE_24LC256, wire);
}

The line in this constructor probably does the same, i.e. _wire is only set on a temporary local copy. You're suggested change would fix this, so does

I2C_eeprom::I2C_eeprom(const uint8_t deviceAddress, TwoWire * wire) : I2C_eeprom(deviceAddress, I2C_PAGESIZE_24LC256, wire) 
{ 
   //No code needed here
}

I am not really a C++ developer but this all makes sense if true. If true then _wire will be nullptr when used which won't work and may crash but a crash is not necessaily guaranteed hence it sometimes not immediately stopping.

[RobTillaart]

Thanks for testing, I will create a PR later today.

If true then _wire will be nullptr when used which won't work and may crash but a crash is not necessaily guaranteed hence it sometimes not immediately stopping.

I've never encountered this problem before - and had several releases with this code (also other libs using a similar construct) I'll do a test on the Arduino UNO to see what the compiler does there. But anyway it should be made robust.

[RobTillaart]

Arduino UNO gives the same problem, Checked the examples of the lib and they all use the 2nd constructor (also in my projects) Good catch!

[RobTillaart]

Merged PR, Again thanks for catching the bug, appreciated!