Code for random alarm in 6 hour shift Arduino uno

This code waits for input from system. System sends relay contact for input every 6 hours.
Code creates a random time within that 6 hours and generates an alarm, then goes back to waiting for input.

Is my concept ok. I am not a programmer, I’m a hardware type so dont bash too hard. The code works and does what I want, I just want an opinion if I could have done it different.

NOTE: delays are only there to help fololow and see the code react. End version the delays will be removed.

int val1 = 0;
unsigned long previousTime = 0;
unsigned long previousMillis = 0;
unsigned long currentMillis = 0;
unsigned long startMillis = 0;
unsigned long trigTime = 0;
int shiftSet = 0;
long startMicros = 0;
long currentMicros = 0;
int hourSeed = 1;
int previousHourSeed = 1;
int minSeed = 1;
int previousMinSeed =1;
int randomHour = 1;
extern volatile unsigned long timer0_millis;
int returnToMain = 0;

//======== setup Function
void setup() {
Serial.begin(9600); 
pinMode(7, INPUT_PULLUP);
pinMode(13, OUTPUT);
randomSeed(analogRead(A0));
} // Close Setup



//======== shiftCalc Function

void shiftCalc(){
  Serial.println("Entering the calculating loop...");

  hourSeed = random(0,5); //sets the hour random number
  minSeed = random(1,55); //sets the min random number

 //this loop checks if previous random number is the same and if it is grabs a different one.
 while (hourSeed == previousHourSeed){ 
  hourSeed = random(0,5);
  Serial.println("Y");
 }
 
//this loop checks if previous random number is the same and if it is grabs a different one. 
while (minSeed == previousMinSeed){
  minSeed = random(1,55);
  Serial.println("Z");
 }
 
previousMinSeed = minSeed;
previousHourSeed = hourSeed;
Serial.print(hourSeed);
Serial.print(" Hours and ");
Serial.print(minSeed);
Serial.println(" minutes.");
delay(2500);
//Now we have the hour and minute the alarm gets triggered

//The below converts hours and minutes to combined miliseconds
long hTimeMS = (hourSeed * 3600000);
long mTimeMS = (minSeed * 60000);
Serial.println(hTimeMS);
Serial.println(mTimeMS);
trigTime = (hTimeMS + mTimeMS);
Serial.println(trigTime);

  Serial.println("Calculations done....");

  //Heading off to sit and wait until alarm time
  inShift();
}
//======== END shiftCalc Function



//======== inShift Function
void inShift(){
      
  startMillis = millis();
  currentMillis = millis();
  while (shiftSet == HIGH){
  Serial.println(currentMillis);
  Serial.println(startMillis);
  Serial.println(trigTime);
  delay(2000);

  //wait have some coffee and maybe drop some whickey in coffee
  //We are waiting until currentMillis gets to startMillis
  
    while (currentMillis - startMillis < trigTime){
      Serial.println("Waiting for shift Trigger");
      Serial.println(currentMillis - startMillis);
      Serial.println(trigTime);
      currentMillis = millis();
      delay(1000);
    }

//This sets up the return to Main loo[p
Serial.println(returnToMain);
 if (returnToMain == 1){
    returnToMain = 0;
    Serial.println("Getting ready to leave this joint and go backl to main");
    shiftSet = 0;
    return;
  }
  
 //Time has come to head over to trigger the alarm
 Serial.println("Heading over to alarm Function.");
 delay(2000);
 alarmTrigger();
   }
 }
//======== END inShift Function ===========

//======== Alarm Function ==================

void alarmTrigger(){
   Serial.println(" Fire alarm bells and evacuate prison");
   digitalWrite(13, HIGH);
   Serial.println("Output 13 should be ON...");
    delay(5000);
   digitalWrite(13, LOW);
    delay(2000);
   Serial.println("Output 13 should be OFF...");

 returnToMain = 1;

}
//======== END Alarm Function ==================



//***************************************************
//==================== MAIN LOOP ====================
//***************************************************

void loop() { 
//Resets millis after 30 days
if (millis() >= 2592000000){
    noInterrupts ();
    timer0_millis = 0;
    interrupts ();
  }
                               
unsigned long currentMillis = millis();
Serial.println(currentMillis);
int shiftTrig = digitalRead(7); //Reads input from card access system.

while (shiftTrig == HIGH){  // loops here until input is active from card access system
  Serial.println("Waiting for input...");
  delay(600);
  Serial.println(millis());
  Serial.println("Still in the Main loop");
  shiftTrig = digitalRead(7);
  } 
  //This is the shift trigger exit
  shiftSet = 1;
    
  while (shiftSet == HIGH){  // This is how shiftSet gets set
  Serial.println("Shift is set");
  delay(600);
  Serial.println("Heading to Calculation loop...");
  delay(3600);
  //shiftSet = 0;
  shiftCalc();
  } //This is the shift loop exit

Serial.println("Main loop executing");

} 
//***************************************************
//==================== END MAIN LOOP ================
//***********

