0
Offline
Jr. Member
Karma: 0
Posts: 58
Arduino rocks
|
 |
« on: January 23, 2013, 06:09:32 pm » |
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); } }
|
|
|
|
|
Logged
|
|
|
|
|
Offline
Sr. Member
Karma: 7
Posts: 386
|
 |
« Reply #1 on: January 23, 2013, 06:12:50 pm » |
Well, for one thing, elapsed timeuntil now should be *unsigned* long to match millis() output and prevent a sudden switch to a negative number.
|
|
|
|
|
Logged
|
|
|
|
|
Offline
Sr. Member
Karma: 12
Posts: 327
The last thing you did is where you should start looking.
|
 |
« Reply #2 on: January 23, 2013, 06:15:15 pm » |
I was under the impression you should use, unsigned long long elapsedTimeUntilNow = millis();
Sorry, I can't help any more than this.
|
|
|
|
|
Logged
|
|
|
|
|
0
Offline
Jr. Member
Karma: 0
Posts: 58
Arduino rocks
|
 |
« Reply #3 on: January 23, 2013, 06:23:27 pm » |
gonna give this a try, thanks for the fast answers
|
|
|
|
|
Logged
|
|
|
|
|
Saskatchewan
Offline
Full Member
Karma: 10
Posts: 222
When the going gets weird, the weird turn pro. - Hunter S. Thompson
|
 |
« Reply #4 on: January 23, 2013, 06:26:29 pm » |
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.
|
|
|
|
|
Logged
|
|
|
|
|
0
Offline
Jr. Member
Karma: 0
Posts: 58
Arduino rocks
|
 |
« Reply #5 on: January 23, 2013, 06:27:37 pm » |
still crashing... if I comment this line the program runs fine //startOfNextPhoto = photosTaken * photoLength;
ideas? :-)
|
|
|
|
|
Logged
|
|
|
|
|
Offline
Sr. Member
Karma: 12
Posts: 327
The last thing you did is where you should start looking.
|
 |
« Reply #6 on: January 23, 2013, 06:35:20 pm » |
Use: Serial.println(photosTaken); //what do you get? Serial.println(photoLength; //what do you get? //startOfNextPhoto = photosTaken * photoLength;
|
|
|
|
|
Logged
|
|
|
|
|
0
Offline
Full Member
Karma: 1
Posts: 222
Arduino rocks
|
 |
« Reply #7 on: January 23, 2013, 06:55:57 pm » |
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.
|
|
|
|
|
Logged
|
|
|
|
|
0
Offline
Jr. Member
Karma: 0
Posts: 58
Arduino rocks
|
 |
« Reply #8 on: January 24, 2013, 04:53:26 am » |
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?
|
|
|
|
|
Logged
|
|
|
|
|
Global Moderator
UK
Online
Brattain Member
Karma: 137
Posts: 19021
I don't think you connected the grounds, Dave.
|
 |
« Reply #9 on: January 24, 2013, 05:10:25 am » |
would it help if I post the whole code? Ten hours into the thread, what do you think?
|
|
|
|
|
Logged
|
Pete, it's a fool looks for logic in the chambers of the human heart.
|
|
|
|
0
Offline
Jr. Member
Karma: 0
Posts: 58
Arduino rocks
|
 |
« Reply #10 on: January 24, 2013, 05:34:58 am » |
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?
|
|
|
|
|
Logged
|
|
|
|
|
Seattle, WA USA
Offline
Brattain Member
Karma: 311
Posts: 35478
Seattle, WA USA
|
 |
« Reply #11 on: January 24, 2013, 05:45:41 am » |
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.
|
|
|
|
|
Logged
|
|
|
|
|
0
Offline
Jr. Member
Karma: 0
Posts: 58
Arduino rocks
|
 |
« Reply #12 on: January 24, 2013, 06:01:58 am » |
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? ;-) I wouldn't say the height of laziness but the height of having clean code which is as short as possible...
|
|
|
|
|
Logged
|
|
|
|
|
Global Moderator
UK
Online
Brattain Member
Karma: 137
Posts: 19021
I don't think you connected the grounds, Dave.
|
 |
« Reply #13 on: January 24, 2013, 06:06:19 am » |
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.
|
|
|
|
|
Logged
|
Pete, it's a fool looks for logic in the chambers of the human heart.
|
|
|
|
Seattle, WA USA
Offline
Brattain Member
Karma: 311
Posts: 35478
Seattle, WA USA
|
 |
« Reply #14 on: January 24, 2013, 06:15:00 am » |
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.
|
|
|
|
|
Logged
|
|
|
|
|
|