[SOLVED] Why are these if statements not working simultaneously?

Projects Programming
1

Hi. In a nut shell, this part of my code is for the led lighting of a fish tank with four channels running off of the PWM channels. I have a statement that will call void l_lsramp() when it is time to start ramping the lights up to a specified brightness on all four channels. However, it only adjusts one channel at a time and I can't seem to see why. They should trigger all relevant 'ifs' at the same time and therefore add the value of ch1pwmincr, or ch2pwmincr, etc at the same time but it sends ch1 up to the desired level, then starts on ch2 and so on.

if (lightEditing == 0)
{

  if ((tm.Hour == lunartolowsunH) && (tm.Minute == lunartolowsunM) && (tm.Second == 0))
    {
      l_lsramp();
    }
}

void l_lsramp()          // Lunar to low sun
{
  currentColor.ch1 == ledch1pin;          // Read current colors
  currentColor.ch2 == ledch2pin;          // DONT KNOW IF THIS NEEDS TO BE HERE??
  currentColor.ch3 == ledch3pin;
  currentColor.ch4 == ledch4pin;
  
    // last color is the starting point of the fade
  lastColor.ch1 = currentColor.ch1;
  lastColor.ch2 = currentColor.ch2;
  lastColor.ch3 = currentColor.ch3;
  lastColor.ch4 = currentColor.ch4;
  
    // target color is low sun
  targetColor.ch1 = lightLowSun.ch1;
  targetColor.ch2 = lightLowSun.ch2;
  targetColor.ch3 = lightLowSun.ch3;
  targetColor.ch4 = lightLowSun.ch4;

    // calculate how long to run the fade for
  int fadeHours = lunartolowsundurH;
  int fadeMins = lunartolowsundurM;  
  fadeDurationSeconds = ((fadeHours*60*60)+(fadeMins*60));

  fadeInProgress = true;
  currentLightMode=4;
  previousMillis = millis();
  
}

   ////////   R A M P    T R I G G E R S   ////////

    
    

if(targetColor.ch1 != lastColor.ch1)
    {
        // Get the change per second for the current color to the target color
        float ch1pwmincr = (float)(targetColor.ch1 - lastColor.ch1) / (float)fadeDurationSeconds;

        Serial.print("ch1pwmincr - ");
        Serial.println(ch1pwmincr);
        Serial.print("currentColor.ch1 - ");
        Serial.println(currentColor.ch1);
  
      if(ch1pwmincr > 0) // If the change is positive
      {
        if(currentColor.ch1 < targetColor.ch1)
        {
            if ((millis() - previousMillis) > 1000)
            {
                currentColor.ch1 = currentColor.ch1 + ch1pwmincr;         // Add the increment amount to the corrent color
                analogWrite(ledch1pin, currentColor.ch1);
                EEPROM.write(43, currentColor.ch1); 
                previousMillis = millis();
            }  
        }
      }
      else // If the change is negative
      {
        if (ch1pwmincr < 0)
        {
          if(currentColor.ch1 > targetColor.ch1)
          {
              if ((millis() - previousMillis) > 1000)
              {
                  // Decrease the current color
                  currentColor.ch1 = currentColor.ch1 + ch1pwmincr;
                  analogWrite(ledch1pin, currentColor.ch1);
                  EEPROM.write(43, currentColor.ch1); 
                  previousMillis = millis();
              }
          }      
        }
      } 
    }


if(targetColor.ch2 != lastColor.ch2)
    {
        // Get the change per second for the current color to the target color
        float ch2pwmincr = (float)(targetColor.ch2 - lastColor.ch2) / (float)fadeDurationSeconds;

        Serial.print("ch2pwmincr - ");
        Serial.println(ch2pwmincr);
        Serial.print("currentColor.ch2 - ");
        Serial.println(currentColor.ch2);
  
      if(ch2pwmincr > 0) // If the change is positive
      {
        if(currentColor.ch2 < targetColor.ch2)
        {
            if ((millis() - previousMillis) > 1000)
            {
                currentColor.ch2 = currentColor.ch2 + ch2pwmincr;         // Add the increment amount to the corrent color
                analogWrite(ledch2pin, currentColor.ch2);
                EEPROM.write(44, currentColor.ch2); 
                previousMillis = millis();
            }  
        }
      }
      else // If the change is negative
      {
        if (ch2pwmincr < 0)
        {
          if(currentColor.ch2 > targetColor.ch2)
          {
              if ((millis() - previousMillis) > 1000)
              {
                  // Decrease the current color
                  currentColor.ch2 = currentColor.ch2 + ch2pwmincr;
                  analogWrite(ledch2pin, currentColor.ch2);
                  EEPROM.write(44, currentColor.ch2); 
                  previousMillis = millis();
              }
          }      
        }
      } 
    }


if(targetColor.ch3 != lastColor.ch3)
    {
        // Get the change per second for the current color to the target color
        float ch3pwmincr = (float)(targetColor.ch3 - lastColor.ch3) / (float)fadeDurationSeconds;

        Serial.print("ch3pwmincr - ");
        Serial.println(ch3pwmincr);
        Serial.print("currentColor.ch3 - ");
        Serial.println(currentColor.ch3);
  
      if(ch3pwmincr > 0) // If the change is positive
      {
        if(currentColor.ch3 < targetColor.ch3)
        {
            if ((millis() - previousMillis) > 1000)
            {
                currentColor.ch3 = currentColor.ch3 + ch3pwmincr;         // Add the increment amount to the corrent color
                analogWrite(ledch3pin, currentColor.ch3);
                EEPROM.write(45, currentColor.ch3); 
                previousMillis = millis();
            }  
        }
      }
      else // If the change is negative
      {
        if (ch3pwmincr < 0)
        {
          if(currentColor.ch3 > targetColor.ch3)
          {
              if ((millis() - previousMillis) > 1000)
              {
                  // Decrease the current color
                  currentColor.ch3 = currentColor.ch3 + ch3pwmincr;
                  analogWrite(ledch3pin, currentColor.ch3);
                  EEPROM.write(45, currentColor.ch3); 
                  previousMillis = millis();
              }
          }      
        }
      } 
    }


