Strange change of variable value without adressing it

Hi!
I have something strange happening in my code. In this part of the code an array of random length is created with random values in it. But a certain variable called _Pos changes after the code fills the array, even though _Pos isn’t adressed in it.
I have this part of the code running in a loop to see what’s going wrong, so the problem does lie here somewhere…
I hope someone can explain what is happening here!

void LedStrip::Delay()
{
  _N_Delays = random(5, 10);
  _Delays[_N_Delays];
  _Pos = 0;
  
  Serial.print("_Pos: ");
  Serial.println(_Pos);

  for (int i; i <= _N_Delays; i++) {
    _Delays[i] = random(50, 800);
    Serial.println(_Delays[i]);
  }
  
  Serial.print("_Pos: ");
  Serial.println(_Pos);
  Serial.print("_Delays[_Pos]: ");
  Serial.println(_Delays[_Pos]);

}

Really? An unreadable image of text? In what universe is that a good idea?

And in C/C++ it's not trivial to create "an array of random length"... But yeah, it you don't post the code, how are we supposed to help?

I have something strange happening in my code

Which, equally strangely, you seem to have forgotten to post.

How's that working for you?

Sorry guys, I'm still editting my post. Haven't really figured out how these image and code stuff in forum messages work.

Maybe post it to Snippets R Us?

Because I don't see you create an array anywhere :wink: I only see you access an array which I have no idea where you created it...

Please post the code here using code tags. It makes it so much easier to copy to an editor for examination

As it happens I have looked at your code.
The array has a length of _N_Delays so its elements will be numbered from 0 to _N_Delays - 1

Then you iterate over the array with an upper limit of <= _N_Delays

WHOOPS, you just wrote outside of the bounds of the array.

The upper limit of the for loop should be < _N_Delays

The Serial. print give:
_Pos: 0
549
373
208
480
322
344
178
223
_Pos: 549
_Delays[_Pos]: 30039

Full code:

int PotMtrPin = 6;   // potentiometer connected to analog pin 3
int PotVal = 0;         // variable to store the read value

int All_OnOff_Pin = 5;

int StripsDone = 0;
int TotStrips = 14;

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

class LedStrip {

  private:

    int _Place;
    int _LedPin;
    bool _LedOnOff;
    bool _LedOnOff_Bfr;
    int _N_Delays;
    int _Delays[];
    int _Pos;
    unsigned long _previousMillis;

  public:

    LedStrip(int Place, int LedPin);
    void Setup();
    void Update(unsigned long _currentMillis);
    void Delay();
    void Go_OnOff();

};

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////


