When motors ON, push button not working

Hi. I creating a simple code with H-bridge, motors and buttons.
Buttons will increment and decrement speed and time of working motor. All that will show on OLED display.

The problem is when motors are started, the Buttons not work.

What I figured is when I delete 'delay()' under digital/analog Write (motors begin OFF) then buttons works fine in Serial, but that delay() is important to motor work.

(Note that I deleted a half of code where is second motor and display.)

int btn2=3;  //button for increment
int btn1=2;   //button for decrement
int sekunde=0;
int sek1;

int incPrev; 
int decPrev;

int in3 = 4; 
int in4 = 5; 
int ENB = 7; 

void setup() {
    Serial.begin(9600); 
    //  display.begin(SH1106_SWITCHCAPVCC, 0x3C); 

pinMode(btn1, INPUT_PULLUP);  //button for increment
pinMode(btn2, INPUT_PULLUP); //button for decrement

pinMode (4, OUTPUT); // motor 2
pinMode (5, OUTPUT); // motor 2
pinMode (7, OUTPUT); // motor 2
}

void loop() {
    int inc = digitalRead(3);
    int dec = digitalRead(2);

//////////////// MOTOR 2 /////////////////////////////////////////
// TURN ON MOTOR
digitalWrite (in4, HIGH); 
digitalWrite (in3, LOW);
analogWrite (ENB, 150);  
delay(250); //Koliko motor radi

//TURN OFF
digitalWrite (in4, LOW);
digitalWrite (in3, LOW);
delay(5000); 
//////////////// MOTOR 2 /////////////////////////////////////////


// BUTTONS /////////////////////////////////////////
 //Increment
    if((inc == HIGH) && (inc != incPrev )) {
     // delay(100);
      sekunde++;
          Serial.println(sekunde); //show 1 decimal
      sek1=sekunde*1000;
          Serial.println(sek1); // show 4 decimals
    }
    
     //Decrement
    if((dec == HIGH) && (dec != decPrev)) {
     // delay(100);
      sekunde--;
        Serial.println(sekunde); 
    sek1=sekunde*1000;    
        Serial.println(sek1); 
    }
    //storing the button states
    incPrev = inc;
    decPrev = dec;
// BUTTONS /////////////////////////////////////////
}

bad idea as this code can be part of your problem

Sure - as long as you want to create a pwm-signal this way you will need delay()
But there is a much better way to create the PWM-signal that controls the speed of the motors

function analogWrite()

best regards Stefan

Don't worry, I tested this code without this deleted parts..

How do you mean? I have analogWrite() in my code already.

  • each iteration of loop() enables the code for 0.25 msec then turns the motor off and waits 5 sec

  • sekunde and sek1 are affected by the button presses but don't affect the motor

  • the buttons are not debounced and can result in multiple inc/decrements

  • i believe only pins 3, 5, 6, 9 10 and 11 are PWM outputs

look this over

int btn2 = 3;  // increment
int btn1 = 2;  // decrement

int in3 = 4;
int in4 = 5;
int enB = 7;    // only 3, 5, 6, 9 10 and 11 are PWM outputs

int incPrev;
int decPrev;

int sekunde = 0;

// -----------------------------------------------------------------------------
void setup () {
    Serial.begin (9600);
    Serial.println ("ready");

    pinMode (btn1, INPUT_PULLUP);  //button for increment
    pinMode (btn2, INPUT_PULLUP); //button for decrement

    pinMode (in3, OUTPUT); // motor 2
    pinMode (in4, OUTPUT); // motor 2
    pinMode (enB, OUTPUT); // motor 2
}

