Add more functionality to existing code… Uno & Stepper Control

Hi everyone, new to this forum and to the Arduino platform. I would really appreciate some help with a little code.

I want to keep this simple so I can learn but I am not sure how to format the code in order to add additional functionality.

As it sits, I have an adafruit stepper which is controlled by a rotary encoder... this functions great and the sketch seems to be working great. The rotary encoder allows me to jog 1 step at a time on the stepper... so far so good.

I wanted to add a momentary switch on a digital input so I can have the stepper run continuously at a faster speed until it is depressed so I don't have to jog a large distance with the rotary encoder.

Can someone help me with the formatting of the code... I gave it a couple tries and both broke my sketch every time. Not sure where to add it in the void setup and loop, what how it needs to be defined so it does not cause conflict with the existing function.

Here is what it looks like so far...

#include <AccelStepper.h>
#include <AFMotor.h>

#define encoderPinA  2
#define encoderPinB  3

int encoderPos = 0;
int encoderPinALast = LOW;
int encoderPinBLast = LOW;
int n = LOW;

AF_Stepper motor2(200, 2);

// wrappers for the motor! SINGLE is default, can also use INTERLEAVE DOUBLE MICROSTEP
void forwardstep2() {  
  motor2.onestep(FORWARD, SINGLE);
}
void backwardstep2() {  
  motor2.onestep(BACKWARD, SINGLE);
}

// Motor shield has two motor ports, now we'll wrap them in an AccelStepper object
AccelStepper stepper2(forwardstep2, backwardstep2);

void setup()
{  
    stepper2.setMaxSpeed(500.0);
    stepper2.setAcceleration(100.0);
    stepper2.moveTo(0);
    
    pinMode(encoderPinA, INPUT_PULLUP); 
    pinMode(encoderPinB, INPUT_PULLUP);
}

void loop()
{  
    n = digitalRead(encoderPinA);
   if ((encoderPinALast != n )) {
     if (((n == HIGH) && (digitalRead(encoderPinB) == LOW)) || 
         ((n == LOW) && (digitalRead(encoderPinB) == HIGH))) {
       motor2.step(1, BACKWARD, INTERLEAVE);
     } else {
       motor2.step(1, FORWARD, INTERLEAVE);
     }
   } else {
     n = digitalRead(encoderPinB);
     if ((encoderPinBLast != n )) {
       if (((n == HIGH) && (digitalRead(encoderPinA) == LOW)) || 
           ((n == LOW) && (digitalRead(encoderPinA) == HIGH))) {
         motor2.step(1, FORWARD, INTERLEAVE);
       } else {
         motor2.step(1, BACKWARD, INTERLEAVE);
       }
     }
   }
   encoderPinALast = digitalRead(encoderPinA);
   encoderPinBLast = digitalRead(encoderPinB);
    
}

I want to keep this simple so I can learn but I am not sure how to format the code in order to add additional functionality.

Adding additional functionality has nothing to do with format.

I wanted to add a momentary switch on a digital input so I can have the stepper run continuously at a faster speed until it is depressed so I don't have to jog a large distance with the rotary encoder.

What are the things you need to consider, then? You have to wire the switch correctly. You have to read the state of the switch. You have to make decisions as to what to do based on the state of the switch, perhaps caring about when the switch BECOMES pressed vs. IS pressed.

What part(s) are you having trouble with?

I gave it a couple tries and both broke my sketch every time.

It's impossible to break a sketch. It is possible to make it not compile. It is possible to make it so it does not do what you want.

You won't learn if we write the code for you. Do something, show what you've done, and describe the problems. Then, we'll help you understand why that didn't work, and suggest ways to fix it.

What it really sounds like is that you do not have firm requirements for how the switch is to be used, so you don't know how to incorporate it in the existing program. We can't help you with the requirements.

I may be using the same language to mean something different but I do believe the format of the code can make it easier to manage.

At the moment you have a chunk of code in loop() that works with your encoder. It may be complicated to add extra functionality into that.

But, if you take all the existing code from loop() into a function called moveWithEncoder() and create a new function moveWithButton() maybe it won't be so difficult.

Then the code in loop() would be reduced to something like this

