First sketch, plase check code.

Hello.

This is my first attempt with arduino, I work as an electrician so I’m probably a bit bound by relay control mindset.
But my daughter has acquired some chickens, and there has to be opened and closed to them daily. so there I saw my chance to getting started with ardiono with a small easy project … I believed … it is a little different from the relay control.

So I’ve read some on this forum and found a number of examples, including a variety of chicken coop door open / close, but I want to make a program myself so I can get some practice with the ardiono. So I found some examples in the learning section here on this site and have adapted them to my task.

I have ardiono mini pro, nano and uno, all kinds of sensors which I have played with on a breadboard including a light sensor module and a stepper / DC motor controller.

I have not installed the entire circuit on breadboard yet, I would like to have my code verified first to see if it makes sense.

Suggestions, criticisms, explanations and ideas will be very much appreciated.

/* 

Open and closing the door in the chicken coop depending on ambient light level.

This sketch was made with cut and paste from:
http://arduino.cc/en/Tutorial/BlinkWithoutDelay
http://arduino.cc/en/Tutorial/Smoothing

*/


//analoge I/O
int currentSensor = 2;             //analog input, current sensor on door motor.
int lightSensor = 3;               //analog input, light level sensor.

//digitale I/O
int motor = 4;                     // digital output, enable motor = pin HIGH, stop motor = pin LOW.
int doorClose = 5;                 // digital output, pin 5 = HIGH and pin 6 = LOW, door close.
int doorOpen = 6;                  // digital output, pin 6 = HIGH and pin 5 = LOW, door open.
int buttomEndStop = 7;             // digital input, buttom end stop.
int topEndStop = 8;                // digital indput, top end stop.

//limit values
int lightTreshold = 600;           // variable, set light treshold.
int currentUpMax = 100;            // variable, set max current motor use when open door.
int currentDownMax = 100;          // variable, set max current motor use when closing door.
const int numReadings1 = 5;        // numbers of readings from witch to calculating average motor current use.
const int numReadings2 = 10;       // numbers of readings from witch to calculating average light level.

//variabels
int currentAverage = 0;            // the average current
int lightAverage = 0;              // the running average light level over 15 min.
int doorTop = LOW;                 // variable, high if door is open.
int doorButtom = LOW;              // variable, high if door is closed.
int doorMoving = LOW;              // variable, high if door is moving.
long previousMillis = 0;           // variable, will store last time 
long interval = 90000;             // variable, interval betveen light readings.
int readings1[numReadings1];       // the readings from the analog input.
int readings2[numReadings2];       // the readings from the analog input.
int index1 = 0;                    // the index of the current avrage current reading.
int index2 = 0;                    // the index of the current average light reading over 15 min.
int total1 = 0;                    // the running total1.
int total2 = 0;                    // the running total2.



void setup() 
{
  for (int thisReading1 = 0; thisReading1 < numReadings1; thisReading1++)
  readings1[thisReading1] = 0;                             // initialize all the current current readings to 0: 
  for (int thisReading2 = 0; thisReading2 < numReadings2; thisReading2++)
  readings2[thisReading2] = 0;                             // initialize all the current light readings to 0:   
  
  
  Serial.begin(9600); 
  pinMode (currentSensor, INPUT);
  pinMode (motor, OUTPUT);
  pinMode (doorClose, OUTPUT);
  pinMode (doorOpen, OUTPUT);
  pinMode (buttomEndStop, INPUT);
  pinMode (topEndStop, INPUT);
  pinMode (lightSensor, INPUT);
}
 
 
 
int currentSensorReading()                                  //read current sensor 5 times and output the average
{ 
  total1= total1 - readings1[index1];         
  readings1[index1] = analogRead(currentSensor);             // read from the sensor:  
  total1= total1 + readings1[index1];                        // add the reading to the total:       
  index1 = index1 + 1;                                       // advance to the next position in the array:                     
  if (index1 >= numReadings1)                                // if we're at the end of the array...              
    index1 = 0;                                              // ...wrap around to the beginning:                            
  currentAverage = total1 / numReadings1;                    // calculate the average:
  return currentAverage;  
  Serial.println(currentAverage);                            // send it to the computer as ASCII digits   
  delay(1);                                                  // delay in between reads for stability            
} 
  
  

