A general hint first: indent your code correctly. It makes it much easier to read and easier to spot logical errors.
Another improvement would be to name the pins with the same names as you name them when explaining the problem to us. I expected to find a int buttonRunPin and int buttonStopPin.
The first issue I see is:
buttonNew=digitalRead(buttonPin);
buttonOff=digitalRead(buttonPin);
This reads the same button twice. You probably wanted to read two different buttons: the "run" button and the "stop" button.
The next issue I see is:
delay(8000);
delay(2000);
During these 10 seconds, you can press and let go the button many times. But since there is no digitalRead() during that time, the button press will not be recognized.
The same applies to
int dt=100;
delay(dt);
If you press the button within these 100 ms, it will not be noticed.
Unfortunately, to fix that, you need to learn a completely new concept: interrupts. And even worse, not all pins support interrupts. On the Arduino Uno, it's only pin 2 and pin 3, which means you would need to change your hardware setup (because you use pin 12 and 13). Put both buttons from the pin to ground, so that it closes when pushed.
You would then prepare the interrupt in the setup() routine:
pinMode(buttonPin, INPUT_PULLUP);
attachInterrupt(digitalPinToInterrupt(buttonPin), toggleRun, CHANGE);
pinMode(buttonPinOff, INPUT_PULLUP);
attachInterrupt(digitalPinToInterrupt(buttonPinOff), toggleStopped, CHANGE);
You also define global variables for the state of the buttons:
volatile bool running = false;
volatile bool stopped = false;
volatile is needed here to prevent the compiler from optimizing too much. The compiler does not know about the interrupts and thus can't know when the variable changes.
Next, you need two methods that are being executed whenever an interrupt occurs, i.e. you press a button:
void toggleRun() {
running = !running;
}
void toggleStopped() {
stopped = !stopped;
}
Note: the above x = !x; pattern is a short and efficient form of
if (x == false) {
x = true;
}
else if (x == true) {
x = false;
}
Once you have all that, you don't need the two digitalRead()s any more.
I'm afraid I haven't understood what your code's supposed to do. You have a lot of state variables that seem to have no use and especially they don't correspond to the motors. E.g. once RELAYGrnState is set to 1, it never changes back to 0.
This is what I have in the end:
int LEDYelState=0;
int LEDRedState=0;
int LEDGrnState=0;
int RELAYRedState=0;
int RELAYYelState=0;
int RELAYGrnState=0;
int LEDRedPin=10;
int LEDYelPin=9;
int LEDGrnPin=8;
int RELAYRedPin=7;
int RELAYYelPin=6;
int RELAYGrnPin=5;
int buttonPin=13;
int buttonNew;
int buttonOld=1;
int dt=100;
int buttonOff;
int buttonPinOff=12;
volatile bool running = false;
void toggleRun() {
running = !running;
}
volatile bool stopped = false;
void toggleStopped() {
stopped = !stopped;
}
void setup() {
// put your setup code here, to run once:
Serial.begin(9600);
pinMode(LEDRedPin, OUTPUT);
pinMode (LEDYelPin,OUTPUT);
pinMode (LEDGrnPin, OUTPUT);
pinMode (RELAYRedPin, OUTPUT);
pinMode (RELAYYelPin, OUTPUT);
pinMode (RELAYGrnPin, OUTPUT);
pinMode(buttonPin, INPUT_PULLUP);
attachInterrupt(digitalPinToInterrupt(buttonPin), toggleRun, CHANGE);
pinMode(buttonPinOff, INPUT_PULLUP);
attachInterrupt(digitalPinToInterrupt(buttonPinOff), toggleStopped, CHANGE);
}
void doSomething() {
digitalWrite(LEDRedPin,HIGH); // red LED is on
LEDRedState=1;
digitalWrite(RELAYRedPin,LOW); // red motor off? others unclear
RELAYRedState=1;
digitalWrite(LEDYelPin, HIGH); // yellow LED and red LED are on
LEDYelState=1;
digitalWrite(RELAYYelPin, LOW); // yellow motor off?
RELAYYelState=1;
delay (8000);
digitalWrite(LEDYelPin,LOW); // red LED on
digitalWrite(RELAYYelPin,HIGH); // yellow motor on
digitalWrite (LEDGrnPin,HIGH); // red and green LED are on
LEDGrnState=1;
digitalWrite (RELAYGrnPin, LOW); // green motor off
RELAYGrnState=1;
delay(2000);
digitalWrite (LEDGrnPin,LOW); // red LED on
digitalWrite (RELAYGrnPin,HIGH); // green motor on
RELAYGrnState=1;
}
void doLess() {
digitalWrite(LEDRedPin, LOW); // red LED off
LEDRedState=0;
digitalWrite(RELAYRedPin, HIGH); // red motor on
RELAYRedState=0;
digitalWrite(LEDYelPin,LOW); // yellow LED off
LEDYelState=0;
}
void loop() {
if(running){
if (!stopped){
doSomething();
}
else{
doLess();
}
}
delay(dt);
}
runfor example ... when it is true, then the engine runs ... when it is false, then the engine stops ... if you press start, then makeruntrue ... if you press start again, then makerunfalse .... when you press stop, then makerunfalse – jsotola Nov 28 '19 at 18:52