Help me improve my code for a daylight/sunrise alarm

Hi,
this is my first post here, please let me know if I am violating the forum etiquette.

This is also my first Arduino project. The last time I coded in C was ten years ago and it amazes me how quickly I forget a skill like this when I don’t use it.

I would like your input on my code architecture.

The project is a daylight alarm: A device that will slowly light up at a set time over the course of 30 minutes in order to simulate a sunrise and wake my wife and me up gently. There are several such projects on the web already but most of them use the delay method which is not suited for my specs. I want this to be an extremely simple device that wont need further maintenance by computer once it is done. It needs a buttons to stop the alarm, to set it to three different alarm times and one button to reset the RTC to 9am should the onboard battery be replaced. Of course the buttons should ba working all the time.

Since I did not receive the RTC yet, the code is not finished but it is working using the placeholder via milis();

I am wondering if there is any way to improve my architecture in a major waay, making it more efficient or just easier to handle. Surely there are endless ways to accomplish my goal but I would like to learn how to chose the path of least resistance (for the processor).

const byte led = 9; // Pin LED is connected to
const byte sixAmPin = 4; // Pin for set-alarm-to-6am-button
const byte sevenAmPin = 5; // Pin for set-alarm-to-7am-button
const byte eightAmPin = 6; // Pin for set-alarm-to-8am-button
const byte stopPin = 7; // Pin for stop-button
const byte timeResetPin; // Pin for reset-RTC-to-9am-button (useful e.g. after battery failure)

byte sixAmButton = 0; // State of 6am-set-button
byte sevenAmButton = 0; // State of 7am-set-button
byte eightAmButton = 0; // State of 8am-set-button
byte stopButton = 0; // State of stopButton
byte timeResetButton = 0; // State of time-set-button

byte brightness = 0;
bool wakeupMode = false;
unsigned long prevStepTime = millis();
unsigned long time;

// Changeable variables here
byte setHour = 6;
byte setMin = 0;
byte fadeDuration = 1; // Duration of fade in minutes
byte afterBurn = 30; // Keep light on after alarm for this many minutes

int keepLight = afterBurn * 60000; // Calculate time to keep LED on after fade (in ms)
int fadeStepLength = fadeDuration * 60000 / 255; // Calculate step duration for increasing LED brightness

void wakeup();
void readButtons();

void setup() {
  // declare LED pin to be an output:
  pinMode(led, OUTPUT);

  // declare button pins as inputs:
  pinMode(stopPin, INPUT);
  pinMode(sixAmPin, INPUT);
  pinMode(sevenAmPin, INPUT);
  pinMode(eightAmPin, INPUT);
  pinMode(timeResetPin, INPUT);
}

void loop() {
  // Setup temporary counter until I get the RTC:
  time = millis();    
  
  // If it is wake-up time and wakeupMode is on, trigger alarm and reset trigger; else turn on wakeupMode
  if ( time > 2500 && time < 3500) {
    if (wakeupMode) {
      wakeup();
      wakeupMode = false;
    }
    else {
      wakeupMode = true;
    }
  }

  // Check for input from buttons
  readButtons();   
      
  // update LED in case stopButton was pressed
  analogWrite(led, brightness);
  
  // Todo: Setup power saving methods

}

void wakeup() {
  // If wake-up-mode is true and brightness < 255, add +1 to brightness every 7 seconds (256 steps)
  while (wakeupMode == true) {
    //update clock
    time = millis();
    
    // If current time is wakeup-time, increase LED brightness slowly over the course of 30 minutes
    if (brightness < 255 ) {
      // delay brightening in steps:
      if (time > prevStepTime + fadeStepLength) {
        brightness ++;
        prevStepTime = millis();
      }
    }
    // Turn off light and exit wakupMode x minutes after max brightness
    else if (brightness == 255 && time > prevStepTime + keepLight) {
      brightness = 0;
      wakeupMode = false; 
    }

    // Check for input from buttons
    readButtons();   
    
    // Set LED brightness accordingly
    analogWrite(led, brightness);
  }
}

void readButtons() {
  // Always run this:
  // Check if alarm gets set to 6, 7 or 8am
  sixAmButton = digitalRead(sixAmPin);
  if (sixAmButton == HIGH) {
    setHour = 6;
  }
  sevenAmButton = digitalRead(sevenAmPin);
  if (sevenAmButton == HIGH) {
    setHour = 7;
  }
  eightAmButton = digitalRead(eightAmPin);
  if (eightAmButton == HIGH) {
    setHour = 8;
  }

  // If stopButton is pressed, turn off LED and reset alarm
  stopButton = digitalRead(stopPin);
  if (stopButton == HIGH) {
    wakeupMode = false;
    brightness = 0;
  }
  
  // Todo: If timeResetButton is pressed, reset clock to 9am
  if (timeResetButton == HIGH) {
    
  }
}

As you can see I am also plannig to implement a few power management functions as the processor won’t need to do anything for most of the day. I have not looked into that. I don’t know whether sleep mode would be viavle since I still would like the buttons to be responsive.

So: Do you see any major flaws in the architecture? Are there obvious ways to make this project more efficient?

Thank you for your time.

Cheers,
Fertchen

int keepLight = afterBurn * 60000;A sixteen bit int will not hold such a value.

unsigned long keepLight = afterBurn * 60000UL;

the DS3231 has internal alarms.
you can set to alarm at each (decade?) change.
each second, or each minute or each hour, or each day.

set for each hour, RTC alarm goes off
Arduino wakes up increments hour since last start hour ++
if hour = (24?) then do the alarm thing

math is integer counts to 24
sleep is all the time, except when counting hours, or making the sun Fade-in for that morning sun rise.

Since the RTC is the timer, the Arduino does not need to do clock times.
if you do want to check the time, you can do that each day during the alarm time to make sure things are still aligned.

AWOL:
A sixteen bit int will not hold such a value.

Thanks a lot for pointing that out. During testing I used much smaller values and therefore did not encounter any problems.

dave-in-nj:
sleep is all the time, except when counting hours, or making the sun Fade-in for that morning sun rise.

I am going to use the PCF8563. Either way, thank you for your suggestion. It sounds like a good idea. But sleep mode means that Arduino is not going to read button inputs, doesn't it?