void loop() {
  moveWithEncoder();
  moveWithButton();
}

I haven't looked at your code carefully so it may be necessary to do something to ensure the existing moveWithEncoder() code would not fight with the new code.

And, if this was my project I would break it up even further so that I had something like

void loop() {
   readEncoder();
   readButtons();
   moveMotor();
}

This is the style illustrated in the Thread planning and implementing a program

...R

PaulS and Robin2, I really appreciate the reply.

I took some time last night and used some software to draw out the electrical part of the project, I hope this helps.

Also posted, is my revised code which I attempted to get the switch working... I tried to define the switch as "int button = 4" and the set the pinMode under the void setup as an input.

I still don't follow 100$ on the void loop Robin. I did read your post/guide but I am still so new to this I am having a hard time following.

Basically this existing code I have was taken from someone else project, and I am trying to modify. Its part of my learning process when I backwards engineer it. I do understand its not the right way, but I had a heck of a time trying to work with a rotary encoder up to this point.

Picture of Project:

#include <AccelStepper.h>
#include <AFMotor.h>

#define encoderPinA  2
#define encoderPinB  3

int encoderPos = 0;
int encoderPinALast = LOW;
int encoderPinBLast = LOW;
int n = LOW;
int button = 4;


AF_Stepper motor2(200, 2);

// wrappers for the motor! SINGLE is default, can also use INTERLEAVE DOUBLE MICROSTEP
void forwardstep2() {  
  motor2.onestep(FORWARD, SINGLE);
}
void backwardstep2() {  
  motor2.onestep(BACKWARD, SINGLE);
}

// Motor shield has two motor ports, now we'll wrap them in an AccelStepper object
AccelStepper stepper2(forwardstep2, backwardstep2);

void setup()
{  
    stepper2.setMaxSpeed(500.0);
    stepper2.setAcceleration(100.0);
    stepper2.moveTo(0);
    
    pinMode(encoderPinA, INPUT_PULLUP); 
    pinMode(encoderPinB, INPUT_PULLUP);
    pinMode(button, INPUT);
    stepper2.setSpeed(500);
}

void loop()
{  
  if(digitalRead(button) == LOW)
  {
   stepper2.runSpeed();
  } 
    n = digitalRead(encoderPinA);
   if ((encoderPinALast != n )) {
     if (((n == HIGH) && (digitalRead(encoderPinB) == LOW)) || 
         ((n == LOW) && (digitalRead(encoderPinB) == HIGH))) {
       motor2.step(1, BACKWARD, INTERLEAVE);
     } else {
       motor2.step(1, FORWARD, INTERLEAVE);
     }
   } else {
     n = digitalRead(encoderPinB);
     if ((encoderPinBLast != n )) {
       if (((n == HIGH) && (digitalRead(encoderPinA) == LOW)) || 
           ((n == LOW) && (digitalRead(encoderPinA) == HIGH))) {
         motor2.step(1, FORWARD, INTERLEAVE);
       } else {
         motor2.step(1, BACKWARD, INTERLEAVE);
       }
     }
   }
   encoderPinALast = digitalRead(encoderPinA);
   encoderPinBLast = digitalRead(encoderPinB);
    
}

It seems to me that you want to do one thing if the switch is pressed, and another if it isn't. That is not what you are doing. You are doing one thing if the switch is pressed and then the other thing, regardless of whether the switch is pressed or not. It seems to me that that interferes with what you do when the switch is pressed.

I think I get what you are saying... I believe I need to isolate the function for both the switch and rotary encoder?

Basically I want the switch and encoder to work independently of each other, but control the same stepper motor.

The encoder jogs the stepper motor 1 step at a time... currently this is how I want it to work as it provides a lot of resolution when trying to move a small amount.

The switch on the other hand I want to run the motor continuously in a direction I specify, and at a faster speed which as of now I want to be software defined in the code... which is why I added the line " stepper2.setSpeed(500);"

Ultimately I want to keep building on this adding more switches which either control direction and or speed.

Is my issue in the void loop? How does the rest of it look?

Thanks

Basically I want the switch and encoder to work independently of each other, but control the same stepper motor.

So, when the switch is pressed, do one thing. Otherwise, do the other thing, that being encoder-driven.

