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 ================
//***********
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…
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
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.
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…