Your topic has been moved here as this forum section is more appropriate than where it was originally posted.

Please take care to post in the correct forum section

The easier you make it to read and copy the code the more likely it is that you will get help

Please follow the advice given in the link below when posting code

Thank you. this is my first post. Didnt realize. I was looking for formatting but quickly got bored. Hope this is better now.

That’s better, thanks. See how much easier it is to read and copy for examination when posted in code tags.

Yeah thanks. Appreciate your help.

A quick look revealed this

  //Resets millis after 30 days
  if (millis() >= 2592000000)
  {
    noInterrupts ();
    timer0_millis = 0;
    interrupts ();
  }

When used properly there is no need to reset millis()

Yeah I read that. I dont understand why this is an issue but I am contemplating on how I can go without it.

the problem is if it resets while it is in the loop waiting for alarm after the shift trigger, then the alarm will never happen in the shift and could skip a shift. Thats the worse case scenario.

I tried searching for reasons why this is an issue and cannot come up with a valid reason not to use it nor a way around not using it. I will study this concept in more detail. Thanks…

Bob Bevins

    while (currentMillis - startMillis < trigTime)
    {
      Serial.println("Waiting for shift Trigger");
      Serial.println(currentMillis - startMillis);
      Serial.println(trigTime);
      currentMillis = millis();
      delay(1000);
    }

Congratulations. You have just invented the delay() function and blocked the code from running freely

Your program would be better if it were structured differently. Start by thinking how many different states it can be in and listing them

I would envisage states such as

  • waiting to start
  • generating random alarm time
  • waiting 6 hours
  • sounding alarm
  • resetting to start

You, of course know better than me what is required but none of these states should block the free running of the program however many states there are

Using named states will make the program more readable and allow for easier debugging, maintenance and revision

I stated in my post the delay is there to allow me to follow the code easier and will be removed on final version. so no congradulation is neccessary.

I am contemplating on using tiny RTC instead of millis.

I wasn’t referring to the delay() but to the while loop as a whole. Ignoring the delay(), how long will the code stay in the while loop ?

Every 6 hours there is an input for a change of shift. then it calculates and goes into a loop waiting for a random time between 0 and 5hours and 55 minutes. after the alarm trigger it goes back to the main then drops into the loop waiting for the shift change.

So how long will be code be in the while loop ?

Which loop? there is the loop in main which will be in there max 6 hours and the loop in the trigger max 6 hours.

The while loop I posted in reply #6

reply#6 is not a while loop.

You are right. See reply #8

while (currentMillis - startMillis < trigTime)
{
Serial.println(“Waiting for shift Trigger”);
Serial.println(currentMillis - startMillis);
Serial.println(trigTime);
currentMillis = millis();
delay(1000);
}

OK… Whats the problem with that? The code does nothing else except wait for that alarm. It has nothing else to do.

What would you rather see? I need to wait until the time picked by the random function to energize a relay.

What would be a better way to do it?

There is nothing actually wrong with it if it does what you want and the code is not required to do anything else during the while loop

Bearing in mind that you said

I would structure the program as a state machine and eliminate the while in favour use an if and let the program run freely with loop() repeating as often as possible. It is all a matter of approach.

If you want to keep it really simple then use a simple delay(), but personally I wouldn’t

Consider using an RTC module. When the shift start signal triggers, calculate the time for the Alarm and save it in EEPROM. When it’s Alarm time, do the alarm and clear the EEPROM. This way, you can recover from power failure and still have the alarm.

Some clock modules will let you store data in them, or you can use the Arduino’s EEPROM. If the latter, remember that it has a finite number of writes and be careful that your code only writes when it needs to.

There’s nothing wrong with delay in the abstract, but the reason it’s discouraged is because we’ve seen too many people bitten by the scenario where they thought up a new feature for their system which requires a complete rewrite which would have been avoided by keeping away from delay initially.

I have contemplated on using an external rtc but a few things that I flag are: what happens on daylight savings time? I dont know whether the hardware rtcs can handle that or I can do it in arduino code and send the change to the rtc and another is the wearyness of adding another component that could be a point of failure. I like having everything in the arduino and not having to worry about external devices.

This is my first arduino project so bear with me if this has been addressed before…

thanks for the reply…