Strange behaviour of "if" statement

Hello,

I'm trying to perfect the date calculation code of my clock. While the clock has no problem when I increase the days (i.e. they eventually fall back to 1 and months are increased by 1), I get into troubles when I decrease them : sometimes it works correctly (i.e. days fall back to 31 or 31 or 28 or 29, depending on the month), but sometimes days get negative, even if I included a statement preventing this. It's really weird.

Here is the code. Everything is in the loop. The "days", "ClockMonthSet" and "ClockYearSet" are set with inputs earlier in the code. The relevant (and non-working) variables are "Jour", "Mois", and "Annee" :

// DATE CALCULATION /////////////////////////////////
// (non-working with decreasing days) ///////////////

  int Jour = days;       //We define new variables to play with without affecting what's above.
  int Mois = ClockMonthSet;
  int Annee = ClockYearSet;
  
  //Number of days in a month:
  byte February;
    if(Annee%4==0)  February=29;
    else  February=28;
  byte MonthArray[13] = {0,31,February,31,30,31,30,31,31,30,31,30,31};  //we'll skip the first entry, so we can use the array's index directly.
  
  //Defining Month incrementation.
  for(byte i=1;i<13;i++){  
    if(Mois==i){
      if(Jour>MonthArray[i]){  //If "Jour" exceeds the number of days in the present month, it is resetted to 1 and we step into the next month.
        Jour -= MonthArray[i];
        Mois++;
      }
      if(Jour<1){    //Similarly if "Jour" is to become 0.
        if(i-1==0)  i=13;  //Before January comes December.
        Jour += MonthArray[i-1];
        Mois--;
      }
    }
  }
    
  //Defining years incrementation.
  if(Mois>12){
    Annee++;
    Mois-=12;
  }
  if(Mois<1){
    Annee--;
    Mois+=12;
  }

The "for" statement at the beginning might seem odd, but this is the solution I found to fix my previous code, which had a bigger problem : the "if" statement seemed to be checked only once. For the first month, things worked nice, but for the second month and after, days would increase with no limits, regardless of my "if" :

// DATE CALCULATION /////////////////////////////////
// (not working at all past the first month) ////////

  int Jour = days;       //We define new variables to play with without affecting what's above.
  int Mois = ClockMonthSet;
  int Annee = ClockYearSet;
  
  //Number of days in a month:
  byte February;
    if(Annee%4==0)  February=29;
    else  February=28;
  byte MonthArray[13] = {0,31,February,31,30,31,30,31,31,30,31,30,31};  //we'll skip the first entry, so we can use the array's index directly.
  
  //Defining Month incrementation.
      if(Jour>MonthArray[Mois]){  //If "Jour" exceeds the number of days in the present month, it is resetted to 1 and we step into the next month.
        Jour -= MonthArray[Mois];
        Mois++;
      }
      if(Jour<1){    //Similarly if "Jour" is to become 0.
        Jour += MonthArray[Mois-1];
        Mois--;
      }
    
  //Defining years incrementation.
  if(Mois>12){
    Annee++;
    Mois-=12;
  }
  if(Mois<1){
    Annee--;
    Mois+=12;
  }

This is by no means urgent, as the clock keeps the date correct day after day (with the first code), but i'd like my code to work not only "apparently".

Any help appreciated.

You might get some help by looking here: Arduino Playground - HomePage

Thanks, I'll probably have the clock running with this library instead.

But I still would like to understand why my first code doesn't always work.

But I still would like to understand why my first code doesn't always work.

You said:

the "if" statement seemed to be checked only once.

And then posted code with half a dozen if statements. You'd like us to guess which one you are referring to?

Where are your Serial.print() statements? You could easily debug the code with some Serial.print() statements.

The problem is with this one :

      if(Jour>MonthArray[Mois]){  
        Jour -= MonthArray[Mois];
        Mois++;
      }

"Jour" are resetted to 1 (substracted by the amount specified in MonthArray) only once. But after, this doesn't happen : they keep increasing. That problem in particular was fixed in the first code via the loop statement but I'm still curious for an explanation.

But after, this doesn't happen : they keep increasing. That problem in particular was fixed in the first code via the loop statement but I'm still curious for an explanation.

It would certain be beneficial to print the values of Jour, Mois, and MonthArray[Mois]. The values may not be what you think they are.

Of course, without seeing all of your code, any suggestions we make are just guesswork, based on too little information. For instance, we can't see where that code is used. Is it in a function? How is the function called? What arguments does the function take? Are Jour and Mois global? Are they possibly assigned values elsewhere? Are there other arrays that are possibly being written beyond, that cause Jour and/or Mois to be modified?

