int on = 1; //detects when machine is on.
int led = 2; //led that turns on when machine is on.
int off = 3; //turns off machine
int up = 4; //open trapdoor that allows off pin to turn off machine.
1: For readability it is good to differentiate global variables by capitalizing the first letter
2: For constant values it is good to declare them 'const' so the compiler can protect you better.
3: As mentioned before: DON'T use Pin 1 and Serial in the same sketch.
4: It makes your pin number constants clearer if you put Pin at the end of the name
const int OnPin = 2; //detects when machine is on.
const int LedPin = 3; //led that turns on when machine is on.
const int OffPin = 4; //turns off machine
const int UpPin = 5; //open trapdoor that allows off pin to turn off machine.
The more meaningful the variable names the easier it will be to read the code
long TrapDelay;
long ArmDelay;
void setup() {
pinMode(OnPin, INPUT);
pinMode(LedPin, OUTPUT);
pinMode(OffPin, OUTPUT);
pinMode(UpPin, OUTPUT);
Serial.begin(9600);
Serial.println("begin");
}
Don't define names for pins and then use the number instead!
void loop() {
digitalWrite(0, HIGH);
TrapDelay = random(5000);
if (on == HIGH) { //insert code to open trapdoor.
digitalWrite(led, HIGH); //led turns on when machine is on.
delay(randomTrap);
digitalWrite(up, HIGH);
Serial.println("trapdoor is open");
if (up == HIGH) {
randomArm = random(5000);
delay(randomArm);
digitalWrite(off, HIGH);
}
} else {
digitalWrite(led, LOW); //led turns off when machine is off.
digitalWrite(up, LOW); //trapdoor closes.
digitalWrite(off, LOW); //turns off off.
}
}
1: You are using digitalWrite() on Pin 0 without declaring a name, using pinMode() to set it as an output
2: DON'T use Pin 0 and Serial in the same sketch.
3: You can't read an input pin by using its pin number! Use digitalRead(pin).
4: Since HIGH==1==true and LOW==0==false then digitalRead(pin) and (digitalRead(pin) == HIGH) are equivalent.
void loop() {
// digitalWrite(0, HIGH); // Don't write to Pin 0 when using Serial
if (digitalRead(OnPin)) { //insert code to open trapdoor.
digitalWrite(LedPin, HIGH); //led turns on when machine is on.
delay(random(5000));
digitalWrite(UpPin, HIGH);
Serial.println("trapdoor is open");
if (digitalRead(UpPin)) { // Yes, you can read an OUTPUT pin but since you JUST set it HIGH this is always true
delay(random(5000));
digitalWrite(OffPin, HIGH);
}
} else {
digitalWrite(LedPin, LOW); //led turns off when machine is off.
digitalWrite(UpPin, LOW); //trapdoor closes.
digitalWrite(OffPin, LOW); //turns off off.
}
}
1: You don't need to store function results in a variable (ArmDelay, TrapDelay) if you use them only once and immediately
2: Even if you are using the results more than once inside a single function you can make your variable local instead of global.