Is my issue in the void loop?

Yes, your issue is in the loop() function. You are missing an else statement and some curly braces around the rest of the code.

PaulS:
So, when the switch is pressed, do one thing. Otherwise, do the other thing, that being encoder-driven.

That is correct... The motor never runs in any direction without direct user input... Either via switch or encoder. The switch is a momentary press, so as soon as the circuit goes open, I want it to stop moving.

PaulS:
Yes, your issue is in the loop() function. You are missing an else statement and some curly braces around the rest of the code.

Ok, I will give this a try again myself and see if I can get it to work.

OK!!!

I added the else statement, braced up the code and it still didn't work... I figured I look into my wiring a little deeper and tried some different I/O pins to troubleshoot, and it started working.

Looks like I make a mistake on my Pin selection. Pin 4 is used by the motor shield so that is why it never worked even with the correct code. Silly me.

I found that Digital Pin 0, 1, and 13 are still free for INPUT use.

Now that its working, I wanted to add a 2nd switch to use at a different speed.

Under my void setup... I added two different speeds but I am not sure how to actually assign the setSpeed value to a specific button. Both button use the last value in the line which is setSpeed(50)

#include <AccelStepper.h>
#include <AFMotor.h>

#define encoderPinA  3
#define encoderPinB  2
#define button  0
#define buttontwo  1

int encoderPos = 0;
int encoderPinALast = LOW;
int encoderPinBLast = LOW;
int n = LOW;


AF_Stepper motor2(200, 2);

// wrappers for the motor! SINGLE is default, can also use INTERLEAVE DOUBLE MICROSTEP
void forwardstep2() {  
  motor2.onestep(BACKWARD, SINGLE);
}
void backwardstep2() {  
  motor2.onestep(FORWARD, SINGLE);
}

// Motor shield has two motor ports, now we'll wrap them in an AccelStepper object
AccelStepper stepper2(forwardstep2, backwardstep2);

void setup()
{  
    stepper2.setMaxSpeed(500.0);
    stepper2.setAcceleration(100.0);
    stepper2.moveTo(0);
    
    pinMode(encoderPinA, INPUT_PULLUP); 
    pinMode(encoderPinB, INPUT_PULLUP);
    pinMode(button, INPUT);
    pinMode(buttontwo, INPUT);
    stepper2.setSpeed(50);  //Slow Speed for button
    stepper2.setSpeed(500);  //Fast Speed for buttontwo
}

void loop()
{  
  if(digitalRead(button) == LOW)
  {
   stepper2.runSpeed();
  } else {}
  if(digitalRead(buttontwo) == LOW)
  {
    stepper2.runSpeed();
  } else {}
    n = digitalRead(encoderPinA);
   if ((encoderPinALast != n )) {
     if (((n == HIGH) && (digitalRead(encoderPinB) == LOW)) || 
         ((n == LOW) && (digitalRead(encoderPinB) == HIGH))) {
       motor2.step(1, BACKWARD, INTERLEAVE);
     } else {
       motor2.step(1, FORWARD, INTERLEAVE);
     }
   } else {
     n = digitalRead(encoderPinB);
     if ((encoderPinBLast != n )) {
       if (((n == HIGH) && (digitalRead(encoderPinA) == LOW)) || 
           ((n == LOW) && (digitalRead(encoderPinA) == HIGH))) {
         motor2.step(1, FORWARD, INTERLEAVE);
       } else {
         motor2.step(1, BACKWARD, INTERLEAVE);
       }
     }
   }
   encoderPinALast = digitalRead(encoderPinA);
   encoderPinBLast = digitalRead(encoderPinB);
    
}
  if(digitalRead(button) == LOW)
  {
   stepper2.runSpeed();
  } else {}

No!

Inside the else block is where the rest of the code in loop goes - the stuff that deals with reading the encoder and moving the motor. Get this working with one switch first. Then, get a potentiometer to use to adjust the speed (using stepper.setSpeed()).

if(digitalRead(button) == LOW)
{
   stepper2.runSpeed();
}
else
{
    // read the encoder data
    // step as needed
}

PaulS,