It would certain be beneficial to print the values of Jour, Mois, and MonthArray[Mois]. The values may not be what you think they are.

Jour and Mois appear "on-screen" on the clock itself, so I see them. These are the very final variables (and they go wrong). MonthArray just tells the number of days in a month and isn't supposed to change (I've checked anyway to be sure -no change).

For instance, we can't see where that code is used. Is it in a function? How is the function called? What arguments does the function take?

As I said in my first post, everything happens in void.loop(), so this is just part of the code. (the only function used is for displaying the values on the clock).

Are Jour and Mois global?

I think they are.

Are they possibly assigned values elsewhere?

No, these numbers are just split between into tens and units for display (and this part is alright).

Are there other arrays that are possibly being written beyond, that cause Jour and/or Mois to be modified?

I don't think so.

I posted the entier code below (minus some non-relevant stuff -like alarm, mode selection, multiplexing, and void.setup() that just contain the pins information- to have a post under 9500 characters), but I think the whole story of Jour and Mois is in the piece of code I provided earlier. Jour is initialized form days (which take only positive values, by constuction), and Mois from ClockMonthSet, which can take negative values (it is an int that is modified with inputs).

unsigned long time = 0;       // Time from when we started.

// default time sets. Time will start at 12:24:45.  Date will start at 01/01/13. Default Alarm is 08:00.
long ClockHourSet = 12;
long ClockMinSet  = 24;
long ClockSecSet  = 45;
long ClockDaySet  = 1;
long ClockMonthSet= 1;
long ClockYearSet = 13;
int AlmHours = 8;       //Those are set directly, no need for time count.
int AlmMins = 0;

//Defining commands
byte Mode = 1;            //0=Sleep, 1=Time, 2=Date, 3=Alarm
int ModeTimer = 0;
long AlmMillis = 0;
byte Set = 0;             //0=Left, 1=Middle, 2=Right
byte ModeButtonPressed = false;
byte IncreaseButtonPressed = false;
byte DecreaseButtonPressed = false;
byte AlmButtonPressed = false;
byte LightCmdPin = false;
byte ALARM = false;
byte AlarmPlaying = false;