int lightSensorReading()                                    //read light sensor 10 times over 15 min. and output the running average.
{
unsigned long currentMillis = millis();
  if(currentMillis - previousMillis > interval) 
  {
  previousMillis = currentMillis;  
  total2= total2 - readings2[index2];                        // subtract the last reading:         
  readings2[index2] = analogRead(lightSensor);               // read from the sensor:   
  total2= total2 + readings2[index2];                        // add the reading to the total:       
  index2 = index2 + 1;                                       // advance to the next position in the array: 
  }  
  if (index2 >= numReadings2)                                // if we're at the end of the array...              
    index2 = 0;                                              // ...wrap around to the beginning:                            
  lightAverage = total2 / numReadings2;                      // calculate the average:
  return lightAverage; 
  Serial.println(lightAverage);                              // send it to the computer as ASCII digits   
  delay(1);                                                  // delay in between reads for stability            
}



void loop()
{
  if (lightAverage < lightTreshold)                           // close door if lightlevel is lower than lighttreshold.
     {doorClose == HIGH; doorOpen == LOW; motor = HIGH;}
  
  if (lightAverage >= lightTreshold)                          // open door if light sensor reads higher than or equal to lightTreshold. 
     {doorOpen = HIGH; doorClose = LOW; motor = HIGH;}

  if (digitalRead (buttomEndStop) == HIGH || digitalRead (topEndStop) == HIGH || (currentAverage) >= currentUpMax || (currentAverage) >= currentDownMax) 
     {motor = LOW;}    // stop open door if buttoEndStop or topEndStop is HIGH, or current reading i higher than currentUpMax/currentDownMax Treshold. 

  if (digitalRead (topEndStop) == HIGH) {doorTop = HIGH; doorButtom = LOW; doorMoving = LOW;}
    else if (digitalRead (buttomEndStop) == HIGH) {doorButtom = HIGH; doorTop = LOW; doorMoving = LOW;}
    else {doorMoving = HIGH; doorTop = LOW; doorButtom = LOW;}                                 //show if door is open, closed or moving.
}

Well, you have to call your sensors reading functions in your main loop().

  if (lightSensorReading()      < lightTreshold)                           // close door if lightlevel is lower than lighttreshold.
     {doorClose == HIGH; doorOpen == LOW; motor = HIGH;}
  
  if (lightSensorReading()      >= lightTreshold)                          // open door if light sensor reads higher than or equal to lightTreshold. 
     {doorOpen = HIGH; doorClose = LOW; motor = HIGH;}

 if ((digitalRead (buttomEndStop) == HIGH) || (digitalRead (topEndStop) == HIGH) || (currentSensorReading()     >= currentUpMax) || (currentSensorReading()     >= currentDownMax) ) 
     {motor = LOW;}    // stop open door if buttoEndStop or topEndStop is HIGH, or current reading i higher than currentUpMax/currentDownMax Treshold. 
...

So your “lightAvarage” will be returned by calling the “lightSensorReading()” function, etc.

Also “return…” shall be the last line in both your functions, ie:

 ... currentAverage = total1 / numReadings1;                    // calculate the average:
  //return currentAverage;  
  Serial.println(currentAverage);                            // send it to the computer as ASCII digits   
  delay(1);                                                  // delay in between reads for stability 
 return currentAverage;  
}

In your concept the loop() must be looped quite frequently as you are checking there various buttons and switches, thus any delays() in your sensor functions will cause the buttons/switches might not be read/served properly. And the chickens will be unhappy… no eggs served at breakfast then :slight_smile:

Hello Pito.

Thanks for the quick reply and suggestions for changes.
Changes have now been made ??in the program. And now I think I have a little better understanding of how the “functions” works, thanks.

/* 

Open and closing the door in the chicken coop depending on ambient light level.

This sketch was made with cut and paste from:
http://arduino.cc/en/Tutorial/BlinkWithoutDelay
http://arduino.cc/en/Tutorial/Smoothing

*/


//analoge I/O
int currentSensor = 2;             //analog input, current sensor on door motor.
int lightSensor = 3;               //analog input, light level sensor.

//digitale I/O
int motor = 4;                     // digital output, enable motor = pin HIGH, stop motor = pin LOW.
int doorClose = 5;                 // digital output, pin 5 = HIGH and pin 6 = LOW, door close.
int doorOpen = 6;                  // digital output, pin 6 = HIGH and pin 5 = LOW, door open.
int buttomEndStop = 7;             // digital input, buttom end stop.
int topEndStop = 8;                // digital indput, top end stop.

