not sure if these things are significant in the total aspect of the code (not 100% on what you're trying to achieve), but:
-
your conditional statements, let's call them A, B, and C, where:
A = if (T >= 0 && pause == 0 )
B = if ( T <= 0 && pause == 0 && horn == 0)
C = if ( horn == 1)
if horn == 0 and T == 0 and pause == 0 then A and B will both execute
if horn == 1 and T >= 0 and pause == 0 then A and C will both execute
perhaps that was the intent...
- efficiency issue... using "currentMillis" is kind of redundant considering you can call millis() for your "currentMillis".
also, instead of performing calculations with each check... actually i'd just replace this:
unsigned long currentMillis = millis();
if(currentMillis - previousMillis > 1000)
with this:
loop(){
static unsigned long previousMillis;
...
if(millis() > previousMillis){
previousMillis = millis() + 1000;
...
that way, your program just has to compare longs rather than subtract and compare.
personally, if i were to achieve what i think you're trying to achieve (if a function completely executes/event occurs then horn will turn on for 4 seconds) i would do it something like this:
void loop(){
static unsigned long LED_timer, HORN_timer; //initialize to 0
static unsigned char LED_flag, HORN_state; //initialize to 0
static unsigned long program_Timer; //placed on its own line for readibility
shotT(); //display countdown and update function
if(millis() > LED_timer){ //executes once immediately (LED_timer initialized to 0)
LED_timer = millis() + 1000; //store time of next execution... right now + 1000ms
programTimer--; //decrement program timer -1second
Serial.print(programTimer,DEC);
Serial.print("\n\n");
}
if(millis() > HORN_timer){
HORN_timer = HORN_timer + 4000; //next HORN event will happen in (now) + 4000ms
if(HORN_state){
HORN_state = 0;
//turn_horn_on_function()
}
else{
HORN_state = 1;
//turn_horn_off_function();
}
}
}
the theory behind the whole storing-the-timer thing is to eliminate busy-wait's. when the processor is in a busy-wait, nothing else can happen, which is inefficient. instead, just think relatively. instead of saying "wait right here for a while" you're saying "do what you need but be here in a while". so your entire application will have 0 busy-waits, and instead of using "delay(x)" you'll use "if(millis() > x)" and "x" will have been assigned in the previous cycle as (previous_cycle_time)+x. your loop becomes a dispatch manager:
#define TOGGLE(a) (a==1)?(0):(1) //toggling 1:0:1:0:1...
#define LED_PIN 13
#define A_cycletime 100 //every 100ms
#define B_cycletime 500 //every 500ms
#define C_cycletime 1000 //every 1s
#define D_cycletime 5000 //every 5s
void setup(){
Serial.begin(9600);
Serial.println("Initialized");
}
void loop(){
static unsigned long timerA, timerB, timerC, timerD;
static unsigned char LED_state;
if(millis() > timerA){
timerA = millis() + A_cycletime; //first event should set the clock for accuracy
subfunctionA(); //call a function for functionA to keep it organized
Serial.println("A");
}
if(millis() > timerB){ //500ms led flasher in here
timerB = millis() + B_cycletime;
digitalWrite(LED_PIN,TOGGLE(LED_state));//LED_state is toggled, pin is turned[on/off]
subfunctionB();
Serial.println("B");
}
if(millis() > timerC){
timerC = millis() + C_cycletime;
subfunctionC();
Serial.println("C");
}
if(millis() > timerD){
timerD = millis() + D_cycletime;
subfunctionD();
Serial.println("D");
}
}
void subfunctionA(){
//some stuff
}
void subfunctionB(){
//some stuff
}
void subfunctionC(){
//some stuff
}
void subfunctionD(){
//some stuff
}
hopefully you can pick a solution out of all that
)