Thanks for your help... surprisingly it was working as intended before you told me to correct the format of my code.

I think I am good to go! Everything works including my 2nd switch with two different speed settings per switch.

I moved the "stepper2.setSpeed(x);" from the void setup to the void loop area and put that after the if statement for each button... worked like a charm.

Here is my final code if anyone is interested.

#include <AccelStepper.h>
#include <AFMotor.h>

#define encoderPinA  3
#define encoderPinB  2
#define button  0
#define buttontwo  1

int encoderPos = 0;
int encoderPinALast = LOW;
int encoderPinBLast = LOW;
int n = LOW;


AF_Stepper motor2(200, 2);

// wrappers for the motor! SINGLE is default, can also use INTERLEAVE DOUBLE MICROSTEP
void forwardstep2() {  
  motor2.onestep(BACKWARD, SINGLE);
}
void backwardstep2() {  
  motor2.onestep(FORWARD, SINGLE);
}

// Motor shield has two motor ports, now we'll wrap them in an AccelStepper object
AccelStepper stepper2(forwardstep2, backwardstep2);

void setup()
{  
    stepper2.setMaxSpeed(500.0);
    stepper2.setAcceleration(100.0);
    stepper2.moveTo(0);
    
    pinMode(encoderPinA, INPUT_PULLUP); 
    pinMode(encoderPinB, INPUT_PULLUP);
    pinMode(button, INPUT);
    pinMode(buttontwo, INPUT);
}

void loop()
{  
  if(digitalRead(button) == LOW)
  {
   stepper2.setSpeed(50);
   stepper2.runSpeed();
  }
  else
  {
  if(digitalRead(buttontwo) == LOW)
  {
    stepper2.setSpeed(500);
    stepper2.runSpeed();
  }
  else
  {
    n = digitalRead(encoderPinA);
   if ((encoderPinALast != n )) {
     if (((n == HIGH) && (digitalRead(encoderPinB) == LOW)) || 
         ((n == LOW) && (digitalRead(encoderPinB) == HIGH))) {
       motor2.step(1, BACKWARD, INTERLEAVE);
     } else {
       motor2.step(1, FORWARD, INTERLEAVE);
     }
   } else {
     n = digitalRead(encoderPinB);
     if ((encoderPinBLast != n )) {
       if (((n == HIGH) && (digitalRead(encoderPinA) == LOW)) || 
           ((n == LOW) && (digitalRead(encoderPinA) == HIGH))) {
         motor2.step(1, FORWARD, INTERLEAVE);
       } else {
         motor2.step(1, BACKWARD, INTERLEAVE);
       }
     }
   }
   encoderPinALast = digitalRead(encoderPinA);
   encoderPinBLast = digitalRead(encoderPinB);
  }
  }
}

I still don't follow 100$ on the void loop Robin. I did read your post/guide but I am still so new to this I am having a hard time following.

This is the code from your original post moved into a function. It should work just the same as your original post.

#include <AccelStepper.h>
#include <AFMotor.h>

#define encoderPinA  2
#define encoderPinB  3

int encoderPos = 0;
int encoderPinALast = LOW;
int encoderPinBLast = LOW;
int n = LOW;

AF_Stepper motor2(200, 2);

// wrappers for the motor! SINGLE is default, can also use INTERLEAVE DOUBLE MICROSTEP
void forwardstep2() {  
  motor2.onestep(FORWARD, SINGLE);
}
void backwardstep2() {  
  motor2.onestep(BACKWARD, SINGLE);
}

// Motor shield has two motor ports, now we'll wrap them in an AccelStepper object
AccelStepper stepper2(forwardstep2, backwardstep2);

void setup()
{  
    stepper2.setMaxSpeed(500.0);
    stepper2.setAcceleration(100.0);
    stepper2.moveTo(0);
    
    pinMode(encoderPinA, INPUT_PULLUP); 
    pinMode(encoderPinB, INPUT_PULLUP);
}

void loop() {
   runWithEncoder();
}