//limit values
int lightTreshold = 600;           // variable, set light treshold.
int currentUpMax = 100;            // variable, set max current motor use when open door.
int currentDownMax = 100;          // variable, set max current motor use when closing door.
const int numReadings1 = 5;        // numbers of readings from witch to calculating average motor current use.
const int numReadings2 = 10;       // numbers of readings from witch to calculating average light level.

//variabels
int currentAverage = 0;            // the average current
int lightAverage = 0;              // the running average light level over 15 min.
int doorTop = LOW;                 // variable, high if door is open.
int doorButtom = LOW;              // variable, high if door is closed.
int doorMoving = LOW;              // variable, high if door is moving.
long previousMillis = 0;           // variable, will store last time 
long interval = 90000;             // variable, interval betveen light readings.
int readings1[numReadings1];       // the readings from the analog input.
int readings2[numReadings2];       // the readings from the analog input.
int index1 = 0;                    // the index of the current avrage current reading.
int index2 = 0;                    // the index of the current average light reading over 15 min.
int total1 = 0;                    // the running total1.
int total2 = 0;                    // the running total2.



void setup() 
{
  for (int thisReading1 = 0; thisReading1 < numReadings1; thisReading1++)
  readings1[thisReading1] = 0;                             // initialize all the current current readings to 0: 
  for (int thisReading2 = 0; thisReading2 < numReadings2; thisReading2++)
  readings2[thisReading2] = 0;                             // initialize all the current light readings to 0:   
  
  
  Serial.begin(9600); 
  pinMode (currentSensor, INPUT);
  pinMode (motor, OUTPUT);
  pinMode (doorClose, OUTPUT);
  pinMode (doorOpen, OUTPUT);
  pinMode (buttomEndStop, INPUT);
  pinMode (topEndStop, INPUT);
  pinMode (lightSensor, INPUT);
}
 
 
 
int currentSensorReading()                                  //read current sensor 5 times and output the running average.
{ 
  total1= total1 - readings1[index1];         
  readings1[index1] = analogRead(currentSensor);             // read from the sensor:  
  total1= total1 + readings1[index1];                        // add the reading to the total:       
  index1 = index1 + 1;                                       // advance to the next position in the array:                     
  if (index1 >= numReadings1)                                // if we're at the end of the array...              
    index1 = 0;                                              // ...wrap around to the beginning:                            
  currentAverage = total1 / numReadings1;                    // calculate the average:
  return currentAverage;           
} 
  
  

int lightSensorReading()                                    //read light sensor 10 times over 15 min. and output the running average.
{
unsigned long currentMillis = millis();
  if(currentMillis - previousMillis > interval) 
  {
  previousMillis = currentMillis;  
  total2= total2 - readings2[index2];                        // subtract the last reading:         
  readings2[index2] = analogRead(lightSensor);               // read from the sensor:   
  total2= total2 + readings2[index2];                        // add the reading to the total:       
  index2 = index2 + 1;                                       // advance to the next position in the array: 
  }  
  if (index2 >= numReadings2)                                // if we're at the end of the array...              
    index2 = 0;                                              // ...wrap around to the beginning:                            
  lightAverage = total2 / numReadings2;                      // calculate the average:
  return lightAverage;          
}



void loop()
{
  if (lightSensorReading() < lightTreshold)                           // close door if lightlevel is lower than lighttreshold.
     {doorClose == HIGH; doorOpen == LOW; motor = HIGH;}
  
  if (lightSensorReading() >= lightTreshold)                          // open door if light sensor reads higher than or equal to lightTreshold. 
     {doorOpen = HIGH; doorClose = LOW; motor = HIGH;}

  if (digitalRead (buttomEndStop) == HIGH || digitalRead (topEndStop) == HIGH || (currentSensorReading() >= currentUpMax) || (currentSensorReading() >= currentDownMax)) 
     {motor = LOW;}               // stop open door if buttoEndStop or topEndStop is HIGH, or current reading i higher than currentUpMax/currentDownMax Treshold. . 

  if (digitalRead (topEndStop) == HIGH) {doorTop = HIGH; doorButtom = LOW; doorMoving = LOW;}
    else if (digitalRead (buttomEndStop) == HIGH) {doorButtom = HIGH; doorTop = LOW; doorMoving = LOW;}
    else {doorMoving = HIGH; doorTop = LOW; doorButtom = LOW;}                     //show if door is open, closed or moving.
}