void loop()     
{
  // Get milliseconds.
  unsigned long runTime = millis();
  
// SETTING SELECTION ////////////////////////////////////////////////////
/////////////////////////////////////////////////////////////////////////
  int LeftInput = digitalRead(A2);
  int RightInput = digitalRead(A3);

  if(LeftInput==LOW && RightInput==HIGH)  Set = 0; //Left couple of tubes will be available for setting
  if(RightInput==LOW && LeftInput==HIGH)  Set = 2; //Right pair of tubes will be available for setting
  if(RightInput==HIGH && LeftInput==HIGH)  Set = 1; //Middle pair of tubes will be available for setting
  
// SETTING THE SELECTED PAIR OF TUBES ////////////////////////////////////
//////////////////////////////////////////////////////////////////////////
  int DecreaseInput = digitalRead(A4);  
  int IncreaseInput  = digitalRead(A5);

  if(DecreaseInput==0)    DecreaseButtonPressed = true;
  if(IncreaseInput==0)    IncreaseButtonPressed = true;
  
  if(DecreaseButtonPressed==true && DecreaseInput==1)
  {
    if(Set==0){
      if(Mode==1)  ClockHourSet--;
      if(Mode==2)  ClockDaySet--;
    }
    if(Set==1){
      if(Mode==1)  ClockMinSet--;
      if(Mode==2)  ClockMonthSet--;
      if(Mode==3)  AlmHours--;
    }
    if(Set==2){
      if(Mode==1)  ClockSecSet--;
      if(Mode==2)  ClockYearSet--;
      if(Mode==3)  AlmMins--;
    }
    DecreaseButtonPressed = false;
  }
  if(IncreaseButtonPressed==true && IncreaseInput==1)
  {
    if(Set==0){
      if(Mode==1)  ClockHourSet++;
      if(Mode==2)  ClockDaySet++;
    }
    if(Set==1){
      if(Mode==1)  ClockMinSet++;
      if(Mode==2)  ClockMonthSet++;
      if(Mode==3)  AlmHours++;
    }
    if(Set==2){
      if(Mode==1)  ClockSecSet++;
      if(Mode==2)  ClockYearSet++;
      if(Mode==3)  AlmMins++;
    }
    IncreaseButtonPressed = false;
  }

// TIME CALCULATION /////////////////////////////////
/////////////////////////////////////////////////////
  // Get time in seconds.
  long time = runTime / 1000; 
  time += ClockSecSet + 60*ClockMinSet + 3600*ClockHourSet + 86400*ClockDaySet;    //time in seconds, based on offset

  // Convert time to days,hours,mins,seconds
  long days  = time / 86400;    time -= days  * 86400; 
  long hours = time / 3600;   time -= hours * 3600; 
  long minutes  = time / 60;    time -= minutes  * 60; 
  long seconds  = time; 

  // Get the high and low order values for hours,min,seconds. 
  byte lowerHours = hours % 10;
  byte upperHours = hours - lowerHours;
  byte lowerMins = minutes % 10;
  byte upperMins = minutes - lowerMins;
  byte lowerSeconds = seconds % 10;
  byte upperSeconds = seconds - lowerSeconds;
  if( upperSeconds >= 10 )   upperSeconds = upperSeconds / 10;
  if( upperMins >= 10 )      upperMins = upperMins / 10;
  if( upperHours >= 10 )     upperHours = upperHours / 10;

// DATE CALCULATION /////////////////////////////////
// (non-working with decreasing days) ///////////////

  int Jour = days;       //We define new variables to play with without affecting what's above.
  int Mois = ClockMonthSet;
  int Annee = ClockYearSet;
  
  //Number of days in a month:
  byte February;
    if(Annee%4==0)  February=29;
    else  February=28;
  byte MonthArray[13] = {0,31,February,31,30,31,30,31,31,30,31,30,31};  //we'll skip the first entry, so we can use the array's index directly.
  
  //Defining Month incrementation.
  for(byte i=1;i<13;i++){  
    if(Mois==i){
      if(Jour>MonthArray[i]){  //If "Jour" exceeds the number of days in the present month, it is resetted to 1 and we step into the next month.
        Jour -= MonthArray[i];
        Mois++;
      }
      if(Jour<1){    //Similarly if "Jour" is to become 0.
        if(i-1==0)  i=13;  //Before January comes December.
        Jour += MonthArray[i-1];
        Mois--;
      }
    }
  }

  // Splitting.    
  byte lowerYears = Annee % 10;
  byte upperYears = Annee - lowerYears;
  byte lowerMonths = Mois % 10;
  byte upperMonths = Mois - lowerMonths;
  byte lowerDays = Jour % 10;
  byte upperDays = Jour - lowerDays;    
  if( upperDays >= 10 )   upperDays = upperDays / 10;
  if( upperMonths >= 10 )      upperMonths = upperMonths / 10;
  if( upperYears >= 10 )     upperYears = upperYears / 10;


// SHOWTIME /////////////////////////////////////////
/////////////////////////////////////////////////////
  // Fill in the Number array used to display on the tubes following the Mode.      
  if(Mode==1){
    NumberArray[3] = upperHours;
    NumberArray[1] = lowerHours;
    NumberArray[5] = upperMins;
    NumberArray[0] = lowerMins;
    NumberArray[4] = upperSeconds;  
    NumberArray[2] = lowerSeconds;
    DisplayFadeNumberString();    // Display time.
  }
  
  if(Mode==2){
    NumberArray[3] = upperDays;
    NumberArray[1] = lowerDays;
    NumberArray[5] = upperMonths;
    NumberArray[0] = lowerMonths;
    NumberArray[4] = upperYears;  
    NumberArray[2] = lowerYears;
    DisplayFadeNumberString();    //Display date.
  }
 
}

I posted the entier code below (minus some non-relevant stuff -like alarm, mode selection, multiplexing, and void.setup() that just contain the pins information- to have a post under 9500 characters)

Well, good luck figuring it out. I thought you wanted us to help. If you do, you will post ALL of the code. You can use the Additional Options link to attach code that exceeds the 9500 character limit.

If the sketch is big and includes a lot of code not relevant to the problem, it would be worth extracting the relevant code into a smaller test sketch that just demonstrates the problem, and post that. Preferably write the sketch to print its results, tell us what results you expected and show us the actual results.

PeterH:
If the sketch is big and includes a lot of code not relevant to the problem, it would be worth extracting the relevant code into a smaller test sketch that just demonstrates the problem, and post that. Preferably write the sketch to print its results, tell us what results you expected and show us the actual results.

That sounds like a good idea, i'm on it.

In the meantime, here is the entire current code, as PaulS requested.

6digit_SN74141_clock.txt (16.4 KB)

