programming Servo motor issues with automatic pet flap.

Trying to build automatic pet flap and feeder.
Have the project nearly built and all code individually tested all ok.
Have now put complete code together but have issue with servo motor.
Basically when pet approaches pet flap rfid reads tag on pet, if access granted servo moves to 90 degrees position to open lock on flap. the pet pushes through the flap and as flap swings back and forth until it settles in vertical position it will have activated reed switches on either side of the flap and this will have incremented two counters. Whichever counter reaches four first will indicate whether the pet is entering or leaving the house and will light a green or red led to alert owner the direction the pet has taken. My problem is as the code shows once the count has reached four it does light the appropriate led but I cant get the servo to move to close the flap. It is added in at the end of the led_ckt code.
I thought to save power I could get servo to go to close position and then detach.
Any help appreciated.
Here is code just for this issue.

void opendoor(unsigned char check)
    {
      if ((check == 35)||(check == 242))
      {
      myservo.attach(9);//attachs the servo on pin 9 to servo object
      myservo.write(90);//goes to 90 degrees and open door
      led_ckt(); 
      }
                                                         
      }

void feeder(void)
{
  digitalWrite(petfeeder,HIGH);       //goes to Pet feeder hardware ckt
  delay(20000);
  digitalWrite(petfeeder,LOW);
}
    void led_ckt(void )
  { 
  
     while ((Counter <= 3)&&(Counter1 <= 3))
    {
      State = digitalRead(entry);
      State1 = digitalRead(leave);
    if (State != lastButtonState) 
    {
      if (State == HIGH) 
    {
          Counter++;
    
    delay(200);//debounce
    }
    }
   
  if (State1 != lastButtonState1) 
    {
       if (State1 == HIGH) 
    {
          Counter1++;
        
    delay(200);//debounce
    }
    }
    }
  lastButtonState = State;
  lastButtonState1 = State1;
           
  if (Counter1 > 3)  // exit led
   {
    digitalWrite(red, HIGH);
    digitalWrite(green, LOW);
    delay(30);
    Counter1 = 0;
    Counter = 0;
    delay(2000);
    //myservo.attach(9);//attachs the servo on pin 9 to servo object
    myservo.write(0);  //close door                                              
    myservo.detach();

     
   } 
  if (Counter >3) //  entry led
   {
    digitalWrite(green, HIGH);
    digitalWrite(red, LOW);
    delay(30);
    Counter = 0;
    Counter1 = 0;
    delay(2000);
    //myservo.attach(9);//attachs the servo on pin 9 to servo object
    myservo.write(0); //close door                                               
    myservo.detach();

   } 
   }

Please post a complete sketch.

I suspect you need a variable to record the desired state of the flap - perhaps call it flapLocked and normally it is true. When the RFID tag is detected the variable is set to false. After the flap finishes flapping the variable is set back to true.

Then the servo lock can be controlled by a little function like this (use the correct angles, of course)

void operateServoLock() {
  if (flapLocked == true) {
      lockServo.write(0);
  }
  else {
      lockServo.write(90);
   }
}

...R

aarg:
Please post a complete sketch.

But use Tools + Auto Format, to fix that horrid indenting.

I will look into that advice Robin2 thanks
Here is the complete code

#include <Servo.h>
#include <AddicoreRFID.h>    
#include <SPI.h>
#include <virtuabotixRTC.h> 

Servo myservo;//create servo object to control a servo
virtuabotixRTC myRTC(6, 7, 8);                   //RTC
 
AddicoreRFID myRFID;  //AddicoreRFID object to control the RFID module
unsigned char serNumA[5];  //4 bytes tag serial number, the first 5 bytes for the checksum byte 

const int activation = 10; 
const int NRSTPD = 5; 

#define MAX_LEN 16   //Maximum length of the array

const int start_curfew =2;      //curfew
const int  End_curfew = 4;

const int  breakfast = 12;       //feeder
const int dinner = 19;
const int petfeeder = 3;

const int red = 4;
const int green = 2;
const int entry = A0;// monitor ckt for green led, normally low, goes high when reed 1 switch activate.  //led ckt
const int leave = A1;// monitor ckt for red led, normally low, goes high when reed 2 switch activate. 

int State = 0;
int State1 = 0;
int Counter1 = 0; // counter for red led
int Counter = 0; // counter for green led
int lastButtonState1 = 0; //last state of leave cct
int lastButtonState = 0; //last state of entry cct

void opendoor();
void tag(void);
void time(void);
void feeder(void);
void led_ckt(void);