What does the doorMoving variable tell you that can't be worked out from doorTop and doorButtom? If both these sensors are low the door [u]must[/u] be somewhere in between them. IE, moving.

This is off topic, but I made an auto chicken coop door with a couple op-amps and it has worked well.

Schematic and video:

I’m new to Arduino as well. 8)

hi Henry_Best.

The door moving, doorTop and doorButtom variable is only added in case I want some status LED.

hi robowarner.

Nice setup, such a control would normally have been my first choice, but the whole purpose of this exercise is to learn a little more about ardiono, I see a lot of potential in Arduino to control all sorts of things here on the farm, first the farm then the world :)

pito:
Well, you have to call your sensors reading functions in your main loop().

  if (lightSensorReading()      < lightTreshold)                           // close door if lightlevel is lower than lighttreshold.

{doorClose == HIGH; doorOpen == LOW; motor = HIGH;}
 
 if (lightSensorReading()      >= lightTreshold)                          // open door if light sensor reads higher than or equal to lightTreshold.
    {doorOpen = HIGH; doorClose = LOW; motor = HIGH;}

if ((digitalRead (buttomEndStop) == HIGH) || (digitalRead (topEndStop) == HIGH) || (currentSensorReading()     >= currentUpMax) || (currentSensorReading()     >= currentDownMax) )
    {motor = LOW;}    // stop open door if buttoEndStop or topEndStop is HIGH, or current reading i higher than currentUpMax/currentDownMax Treshold.

Trouble with that is that you do all of the calculations four times on each loop when only twice is necessary.

Notice the difference:

  int reading = lightSensorReading();
  if (reading < lightTreshold)                           // close door if lightlevel is lower than lighttreshold.
     {doorClose == HIGH; doorOpen == LOW; motor = HIGH;}
  
  if (reading >= lightTreshold)                          // open door if light sensor reads higher than or equal to lightTreshold. 
     {doorOpen = HIGH; doorClose = LOW; motor = HIGH;}
  reading = currentSensorReading();
 if (digitalRead (buttomEndStop) || digitalRead (topEndStop)  || (reading  >= currentUpMax) || (reading  >= currentDownMax) ) 
     {motor = LOW;}    // stop open door if buttoEndStop or topEndStop is HIGH, or current reading i higher than currentUpMax/currentDownMax Treshold. 
...

I have also removed the “== HIGH” part of the if statement for digitalRead() as it is redundant. digitalRead() returns HIGH or LOW yes, but in the case of boolean logic, anything that is not 0 (LOW) is considered true.

Trouble with that is that you do all of the calculations four times on each loop when only once is necessary.

He calls two different functions!.. So he does it 2x2 what could be done better as Tom said, for example:

  int reading_light = lightSensorReading();
  if (reading_light< lightTreshold)                           // close door if lightlevel is lower than lighttreshold.
     {doorClose == HIGH; doorOpen == LOW; motor = HIGH;}
  
  if (reading_light >= lightTreshold)                          // open door if light sensor reads higher than or equal to lightTreshold. 
     {doorOpen = HIGH; doorClose = LOW; motor = HIGH;}

int reading_current = currentSensorReading();
 if (digitalRead (buttomEndStop) || digitalRead (topEndStop)  || (reading_current  >= currentUpMax) || (reading_current  >= currentDownMax) ) 
     {motor = LOW;}    // stop open door if buttoEndStop or topEndStop is HIGH, or current reading i higher than currentUpMax/currentDownMax Treshold. 
...

or

if (lightSensorReading()< lightTreshold) {      // close door if lightlevel is lower than lighttreshold.
  doorClose == HIGH; 
  doorOpen == LOW; 
  motor = HIGH;
}  
else  {      // open door if light sensor reads higher than or equal to lightTreshold. 
  doorOpen = HIGH; 
  doorClose = LOW; 
  motor = HIGH;
}

int reading_current = currentSensorReading();
if (digitalRead (buttomEndStop) ||               \
     digitalRead (topEndStop)  ||                 \
     (reading_current  >= currentUpMax) ||       \
     (reading_current  >= currentDownMax) )       \
         {
  motor = LOW;     // stop open door if buttoEndStop or topEndStop is HIGH, 
                  // or current reading is higher than currentUpMax/currentDownMax Treshold. 
}    
...

