My alarm goes off when it shold'ent [SOLVED]

Heres the code:

/************************************************************
 * TOOLCABIN LIGHT IN GOTHENBURG VERSION 0.90 DATE: 11. MARCH 2014
 * Led dimmer using a TIP120 Darlington Transistor
 * Supply voltage is from 14.4v battery and down/ LED build for 12V, 
 * But voltagedrop over Transistor and wires makes it ok.
 * PIR-sensor power is fed from Arduino 5V / GND-pins. 
 * PIR output to digital pin (D3) - polled
 * PIR can be retriggable or non retriggable
 * LED switches on when PIR detects movement
 * LED output will stay 'ON' until 5 sec of 'PIR silence'
 * LED (transistor gate) MUST be connected to PWM pin. (D11)
 * The Alarm is made from a Velleman MK174 where it is possible to record
 * a custom message, mine is "Low Voltage, Please Recharge."
 * The alarm is triggered from pin 7 wich goes HIGH for 10 ms and low again.
 * There is a voltage divider (wiki that if you don't know") connected to
 * pin A2 to keep track of the battery status, there might be a ampare
 * sensor board on there sometime, but for now this is ok.
 * This code can be drastically redused on volume - made for readability
 ****************************************************************/
// declare global constants 
const byte led = 11;         // connect to gate
const byte pir =  3;         // PIR signal out 
const byte alarm = 7;        // The positive to the alarm trigger, have common ground whit power.
const byte turnOnTime = 1;   // turn on/off time in seconds !(will be forced:  50 > time > 0)
const byte secondsToOff = 5; // seconds of 'silence' before "lights out" !! ALTER THIS VALUE
const boolean debug=false;

// declare global variables
boolean alreadyBlipped = true; //Have there been a "Low Voltage Alarm"?
boolean lightIsOn = false;   // remember LED-status 'now'
unsigned long timeToTurnOff; // time when led  will turn off
unsigned long timeNow;       // current time

//Setup for measuring voltage
// number of analog samples to take per reading
#define NUM_SAMPLES 10

int sum = 0;                    // sum of samples taken
unsigned char sample_count = 0; // current sample number
float voltage = 0.0;            // calculated voltage


// one time setup
void setup() 
{
  // set status og used I/O-pins
  pinMode(led,OUTPUT);
  pinMode(pir,INPUT);
  pinMode(alarm,OUTPUT);
  Serial.begin(9600);
}
//********************************
// small functions and procedures - to make reading of main prog easier
boolean heWantsLightsOn()
{
  return (digitalRead(pir)==HIGH);  
}
//********************************
void turnLightsOn(byte period)
{
  // now the LED shall increase light intensity over "turnOnTime" seconds
  long delayTime = period * 5 ;
  for (byte i=0; i<255; i++) 
  {
    analogWrite(led,i);
    delay(delayTime);
  }
  digitalWrite(led,HIGH); // no PWM needed - turn on 100%
  lightIsOn=true;         // remember status
  Serial.println("Light Is On"); //Tell me that the light is on
}
//**********************************
void turnLightsOff(byte period)
{
  // now the LED shall decrease light intensity - slowly to zero 
  long delayTime = period*5 ;
  for (byte i=254; i>0; i--) 
  {
    analogWrite(led,i);
    delay(delayTime);
  }
  digitalWrite(led,LOW); // no PWM needed - turn off
  lightIsOn=false;         // remember status
  Serial.println("Light Is Off"); //Tell me that the light is off

  alreadyBlipped = false;

}

void loop() 
{
  timeNow=millis();  // note time now
  if (heWantsLightsOn()) // if movement -> be light! 
  {  
    if (!lightIsOn) turnLightsOn(turnOnTime);   // if still dark - be light! (else light were on already)
    // set 'new' time for off -since you just told 'stay on for some mor minutes..)
    timeToTurnOff=timeNow + secondsToOff * 1000;  // set 'off-time' some minutes into the future (milliseconds used)
    if (debug) Serial.println("renew for one minute");
  }
  // now test for "Elvis  has left the building"
  if (timeNow > timeToTurnOff) // is it time to turn off ??
  {
    if (lightIsOn) turnLightsOff(turnOnTime);  // if light still are on: turn them off
  }
  {
    // take a number of analog samples and add them up
    while (sample_count < NUM_SAMPLES) {
      sum += analogRead(A2);
      sample_count++;
      delay(100);  //Wait 100ms before reading voltage again
    }
    // calculate the voltage
    // use 5.0 for a 5.0V ADC reference voltage
    voltage = ((float)sum / (float)NUM_SAMPLES * 4.73) / 1024.0;
    // send voltage for display on Serial Monitor
    // voltage multiplied by 3 when using voltage divider that
    // divides by 3. 3 is the calibrated voltage divide
    // value
    Serial.print(voltage * 3);
    Serial.println (" V");
    sample_count = 0;
    sum = 0;
  }
  if (voltage < 11.5 && lightIsOn == true && alreadyBlipped == false);
  //  if (lightIsOn == true && alreadyBlipped == false);   //test if it's time to give the alarm
  {
    digitalWrite(alarm,HIGH); //open the trigger pin
    delay(1);                //let it stay on a bit
    digitalWrite(alarm,LOW);  //close it again
    Serial.println("Low Voltage, Please Recharge"); //Tell me to recharge, but on-screen
    alreadyBlipped = true; //Yes now the alarm has gone off!
  }

}
//***************END***********************

My question is: why does this alarm goes off when the light is off?
and why does it triggers every time through the code and not only once after the light is 100% on?
and it keeps printing "Low voltage" way above 11.5 volts
I'm sure it's just a small stupid thing I missed...

Is 'the alarm' the light coming on?

The code calculating the voltage from the analog value has got some strange magic numbers in it. Is it printing out the correct voltage?

The loop to calculate the average would be a lot cleaner as a FOR loop rather than as a while loop, and this would eliminate some global data too.

When testing for the end of an interval, it's better to use subtraction rather than addition so that timer overflow is handled correctly. In other words, instead of:

if (timeNow > timeToTurnOff)

This is preferred:

if (timeNow - startTime > duration)

This statement shouldn't have a semicolon after it and would cause lines 127 .. 131 to be executed unconditionally:

  if (voltage < 11.5 && lightIsOn == true && alreadyBlipped == false);

The alarm is a Soundkit with a recorded message, being triggered.
And it should only be triggered when the battery is low and the light is on.

But I'll try what you suggested after the weekend, it did make sense so it was propebly that.

I actually modified the voltage, it printed the correct voltage, but stored a wrong one. That has been taken care of, but did not make a change to the alarm being triggered all the time.

The code works well on the simulator now :slight_smile:
Have to test it monday but I think it works :slight_smile:
Thanks PeterH

I did not do anything by the way to handle the time, maybe I'll do it one day, but for now it's ok...