// -----------------------------------------------------------------------------
void loop () {
    int inc = digitalRead (btn2);
    if (inc != incPrev ) {
        incPrev = inc;
        delay (20);
        if (LOW == inc)  {
            sekunde += 16;
            if (255 < sekunde)
                sekunde = 255;
            Serial.println (sekunde);
        }
    }

    int dec = digitalRead (btn1);
    if (dec != decPrev) {
        decPrev = dec;
        delay (20);
        if (LOW == dec) {
            sekunde -= 16;
            if (0 > sekunde)
                sekunde = 0;
            Serial.println (sekunde);
        }
    }

    // MOTOR
    digitalWrite (in4, HIGH);
    digitalWrite (in3, LOW);
    analogWrite  (enB, sekunde);
}
1 Like

Yes you are right. my fault.

OK you switch on motor 2 for 0,25 seconds and then you switch the motor off again
and freeze the microcontroller for 5 seconds.

Why is this 0,25 seconds switch motor on
then switch off for 5 seconds

??
seems very odd to me.

You should write a functional description what you want to do.
Functional description means to avoid using code because you might have misconeptions about how the code works.

Good try. But now motor works all time.

There is a misunderstanding here. Sorry, my fault. I confuse you with my language: sekunde is not speed,
they was second*1000!
But now I renamed all again to easy figure out.

a) When press button, seconds must go by 1 (like 1,2,3..).
When press other button must go reverse (like 3, 2, 1).
Must be 1 decimal point, because that seconds will show on display. We call it 'seconds'.

b) I added +1000 because convert that seconds to milisedoncs delay(), which must go 1000, 2000, 3000.
That will no go on display, and must be 4 decimal points.
So, a) and b) will go same time: when 1, then 1000, when 2, then 2000. We call it 'mseconds'.

Now, can you please try not edit SPEED for now? Let we set it default for 150(for now), like bellow.
We will now use only SECONDS -> How long motor will be SHUTT OFF. Can you only do that please?
(That will be miliseconds, like delay(5000); motor shut of 5 seconds.)

Btw I working on Arduino Mega in this case.

int btn2 = 3;  // increment
int btn1 = 2;  // decrement

int in3 = 4;
int in4 = 5;
int enB = 7;    // only 3, 5, 6, 9 10 and 11 are PWM outputs

int incPrev;
int decPrev;

int seconds = 0; //seconds count
int mseconds; //miliseconds

// -----------------------------------------------------------------------------
void setup () {
    Serial.begin (9600);
    Serial.println ("ready");

    pinMode (btn1, INPUT_PULLUP);  //button for increment
    pinMode (btn2, INPUT_PULLUP); //button for decrement

    pinMode (in3, OUTPUT); // motor 2
    pinMode (in4, OUTPUT); // motor 2
    pinMode (enB, OUTPUT); // motor 2
}

// -----------------------------------------------------------------------------
void loop () {

    int inc = digitalRead (btn2);
    if (inc != incPrev ) {
        incPrev = inc;
        delay (20);
        if (LOW == inc)  {
         
         delay(100);
      seconds++;
          Serial.println(seconds); //show seconds - later for 'display'
      mseconds=seconds*1000;
          Serial.println(mseconds); // show miliseconds - later for 'delay(xxxx)' 

        }
    }

    int dec = digitalRead (btn1);
    if (dec != decPrev) {
        decPrev = dec;
        delay (20);
        if (LOW == dec) {
            delay(100);
      seconds--;
        Serial.println(seconds); //show seconds - later for 'display'
    mseconds=seconds*1000;    
        Serial.println(mseconds); // show miliseconds - later for 'delay(xxxx)' 
        }
    }

    // MOTOR
    digitalWrite (in4, HIGH);
    digitalWrite (in3, LOW);
    analogWrite  (enB, 150); //default speed for now we will use 150

//now, set motor to work like 1 secons, and be OFF like 5 secons, from beginning ?

}

Hi, @danyCro
Can we please have a circuit diagram?
An image of a hand drawn schematic will be fine, include ALL power supplies, component names and pin labels.

Can you post some images of your project?
So we can see your component layout.

Thanks.. Tom.. :smiley: :+1: :coffee: :australia:

If you want the code to be responsive to button-presses all the time
your code must be re-written to be non-blocking.

Using delay() means the microcontroller stops code-execution.
The function delay should have better be named