if(targetColor.ch4 != lastColor.ch4)
    {
        // Get the change per second for the current color to the target color
        float ch4pwmincr = (float)(targetColor.ch4 - lastColor.ch4) / (float)fadeDurationSeconds;

        Serial.print("ch4pwmincr - ");
        Serial.println(ch4pwmincr);
        Serial.print("currentColor.ch4 - ");
        Serial.println(currentColor.ch4);
  
      if(ch4pwmincr > 0) // If the change is positive
      {
        if(currentColor.ch4 < targetColor.ch4)
        {
            if ((millis() - previousMillis) > 1000)
            {
                currentColor.ch4 = currentColor.ch4 + ch4pwmincr;         // Add the increment amount to the corrent color
                analogWrite(ledch4pin, currentColor.ch4);
                EEPROM.write(46, currentColor.ch4); 
                previousMillis = millis();
            }  
        }
      }
      else // If the change is negative
      {
        if (ch4pwmincr < 0)
        {
          if(currentColor.ch4 > targetColor.ch4)
          {
              if ((millis() - previousMillis) > 1000)
              {
                  // Decrease the current color
                  currentColor.ch4 = currentColor.ch4 + ch4pwmincr;
                  analogWrite(ledch4pin, currentColor.ch4);
                  EEPROM.write(46, currentColor.ch4); 
                  previousMillis = millis();
              }
          }      
        }
      } 
    }
}
2 
  currentColor.ch1 == ledch1pin;          // Read current colors
  currentColor.ch2 == ledch2pin;          // DONT KNOW IF THIS NEEDS TO BE HERE??
  currentColor.ch3 == ledch3pin;
  currentColor.ch4 == ledch4pin;

==?

3

PaulS:

  currentColor.ch1 == ledch1pin;          // Read current colors

currentColor.ch2 == ledch2pin;          // DONT KNOW IF THIS NEEDS TO BE HERE??
  currentColor.ch3 == ledch3pin;
  currentColor.ch4 == ledch4pin;



==?

Oh, that might need to come out. That is basically just to read the values that the PWM pins are giving out at the time. It will probably be done another way later and put somewhere else. It was something I was playing with earlier.

4

Without the rest of the sketch it's hard to tell what is going wrong.

Are you color channel values integers? If so, adding a floating increment less than 1 and greater than -1 will do nothing.

If you want a long lifespan for your device I would not write to eeprom every time a channel increments or decrements.

5

From the sticky post, "How to use this forum - please read," linked at or near the top of each topic listing in the forum:

  1. Tips for getting the most out of your post

Mention which Arduino you have. Is it a Uno? Leonardo? Due? Mini? Mega? The problem might be specific to a certain model.

I'm guessing it's an Uno.

Post a complete sketch (program code)! If you don't you waste time while people ask you to do that. However, with coding problems, if possible post a "minimal" sketch that demonstrates the problem - not hundreds of lines of code. If the problem goes away in the minimal sketch, it wasn't where you thought it was.

You've shown us a lot of code. There's a lot missing. With the code incomplete, we can't even tell what kind of objects most of your variables are. Is it possible for you to craft a small sketch that illustrates the problem? The smaller and more direct it is, the larger the number of readers that are likely to stop and examine the code.

That said, I think that the problem that you described lies in the management of previousmillis. When the code finds that it's time to change the LED brightness, it adjusts LED1 and sets previousmillis to the current time. All the other LEDs will find that the interval hasn't yet elapsed, because previousmillis will be quite close to the current time. When LED1 has been fully ramped, its code won't execute, and LED2 will have a chance. But it will do the same thing - set previousmillis to the current time before LED3 and LED4 can get a look at it. I think that explains the issue you described.

For a quick solution, I'll recommend that you wait until the code is completely finished with previousmills before updating it. From what we can see in your post, that would mean after the fourth LED has checked it.

6

Thanks John, I'll look into re writing the eeprom read and writes. I was planning on doing some spring cleaning once I know it's all working.

I only posted the parts I thought were relevant to the problem as its really only the three parts there that are connected, other than the fade time start screen and brightness input screen.

Yes they are all integers. And the float values seem to work perfectly in that the currentcolor.ch1 only displays the integer value and so rounds the float up or down which is exactly what I wanted.

It's just the issue of the if statements only running one after the other and not simultaneously that has stumped me. I figured it might be a slight misplaced code or a different way of writing it to solve it.

Can you see from what I posted any reason as to why it would only run one at a time? Anything to do with the millis?

7

Tmd you are spot on. That is exactly what I reckon it is. I had a hunch it was something to do with the millis but could fathom it out.

And apologies about breaking the basic rules but was pretty sure that the problem was only minor, as you have found the solution, and Didn't want to waste server space and people's time with reading 4000 lines of code when only 100 are relevant. The same goes for what unit I'm using. It is a mega btw with a cheapo eBay 3.6 touch screen which glawrence helped me to get running.

Will post more info in the future.

Thanks.