next topic »

[on:]

I ran out of time today, and I need to figure out what went wrong.
My understanding of C is minimal, as you can tell, and I think my problem is the code.
I want to take my time and understand what I did wrong, but I also want to finish this thing.
if you see anything glaringly obvious, please tell how that operation should work, so I can learn from my mistake.

Code:

/*
This is my first attempt at writing a functional code.
My goal is to automate the opening, closing and locking
of the door to my chicken coop. If you notice any problems
that will prevent the operation let me know
jakedixon@hotmail.com   
*/
#include <Servo.h>
Servo Latch;
Servo Door;
int S0=0;  //Swithch state one
int S1=0;  //Swithch state two
int S2=0;  //Swithch state three
int S3=0;  //Swithch state four
int CD=0;  //Close disable
int OD=0;  //Open Disable
int TM=0;  //time variable
int MD=0;  //Mode state
int Val=0; //Light Value
int sensorPin = A0; //Analog input pin

void setup() {               
 Latch.attach(9);
 Door.attach(10);   
}

void loop() {        //Read 4 position selector switch
S0= digitalRead(0);   //Off
S1= digitalRead(1);   //Manual open
S2= digitalRead(2);   //Manual Close
S3= digitalRead(3);   //Auto


if (S0=1){MD=0;}      //convert switch to mode
if (S1=1){MD=1;}
if (S2=1){MD=2;}
if (S3=1){Val=analogRead(sensorPin);
  if (Val>150){MD=1;}
  if (Val<100){MD=2;}
}
if (MD=1){
  if (CD=0){TM++;delay (20);
    if(TM<16){Latch.write(135);}         //.2 sec/30deg of movement therefore Rotate 90deg = 300mS or 15 cycles
    if(TM>20){Door.write(170);}          // delay, open main door 180deg=600ms 600ms/20 per cycle= 30 cycles
    if(TM>52){CD=1;OD=0;TM=0;}            //disable close, enable open reset TiMer
  }
}
if (MD=2){
  if (OD=0){TM++;delay (20);
    if(TM<32){Door.write(10);}         //.2 sec/30deg of movement therefore Rotate 90deg = 300mS or 15 cycles
    if(TM>40){Latch.write(45);}          // delay, open main door 180deg=600ms 600ms/20 per cycle= 30 cycles
    if(TM>52){OD=1;CD=0;TM=0;}    //disable close, enable open reset TiMer
  }
}
}

[Reply #1 on:]

All your if statements are like this:

Code:

if (MD=1)

when they should be like this:

Code:

if (MD==1)

[Reply #2 on:]

There is also a convention, that I strongly urge you to follow, of one statement per line. I also suggest that for readability purposes, open and closing braces so no belong on the same line as the statement that they go with. Use white space around operators and proper indenting, so one can read the code.
This:

Code:

if (MD=2){
  if (OD=0){TM++;delay (20);
    if(TM<32){Door.write(10);}         //.2 sec/30deg of movement therefore Rotate 90deg = 300mS or 15 cycles
    if(TM>40){Latch.write(45);}          // delay, open main door 180deg=600ms 600ms/20 per cycle= 30 cycles
    if(TM>52){OD=1;CD=0;TM=0;}    //disable close, enable open reset TiMer
  }
}

should be:

Code:

  if (MD == 2)
  {
    if (OD == 0)
    {
      TM++;
      delay (20);
      if(TM < 32)
      {
         Door.write(10);
      }         //.2 sec/30deg of movement therefore Rotate 90deg = 300mS or 15 cycles
      if(TM > 40)
      {
         Latch.write(45);
      }          // delay, open main door 180deg=600ms 600ms/20 per cycle= 30 cycles
      if(TM > 52)
      {
         OD = 1;
         CD = 0;
         TM = 0;
      }    //disable close, enable open reset TiMer
   }
}

One can much more easily read this code, even though it takes more lines, and one can see where the blocks stop and end.

Another thing to consider is that there is convention that all capital letter names are constants, and one does not assign values to constants. Variables should have obvious names. OD, CD, and TM don't mean squat. Door and Latch are, on the other hand, good names. It is quite clear what they are.