Also mind when reading sensors and actuating doors based on its values - the sensor’s values read may jump +/ few digits (especially when measuring slow changing stuff, like light level), so when you do an “exact compare” with your threshold the doors may open/close erratically each loop. So you will get a “horror coop” then :slight_smile:

I would introduce a “hysteresis”, it means (an example only):

int light_hysteresis = 5;
if (reading_light < (lightTreshold - light_hysteresis) ) { 
    do something with your doors - close the door;
 }
if (reading_light > (lightTreshold + light_hysteresis) ) { 
    do something else with your doors - open the door;
}
..

Mind you cannot use single “if…else…” then, because there is a “dead zone” between the two “if-s” above. The “dead zone” is 2*5=10 in the above example (so the light level must change at least by 10 in order to change the door’s previous state). Try to introduce the hysteresis in both light and current comparisons with your thresholds. The hysteresis values needed - that is matter of experimenting…

pito:
He calls two different functions!..

Missed that, post modified.

Hi.

Again thanks for the time and input.

Sketch has been slightly modified, among other things with a hysteresis for light sensor.

/* 

Open and closing the door in the chicken coop depending on ambient light level.

This sketch was made with cut and paste from:
http://arduino.cc/en/Tutorial/BlinkWithoutDelay
http://arduino.cc/en/Tutorial/Smoothing

And a lot of good ideas, help and suggestions from:
http://arduino.cc/en/

*/


//analoge I/O
int currentSensor = 2;             //analog input, current sensor on door motor.
int lightSensor = 3;               //analog input, light level sensor.

//digitale I/O
int motor = 4;                     // digital output, enable motor = pin HIGH, stop motor = pin LOW.
int doorClose = 5;                 // digital output, pin 5 = HIGH and pin 6 = LOW, door close.
int doorOpen = 6;                  // digital output, pin 6 = HIGH and pin 5 = LOW, door open.
int buttomEndStop = 7;             // digital input, buttom end stop.
int topEndStop = 8;                // digital indput, top end stop.

//limit values
int light_hysteresis = 50;         // variable for light hysteresis
int lightTreshold = 600;           // variable, set light treshold.
int currentUpMax = 100;            // variable, set max current motor use when open door.
int currentDownMax = 100;          // variable, set max current motor use when closing door.
const int numReadings1 = 5;        // numbers of readings from witch to calculating average motor current use.
const int numReadings2 = 10;       // numbers of readings from witch to calculating average light level.
long interval = 90000;             // variable, interval betveen light readings.

//variabels
int motorCurrentAlarm = LOW;       // to show there has been a motor over current       
int currentAverage = 0;            // the average current
int lightAverage = 0;              // the running average light level over 15 min.
int doorTop = LOW;                 // variable, high if door is open.
int doorButtom = LOW;              // variable, high if door is closed.
int doorMoving = LOW;              // variable, high if door is moving.
long previousMillis = 0;           // variable, will store last time 
int readings1[numReadings1];       // the readings from the analog input.
int readings2[numReadings2];       // the readings from the analog input.
int index1 = 0;                    // the index of the current avrage current reading.
int index2 = 0;                    // the index of the current average light reading over 15 min.
int total1 = 0;                    // the running total1.
int total2 = 0;                    // the running total2.



void setup() 
{
  for (int thisReading1 = 0; thisReading1 < numReadings1; thisReading1++)
  readings1[thisReading1] = 0;                             // initialize all the current current readings to 0: 
  for (int thisReading2 = 0; thisReading2 < numReadings2; thisReading2++)
  readings2[thisReading2] = 0;                             // initialize all the current light readings to 0:   
  
  
  Serial.begin(9600); 
  pinMode (currentSensor, INPUT);
  pinMode (motor, OUTPUT);
  pinMode (doorClose, OUTPUT);
  pinMode (doorOpen, OUTPUT);
  pinMode (buttomEndStop, INPUT);
  pinMode (topEndStop, INPUT);
  pinMode (lightSensor, INPUT);
}
 
 
 