void setup()
{
  Serial.begin(9600);       // RFID reader SOUT pin connected to Serial RX pin at 9600bps      //universal
  myRTC.setDS1302Time(00, 5, 21, 6, 10, 1, 2014); 

  pinMode(petfeeder,OUTPUT);     //feeder

  SPI.begin();              // start the SPI library:
  pinMode(activation, OUTPUT);        // Set digital pin 10 as OUTPUT to connect it to the RFID /ENABLE pin
  digitalWrite(activation, LOW);      // Activate the RFID reader 
  pinMode(NRSTPD, OUTPUT);               // Set digital pin 10 , Not Reset and Power-down       //tag
  digitalWrite(NRSTPD, HIGH);
  myRFID.AddicoreRFID_Init();

   pinMode(red,OUTPUT);
   pinMode(green,OUTPUT);
                                                           // led ckt
   pinMode(entry,INPUT);// reed switch for green led
   pinMode(leave,INPUT);// reed switch for red led

  myservo.attach(9);//attachs the servo on pin 9 to servo object
  myservo.write(0);//back to 0 degrees      //servo
  delay(500);//wait for half a second
}
void loop()
{          // changes curfew time
 if((myRTC.minutes >= start_curfew) && (myRTC.minutes <= End_curfew)) 
{
  time();
}
else
{
  tag(); 
  time();
  //led_ckt(); 
} 
 if((myRTC.hours == breakfast)&&(myRTC.minutes == 0)&&(myRTC.seconds == 0))// Not sure whether to put this in loop or feeder function  //
  {
    feeder();
  }
  if((myRTC.hours == dinner)&&(myRTC.minutes == 0)&&(myRTC.seconds == 0))
  {
    feeder();
  }
}
void time(void)
{     
   // This allows for the update of variables for time or accessing the individual elements. 
  myRTC.updateTime();
         
// Start printing elements as individuals        
  Serial.print("Current Date / Time: ");       
  Serial.print(myRTC.dayofmonth);        
  Serial.print("/");        
  Serial.print(myRTC.month);      
  Serial.print("/"); 
  Serial.print(myRTC.year);
  Serial.print("  ");
  Serial.print(myRTC.hours);  
  Serial.print(":");  
  Serial.print(myRTC.minutes); 
  Serial.print(":");  
  Serial.println(myRTC.seconds);
  }
void tag(void)
{
  myRFID.AddicoreRFID_Init();
  unsigned char checksum;
  unsigned char status;
  unsigned char str[MAX_LEN];
  
  //Find tag, return tag detected
  status = myRFID.AddicoreRFID_Request(PICC_REQIDL, str);
  if (status == MI_OK)
  {
    Serial.println("RFID tag detected");
  }

  //Anti-collision, return tag serial number 
  status = myRFID.AddicoreRFID_Anticoll(str);
  if (status == MI_OK)
  {
    checksum = str[0] ^ str[1] ^ str[2] ^ str[3];
    Serial.println("The tag's number is : ");
    Serial.println(checksum);    //Gets each of the 4 bypes and exor's them to get the checksum value, this is set to be the cards number 
  
  opendoor(checksum);
  }
  myRFID.AddicoreRFID_Halt();             //Command tag into hibernation 
}

void opendoor(unsigned char check)
    {
      if ((check == 35)||(check == 242))
      {
      myservo.attach(9);//attachs the servo on pin 9 to servo object
      myservo.write(90);//goes to 90 degrees and open door
      led_ckt(); 
      }
      }
void feeder(void)
  {
  digitalWrite(petfeeder,HIGH);       //goes to Pet feeder hardware ckt
  delay(20000);
  digitalWrite(petfeeder,LOW);
  }
    void led_ckt(void )
  { 
       while ((Counter <= 3)&&(Counter1 <= 3))
    {
      State = digitalRead(entry);
      State1 = digitalRead(leave);
    if (State != lastButtonState) 
    {
      if (State == HIGH) 
    {
          Counter++;
    
    delay(200);//debounce
    }
    }
     if (State1 != lastButtonState1) 
    {
       if (State1 == HIGH) 
    {
          Counter1++;
        
    delay(200);//debounce
    }
    }
    }
  lastButtonState = State;
  lastButtonState1 = State1;
           
  if (Counter1 > 3)
   {
    digitalWrite(red, HIGH);
    digitalWrite(green, LOW);
    delay(30);
    Counter1 = 0;
    Counter = 0;
    delay(2000);
    //myservo.attach(9);//attachs the servo on pin 9 to servo object
    myservo.write(0);  //close door
    myservo.detach();
    } 
  if (Counter >3) 
   {
    digitalWrite(green, HIGH);
    digitalWrite(red, LOW);
    delay(30);
    Counter = 0;
    Counter1 = 0;
    delay(2000);
    //myservo.attach(9);//attachs the servo on pin 9 to servo object
    myservo.write(0); //close door 
    myservo.detach();
   } 
   }
  pinMode(activation, OUTPUT);        // Set digital pin 10 as OUTPUT to connect it to the RFID /ENABLE pin
  digitalWrite(activation, LOW);      // Activate the RFID reader
  pinMode(NRSTPD, OUTPUT);               // Set digital pin 10 , Not Reset and Power-down       //tag
  digitalWrite(NRSTPD, HIGH);