LedStrip::LedStrip(int Place, int LedPin)
{
  _Place = Place;
  _LedPin = LedPin;
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

void LedStrip::Setup()
{
  pinMode(_LedPin, OUTPUT);
  digitalWrite(_LedPin, LOW);
  _LedOnOff = false;
  _LedOnOff_Bfr = false;
  _previousMillis = 0;
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

void LedStrip::Delay()
{
  _N_Delays = random(5, 10);
  _Delays[_N_Delays];
  _Pos = 0;
  
  Serial.print("_Pos: ");
  Serial.println(_Pos);

  for (int i; i <= _N_Delays; i++) {
    _Delays[i] = random(50, 800);
    Serial.println(_Delays[i]);
  }
  
  Serial.print("_Pos: ");
  Serial.println(_Pos);
  Serial.print("_Delays[_Pos]: ");
  Serial.println(_Delays[_Pos]);

}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

void LedStrip::Update(unsigned long _currentMillis) {
  Serial.print("going update  ");
  Serial.print(_currentMillis);
  Serial.print("  ");
  Serial.print(_previousMillis);
  Serial.print("  ");
  Serial.println(_Delays[_Pos]);
  if (_currentMillis - _previousMillis > _Delays[_Pos]) {
    Serial.println("going inside if form update");
    Go_OnOff();
    _previousMillis = _currentMillis;
    _Pos += 1;
  }
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

void LedStrip::Go_OnOff()
{
  Serial.println("going go on off");
  if ( _Pos == _N_Delays ) {
    if (_LedOnOff_Bfr == true && _LedOnOff == true) {
      digitalWrite(_LedPin, LOW);
      _LedOnOff = false;
      _LedOnOff_Bfr = false;
    }
    else if ( _LedOnOff_Bfr == false && _LedOnOff == false) {
      digitalWrite(_LedPin, HIGH);
      _LedOnOff = true;
      _LedOnOff_Bfr = true;
    }
    StripsDone += 1;
  }
  else {
    if (_LedOnOff) {
      digitalWrite(_LedPin, LOW);
      _LedOnOff = false;
    }
    else {
      digitalWrite(_LedPin, HIGH);
      _LedOnOff = true;
    }
  }
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

LedStrip LS_A_1(1, 6);
LedStrip LS_A_2(2, 2);
LedStrip LS_M_1(3, 3);
LedStrip LS_M_2(4, 3);
LedStrip LS_M_3(5, 3);
LedStrip LS_B_1(6, 4);
LedStrip LS_B_2(7, 4);
LedStrip LS_I1(8, 5);
LedStrip LS_T_1(9, 6);
LedStrip LS_T_2(10, 6);
LedStrip LS_I2(11, 7);
LedStrip LS_O(12, 8);
LedStrip LS_N_1(13, 8);
LedStrip LS_N_2(14, 8);


//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////


void setup()
{

  LS_A_1.Setup();
  LS_A_2.Setup();
  LS_M_1.Setup();
  LS_M_2.Setup();
  LS_M_3.Setup();
  LS_B_1.Setup();
  LS_B_2.Setup();
  LS_I1.Setup();
  LS_T_1.Setup();
  LS_T_2.Setup();
  LS_I2.Setup();
  LS_O.Setup();
  LS_N_1.Setup();
  LS_N_2.Setup();

  Serial.begin(9600);
}


//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////



void loop()
{

  PotVal = analogRead(PotMtrPin);   // read the input pin
  analogWrite(All_OnOff_Pin, PotVal / 4); // analogRead values go from 0 to 1023, analogWrite values from 0 to 255

  LS_A_1.Delay();
  /*
  while (StripsDone < 1) {
    LS_A_1.Update(millis());
    Serial.println("going whileloop");
    /*
      LS_A_2.Update();
      LS_M_1.Update();
      LS_M_2.Update();
      LS_M_3.Update();
      LS_B_1.Update();
      LS_B_2.Update();
      LS_I1.Update();
      LS_T_1.Update();
      LS_T_2.Update();
      LS_I2.Update();
      LS_O.Update();
      LS_N_1.Update();
      LS_N_2.Update();
    *
  }*/


}

This

int _Delays[];

ONLY creates a reference to an array but NO room for the array itself. Aka, an array with size 0 (zero)

_Delays[_N_Delays];

Only access the '_N_Delays’th element of ‘_Delays’. But as ‘_Delays’ is zero long that’s outside the range. Here it doesn’t matter because you throw away the result…

_Delays[i] = random(50, 800);

But now you write beyond the array for sure because it’s still size 0.

Like I said, an array with ransom length isn’t trivial in C/C++. Easiest (and I think the neatest here as well), just create an array with the max size you want aka:

int _Delays[9];

And now you can just use random to determine how far you want to fill it.

Thanks so much!
I'm used to Python so I have to get used to the different approaches. But it works perfectly now!

Thing is, it's not impossible in C/C++ either. But thing is, then the microcontroller has to do all the memory management and that's not a light task. Not a problem on you're Intel i7 and 16GB of RAM, but it is on a 8-bit micro with 2kB of RAM. So if you ca avoid it it's usually easier/quicker to let the compiler (running on the PC with loads of resources) do the memory management so the micro does not need to worry about it on run time.

Aka, it's with a good reason it's not common in micro controller-land. And C/C++ makes it easy to do so.

For good measures; In C you usually initialize the iterator/loop controller of a for loop:

for (int i; i <= _N_Delays; i++) //"i" is not explicitly initialized here
for (int i = 0; i <= _N_Delays; i++) //but here it is

@Danois90, correct!

But now I spot another error… ‘_N_Delays’ is with random(5, 10); so largest it can be is 9. That’s why I suggested to make ‘_Delays’ 9 big.

BUT

for (int i = 0; i <= _N_Delays; i++)

Goes to 10 is ‘_N_Delays’ is indeed set to 9 because of <=

So logical fix:

for(byte i = 0; i < _N_Delays; i++)

Also changed to byte because it’s more then large enough to hold all possible delay sizes.