int currentSensorReading()                                  //read current sensor 5 times and output the running average.
{ 
  total1= total1 - readings1[index1];         
  readings1[index1] = analogRead(currentSensor);             // read from the sensor:  
  total1= total1 + readings1[index1];                        // add the reading to the total:       
  index1 = index1 + 1;                                       // advance to the next position in the array:                     
  if (index1 >= numReadings1)                                // if we're at the end of the array...              
    index1 = 0;                                              // ...wrap around to the beginning:                            
  currentAverage = total1 / numReadings1;                    // calculate the average:
  return currentAverage;           
} 
  
  

int lightSensorReading()                                    //read light sensor 10 times over 15 min. and output the running average.
{
unsigned long currentMillis = millis();
  if(currentMillis - previousMillis > interval) 
  {
  previousMillis = currentMillis;  
  total2= total2 - readings2[index2];                        // subtract the last reading:         
  readings2[index2] = analogRead(lightSensor);               // read from the sensor:   
  total2= total2 + readings2[index2];                        // add the reading to the total:       
  index2 = index2 + 1;                                       // advance to the next position in the array: 
  }  
  if (index2 >= numReadings2)                                // if we're at the end of the array...              
    index2 = 0;                                              // ...wrap around to the beginning:                            
  lightAverage = total2 / numReadings2;                      // calculate the average:
  return lightAverage;          
}



void loop()
{
  if (lightSensorReading() < lightTreshold - light_hysteresis)          // close door if lightlevel is lower than lighttreshold.
     {
     doorClose == HIGH; 
     doorOpen == LOW; 
     motor = HIGH;
     }
  
  if (lightSensorReading() >= lightTreshold + light_hysteresis)         // open door if light sensor reads higher than or equal to lightTreshold. 
     {
     doorOpen = HIGH; 
     doorClose = LOW; 
     motor = HIGH;
     }

  if (digitalRead (buttomEndStop) == HIGH ||             // stop open door if buttoEndStop or topEndStop is HIGH,
      digitalRead (topEndStop) == HIGH)
     {motor = LOW;}
    
  int reading_current = currentSensorReading(); 
  if (reading_current >= currentDownMax);        // stop door if current reading i higher than currentDownMax Treshold.
     {
      motor = LOW; 
      motorCurrentAlarm = HIGH;
     }

  if (reading_current >= currentUpMax)           // stop door if current reading i higher than currentUpMax Treshold.
     {
      motor = LOW; 
      motorCurrentAlarm = HIGH;
     }       

  if (digitalRead (topEndStop) == HIGH) 
     {
      doorTop = HIGH; 
      doorButtom = LOW; 
      doorMoving = LOW;
     }
    else if (digitalRead (buttomEndStop) == HIGH) 
     {
      doorButtom = HIGH; 
      doorTop = LOW; 
      doorMoving = LOW;
     }
    else 
     {
      doorMoving = HIGH; 
      doorTop = LOW; 
      doorButtom = LOW;
     }                     //just to show if door is open, closed or moving.
}

For now I think that I let the redundant “== HIGH” part be, if it does not matter easier for me to understand for now :slight_smile:

Current sensor readings will only be used to see if the reading is bigger or equal to the threshold, there is no equal to or lower comparison.
And with a few test runs with the motor, I can measure how much current the motor uses and adjust the threshold in relation. And increasing the number of readings as it must calculate the average of.

And what is the difference between:

 int reading = lightSensorReading();
  if (reading < lightTreshold)

and:

if (lightSensorReading() < lightTreshold)

I would do it in this way - when doing comparison be careful with execution order so I rather use parentheses. Reading “reading_light” outside those ifs has the advantage you do it only once.

  int reading_light = lightSensorReading();       // here we read the light sensor

  if (reading_light  < (lightTreshold - light_hysteresis) )          // close door if lightlevel is lower than lighttreshold.
     {
....
     }
  
  if (reading_light  >= (lightTreshold + light_hysteresis) )        // open door if light sensor reads higher than or equal to lightTreshold. 
     {
....
     }

I am not sure whether I do understand what you do in the int lightSensorReading() function.

  if (lightSensorReading() < lightTreshold - light_hysteresis)          // close door if lightlevel is lower than lighttreshold.
...
  if (lightSensorReading() >= lightTreshold + light_hysteresis)         // open door if light sensor reads higher than or equal to lightTreshold. 
...

You call the function twice. Two things wrong with that: (1) it takes longer because all the calculations have to be repeated. (2) what if the value changes between the first and second calls?