If the sketch is big and includes a lot of code not relevant to the problem, it would be worth extracting the relevant code into a smaller test sketch that just demonstrates the problem, and post that.

I second that. Often the process of doing so will trigger a "WTF was I thinking" moment that will eliminate the need to continue.

Here is the serial version. The date is printed on serial. To increase days, type D[increment].. From January to February, the code works fine, but afterwards you got things like 31/02/13.

int NumberArray[6] = {0,0,0,0,0,0};

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

void DisplayFadeNumberString() {
  Serial.print(NumberArray[3]);
  Serial.print(NumberArray[1]);
  Serial.print(":");
  Serial.print(NumberArray[5]);
  Serial.print(NumberArray[0]);
  Serial.print(":");
  Serial.print(NumberArray[4]);
  Serial.print(NumberArray[2]);
  Serial.print("\n");
  delay(200);
}
  
  
// Defines
long time = 0;       // Time from when we started.

// default time sets. Time will start at 12:14:50.  Date will start at 01/01/13.
long ClockHourSet = 12;
long ClockMinSet  = 14;
long ClockSecSet  = 50;
long ClockDaySet  = 1;
long ClockMonthSet= 1;
long ClockYearSet = 13;


////////////////////////////////////////////////////////////////////////
//
//
////////////////////////////////////////////////////////////////////////
void loop()     
{
  // Get milliseconds.
  unsigned long runTime = millis();
  
  //Settings
int DayIncr=0;

while(Serial.available()){
  if(Serial.findUntil("D","."))  DayIncr=Serial.parseInt();
}
  ClockDaySet+=DayIncr;
  
  
  // TIME CALCULATION /////////////////////////////////
  /////////////////////////////////////////////////////
  // Get time in seconds.
  long time = runTime / 1000; 
  time += ClockSecSet + 60*ClockMinSet + 3600*ClockHourSet + 86400*ClockDaySet;    //time in seconds, based on offset

  // Convert time to days,hours,mins,seconds
  long days  = time / 86400;    time -= days  * 86400; 
  long hours = time / 3600;   time -= hours * 3600; 
  long minutes  = time / 60;    time -= minutes  * 60; 
  long seconds  = time; 

// DATE CALCULATION /////////////////////////////////
// (non-working pas the first month) ///////////////
  int Jour = days;       //We define new variables to play with without affecting what's above.
  int Mois = ClockMonthSet;
  int Annee = ClockYearSet;
  
  //Number of days in a month:
  byte February;
    if(Annee%4==0)  February=29;
    else  February=28;
  byte MonthArray[13] = {0,31,February,31,30,31,30,31,31,30,31,30,31};  //we'll skip the first entry, so we can use the array's index directly.
  
  //Defining Month incrementation.
      if(Jour>MonthArray[Mois]){  //If "Jour" exceeds the number of days in the present month, it is resetted to 1 and we step into the next month.
        Jour -= MonthArray[Mois];
        Mois++;
      }
      if(Jour<1){    //Similarly if "Jour" is to become 0.
        Jour += MonthArray[Mois-1];
        Mois--;
      }
    
  //Defining years incrementation.
  if(Mois>12){
    Annee++;
    Mois-=12;
  }
  if(Mois<1){
    Annee--;
    Mois+=12;
  }
    
  // Splitting.    
  int lowerYears = Annee % 10;
  int upperYears = Annee - lowerYears;
  int lowerMonths = Mois % 10;
  int upperMonths = Mois - lowerMonths;
  int lowerDays = Jour % 10;
  int upperDays = Jour - lowerDays;    
  if( upperDays >= 10 )   upperDays = upperDays / 10;
  if( upperMonths >= 10 )      upperMonths = upperMonths / 10;
  if( upperYears >= 10 )     upperYears = upperYears / 10;

  // SHOWTIME  /////////////////////////////////
  //////////////////////////////////////////////
    NumberArray[3] = upperDays;
    NumberArray[1] = lowerDays;
    NumberArray[5] = upperMonths;
    NumberArray[0] = lowerMonths;
    NumberArray[4] = upperYears;  
    NumberArray[2] = lowerYears;
    DisplayFadeNumberString();    //Display date.

}

PaulS:

If the sketch is big and includes a lot of code not relevant to the problem, it would be worth extracting the relevant code into a smaller test sketch that just demonstrates the problem, and post that.

I second that. Often the process of doing so will trigger a "WTF was I thinking" moment that will eliminate the need to continue.

Agreed