void runWithEncoder()
{  
    n = digitalRead(encoderPinA);
   if ((encoderPinALast != n )) {
     if (((n == HIGH) && (digitalRead(encoderPinB) == LOW)) || 
         ((n == LOW) && (digitalRead(encoderPinB) == HIGH))) {
       motor2.step(1, BACKWARD, INTERLEAVE);
     } else {
       motor2.step(1, FORWARD, INTERLEAVE);
     }
   } else {
     n = digitalRead(encoderPinB);
     if ((encoderPinBLast != n )) {
       if (((n == HIGH) && (digitalRead(encoderPinA) == LOW)) || 
           ((n == LOW) && (digitalRead(encoderPinA) == HIGH))) {
         motor2.step(1, FORWARD, INTERLEAVE);
       } else {
         motor2.step(1, BACKWARD, INTERLEAVE);
       }
     }
   }
   encoderPinALast = digitalRead(encoderPinA);
   encoderPinBLast = digitalRead(encoderPinB);
    
}

I did this simply by changing the name of loop() and adding a new loop() function.

Now all that is necessary is to add a new function that makes the motor move when a button is pressed - something like this pseudo code

#include <AccelStepper.h>
#include <AFMotor.h>

#define encoderPinA  2
#define encoderPinB  3

int encoderPos = 0;
int encoderPinALast = LOW;
int encoderPinBLast = LOW;
int n = LOW;

AF_Stepper motor2(200, 2);

// wrappers for the motor! SINGLE is default, can also use INTERLEAVE DOUBLE MICROSTEP
void forwardstep2() {  
  motor2.onestep(FORWARD, SINGLE);
}
void backwardstep2() {  
  motor2.onestep(BACKWARD, SINGLE);
}

// Motor shield has two motor ports, now we'll wrap them in an AccelStepper object
AccelStepper stepper2(forwardstep2, backwardstep2);

void setup()
{  
    stepper2.setMaxSpeed(500.0);
    stepper2.setAcceleration(100.0);
    stepper2.moveTo(0);
    
    pinMode(encoderPinA, INPUT_PULLUP); 
    pinMode(encoderPinB, INPUT_PULLUP);
}

void loop() {
   runWithEncoder();
   runWithButton();
}


void runWithEncoder()
{  
    n = digitalRead(encoderPinA);
   if ((encoderPinALast != n )) {
     if (((n == HIGH) && (digitalRead(encoderPinB) == LOW)) || 
         ((n == LOW) && (digitalRead(encoderPinB) == HIGH))) {
       motor2.step(1, BACKWARD, INTERLEAVE);
     } else {
       motor2.step(1, FORWARD, INTERLEAVE);
     }
   } else {
     n = digitalRead(encoderPinB);
     if ((encoderPinBLast != n )) {
       if (((n == HIGH) && (digitalRead(encoderPinA) == LOW)) || 
           ((n == LOW) && (digitalRead(encoderPinA) == HIGH))) {
         motor2.step(1, FORWARD, INTERLEAVE);
       } else {
         motor2.step(1, BACKWARD, INTERLEAVE);
       }
     }
   }
   encoderPinALast = digitalRead(encoderPinA);
   encoderPinBLast = digitalRead(encoderPinB);
    
}

void runWithButton() {
   if (digitalRead(buttonPin == LOW) { // assumes active LOW
      // stuff to run motor
   }

}

...R

Robin2,

Thanks, that helps a lot, and looks a lot more organized than the way I have it setup!

I have it working already, but I am going to revise my code and see if I can accomplish the same thing using your tips... should be good practice either way.

So just to recap and to make sure I understand what you are saying... is to separate functions within the void loop chain into "sub processes" and then define them below?

So this being the "root" of the void loop.

void loop() {
   runWithEncoder();
   runWithButton();
   runWithButtontwo();
   runWithButtonthree();
   runWithStopSensor();
}

Then define each process under that root...

void runWithXXXXXXXXXX() {
if (digitalRead(buttonPin == LOW) { // assumes active LOW
// stuff to run motor
}

}

THANKS!

ProtoNoob:
So just to recap and to make sure I understand what you are saying... is to separate functions within the void loop chain into "sub processes" and then define them below?

Yes, that seems right. As I said at the bottom of Reply #2 it would should be possible to separate things further and simplify more - for example just have one function that actually makes the stepper move that is shared by all the inputs.

But leave that for later.

...R