Millis problem if interval above 1000

I have a problem using the Milli function in the code posted below.
My fade works perfectly if I use a fadeInterval of 1000(milliseconds) combined with a delay of 1000 in my loop. As soon a I remove the delay in my loop, or change the fadeInterval to a higher value the fading is not happening. Could you tell me where I am going wrong here?

Short description of what i want the code to do:

At set time points I want the LEDs to fade up, burn at a steady level, fade down or just switch off. I want to use Millis because I want the Arduino to do more in the mean time. I use states to make sure the Loop keeps calling the function

#include <Arduino.h>
#include <Wire.h>         // this #include still required because the RTClib depends on it
#include "RTClib.h" 

// Date and time functions using just software, based on millis() & timer

#define UP 0
#define STEADY 1
#define DOWN 2

byte fadeDirection = UP;

const byte ledPin1 = 6;
const byte ledPin2 = 5;

const byte maxPWM = 80;
const byte minPWM = 1;

byte fadeValue = 0;
byte fadeAmount = 5;
byte state = 0;

int myhour = 0;
int myminute = 0;

unsigned long fadeMillis;    //Waarde voor LED fade

int fadeInterval = 1000;

RTC_Millis rtc;

void setup () {

pinMode(ledPin1,OUTPUT);

pinMode(ledPin2,OUTPUT);

    Serial.begin(57600);
    // following line sets the RTC to the date & time this sketch was compiled
    rtc.begin(DateTime(F(__DATE__), F(__TIME__)));

    DateTime now = rtc.now();
    
    myhour = now.hour();
    myminute = now.minute();

     Serial.print(myhour);
     Serial.print(":");
     Serial.println(myminute);

}

void loop () {

unsigned long currentMillis = millis();
  
    DateTime now = rtc.now();
    myhour = now.hour();
    myminute = now.minute();

     Serial.print(myhour);
     Serial.print(":");
     Serial.println(myminute);

if(myhour == 21 && myminute == 18) state = 1;
else if(myhour == 21 && myminute == 00) state = 2;
else if(myhour == 21 && myminute == 10) state = 3;
else if(myhour == 23 && myminute == 00) state = 4;

switch(state) {
  case(1): sunrise(currentMillis); break;
    case(2): daytime(); break;
      case(3): sunset(currentMillis); break;
        case(4): evening(); break;
}
delay(1000);
  }




void sunrise(unsigned long sunriseMillis)      // Functie voor starten LED
{
  
  
    if (sunriseMillis - fadeMillis >= fadeInterval){
    if (fadeDirection == UP) {
      fadeValue = fadeValue + fadeAmount;  
      if (fadeValue >= maxPWM) {
        // At max, limit and change direction
        fadeValue = maxPWM;
        fadeDirection = STEADY;
      }
    } 
    }
      analogWrite(ledPin1,fadeValue);
          Serial.print("UP");
          Serial.print(fadeValue);
          Serial.print(fadeDirection);
       
      analogWrite(ledPin2,fadeValue);
          Serial.print("UP");
          Serial.print(fadeValue);
          Serial.print(fadeDirection);
          
    fadeMillis = sunriseMillis; //IMPORTANT to save start time of current LED

    }

void daytime()      // Functie voor starten LED
{
      analogWrite(ledPin1,fadeValue);
          Serial.print(fadeDirection);
         
       
      analogWrite(ledPin2,fadeValue);
           Serial.print(fadeDirection);;
          
      }

void sunset(unsigned long sunsetMillis)      // Functie voor starten LED
{
  
      if (sunsetMillis - fadeMillis >= fadeInterval){
    if (fadeDirection == DOWN) {
      fadeValue = fadeValue - fadeAmount;  
      if (fadeValue >= minPWM) {
        // At max, limit and change direction
        fadeValue = minPWM;
        fadeDirection = UP;
      }
    } 
    }
      analogWrite(ledPin1,fadeValue);
           Serial.print(fadeDirection);
       
      analogWrite(ledPin2,fadeValue);
           Serial.print(fadeDirection);;
          
    fadeMillis = sunsetMillis; //IMPORTANT to save start time of current LED

    }
  
void evening()      // Functie voor starten LED
{
  
  
      analogWrite(ledPin1,0);
      analogWrite(ledPin2,0);
           
       
       
      }

Can't answer your question, but why code this way:

if(myhour == 21 && myminute == 18) state = 1;
else if(myhour == 21 && myminute == 00) state = 2;
else if(myhour == 21 && myminute == 10) state = 3;
else if(myhour == 23 && myminute == 00) state = 4;

The else may not be doing what you expect and is superfluous.

Just make all 4 "IF" with no "else".

Paul

mDutch:
My fade works perfectly if I use a fadeInterval of 1000(milliseconds) combined with a delay of 1000 in my loop.

What happens if you take the delay() out of your program?

My guess is that the delay(1000) is conflicting with a millis() interval of 1000. After the delay() the entire period of the 1000 millis() interval will already be used up,

If you need the wait caused by delay(1000) then you should also implement that using millis().

...R

Hi Paul

Would having 4'if's not mean all 4 have to be executed, where as 'else if's won’t be checked when a previous condition has be confirmed, faster code execution?

Not sure why delay() is being used with BWD.

larryd:
Hi Paul

Would having 4'if's not mean all 4 have to be executed, where as 'else if's won’t be checked when a previous condition has be confirmed, faster code execution?

Not sure why delay() is being used with BWD.

Wouldn't take any more time than having to decide to skip each test,

Paul

You are using fadeMillis here

 if (sunriseMillis - fadeMillis >= fadeInterval){

before setting it to a value. It gets set to a value in sunrise() or sunset(), if you wait a second before the next loop() then is was set over a second ago.

I think you want the writes to the LED pins and the assignment to fadeMillis inside the if statement which checks to see if it is time to fade.

You might need some code restructuring to get the fade happening as soon as the state changes.

Thank you Blue Eyes, i indeed did not upload the currentMillis within the if statement. After moving the brackets it is working!