I don't know if it's causing your problem, but I see that you are not wrapping your month number before you use it as an index into MonthArray. You need to handle the month number wrapping at 12 on increment, and at 1 on decrement, before you use it as an index into the array.

PeterH:
I don't know if it's causing your problem, but I see that you are not wrapping your month number before you use it as an index into MonthArray. You need to handle the month number wrapping at 12 on increment, and at 1 on decrement, before you use it as an index into the array.

I made the change, it makes indeed much more sense as you put it.

...But the glitch remains.

Post your (updated) code.

Yeah sorry, here it is :

int NumberArray[6] = {0,0,0,0,0,0};

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

void DisplayFadeNumberString() {
  Serial.print(NumberArray[3]);
  Serial.print(NumberArray[1]);
  Serial.print(":");
  Serial.print(NumberArray[5]);
  Serial.print(NumberArray[0]);
  Serial.print(":");
  Serial.print(NumberArray[4]);
  Serial.print(NumberArray[2]);
  Serial.print("\n");
  delay(200);
}
  
  
// Defines
long time = 0;       // Time from when we started.

// default time sets. Time will start at 12:14:50.  Date will start at 01/01/13.
long ClockHourSet = 12;
long ClockMinSet  = 14;
long ClockSecSet  = 50;
long ClockDaySet  = 1;
long ClockMonthSet= 1;
long ClockYearSet = 13;


////////////////////////////////////////////////////////////////////////
//
//
////////////////////////////////////////////////////////////////////////
void loop()     
{
  // Get milliseconds.
  unsigned long runTime = millis();
  
  //Settings
int DayIncr=0;

while(Serial.available()){
  if(Serial.findUntil("D","."))  DayIncr=Serial.parseInt();
}
  ClockDaySet+=DayIncr;
  
  
  // TIME CALCULATION /////////////////////////////////
  /////////////////////////////////////////////////////
  // Get time in seconds.
  long time = runTime / 1000; 
  time += ClockSecSet + 60*ClockMinSet + 3600*ClockHourSet + 86400*ClockDaySet;    //time in seconds, based on offset

  // Convert time to days,hours,mins,seconds
  long days  = time / 86400;    time -= days  * 86400; 
  long hours = time / 3600;   time -= hours * 3600; 
  long minutes  = time / 60;    time -= minutes  * 60; 
  long seconds  = time; 

// DATE CALCULATION /////////////////////////////////
// (non-working pas the first month) ///////////////
  int Jour = days;       //We define new variables to play with without affecting what's above.
  int Mois = ClockMonthSet;
  int Annee = ClockYearSet;

  //Defining years incrementation.
  if(Mois>12){
    Annee++;
    Mois-=12;
  }
  if(Mois<1){
    Annee--;
    Mois+=12;
  }
  
  //Number of days in a month:
  byte February;
    if(Annee%4==0)  February=29;
    else  February=28;
  byte MonthArray[13] = {0,31,February,31,30,31,30,31,31,30,31,30,31};  //we'll skip the first entry, so we can use the array's index directly.
  
  //Defining Month incrementation.
      if(Jour>MonthArray[Mois]){  //If "Jour" exceeds the number of days in the present month, it is resetted to 1 and we step into the next month.
        Jour -= MonthArray[Mois];
        Mois++;
      }
      if(Jour<1){    //Similarly if "Jour" is to become 0.
        Jour += MonthArray[Mois-1];
        Mois--;
      }

    
  // Splitting.    
  int lowerYears = Annee % 10;
  int upperYears = Annee - lowerYears;
  int lowerMonths = Mois % 10;
  int upperMonths = Mois - lowerMonths;
  int lowerDays = Jour % 10;
  int upperDays = Jour - lowerDays;    
  if( upperDays >= 10 )   upperDays = upperDays / 10;
  if( upperMonths >= 10 )      upperMonths = upperMonths / 10;
  if( upperYears >= 10 )     upperYears = upperYears / 10;

  // SHOWTIME  /////////////////////////////////
  //////////////////////////////////////////////
    NumberArray[3] = upperDays;
    NumberArray[1] = lowerDays;
    NumberArray[5] = upperMonths;
    NumberArray[0] = lowerMonths;
    NumberArray[4] = upperYears;  
    NumberArray[2] = lowerYears;
    DisplayFadeNumberString();    //Display date.

}

The code looks as if it is incomplete, but from the part we can see it still doesn't seem to be handling the month wraparound before accessing MonthArray.

Fixed. Copypaste error.

I see you need to type stuff into the serial monitor to adjust the date. Have you got a specific scenario where the adjustment produces the wrong answer?