freeze_Microcontroller_Stop_Code_Execution_Until_FreezingTime_is_Over(5000);

There is a very poorly written example-code that demonstrate this.
My recommendation is to not use this poorly written example-code most users here refer to
but
instead reading this tutorial how non-blocking code works.

best regards Stefan

look this over

int btn2 = 3;  // increment
int btn1 = 2;  // decrement

int in3 = 4;
int in4 = 5;
int enB = 7;    // only 3, 5, 6, 9 10 and 11 are PWM outputs

#define Off 0

int incPrev;
int decPrev;


enum { Idle, Run, Pause, Stop };
int state;

unsigned long msecPeriod;
unsigned long msecLst;
unsigned long sekunde = 0;

// -----------------------------------------------------------------------------
void setup () {
    Serial.begin (9600);
    Serial.println ("ready");

    pinMode (btn1, INPUT_PULLUP);  //button for increment
    pinMode (btn2, INPUT_PULLUP); //button for decrement

    pinMode (in3, OUTPUT); // motor 2
    pinMode (in4, OUTPUT); // motor 2
    pinMode (enB, OUTPUT); // motor 2

    digitalWrite (in4, HIGH);
    digitalWrite (in3, LOW);
    analogWrite  (enB, Off);
}

// -----------------------------------------------------------------------------
void loop ()
{
    unsigned long msec = millis ();

    if (state && msec - msecLst >= msecPeriod)  {
        msecLst = msec;

        Serial.print   (" state ");
        Serial.println (state);

        if (Run == state)  {
            state = Pause;
            msecPeriod = sekunde * 1000;
            analogWrite  (enB, 150);
        }
        else if (Pause == state)  {
            state = Run;
            msecPeriod = 200;
            analogWrite  (enB, Off);
        }
        else  {
            msecPeriod = 0;
            analogWrite  (enB, Off);
        }
    }

    int inc = digitalRead (btn2);
    if (inc != incPrev ) {
        incPrev = inc;
        delay (20);
        if (LOW == inc)  {
            sekunde ++;
            if (0 < sekunde)
                state = Run;
            Serial.println (sekunde);
        }
    }

    int dec = digitalRead (btn1);
    if (dec != decPrev) {
        decPrev = dec;
        delay (20);
        if (LOW == dec) {
            sekunde --;
            if (0 >= sekunde) {
                sekunde = 0;
                state = Stop;
            }
            Serial.println (sekunde);
        }
    }
}

Thank you @StefanL38, that is much useful examples :slight_smile:
Thank you very much @gcjr.
Working good now, I just replace
msecPeriod = 1000; with msecPeriod = sekunde * 1000; from Pause to Run state, so now I controled Paused, and working motor seconds are fixed.

Let's stay this topic open if I need it.

Now I will try that put in my OLED display, add more two buttons for speed and new state, but let me do I try that first :slight_smile:

This "traditional" code-structure of non-blocking timing has these disadvantages:

  1. you have to update the timing-variable in its own line of code
  2. the if-condition is not really self-explaining
  3. you have to update a second variable
    unsigned long msec = millis (); // 1. update timing-variable

    if (state && msec - msecLst >= msecPeriod)  { // 2. not really self-explaining code
        msecLst = msec; // 3. update a second variable

this reduces to a single line of code in my version which uses a self-explaining name

  if (state != Idle &&  TimePeriodIsOver(myTimerVar, msecPeriod ) )  {

So the code looks like this

int btn2 = 3;  // increment
int btn1 = 2;  // decrement

int in3 = 4;
int in4 = 5;
int enB = 7;    // only 3, 5, 6, 9 10 and 11 are PWM outputs

#define Off 0

int incPrev;
int decPrev;


enum { Idle, Run, Pause, Stop };
int state;

unsigned long myTimerVar;
unsigned long msecPeriod;
//unsigned long msecLst;
unsigned long sekunde = 0;

const byte    OnBoard_LED = 13;


// -----------------------------------------------------------------------------
void setup () {
  Serial.begin (9600);
  Serial.println("Setup-Start");

  pinMode (btn1, INPUT_PULLUP);  //button for increment
  pinMode (btn2, INPUT_PULLUP); //button for decrement

  pinMode (in3, OUTPUT); // motor 2
  pinMode (in4, OUTPUT); // motor 2
  pinMode (enB, OUTPUT); // motor 2

  digitalWrite (in4, HIGH);
  digitalWrite (in3, LOW);
  analogWrite  (enB, Off);
  myTimerVar = millis(); // initialise Timer-variable
}

// -----------------------------------------------------------------------------
void loop () {

  if (state != Idle && TimePeriodIsOver(myTimerVar, msecPeriod) ) {

    Serial.print   (" state ");
    Serial.println (state);

    if (Run == state)  {
      state = Pause;
      msecPeriod = sekunde * 1000;
      analogWrite  (enB, 150);
    }
    else if (Pause == state)  {
      state = Run;
      msecPeriod = 200;
      analogWrite(enB, Off);
    }
    else  {
      msecPeriod = 0;
      analogWrite(enB, Off);
    }
  }


  int inc = digitalRead(btn2);
  if (inc != incPrev ) { // if IO-pin-state has changed
    incPrev = inc;       // update comparing variable
    delay (20);          // wait unil bouncing is over
    if (LOW == inc)  {   // if button is pressed down
      sekunde ++;
      if (sekunde != 0)
        state = Run;

      Serial.println(sekunde);
    }
  }

  int dec = digitalRead(btn1);
  if (dec != decPrev) { // if IO-pin-state has changed
    decPrev = dec;      // update comparing variable
    delay (20);         // wait unil bouncing is over
    if (LOW == dec) {   // if button is pressed down
      sekunde --;
      if (0 >= sekunde) {
        sekunde = 0;
        state = Stop;
      }
      Serial.println (sekunde);
    }
  }
}

// easy to use helper-function for non-blocking timing
boolean TimePeriodIsOver (unsigned long &startOfPeriod, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();
  if ( currentMillis - startOfPeriod >= TimePeriod ) {
    // more time than TimePeriod has elapsed since last time if-condition was true
    startOfPeriod = currentMillis; // a new period starts right here so set new starttime
    return true;
  }
  else return false;            // actual TimePeriod is NOT yet over
}

C++ offers to declare your own functions. This enables to have better structured code where each function does one thing

  • controlling the motor
  • reading in the buttons and inc/dec a variable

Here is a version of your code where loop() looks very short because
controlling the motor
and reading in the buttons is in its own functions

void loop () {
  controlMotor2();
  readButtonsIncDec();
}

here is the complete code. If you look into the functions you will discover that the lines of code have just moved into the functions

int btn2 = 3;  // increment
int btn1 = 2;  // decrement

int in3 = 4;
int in4 = 5;
int enB = 7;    // only 3, 5, 6, 9 10 and 11 are PWM outputs

const byte Off = 0;
const byte pressed = LOW;

// equals to integer-values    0    1     2      3
enum                       { Idle, Run, Pause, Stop };
int state;

unsigned long msecPeriod;
unsigned long sekunde = 0;

const byte    OnBoard_LED = 13;
unsigned long myTimerVar;

// -----------------------------------------------------------------------------
void setup () {
  Serial.begin (9600);
  Serial.println("Setup-Start");

  pinMode (btn1, INPUT_PULLUP); // button for increment
  pinMode (btn2, INPUT_PULLUP); // button for decrement

  pinMode (in3, OUTPUT); // motor 2
  pinMode (in4, OUTPUT); // motor 2
  pinMode (enB, OUTPUT); // motor 2

  digitalWrite (in4, HIGH);
  digitalWrite (in3, LOW);
  analogWrite  (enB, Off);
  myTimerVar = millis ();
}


// -----------------------------------------------------------------------------
void loop () {
  controlMotor2();
  readButtonsIncDec();
}

void controlMotor2() {

  if (state != Idle && TimePeriodIsOver(myTimerVar, msecPeriod) ) {

      Serial.print   (" state ");
      Serial.println (state);

      if (Run == state)  {
        state = Pause;
        msecPeriod = sekunde * 1000;
        analogWrite  (enB, 150);
      }
      else if (Pause == state)  {
        state = Run;
        msecPeriod = 200;
        analogWrite(enB, Off);
      }
      else  {
        msecPeriod = 0;
        analogWrite(enB, Off);
      }
    }
  }
}


void readButtonsIncDec() {
  int inc;
  static int incPrev; // must be static to preserve value

  int dec;
  static int decPrev;

  inc = digitalRead(btn2);
  if (inc != incPrev ) {  // if IO-pin-state has changed
    incPrev = inc;        // update comparing variable
    delay (20);           // wait until bouncing is over
    if (inc == pressed) { // if button is pressed down
      sekunde++;
      if (sekunde != 0)
        state = Run;

      Serial.println(sekunde);
    }
  }

  dec = digitalRead(btn1);
  if (dec != decPrev) {   // if IO-pin-state has changed
    decPrev = dec;        // update comparing variable
    delay (20);           // wait until bouncing is over
    if (dec == pressed) { // if button is pressed down
      sekunde--;
      if (0 >= sekunde) {
        sekunde = 0;
        state = Stop;
      }
      Serial.println (sekunde);
    }
  }
}

// easy to use helper-function for non-blocking timing
boolean TimePeriodIsOver (unsigned long &startOfPeriod, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();
  if ( currentMillis - startOfPeriod >= TimePeriod ) {
    // more time than TimePeriod has elapsed since last time if-condition was true
    startOfPeriod = currentMillis; // a new period starts right here so set new starttime
    return true;
  }
  else return false;            // actual TimePeriod is NOT yet over
}

best regards Stefan

Shanks StefanL38.
Warnins says
Compilation error: 'readButtonsIncDec' was not declared in this scope .
I try 'int readButtonsIncDec'.

I am sorry there was one curly closing brace too much
I corrected it and I recompiled the code successfully

int btn2 = 3;  // increment
int btn1 = 2;  // decrement

int in3 = 4;
int in4 = 5;
int enB = 7;    // only 3, 5, 6, 9 10 and 11 are PWM outputs

const byte Off = 0;
const byte pressed = LOW;

// equals to integer-values    0    1     2      3
enum                       { Idle, Run, Pause, Stop };
int state;

unsigned long msecPeriod;
unsigned long sekunde = 0;

const byte    OnBoard_LED = 13;
unsigned long myTimerVar;

// -----------------------------------------------------------------------------
void setup () {
  Serial.begin (9600);
  Serial.println("Setup-Start");

  pinMode (btn1, INPUT_PULLUP); // button for increment
  pinMode (btn2, INPUT_PULLUP); // button for decrement

  pinMode (in3, OUTPUT); // motor 2
  pinMode (in4, OUTPUT); // motor 2
  pinMode (enB, OUTPUT); // motor 2

  digitalWrite (in4, HIGH);
  digitalWrite (in3, LOW);
  analogWrite  (enB, Off);
  myTimerVar = millis ();
}


// -----------------------------------------------------------------------------
void loop () {
  controlMotor2();
  readButtonsIncDec();
}

void controlMotor2() {

  if (state != Idle && TimePeriodIsOver(myTimerVar, msecPeriod) ) {

    Serial.print   (" state ");
    Serial.println (state);

    if (Run == state)  {
      state = Pause;
      msecPeriod = sekunde * 1000;
      analogWrite  (enB, 150);
    }
    else if (Pause == state)  {
      state = Run;
      msecPeriod = 200;
      analogWrite(enB, Off);
    }
    else  {
      msecPeriod = 0;
      analogWrite(enB, Off);
    }
  }
}



void readButtonsIncDec() {
  int inc;
  static int incPrev; // must be static to preserve value

  int dec;
  static int decPrev;

  inc = digitalRead(btn2);
  if (inc != incPrev ) {  // if IO-pin-state has changed
    incPrev = inc;        // update comparing variable
    delay (20);           // wait until bouncing is over
    if (inc == pressed) { // if button is pressed down
      sekunde++;
      if (sekunde != 0)
        state = Run;

      Serial.println(sekunde);
    }
  }

  dec = digitalRead(btn1);
  if (dec != decPrev) {   // if IO-pin-state has changed
    decPrev = dec;        // update comparing variable
    delay (20);           // wait until bouncing is over
    if (dec == pressed) { // if button is pressed down
      sekunde--;
      if (0 >= sekunde) {
        sekunde = 0;
        state = Stop;
      }
      Serial.println (sekunde);
    }
  }
}

// easy to use helper-function for non-blocking timing
boolean TimePeriodIsOver (unsigned long &startOfPeriod, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();
  if ( currentMillis - startOfPeriod >= TimePeriod ) {
    // more time than TimePeriod has elapsed since last time if-condition was true
    startOfPeriod = currentMillis; // a new period starts right here so set new starttime
    return true;
  }
  else return false;            // actual TimePeriod is NOT yet over
}

best regards Stefan

Thanks Stefan, that working too.

Hello danyCro

Should the sketch use one button to accelerate the motor from 0 to Vmax and use the second button to decelerate back to 0?

And then also switch the direction of rotation?

Or have I not understood your task?

Hi. I using two butons as described in post #6. Same fixed speed motor, same rotation, only paused by buttons time.
But don't waste time, we solve it. Thanks

1 Like

Hi StefanL38.

I added two buttons wihich working good, and two motors, which one working, other not.
I just everywhere added "2" (like "msecPeriod","msecPeriod2"). Do I need for second button and motor a new separate function?
This is now my full code (one motor not work).

//code by Stefan

#include <SPI.h> // za moj display
#include <Wire.h> // za moj display
#include <Adafruit_GFX.h> // za moj display
#include <Adafruit_SH1106.h> // za moj display

int btn2 = 9;  // increment motor 2
int btn1 = 8;  // decrement motor 2

int btn4 = 11;  // increment motor 1  
int btn3 = 10;  // decrement motor 1

int enA = 2; // motor 1
int in1 = 3; // motor 1
int in2 = 4; //motor 1

int enB = 5; // motor 2 // only 3, 5, 6, 9 10 and 11 are PWM outputs
int in3 = 6; // motor 2
int in4 = 7; // motor 2

const byte Off = 0;
const byte pressed = LOW;

// equals to integer-values    0    1     2      3
enum                       { Idle, Run, Pause, Stop };
int state;

unsigned long msecPeriod;
unsigned long msecPeriod2;
unsigned long seconds = 0;

//const byte    OnBoard_LED = 13;
unsigned long myTimerVar;

#define OLED_RESET -1 //4 // za moj display
Adafruit_SH1106 display(OLED_RESET); // za moj display

// -----------------------------------------------------------------------------
void setup () {
  Serial.begin (9600);
  display.begin(SH1106_SWITCHCAPVCC, 0x3C); 

  pinMode (btn1, INPUT_PULLUP); // button for increment
  pinMode (btn2, INPUT_PULLUP); // button for decrement
  pinMode (btn3, INPUT_PULLUP); // button for increment
  pinMode (btn4, INPUT_PULLUP); // button for decrement

  pinMode (in1, OUTPUT); // motor 1
  pinMode (in2, OUTPUT); // motor 1
  pinMode (enA, OUTPUT); // motor 1

  pinMode (in3, OUTPUT); // motor 2
  pinMode (in4, OUTPUT); // motor 2
  pinMode (enB, OUTPUT); // motor 2

  digitalWrite (in4, HIGH); // motor 2
  digitalWrite (in3, LOW); // motor 2
  analogWrite  (enB, Off); // motor 2
  myTimerVar = millis ();

  digitalWrite (in2, HIGH); // motor 1
  digitalWrite (in1, LOW); // motor 1
  analogWrite  (enA, Off); // motor 1

}


// -----------------------------------------------------------------------------
void loop () {
  controlMotor1();
  controlMotor2();
  readButtonsIncDec();

/////////////// DISPLAY ///////////////
  display.clearDisplay();
  display.setTextSize(3);
  display.setTextColor(WHITE);
  display.setCursor(0,10);  // first digit
  display.print(seconds);
    display.setCursor(70,10);  //second digit
  display.print(seconds2);
 // display.setTextColor(BLACK, WHITE); // 'inverted' text
  display.display();
/////////////// DISPLAY ///////////////
}

void controlMotor2() {

  if (state != Idle && TimePeriodIsOver(myTimerVar, msecPeriod) ) {
    if (Run == state)  {
      state = Pause;
           msecPeriod = 1000;

      analogWrite  (enB, 150); 
    }
    else if (Pause == state)  {
      state = Run;
      msecPeriod = seconds * 1000;
      analogWrite(enB, Off);
    }
    else  {
      msecPeriod = 0;
      analogWrite(enB, Off);
    }
  }
}


void controlMotor1() {
  if (state != Idle && TimePeriodIsOver(myTimerVar, msecPeriod2) ) {
    if (Run == state)  {
      state = Pause;
           msecPeriod2 = 1000;

      analogWrite  (enA, 150); ////////////////UGASI MOTOR AKO RADIŠ ////////////////////////
    }
    else if (Pause == state)  {
      state = Run;
      msecPeriod2 = seconds * 1000;
      analogWrite(enA, Off);
    }
    else  {
      msecPeriod2 = 0;
      analogWrite(enA, Off);
    }
  }
}
void readButtonsIncDec() {
  int inc;
  static int incPrev; // must be static to preserve value
  int dec;
  static int decPrev;

  inc = digitalRead(btn2);
  if (inc != incPrev ) {  // if IO-pin-state has changed
    incPrev = inc;        // update comparing variable
    delay (20);           // wait until bouncing is over
    if (inc == pressed) { // if button is pressed down
      seconds++;
      if (seconds != 0)
        state = Run;

      Serial.println(seconds);
    }
  }

  dec = digitalRead(btn1);
  if (dec != decPrev) {   // if IO-pin-state has changed
    decPrev = dec;        // update comparing variable
    delay (20);           // wait until bouncing is over
    if (dec == pressed) { // if button is pressed down
      seconds--;
      if (0 >= seconds) {
        seconds = 0;
        state = Stop;
      }
      Serial.println (seconds);
    }
  }

  int inc2;
  static int incPrev2; // must be static to preserve value
  int dec2;
  static int decPrev2;

 inc2 = digitalRead(btn4);
  if (inc2 != incPrev2 ) {  // if IO-pin-state has changed
    incPrev2 = inc2;        // update comparing variable
    delay (20);           // wait until bouncing is over
    if (inc2 == pressed) { // if button is pressed down
      seconds2++;
      if (seconds2 != 0)
        state = Run;

      Serial.println(seconds2);
    }
  }

  dec2 = digitalRead(btn3);
  if (dec2 != decPrev2) {   // if IO-pin-state has changed
    decPrev2 = dec2;        // update comparing variable
    delay (20);           // wait until bouncing is over
    if (dec2 == pressed) { // if button is pressed down
      seconds2--;
      if (0 >= seconds2) {
        seconds2 = 0;
        state = Stop;
      }
      Serial.println (seconds2);
    }
  }
}

// easy to use helper-function for non-blocking timing
boolean TimePeriodIsOver (unsigned long &startOfPeriod, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();
  if ( currentMillis - startOfPeriod >= TimePeriod ) {
    // more time than TimePeriod has elapsed since last time if-condition was true
    startOfPeriod = currentMillis; // a new period starts right here so set new starttime
    return true;
  }
  else return false;            // actual TimePeriod is NOT yet over
}

don't you need separate state and myTimeVar variables
should consider arrays, for buttons and motor pins as well

1 Like

see if this interests you (127 lines vs 208)

// multiple motor controller

enum { Idle, Run, Pause, Stop };
const int Nbut = 2;

struct Motor {
    const byte  PinEn;
    const byte  PinInA;
    const byte  PinInB;

    const byte  PinBut [Nbut];

    const char *label;

    byte        butState [Nbut];
    int         state;

    int           sekunde;
    unsigned long msecPeriod;
    unsigned long msecLst;
};

Motor motors [] = {
#undef MyHW         // for testing on my hardware
#ifdef MyHW
    { 10, 12, 8, { A1, A2 }, "Motor-A" },
    { 11, 13, 9, { A3, A4 }, "Motor-B" },

#else
    { 2, 3, 4, {  8,  9 }, "Motor-A" },
    { 5, 6, 7, { 10, 11 }, "Motor-B" },
#endif
};
const int Nmotor = sizeof(motors) / sizeof(Motor);

#ifdef MyHW
const byte  MotorOn  = 255;             // needed to test with LED
#else
const byte  MotorOn  = 150;
#endif
const byte  MotorOff = 0;

unsigned long msec;

// -----------------------------------------------------------------------------
void
chkMotor (
    Motor *m )
{
    if (Idle != m->state && msec - m->msecLst >= m->msecPeriod)  {
        m->msecLst = msec;

        Serial.println (m->label);

        if (Run == m->state)  {
            m->state      = Pause;
            m->msecPeriod = m->sekunde * 1000;
            analogWrite  (m->PinEn, MotorOn);
        }
        else if (Pause == m->state)  {
            m->state      = Run;
            m->msecPeriod = 200;
            analogWrite  (m->PinEn, MotorOff);
        }
        else    // Stop
            analogWrite  (m->PinEn, MotorOff);
    }

    for (int b = 0; b < Nbut; b++)  {
        byte but = digitalRead (m->PinBut [b]);
        if (m->butState [b] != but)  {
            m->butState [b] = but;
            delay (20);             // debounce
            if (LOW == but)  {
                if (0 == b)  {
                    m->sekunde ++;
                    m->state = Run;
                }
                else {
                    m->sekunde --;
                    if (0 > m->sekunde)  {
                        m->sekunde = 0;
                        m->state = Stop;
                    }
                }

                Serial.print (m->sekunde);
                Serial.print   (" ");
                Serial.println (m->label);
            }
        }
    }
}

// -----------------------------------------------------------------------------
void
loop ()
{
    msec = millis ();

    Motor *m = motors;
    for (int n = 0; n < Nmotor; n++, m++)
        chkMotor (m);
}

// -----------------------------------------------------------------------------
void
setup ()
{
    Serial.begin (9600);

    Motor *m = motors;
    for (int n = 0; n < Nmotor; n++, m++)  {
        for (int i = 0; i < Nbut; i++)  {
            pinMode (m->PinBut [i], INPUT_PULLUP);
            m->butState [i] = digitalRead (m->PinBut [i]);
        }

        pinMode (m->PinEn,  OUTPUT);
        pinMode (m->PinInA, OUTPUT);
        pinMode (m->PinInB, OUTPUT);

        analogWrite  (m->PinEn,  0);
        digitalWrite (m->PinInA, LOW);
        digitalWrite (m->PinInB, HIGH);
    }
}

Yes and no.

You don't need it. But I recommend to structure code in a way that each function does one thing.

Examples for the meaning of "one thing"

  • control one motor
  • read in buttons that control one motor
  • the timing-function TimePeriodIsoVer()
  • pinMode()
  • digitalWrite()
  • analogWrite()

are all functions. Some of them written by the Arduino-Team
some written by other users, some written by you

Take a look into this tutorial:

Arduino Programming Course

It is easy to understand and has a good mixture between explaining important concepts and example-codes to get you going. So give it a try and report your opinion about this tutorial.

best regards Stefan