Memory alloc problem in example code / SevSegShift::begin #9
Description
Hi there,
I think there is a bug in all examples of SevSegShift which use the following (or similar) setup function:
void setup() {
byte numDigits = 4;
byte digitPins[] = {8+2, 8+5, 8+6, 2}; // of ShiftRegister(s) | 8+x (2nd Register)
byte segmentPins[] = {8+3, 8+7, 4, 6, 7, 8+4, 3, 5}; // of Shiftregister(s) | 8+x (2nd Register)
bool resistorsOnSegments = false; // 'false' means resistors are on digit pins
byte hardwareConfig = COMMON_ANODE; // See README.md for options
bool updateWithDelays = false; // Default 'false' is Recommended
bool leadingZeros = false; // Use 'true' if you'd like to keep the leading zeros
bool disableDecPoint = false; // Use 'true' if your decimal point doesn't exist or isn't connected
sevseg.begin(hardwareConfig, numDigits, digitPins, segmentPins, resistorsOnSegments,
updateWithDelays, leadingZeros, disableDecPoint);
sevseg.setBrightness(90);
}
The problem lies herein ( https://github.com/bridystone/SevSegShift/blob/ShiftRegister/SevSegShift.cpp#L51 ):
void SevSegShift::begin(
byte hardwareConfig,
byte numDigitsIn,
byte shiftRegisterMapDigits[],
byte shiftRegisterMapSegments[],
bool resOnSegmentsIn,
bool updateWithDelaysIn,
bool leadingZerosIn,
bool disableDecPoint
) {
// here the digit"pins" of the Shift register is stored
// pins are here the OUTPUT PINs of the Shift Register
_shiftRegisterMapDigits = shiftRegisterMapDigits;
// here the segment"pins" of the Shift register is stored
// pins are here the OUTPUT PINs of the Shift Register
_shiftRegisterMapSegments = shiftRegisterMapSegments;
SevSegShift::begin assigns the pointers to the array of pins passed as digitPins and segmentPins in the above examples to the corresponding internal variables. Since digitPins and segmentPins are declared locally in setup() however, they might get deallocated as soon as the program leaves the scope (i.e. the setup function ends) - in any case this probably is undefined behaviour according to the C standard.
At least this is what happens on the ESP32.
Compare this to how SevSeg copies the values explicitly:
https://github.com/DeanIsMe/SevSeg/blob/master/SevSeg.cpp#L170
// Save the input pin numbers to library variables
for (uint8_t segmentNum = 0 ; segmentNum < numSegments ; segmentNum++) {
segmentPins[segmentNum] = segmentPinsIn[segmentNum];
}
for (uint8_t digitNum = 0 ; digitNum < numDigits ; digitNum++) {
digitPins[digitNum] = digitPinsIn[digitNum];
}
A quick fix would just be moving the declaration of digitPins and segmentPins in the examples to the global scope. However I would advise against doing so, since the difference in behaviour compared to SevSeg is certainly unexpected and makes switching between the libraries harder.
A better solution would be to just malloc the member variables (_shiftRegisterMap*) in SegSevShift::begin and memcpy the passed values to them. Or copy the values by hand as SevSeg does (which probably is a tad slower).
Cheers!