You have two names for pin 10?

// changes curfew time
 if((myRTC.minutes >= start_curfew) && (myRTC.minutes <= End_curfew))
{
  time();
}

No, that code does not change curfew time. Or, if it does, that’s a dumb name for the function.

But use Tools + Auto Format, to fix that horrid indenting.

You forgot something. I can’t read that mess.

For PaulS

I am an old man trying to come to terms with all this new technology. My computer and programming skills are slowly increasing.
I just recently got interested in programming and I am trying to get to grips with it and clearly I have not got your expertise in these areas.
I joined the forum to get some help from members if I ran into problems.
There is one thing I have learned over the years and that is if you want to encourage people to try and improve, starting off by insulting them usually does not get the result required.

starting off by insulting them usually does not get the result required.

Well, perhaps you can show where I insulted you.

I asked some questions. I (perhaps bluntly) suggested that either the comment or the function name was wrong. I pointed out that your indenting was less than acceptable. I pointed out that there was a dirt simple solution for that. I commented that I don’t read much of code that is not acceptably indented.

If that ruffled your feathers so badly, calm down.

Hi Robin2

I did as you said and tried it on a breadboard with push button switches.
I have added a 4 second delay to allow the flap to settle. The rfid code works fine.
It seems to work fine except for the entry led (green) part of the code. The first time the switches are operated for the entry code the green led comes on but no delay and the servo operates straight away. After that no problem, it works every time for both green and red led code. If I start with the exit code, red led, no problem 4 second delay. Strange one.
I am very happy with the solution you advised and thanks again.
I will work through the rest of the code and hope no issues.
This is the way I wrote the code. (I know there is room for improvement but if it works I can live with that for now.)

#include <Servo.h>
#include <AddicoreRFID.h>
#include <SPI.h>
#include <virtuabotixRTC.h>

Servo myservo;//create servo object to control a servo
virtuabotixRTC myRTC(6, 7, 8);                   //RTC

AddicoreRFID myRFID;  //AddicoreRFID object to control the RFID module
unsigned char serNumA[5];  //4 bytes tag serial number, the first 5 bytes for the checksum byte

const int activation = 10;
const int NRSTPD = 5;

#define MAX_LEN 16   //Maximum length of the array

const int start_curfew = 2;     //curfew
const int  End_curfew = 4;

const int  breakfast = 12;       //feeder
const int dinner = 19;
const int petfeeder = 3;

const int red = 4;
const int green = 2;
const int entry = A0;// monitor ckt for green led, normally low, goes high when reed 1 switch activate.  //led ckt
const int leave = A1;// monitor ckt for red led, normally low, goes high when reed 2 switch activate.
int flapLocked;
int State = 0;
int State1 = 0;
int Counter1 = 0; // counter for red led
int Counter = 0; // counter for green led
int lastButtonState1 = 0; //last state of leave cct
int lastButtonState = 0; //last state of entry cct

void opendoor();
void tag(void);
void time(void);
void feeder(void);
void led_ckt(void);
void operateServoLock();