Also:

 if (lightSensorReading() < lightTreshold - light_hysteresis)          // close door if lightlevel is lower than lighttreshold.
     {
     doorClose == HIGH; 
     doorOpen == LOW; 
     motor = HIGH;
     }

Those should be ‘=’ not ‘==’ - its an assignment, not a comparison.
But actually in this case they shouldn’t be either. You are trying to write to digital output pins, but actually all you are doing is assigning 1 or 0 to a variable. Perhaps what you are trying to do is:

 if (lightSensorReading() < lightTreshold - light_hysteresis)          // close door if lightlevel is lower than lighttreshold.
     {
     digitalWrite(doorClose,HIGH); 
     digitalWrite(doorOpen,LOW); 
     digitalWrite(motor,HIGH);
     }

My understanding of what you want to do in the light reading function is as follows:

int lightSensorReading()            //read light sensor 10 times over 15 min. and output the running average.
{
unsigned long currentMillis = millis();
  if(currentMillis - previousMillis > interval) 
  {
  previousMillis = currentMillis;  
  readings2[index2] = analogRead(lightSensor);               // read from the sensor into the array:   
  index2 = index2 + 1;                                       // advance to the next position in the array: 
  }  
  if (index2 >= numReadings2)  
    {                                                         // if we're at the end of the array...              
     index2 = 0;                                              // ...wrap around to the beginning:  
    }

  lightAverage = 0;
  for(int i=0; i<numReadings2; i++)
      {
      lightAverage = lightAverage + readings2[i];             // calculate the average from actual array content:
      }
  lightAverage = lightAverage/numReadings2;
  
  return lightAverage;          
}

The issue here is you will not get a correct lightAverage value within first 15minutes after a reset.

Hi.

Tom Carpenter> you’re right, good catch, it would be best if the door opens :slight_smile:
Now I think that I have understood the advantage of a function return its value to a variable and then call the variable.
Apparently there are many places where you can omit the “=” sign.

pito> My idea was with a number of measurements to measure an average of the light level over a period of 15min.
The 15min. was my “hysteresis” attempts to make open / close time a little stable.
I found the smoothing example in the learning section, but the time period the smoother over to was too small, so I’ve tried to combine it with Blink Without Delay example, but I can see that it seems a bit messy and I’m sure that there are a more elegant way to do it.

I think smoothing example works fine for current measurements.

/* 

Open and closing the door in the chicken coop depending on ambient light level.

This sketch was made with cut and paste from:
http://arduino.cc/en/Tutorial/BlinkWithoutDelay
http://arduino.cc/en/Tutorial/Smoothing

And a lot of good ideas, help and suggestions from:
http://arduino.cc/en/

*/


//analoge I/O
int currentSensor = 2;             //analog input, current sensor on door motor.
int lightSensor = 3;               //analog input, light level sensor.

//digitale I/O
int motor = 4;                     // digital output, enable motor = pin HIGH, stop motor = pin LOW.
int doorClose = 5;                 // digital output, pin 5 = HIGH and pin 6 = LOW, door close.
int doorOpen = 6;                  // digital output, pin 6 = HIGH and pin 5 = LOW, door open.
int buttomEndStop = 7;             // digital input, buttom end stop.
int topEndStop = 8;                // digital indput, top end stop.

//limit values
int light_hysteresis = 50;         // variable for light hysteresis
int lightTreshold = 600;           // variable, set light treshold.
int currentUpMax = 100;            // variable, set max current motor use when open door.
int currentDownMax = 100;          // variable, set max current motor use when closing door.
const int numReadings1 = 5;        // numbers of readings from witch to calculating average motor current use.
const int numReadings2 = 10;       // numbers of readings from witch to calculating average light level.
long interval = 90000;             // variable, interval betveen light readings.

//variabels
int motorCurrentAlarm = LOW;       // to show there has been a motor over current       
int currentAverage = 0;            // the average current
int lightAverage = 0;              // the running average light level over 15 min.
int doorTop = LOW;                 // variable, high if door is open.
int doorButtom = LOW;              // variable, high if door is closed.
int doorMoving = LOW;              // variable, high if door is moving.
long previousMillis = 0;           // variable, will store last time 
int readings1[numReadings1];       // the readings from the analog input.
int readings2[numReadings2];       // the readings from the analog input.
int index1 = 0;                    // the index of the current avrage current reading.
int index2 = 0;                    // the index of the current average light reading over 15 min.
int total1 = 0;                    // the running total1.
int total2 = 0;                    // the running total2.



