Basic If statement issue in a large program

Projects Programming
1

See attached programme.

This is a very much cut down version of the whole program, but it is still bugging me.

On line 323 "if (millis() >= testing)" for some reason this If statement is not If-ing when Millis() is greater than or equal to "testing" until much later.

A few lines before I have set testing to be 4000 larger than the then current millis(), which is located inside an "if autoApos == 2". Then autoApos is changed to 3 and the next If statement should run if autoApos == 3 && millis() >= testing.

What I am finding is that millis() can be 40000 when "testing" is 30000 (ish) before the If statement runs. I have tried many variants but I always get the same result, millis() is always 10000 to 20000 larger than the "testing" value.

No doubt someone will spot the stupid and obvious error instantly, my eyes are still fuzzy after writing the 30k programme (only 9.4k uploaded, the rest isn't in use at this point).

I have been using Serial.print to tell me the values of different variables, out of all the different parts it only appears to be the If statement on line 323 which doesn't add up.

If I remove the millis() if statement and just have the "if autoApos == 3" it works fine (but instantly).

The final idea is to have a random time form 3 to 10 seconds for the delay.

Another interesting thing is when the time delay If was inside the autoApos == 3 If, I had a Serial.print after the dodgy combined If autoApos && delay statement which should have ran all the time for testing (causing the MCU to run slow). This would work fine until autoApos == 3, where it would stop until the If statement was satisfied. This made no sense as the Serial.print was after the If statement (outside completely) and would otherwise run except if the If statement was 1/2 true (when autoApos == 3).

I know the If statements should me If (previous_time <= millis() - number). I've currently written them with a + for simplicity. This program will not run for more than 10 hours.

To_upload.ino (57.2 KB)

2

On line 323 "if (millis() >= testing)" for some reason this If statement is not If-ing when Millis() is greater than or equal to "testing" until much later.

If statements don't if. They evaluate to true or false.

Do you have some proof of this assertion?

A few lines before I have set testing to be 4000 larger than the then current millis(), which is located inside an "if autoApos == 2". Then autoApos is changed to 3 and the next If statement should run if autoApos == 3 && millis() >= testing.

Addition of unsigned longs is not guaranteed, and is not a good idea. Record when events happen, and see if now minus then exceeds some interval. Do not try to define when the next event should happen.

3

Right, I've just tried this:

the If statement is now "if (testing <= millis() - 4000)"

"testing" being the "before millis()"

Roughly once every 0.7 seconds I get Serial data of what the millis() is and what "testing" is set to.

Once the If statement is True, I get another set of serial data telling me the same info from inside the If statement:

millis in Loop: 28825 - Programme running nicely
testing in Loop: 0
millis in Loop: 29447

Bulb A0 Max Vol - "Max Vol" Reached, which means autoApos = 3
Bulb A0 down in ms

testing in Loop: 29877
millis in Loop: 30067
testing in Loop: 29877
millis in Loop: 30681
testing in Loop: 29877
millis in Loop: 31292
testing in Loop: 29877
millis in Loop: 31904

*continues to count up as you would expect

millis in Loop: 47816
testing in Loop: 29877
millis in Loop: 48428
testing in Loop: 29877
millis in Loop: 49041
testing in Loop: 29877
millis in Loop: 49652

Bulb A0 ran ok, Bulb A0 Finished. Clear - At this point the If statement is now true.

testing in If: 29877 - This is "testing" in the If statement
millis in If: 49892 - This is millis() in the If statement

testing in Loop: 29877
millis in Loop: 50284
testing in Loop: 29877
millis in Loop: 50896
testing in Loop: 29877
millis in Loop: 51508

My problem is that the If statement is "if (testing <= millis() - 4000)"

Unless my maths is wrong, 29877 <= 49892 - 4000 is true, but it should have been true over 16 seconds ago. The difference of 16015 is a common thing I keep seeing.

Anyone?

4

My problem is that the If statement is "if (testing <= millis() - 4000)"

I'd restructure that statement:

if(millis() - testing >= 4000)

and see what happened.

5

PaulS:

My problem is that the If statement is "if (testing <= millis() - 4000)"

I'd restructure that statement:

if(millis() - testing >= 4000)

and see what happened.

Done that, still the same. Got a time difference of 20015 this time (should be 4000).

6

Am I the only one who can't see any code?

7

Am I the only one who can't see any code?

Have you looked in the attached file? Loads of it in there!

8

Can you post your amended code, as you seem to have changed it?

9

I have amended the code - see attached.

If no one can think of any reason why it is doing what it is doing I will start taking it to bits at piece at a time. I'll be kicking myself when I see the fault I know it.

To_upload_2.ino (57.2 KB)

10

I would highly recommend you to walk through the code and add parenthesis in all your if( ...) where you do complex comparisons with calcs.

example:
if(modeType == 3 && millis() - modeTimer >= 1000)
if( (modeType == 3) && ( (millis() - modeTimer) >= 1000 ) )
..
if(millis() >= autoAa + autoRuntime - rndTimeA || autoAmax >= cost * AlightMax)
if( (millis() >= (autoAa + autoRuntime - rndTimeA)) || (autoAmax >= (cost * AlightMax)) )
..
etc.
11

I have added parenthesis before and I have known it to fail because of them, but its worth a go.

12

Do it everywhere where you do &&ing or ||ing or where you do any calcs/comparisions in all your ifs(). It increases readability much and makes the avr-gcc much happier as well :).

13

I have added parenthesis before and I have known it to fail because of them

Examples?

14

Examples?

Sadly I don't have any to show as I removed them. I'll post some next time I come across them.

It may have been when I used double brackets when a single would have done the same job.

Or it could have been along the lines of

if (something == (a + (b * c)))

I remember removing the bracket before "a" and one off the end and it would work.

15

I can't help thinking this line is definitely wrong:

      if (testing <= millis() - 4000)

Consider if millis() is currently 1000. Now we have:

      if (testing <= 1000 - 4000)

Which is:

      if (testing <= -3000)

However testing (and the result from millis() ) is unsigned long so we really have:

      if (testing <= 4294964296)

Which will be true immediately.

16

Very true Nick, however "testing" will never be greater than 20,000 and millis() will always be greater at this point due to a 20 second delay to avoid such an issue.

17

You are adding a 20 second delay to work around this problem? Hmmm. Why not just code it another way?

For example:

  if (millis() - testing >= 4000)

Your objective should be to write code that "always works". Not code that requires a 20-second delay at the start, and can't run for more than 3 hours, or some such limitations.

18

That is the overall objective, but to try and sort out the strange issue of the If statement not working properly I have made it simple.

Once running properly I will use the millis() function to set the "testing" variable so it will be correct. The 20 second delay is actually for the external hardware, which is out of my control.

19

I don't know about the others, but I'm not interested in trying to work why code, that is obviously wrong, fails. I suggest you fix up that line and we can take it from there. I'm not asking you to re-code the whole thing.

20

That is a perfectly valid argument.

Code attached.

The code is now "if (timeout <= millis() - bulbAtimer)"

You'll notice that timeout = millis() just above it, and "bulbAtimer" is a random number generated between 3 and 10, then multiplied by 100 to give 3000 to 10000 (3 to 10 seconds).

To_upload_3.ino (57.1 KB)