void setup()
{ boolean flapLocked = true;
  Serial.begin(9600);       // RFID reader SOUT pin connected to Serial RX pin at 9600bps      //universal
  //myRTC.setDS1302Time(00, 5, 21, 6, 10, 1, 2014);

  pinMode(petfeeder, OUTPUT);    //feeder

  SPI.begin();              // start the SPI library:
  pinMode(activation, OUTPUT);        // Set digital pin 10 as OUTPUT to connect it to the RFID /ENABLE pin
  digitalWrite(activation, LOW);      // Activate the RFID reader
  pinMode(NRSTPD, OUTPUT);               // Set digital pin 5 , Not Reset and Power-down       //tag
  digitalWrite(NRSTPD, HIGH);
  myRFID.AddicoreRFID_Init();

  pinMode(red, OUTPUT);
  pinMode(green, OUTPUT);
  // led ckt
  pinMode(entry, INPUT); // reed switch for green led
  pinMode(leave, INPUT); // reed switch for red led

  myservo.attach(9);//attachs the servo on pin 9 to servo object
  myservo.write(0);//back to 0 degrees      //servo
  delay(500);//wait for half a second
}
void loop()
{ // changes curfew time
  if ((myRTC.minutes >= start_curfew) && (myRTC.minutes <= End_curfew))
  {
    time();
  }
  else
  {
    tag();
    time();
    //led_ckt();
  }
  if ((myRTC.hours == breakfast) && (myRTC.minutes == 0) && (myRTC.seconds == 0)) // Not sure whether to put this in loop or feeder function  //
  {
    feeder();
  }
  if ((myRTC.hours == dinner) && (myRTC.minutes == 0) && (myRTC.seconds == 0))
  {
    feeder();
  }
}
void time(void)
{
  // This allows for the update of variables for time or accessing the individual elements.
  myRTC.updateTime();

  // Start printing elements as individuals
  Serial.print("Current Date / Time: ");
  Serial.print(myRTC.dayofmonth);
  Serial.print("/");
  Serial.print(myRTC.month);
  Serial.print("/");
  Serial.print(myRTC.year);
  Serial.print("  ");
  Serial.print(myRTC.hours);
  Serial.print(":");
  Serial.print(myRTC.minutes);
  Serial.print(":");
  Serial.println(myRTC.seconds);
}
void tag(void)
{
  myRFID.AddicoreRFID_Init();
  unsigned char checksum;
  unsigned char status;
  unsigned char str[MAX_LEN];

  //Find tag, return tag detected
  status = myRFID.AddicoreRFID_Request(PICC_REQIDL, str);
  if (status == MI_OK)
  {
    Serial.println("RFID tag detected");
  }

  //Anti-collision, return tag serial number
  status = myRFID.AddicoreRFID_Anticoll(str);
  if (status == MI_OK)
  {
    checksum = str[0] ^ str[1] ^ str[2] ^ str[3];
    Serial.println("The tag's number is : ");
    Serial.println(checksum);    //Gets each of the 4 bypes and exor's them to get the checksum value, this is set to be the cards number

    opendoor(checksum);
  }
  myRFID.AddicoreRFID_Halt();             //Command tag into hibernation
}

void opendoor(unsigned char check)
{
  if ((check == 35) || (check == 242))
  { flapLocked = !flapLocked;
    myservo.attach(9);//attachs the servo on pin 9 to servo object
    myservo.write(90);//goes to 90 degrees and open door
    led_ckt();
  }
}
void feeder(void)
{
  digitalWrite(petfeeder, HIGH);      //goes to Pet feeder hardware ckt
  delay(20000);
  digitalWrite(petfeeder, LOW);
}
void led_ckt(void )
{
  while ((Counter <= 3) && (Counter1 <= 3))
  {
    State = digitalRead(entry);
    State1 = digitalRead(leave);
    if (State != lastButtonState)
    {
      if (State == HIGH)
      {
        Counter++;

        delay(200);//debounce
      }
    }
    if (State1 != lastButtonState1)
    {
      if (State1 == HIGH)
      {
        Counter1++;

        delay(200);//debounce
      }
    }
  }
  lastButtonState = State;
  lastButtonState1 = State1;

  if (Counter1 > 3)
  {
    digitalWrite(red, HIGH);
    digitalWrite(green, LOW);
    delay(30);
    Counter1 = 0;
    Counter = 0;
    delay(4000);
    flapLocked = true;
  }

  { void operateServoLock();
    if (flapLocked == true) {
      myservo.write(0);
    }
    else {
      myservo.write(90);
    }
  }
  if (Counter > 3)
  {
    digitalWrite(green, HIGH);
    digitalWrite(red, LOW);
    delay(30);
    Counter = 0;
    Counter1 = 0;
    delay(4000);

    flapLocked = true;
  }
  { void operateServoLock();
    if (flapLocked == true) {
      myservo.write(0);
    }
    else {
      myservo.write(90);
    }
  }

}

My style (and it is no more than that) would be to write loop() like this

void loop() {
        time();
        feeder();
        tag();
        led_ckt();
        operateLock();
}

and have any checks (the feeding time, for instance) inside the relevant function.

I would have the tag() function set flapLocked to false if a valid tag is detected.
I would have led)ckt() set it back to true when appropriate.
Then the function operateLock() would just depend on that being true or false and it would contain the servo code.

Put Servo.attach() in setup() and do not use .detach() if you want the servo to hold in the locked position.

DON'T use delay() anywhere. Have a look at how millis() is used to manage timing without blocking in several things at a time

I have not managed to make sense of the led_ckt() function - perhaps because its name means nothing to me.

I suggest converting the time to a whole number of minutes (or seconds if you need the precision) and use that to make the tests simpler.

...R

Thanks for your help Robin2.

Will follow your advice.