void setup() 
{
  for (int thisReading1 = 0; thisReading1 < numReadings1; thisReading1++)
  readings1[thisReading1] = 0;                             // initialize all the current current readings to 0: 
  for (int thisReading2 = 0; thisReading2 < numReadings2; thisReading2++)
  readings2[thisReading2] = 0;                             // initialize all the current light readings to 0:   
  
  
  Serial.begin(9600); 
  pinMode (currentSensor, INPUT);
  pinMode (motor, OUTPUT);
  pinMode (doorClose, OUTPUT);
  pinMode (doorOpen, OUTPUT);
  pinMode (buttomEndStop, INPUT);
  pinMode (topEndStop, INPUT);
  pinMode (lightSensor, INPUT);
}
 
 
 
int currentSensorReading()                                  //read current sensor 5 times and output the running average.
{ 
  total1= total1 - readings1[index1];         
  readings1[index1] = analogRead(currentSensor);             // read from the sensor:  
  total1= total1 + readings1[index1];                        // add the reading to the total:       
  index1 = index1 + 1;                                       // advance to the next position in the array:                     
  if (index1 >= numReadings1)                                // if we're at the end of the array...              
    index1 = 0;                                              // ...wrap around to the beginning:                            
  currentAverage = total1 / numReadings1;                    // calculate the average:
  return currentAverage;           
} 
  
  

int lightSensorReading()                                    //read light sensor 10 times over 15 min. and output the running average.
{
unsigned long currentMillis = millis();
  if(currentMillis - previousMillis > interval) 
  {
  previousMillis = currentMillis;  
  total2= total2 - readings2[index2];                        // subtract the last reading:         
  readings2[index2] = analogRead(lightSensor);               // read from the sensor:   
  total2= total2 + readings2[index2];                        // add the reading to the total:       
  index2 = index2 + 1;                                       // advance to the next position in the array: 
  }  
  if (index2 >= numReadings2)                                // if we're at the end of the array...              
    index2 = 0;                                              // ...wrap around to the beginning:                            
  lightAverage = total2 / numReadings2;                      // calculate the average:
  return lightAverage;          
}



void loop()
{
  int lightReading = lightSensorReading();
  if (lightReading < lightTreshold - light_hysteresis)          // close door if lightlevel is lower than lighttreshold.
     {
     digitalWrite(doorClose,HIGH); 
     digitalWrite(doorOpen,LOW); 
     digitalWrite(motor,HIGH);
     }
  
  if (lightReading >= lightTreshold + light_hysteresis)         // open door if light sensor reads higher than or equal to lightTreshold. 
     {
     digitalWrite(doorClose,LOW); 
     digitalWrite(doorOpen,HIGH); 
     digitalWrite(motor,HIGH);
     }

  if (digitalRead (buttomEndStop) == HIGH ||             // stop open door if buttoEndStop or topEndStop is HIGH,
      digitalRead (topEndStop) == HIGH)
     {digitalWrite (motor, LOW);}
    
    
    
  int currentReading = currentSensorReading(); 
  if (currentReading >= currentDownMax);        // stop door if current reading i higher than currentDownMax Treshold.
     {
      digitalWrite (motor, LOW); 
      motorCurrentAlarm = HIGH;
     }

  if (currentReading >= currentUpMax)           // stop door if current reading i higher than currentUpMax Treshold.
     {
      digitalWrite (motor, LOW); 
      motorCurrentAlarm = HIGH;
     }       


  if (digitalRead (topEndStop) == HIGH) 
     {
      doorTop = HIGH; 
      doorButtom = LOW; 
      doorMoving = LOW;
     }
    else if (digitalRead (buttomEndStop) == HIGH) 
     {
      doorButtom = HIGH; 
      doorTop = LOW; 
      doorMoving = LOW;
     }
    else 
     {
      doorMoving = HIGH; 
      doorTop = LOW; 
      doorButtom = LOW;
     }                     //just to show if door is open, closed or moving.
}

Now I think that I have understood the advantage of a function return its value to a variable and then call the variable.

Well, you can't call a variable, so your understand is still a bit wrong.