runtime error... memory overload?

my programm keeps crashing and I don't have a clue why. Does anyone spot an error in this code?
Thanks a lot

void runTimeLapse(){
  long elapsedTimeUntilNow = millis();

  int totalPhotos = ceil(runFor / (timeBetween + triggerFor));
  int photosTaken = 0;
  boolean trigger = false;
  int elapsedTime = 0;
  long startOfNextPhoto = 0;
  int photoLength = timeBetween + triggerFor;

  while((millis() - elapsedTimeUntilNow) / 1000 < runFor){
    elapsedTime = (millis() - elapsedTimeUntilNow) / 1000;
    startOfNextPhoto = photosTaken * photoLength;
    if(elapsedTime >= startOfNextPhoto && elapsedTime < startOfNextPhoto + triggerFor){
      trigger = true;
      printRemainingTime(0,1);
      digitalWrite(triggerPin,HIGH);
      lcd.setCursor(15,1);
      lcd.print("X");
    }
    else{
      if(trigger){
        photosTaken++;
        printPhotosTaken(photosTaken, totalPhotos);
      }
      digitalWrite(triggerPin,LOW);
      lcd.setCursor(15,1);
      lcd.print(" ");
      trigger = false;
      printRemainingTime(startOfNextPhoto - elapsedTime, 1);
    }
    printRemainingTime(runFor - ((millis() - elapsedTimeUntilNow) / 1000), 0);
  }
}

Well, for one thing, elapsed timeuntil now should be unsigned long to match millis() output and prevent a sudden switch to a negative number.

I was under the impression you should use, unsigned long

long elapsedTimeUntilNow = millis();

Sorry, I can't help any more than this.

gonna give this a try, thanks for the fast answers

I see a bunch of mixing of types. I also don't see all the code. All variables that I use in calculations involving millis() I declare as unsigned long. Any constants I use in such calculations get UL at the end.

I see some division by 1000 to compare to runFor. If runFor was multiplied by 1000 then this division wouldn't be needed. I'm guessing runFor represents seconds. It would be better to have it represent millis().

That's some stuff I see but I'm just a hack. Some smarter than me guys will no doubt find some other issues.

still crashing...
if I comment this line the program runs fine

//startOfNextPhoto = photosTaken * photoLength;

ideas? :slight_smile:

Use:
Serial.println(photosTaken); //what do you get?
Serial.println(photoLength; //what do you get?
//startOfNextPhoto = photosTaken * photoLength;

startOfNextPhoto = photosTaken * photoLength;

If you anticipate that photosTaken * photoLength will be greater than the value of an int (32,767) then I think you need to tell the compiler to work with long values in the multiplication:

startOfNextPhoto = (long)photosTaken * (long)photoLength;

Otherwise the result of the multiplication will be an int rather than a long.

Of course unless you are using negative numbers you might as well use unsigned ints and unsigned longs.

LarryD:
Use:
Serial.println(photosTaken); //what do you get?
Serial.println(photoLength; //what do you get?
//startOfNextPhoto = photosTaken * photoLength;

I get 1 and 12...

the cast didn't work either

startOfNextPhoto = (long)photosTaken * (long)photoLength;

man this starts to get awkward.. would it help if I post the whole code?

would it help if I post the whole code?

Ten hours into the thread, what do you think?

I think I got the problem...

void printPhotosTaken(int photosTaken, int totalPhotos){
    lcd.setCursor(9,0);
    lcd.print(String(photosTaken) + "/" + String(totalPhotos));
}

if I comment out this function it works... so what's the problem with this? the String(..) convertion?

if I comment out this function it works... so what's the problem with this? the String(..) convertion?

I'd like to have a nickel for every time we've had to tell people that the String class has serious problems and should not be used until they are fixed.

Using the String class to avoid having three calls to lcd.print() is the height of laziness.

PaulS:

if I comment out this function it works... so what's the problem with this? the String(..) convertion?

I'd like to have a nickel for every time we've had to tell people that the String class has serious problems and should not be used until they are fixed.

Using the String class to avoid having three calls to lcd.print() is the height of laziness.

how should I know? :wink:

I wouldn't say the height of laziness but the height of having clean code which is as short as possible...

but the height of having clean code which is as short as possible..

But when the underlying code is known to be grubby, the dirt can show through.

I wouldn't say the height of laziness but the height of having clean code which is as short as possible...

I'd agree with the clean code bit. "As short as possible" I do not agree with. "As short as is reasonable" I'd agree with.

    lcd.print(photosTaken);
    lcd.print("/");
    lcd.print(totalPhotos);

is hardly going to break the "lines of code" bank. It uses far less resources, and is NOT prone to random crashing.

But, hey, it's your choice.

yes you're right the as short as possible is wrong! but you know what I mean and I didn't know that there is a risk of using String()
Anyways, thanks for helping :slight_smile: