if loop not triggering

Can someone please point out where I am wrong please';

I had expected that the code in the first instance of the "if" code would trigger when the millis time first reached the millis(10000) but it never triggers??

Where have I gone wrong?

 long interval10 = 0; 
 long interval11 = 0; 
 long interval12 = 0; 
 long interval13 = 0; 
 long interval14 = 0;
 long interval15 = 0; 
 long interval16 = 0; 
 long interval17 = 0; 
 

unsigned long previousTime_10 = 0;
unsigned long previousTime_11 = 0;
unsigned long previousTime_12 = 0;
unsigned long previousTime_13 = 0;
unsigned long previousTime_14 = 0;
unsigned long previousTime_15 = 0;
unsigned long previousTime_16 = 0;
unsigned long previousTime_17 = 0;

int station  = 0;
int interval = 0;
void setup() {
  Serial.begin(9600);
}
 
void loop() 
{
          unsigned long currentTime = millis();
 
 station =10;interval10=10000;
 Serial.print(currentTime); Serial.print( " /  "); Serial.print(previousTime_10); 
 Serial.print("  /  "); Serial.print(currentTime-previousTime_10);
 Serial.print("  /  "); Serial.println(interval10);delay(100);
 
  
       if ((station==10)&&(interval>0) &&( currentTime - previousTime_10 >= interval10))
    {
    Serial.println(" 10 ");delay (3000);
    previousTime_10 = currentTime;
    }

        if ((station==11)&&(interval>0) &&( currentTime - previousTime_11 >= interval11))
    {
    Serial.println(" 11 ");
    previousTime_11 = currentTime;
    }
 
      if ((station==12)&&(interval>0) &&( currentTime - previousTime_12 >= interval12))
    {
    Serial.println(" 12 ");
    previousTime_12 = currentTime;
    }

       if ((station==13)&&(interval>0) &&( currentTime - previousTime_13 >= interval13))
    {
    Serial.println(" 13 ");
    previousTime_13 = currentTime;
    }
       if ((station==14)&&(interval>0) &&( currentTime - previousTime_14 >= interval14))
    {
    Serial.println(" 14 ");
    previousTime_14 = currentTime;
    }
       if ((station==15)&&(interval>0) &&( currentTime - previousTime_15 >= interval15))
    {
    Serial.println(" 15 ");
    previousTime_15 = currentTime;
    }
       if ((station==16)&&(interval>0) &&( currentTime - previousTime_16 >= interval16))
    {
    Serial.println(" 16 ");
    previousTime_16 = currentTime;
    }
       if ((station==17)&&(interval>0) &&( currentTime - previousTime_17 >= interval17))
    {
    Serial.println(" 17 ");
    previousTime_17 = currentTime;
    }
  }

Your if() statement depends on several variables, one of them being 'interval' which always has the value 0 so it never executes. You do assign a value to 'interval10' which is a completely different variable.

Based on your code, you would do well to study arrays and use them. E.g. 'station[stationNumber' and 'interval[intervalNumber]' etc.

@Farticus, I reckon you have been around long enough now to know that IF is not a loop.

...R

Thanks BLH64. So simple when you point it out.

I appreciate your comments re "arrays".

Will have a go at using them.

Thanks again.

PS Robin. Ive been shufflin around on this mortal coil for over 79 years and still call lots of things by

incorrect names so I hope you can forgive me my terminological mistakes, but I am still pretty new at this

Arduino stuff.After Ive been at it few more years hopefully I might improve. :slight_smile: :slight_smile: :slight_smile:

Farticus:
After Ive been at it few more years hopefully I might improve. :slight_smile: :slight_smile: :slight_smile:

I hope you have at least 20 years in which to polish your programming. :slight_smile:

...R

Have tried out the "arrays" but I am not sure it is of any advantage for my original code.

I have used "arrays" in the attached Millis section of the timing sketch and it does seem

to do what I want it to do, but I am unsure how the use of "arrays" is any better or worse than

not using "arrays".Perhaps you could comment on this?

My project is an attempt to create a programmed irrigation controller.

I could go out and buy a "store bought" one but where's the fun in that.

My overall problem is one of scale.

The controller will control 15 stations all with different on and off times and different durations and

later on a mixture control function.

Each station must have a manual on off function also.

The overall program will be controlled by a master Arduino and wirelessly transmitted to a

slave Arduino which will instigate the function control.

I have broken up the project into sections and with the kind assistance from people like

youselves,have got most of the individual sections working on 5 stations.

My question today is seeking comment and advice on the Millis timing section of the code.

I have tried to use the "array" function just for practise.

Basically this section will receive 2 variables containing a station number and a time duration.

The function should be able to keep track of up to 15 stations and time durations and "Turn Off"

each station at the expiration of the "time duration" period.

Would appreciate your help and advice.

Thanks

 long intervaL[16];

 long interval10 = 0; long interval11 = 0; long interval12 = 0; long interval13 = 0; 
 long interval14 = 0; long interval15 = 0;long interval16 = 0; long interval17 = 0; 
 

unsigned long previousTime_10 = 0;unsigned long previousTime_11 = 0;unsigned long previousTime_12 = 0;
unsigned long previousTime_13 = 0;unsigned long previousTime_14 = 0;unsigned long previousTime_15 = 0;
unsigned long previousTime_16 = 0;unsigned long previousTime_17 = 0;

int station  = 0;
int interval = 0;
void setup() {
  Serial.begin(9600);
}
 
void loop() 
{
 unsigned long currentTime = ((millis())/60000);
 //data received Stat No 10,Time on 30minutes,And on/off (0/1)
 //data received Stat No 11,Time on 15minutes,And on/off (0/1)
 //data received Stat No 12,Time on 240minutes,And on/off (0/1)
//data received Stat No 13,Time on 4minutes,And on/off (0/1)
 //data received Stat No 14,Time on 12minutes,And on/off (0/1)
 intervaL[0] = 2;
 intervaL[1] = 5;
 intervaL[2] = 4;
 intervaL[3] = 1;
 intervaL[4] = 3;
   if( currentTime>(previousTime_10 + intervaL[0]) ) {
    Serial.println(" Station 10 ");     // Would also include a "digitalWrite  LOW" to turn off led and relay
    previousTime_10 = currentTime;
    }
     if( currentTime>(previousTime_11 + intervaL[1]) ) {
    Serial.println(" Station 11 ");     //                      "ditto"
    previousTime_11 = currentTime;
    }
     if( currentTime>(previousTime_12 + intervaL[2]) ) {
    Serial.println(" Station 12 ");    //                      "ditto"
    previousTime_12 = currentTime;
    }
       if( currentTime>(previousTime_13 + intervaL[3]) ) {
    Serial.println(" Station 13 ");    //                      "ditto"
    previousTime_13 = currentTime;
    }
     if( currentTime>(previousTime_14 + intervaL[4]) ) {
    Serial.println(" Station 14 ");    //                      "ditto"
    previousTime_14 = currentTime;
    }
    }

You should use subtraction instead of addition in your millis() math to avoid problems when the millis() counter rolls over from 4294967296 to 0 every 49 days 17 hours 2 minutes and 47.296 seconds. :slight_smile:

 if( currentTime - previousTime_10 > intervaL[0]){
    previousTime_10 = currentTime;
    Serial.println(" Station 10 ");     // Would also include a "digitalWrite  LOW" to turn off led and relay
  }

Google “millis rollover”.

  1. Don't put two statements on one line. While it's legal just to put ";" between two statements, it's not a good idea.

  2. Use the auto-format function in the Arduino IDE (Tools menu.) If it appears to scramble your code then the code was wrong.

You have 15 stations with 15 intervals with 15 "previous". Those should all be arrays. Any time you have numbers in your variable names, you should use an array instead.

JCA79B's solution to the millis rollover problem won't work here. The code that both of you have written will fail after 49 days. Keep JCA79B's advice but only use milliseconds in intervals. If you want an interval of 5 minutes, then multiply 5601000UL (The UL forces it to use unsigned long in the intermediate calculation, which is not the default.)

When you have more than a few variables that are the same except for a suffix, it’s time to put them in an array:

const byte StationCount = 8;
const byte StationNumbers[StationCount] = {10, 11, 12, 13, 14, 15, 16, 17};
unsigned long MinuteIntervals[StationCount] =  {2, 5, 4, 1, 3, 0, 0, 0};
const unsigned long MillisecondsPerMinute = 60000UL;
unsigned long PreviousTime[StationCount];  // (globals default to zero)

void setup()
{
  Serial.begin(9600);
}

void loop()
{
  unsigned long currentTime = millis();

  for (byte station = 0; station < StationCount; station++)
  {
    unsigned long interval = MinuteIntervals[station] * MillisecondsPerMinute;
    if (currentTime -  PreviousTime[station] >= interval)
    {
      PreviousTime[station] += interval;
      Serial.print(" Station ");
      Serial.print(StationNumbers[station]);
      Serial.println(' ');
      // digitalWrite(StationPins[station], LOW);
    }
  }
}

John Thank you for your excellent reply.

When I see it up and running I think I understand a bit better.

I have a few questions

("const byte StationCount = 8;")
What is the reason to use "byte" in lieu of "int"?

("PreviousTime[station] += interval;")
Is the use of ("+=") just basically short for
(PrevT = PrevT + interv) or does it have a specific function?

("PreviousTime[station] += interval;")
Using "interval" to update "PreviousTime"
rather than " Millis or currentTime"?
Is this something to do with the Millis
"Overflow"?

Thanks

byte, 00001000, only 8 bits needed.
Why use int 0000000000001000 for the same?
byte will hold 0 to 255 decimal just fine.

int needs 2 bytes, one of which is all 0s.

Time values, use unsigned long for them all.
millis() returns unsigned long.

("PreviousTime[station] += interval;")
Using "interval" to update "PreviousTime"
rather than " Millis or currentTime"?
Is this something to do with the Millis
"Overflow"?

If something happens at a particular time and you want it to happen again after an interval you add the required period to the time that the event occurred to get the time of the next event.

Thanks Guys for your response.

Appreciated

Farticus:
PS Robin. Ive been shufflin around on this mortal coil for over 79 years and still call lots of things by

incorrect names so I hope you can forgive me my terminological mistakes, but I am still pretty new at this

Arduino stuff.After Ive been at it few more years hopefully I might improve. :slight_smile: :slight_smile: :slight_smile:

That's got to be worth ++Karma :slight_smile:

Farticus:
I have a few questions

(“const byte StationCount = 8;”)
What is the reason to use “byte” in lieu of “int”?

Memory on an Arduino UNO is a very limited resource so for small positive numbers, like counts and Pin numbers, I like to use ‘byte’. In this case it probably won’t make any difference since the compiler is smart enough to treat a small constant as a byte.

Farticus:
(“PreviousTime[station] += interval;”)
Is the use of ("+=") just basically short for
(PrevT = PrevT + interv) or does it have a specific function?

It’s a shortcut built into the C/C++ language since “lvalue = lvalue operator expression” is so common. It also reduces coding errors because it eliminates having to type the lvalue (a value that can appear on the left side of an assignment) twice. It also saves typing because “PreviousTime[station]” is not trivial to type. :slight_smile: . It works for +=, -=, *=, /=, %= (modulo), <<= (shift left), >>= (shift right), &=, |=, ^= (bitwise AND, OR and XOR)

Farticus:
(“PreviousTime[station] += interval;”)
Using “interval” to update “PreviousTime” rather than Millis or currentTime"?
Is this something to do with the Millis “Overflow”?

Nothing to do with overflow. If your loop gets delayed, this method will get your timing back on track. Say you start your sketch on the minute and have a timer that repeats every minute (60000 milliseconds). If your loop gets delayed by 100 milliseconds (0.1 second) your action will trigger in 60100 milliseconds: 0.1 seconds after the minute. If you use currentTime or millis() the next trigger will be in another 60000 which will again be 0.1 seconds after the minute. Each time the loop hits a delay the timing will drift. If, instead of currentTime or millis() you add the interval, you are setting the start time to the time the trigger SHOULD have happened (0.1 seconds ago) and the next trigger will be 60000 milliseconds from when the previous trigger SHOULD have happened, which will be in another 59900 milliseconds, right on the minute!

Note: That is also why I declared a local variable named ‘interval’. I needed the value twice: once in the trigger detection and once in the timer update.

johnwasser:
Memory on an Arduino UNO is a very limited resource so for small positive numbers, like counts and Pin numbers, I like to use 'byte'. In this case it probably won't make any difference since the compiler is smart enough to treat a small constant as a byte.

Actually, the compiler is clever enough to completely remove the const variable and replace it with the actual value so it really doesn't matter if you make your const variables byte or int. They both will not consume any of the precious UNO memory as they will be encoded into the code.

Thanks John.

I have been playing around with the sample of code you submitted
trying to stop the “IF” statement from triggering unless there is
a number greater than (“0”) in the “interval” variable.

I had thought by adding a conditional (&&) to the mix,that the logic of the
“IF” statement would fail unless the (&&) condition was met.This does not seem to work and I dont understand my error.

In my ultimate code the variable (“interval”) would only be supplied within the
“void loop” so I only want the (“if”) statment actioned ONLY if there is a value
in the (“interval”) variable.

Forgive the waffling I hope you understand my meaning.

How can I get the (“IF”) statement to do this??

thanks

const byte StationCount = 8;
const byte StationNumbers[StationCount] = {10, 11, 12, 13, 14, 15, 16, 17};
unsigned long MinuteIntervals[StationCount] =  {0, 0, 0, 0, 0, 0, 0, 0};
const unsigned long MillisecondsPerMinute = 60000UL;
unsigned long PreviousTime[StationCount];  // (globals default to zero)

void setup()
{
  Serial.begin(9600);
}

void loop()
{
  unsigned long currentTime = millis();

  for (byte station = 0; station < StationCount; station++)

  {
    unsigned long interval = MinuteIntervals[station] * MillisecondsPerMinute;
   

    if ((currentTime -  PreviousTime[station] >= interval) [u]&& (interval > 0)[/u]);
    {
      PreviousTime[station] += interval;
      Serial.print(" Station ");
      Serial.print(StationNumbers[station]);
      Serial.println(' ');
      // digitalWrite(StationPins[station], LOW);

      delay (1000);
    }
  }
}
if ((currentTime -  PreviousTime[station] >= interval) && (interval > 0));

Loose the semicolon at the end of the line.

This is a common error, and Nick Gammon has enshrined it as Trap #13 in his Tips, Traps, and Style Guide.
http://www.gammon.com.au/tips

Thanks Cattledog.

I make this mistake once a day and usually figure it out but this time I guess

i’ll put it down to “brain freeze”.

Tx again

Hi again. Could someone please point out my “idiot” mistake for the day?

I am trying to get the code to cease transmitting when the parameters are reached,

“” if ((mn1 < 6) && ((datasent[0]) < 15)) {
transMit();
} “”

but even when parameters are exceeded it continues to transmit the "last Transmission

over and over.

I have had to truncate the code to fit the “limits” of 9000 characters.

Would appreciate any help.

Thanks

void transMit() {
  radio.stopListening();
  Serial.print("        MN1   IS  "); Serial.print(mn1);
  Serial.print("    DA[0]   IS  "); Serial.println(datasent[0]);

  if (radio.write( &datasent, sizeof(datasent) ))
    Serial.print ("SENT  ... "); Serial.print (datasent[0]); Serial.print ("/"); Serial.print (datasent[1]);
  Serial.print ("/"); Serial.print (datasent[2]); Serial.print ("/"); Serial.print (datasent[3]);
  Serial.print ("/"); Serial.print (datasent[4]);
  digitalWrite (6, HIGH); delay(500); digitalWrite(6, LOW);
  radio.startListening();
}
mn1 ++;
  if (mn1 > 300) {
    mn1 = 0;
  }
  if (mn1 < 6) {




    if (mn1 == 1)         {
      datasent[0] = 10;
      datasent[1] = 1;
      datasent[2] = 0;
    }

    if (mn1 == 2)             {
      datasent[0] = 11;
      datasent[1] = 2;
      datasent[2] = 0;
    }

    if  (mn1 == 3)          {
      datasent[0] = 12;
      datasent[1] = 3;
      datasent[2] = 0;
    }

    if (mn1 == 4)          {
      datasent[0] = 13;
      datasent[1] = 4;
      datasent[2] = 0;
    }

    if  (mn1 == 5)            {
      datasent[0] = 14;
      datasent[1] = 4;
      datasent[2] = 0;
    }



  }







  //,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,NRF24,,,,,,,,,,,,,,,,,,,,,,,,




  //,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,TRANSMITTING,,,,,,,,,,,,,,,,,,,,,,,,

  Serial.print("        MN1   IS  "); Serial.print(mn1);
  Serial.print("    DA[0]   IS  "); Serial.println(datasent[0]);

  if ((mn1 < 6) && ((datasent[0]) < 15)) {
    transMit();
  }