Creating subroutines and functions

Hello all. I am writing a program to control a model elevator and am basing it off a simple "if else" method for determining movement and a stepper motor and variable distances to reach the floor. The logic behind it is that with each floor reads two sensors, a photocell which will read if the elevator is on that floor, and a button on each floor. So there are only 9 variable actions in total. Example: if elevator is on floor one then photocell 4 is reading and then if button on floor 2 is pushed the subsequent code written for this "if else" statement becoming true will control the actions. My trouble right now is the way I am coding is not creating mutually exclusive results for my "if" statements. In my mind I want to have this "if" statement be read as true and then have a big chunk of commands and subsequent "if" statements written off to the side in parentheses that only effect anything if accessed because of a true "if else" statement.

I imagine this is done with a function or a subroutine but all I can find through research and examples are one liner functions. Not anything that contains further "if" statements or variable changes. If I can accomplish this then I think I can have 9 different sets of result codes for the 9 potentialities from the 3 elevator floors. And then run the "if else" to determine which to operate.

Thank you for your advice and help!

#include <AccelStepper.h>
#include <MultiStepper.h>

#define DISTANCE1 1000
#define DISTANCE2 4000

int StepCounter = 0;
int Stepping = false;

void setup() {
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  digitalWrite(8, LOW);
  digitalWrite(9, LOW);

  pinMode(1, INPUT);
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  pinMode(4, INPUT);
  pinMode(5, INPUT);
  pinMode(6, INPUT);
}

void loop() {

  //FLOOR ONE BUTTON 1

  {
    if (digitalRead(4) == LOW && digitalRead(1) == LOW && Stepping == false)
    {
      digitalWrite(8, HIGH);
      Stepping = false;
    }
    if (Stepping == true)
    {
      digitalWrite(9, HIGH);
      delay(1);
      digitalWrite(9, LOW);
      delayMicroseconds(100);

      StepCounter = StepCounter + 1;

      if (StepCounter == DISTANCE1)
      {
        StepCounter = 0;
        Stepping = false;

      }
    }
  }

  //FLOOR ONE BUTTON 2

  {
    if (digitalRead(4) == LOW && digitalRead(2) == LOW && Stepping == false)
    {
      digitalWrite(8, HIGH);
      Stepping = true;
    }
    if (Stepping == true)
    {
      digitalWrite(9, HIGH);
      delay(1);
      digitalWrite(9, LOW);
      delayMicroseconds(100);

      StepCounter = StepCounter + 1;

      if (StepCounter == DISTANCE1)
      {
        StepCounter = 0;
        Stepping = false;
      }
    }
  }


  //FLOOR ONE BUTTON 3

  {
    if (digitalRead(4) == LOW && digitalRead(3) == LOW && Stepping == false)
    {
      digitalWrite(8, HIGH);
      Stepping = true;
    }
    if (Stepping == true)
    {
      digitalWrite(9, HIGH);
      delay(1);
      digitalWrite(9, LOW);
      delayMicroseconds(100);

      StepCounter = StepCounter + 1;

      if (StepCounter == DISTANCE2)
      {
        StepCounter = 0;
        Stepping = false;
      }
    }
  }


}

First, please take all the unnecessary white space out of your code. It makes it very difficult to follow. Maybe the AutoFormat tool will do that automatically. It will also indent the code properly.

I think you are getting your actions and your logic mixed up. You should keep them separate. Moving the motor will be pretty much the same whatever floor it is at. Likewise for detecting sensors.

I would make a function that reads all the sensors and stores their values.

Then I would have another function that checks those saved values to figure what to do. If it decides that it needs to move a motor it will set another variable appropriately.

And I would have another function that checks the value(s) for the motor control variable to decide if the motor needs to move.

If the purpose of some of the sensors is to identify the fact that the motor needs to stop then there could also be a function for that - again using the stored sensor values.

To get all that to work it is essential that loop() can repeat very frequently so there should be no delay()s in the code.

For this project my loop() might look like this

void loop() {
   readSensors();
   checkShouldMotorStop();
   checkForNextMove();
   moveMotor();
}

Have a look at Planning and Implementing a Program. And notice how millis() is used to manage all the timing.

...R
PS. I forgot to say earlier that your code will be enormously easier to follow if you give names to I/O pins so, for example, instead of

if (digitalRead(4) == LOW && digitalRead(3) == LOW

you have

if (digitalRead(floor4pin) == LOW && digitalRead(floor3pin) == LOW
  • obviously use sensible names for the pins - I have no idea what they should be called.

And, of course, following my earlier advice it should really be

floor4Val = digitalRead(floor4pin);
....
if (floor4Val == LOW   // etc

Robin,
Thank you for your suggestions and the link to planning and implementing. It will be a useful resource. I appreciate your time and help.

I imagine this is done with a function or a subroutine but all I can find through research and examples are one liner functions.

Here's a hint. loop() is a function.

I'm thinking of doing an elevator project too, although I'll call it a "lift" ;).

Seems to me a state machine approach is called for, where in each time through loop() you listen for the sensors as Robin2 suggested, and then act on those depending what state you're in. I'm inclined to think switch...case here. I've also been looking at the FSM library.

ardy_guy:
you listen for the sensors as Robin2 suggested, and then act on those depending what state you're in

Agreed. I am always a little wary of scaring people with the phrase "state machine"

...R

Robin2:
Agreed. I am always a little wary of scaring people with the phrase "state machine"

...R

Yeah, I saw a "state machine" take out a gazelle once.

Oh, wait, that was a lion.

Maybe "state machine"s aren't that scary....

BulldogLowell:
Maybe "state machine"s aren't that scary....

I suspect you have the experience and courage needed to aim at the lion's eye !

Never underestimate the frightening effect of jargon.

...R

Robin2:
Never underestimate the frightening effect of jargon.

Interesting... then perhaps we should rename them! Something like "soft warm fuzzy do-er thingies." works for me.

:wink:

BulldogLowell:
Interesting... then perhaps we should rename them! Something like "soft warm fuzzy do-er thingies." works for me.

I find it disappointing that you are making fun of something that can be a significant obstacle to learning for some newbies. A teacher must first engage with the student on terms that the student is comfortable with.

...R

Robin2:
I find it disappointing that you are making fun of something that can be a significant obstacle to learning for some newbies.

Oh, no you completely missed my point, I apologize if that was your take-away so forgive me for that. I am not making fun of "something that can be a significant obstacle to learning." No, not at all. I am making fun of you for being patronizing.

Robin2:
I am always a little wary of scaring people with the phrase "state machine"

I guess you will let us know when we are ready for the big-boy words. Meanwhile we'll stick the the ten hundred we can handle.

BulldogLowell:
I am making fun of you for being patronizing.

I don't consider it patronizing to avoid using jargon that a newcomer would not understand and might interpret as some arcane system that is far beyond his/her capability. I do agree with you that such an interpretation would be wrong - but I think it is best to avoid the possibility.

I still think you are interpreting my comments from the perspective of your own extensive knowledge and experience rather than from the viewpoint of a newbie.

...R

Dealing with questions here is a difficult process. Usually there is little or no information as to the background and/or experience of the questioner and I for one find it difficult to strike a balance between short and sweet answers such as "use a state machine" and posting a full solution.

Often it is not until a couple of exchanges of posts that the level of help needed becomes apparent and all too frequently different and sometimes contradictory solutions are offered. All of this is mixed in with the usual "please put the code in code tags", "please post the whole program", "how are the servos powered", "are they actually servos ?", "have you downloaded the library file ?" and "how are the switches wired ?" questions.

I am not an advocate of taking things to PMs and actively discourage people asking me for help that way but I have given help via PMs a few times when things here got complicated and the OP seemed to be having trouble seeing the wood for the trees.

Robins point about seeing things from our own perspective is a very valid one. As soon as I think "state machine" I can visualise an enum holding the states, switch/case controlling which code is executed and the need to decide on entry and exit conditions for the states but what is someone unfamiliar with them supposed to make of it ? I must admit that until I used the Arduino I had no idea what a state machine was and until I did some research I was prepared to believe that it was some special form of computer, ie an actual machine.

So, what is the solution to this conundrum ?

I don't know !

The OP might or might not have read my link to the Wikpedia article on state machines. If the OP did read it, then he or she will have understood it or not, and or have been enticed to research further, or not. If OP didn't read the Wiki article, no harm done. All I did was point the OP at some material which he or she is free to embrace or ignore as he or she feels fit.

UKHeliBob:
Robins point about seeing things from our own perspective is a very valid one.

Yes, but alas everyone's perspective is valid... since perspective is based on the sum-total of your learning.

This is all off-topic, and I apologize to the OP for the tangent.

I just don't like to see people coming out of the gate at the lowest common denominator. This is the era of Google, after all. if someone has the intellectual curiosity to pursue something that was presented (i.e. @ardy_guy 's excellent suggestion of a State Machine approach) I wouldn't even think to dissuade them. If they don't have that curiosity/ambition/gumption then C'est la vie.

Robin2:
... I am always a little wary of scaring people with the phrase "state machine"

sounds a lot like:

Here be dragons

Beyond neither exist Sea Monsters...

So, what is the solution to this conundrum ?

Since we are talking specifically about state machines, I usually begin by pointing out some (obvious) states (waiting for user to press start switch; tube being fed out; tube being cut; etc.) and asking people to draw a picture with the states in bubbles. I ask them to draw all the states that they can think of. I ask them to draw lines between the bubbles, to show what transitions make sense. I ask them to label the lines with what triggers the transition on one side and what has to happen on the other.

When they have a "flow chart", then we can discuss how to implement that in code. An enum is useful, sometimes. But, what often happens is that you have overlapping state machines. Think about a robot that has a manual mode and an automatic mode. Two states, but in each mode, there is a whole other state machine going on.

I prefer to teach about state machines, and how they are visually depicted, and then proceed to discussing how they would be implemented on the Arduino.

BulldogLowell:

... I am always a little wary of scaring people with the phrase "state machine"

sounds a lot like:

Here be dragons

Beyond neither exist Sea Monsters...

That, too, is taken out of context because I issued no warning about State Machines (or Dragons) in Reply #1. I deliberately did not mention them at all.

I suspect @ardy_guy of being argy-mentative :slight_smile:

...R

Robin2:
That, too, is taken out of context because I issued no warning about State Machines (or Dragons) in Reply #1. I deliberately did not mention them at all.

No, not at all out of context, I was responding only to your Post #5, which was a dissuasive response to the previous post.

I suspect @ardy_guy of being argy-mentative :slight_smile:

I didn't see it that way. I saw it as offering up another concept in a thoughtful, helpful manner... with links!

This is a public meeting place for open discussion. Introducing alternative approaches is not an argument... it is virtuous.

:slight_smile:

BulldogLowell:
is not an argument...

Two Monty Pythonesque threads in one afternoon.... luuuxury.

Well as OP I assume it is my duty to welcome you all to this thread. the topic got wildly off base from what I was posting about (understandable) but I did get a good lead off point from Robin and hindered by a very busy schedule went from there.

ardy_guy:
I'm thinking of doing an elevator project too, although I'll call it a "lift" ;).

Seems to me a state machine approach is called for, where in each time through loop() you listen for the sensors as Robin2 suggested, and then act on those depending what state you're in.

ardy I did look into a state machine model and appreciate the link. It influenced the further development of my code. Though I did not use a switch case model. What I ended up doing was very close to Robins suggestion "For this project my loop() might look like this"

Robin2:
For this project my loop() might look like this

void loop() {

readSensors();
  checkShouldMotorStop();
  checkForNextMove();
  moveMotor();
}

If I implemented everything correctly I think my steps are in alternative terms

void loop() {
setFloorNumber();              //Read sensors
from(variable)floor();         //Wait for inputs to check for motor next move
moveMotor();                   // move predetermined amount of steps to reach new floor
}

I am still troubleshooting through this program and I would also like to add a fourth floor. By replicating the model I have with two new inputs (push button and photosensor) I should be able to copy code for the new variables and run it the same way. I would love to hear more feedback from all of you as well as any help troubleshooting the functionality. I know my first bit of program was very weakly constructed but I was definitely in a place where I couldn't see the forest through the trees and you all at least gave me a flashlight.

Here is my further developed code

#include <AccelStepper.h>
#include <MultiStepper.h>

#define DISTANCE1 1000
#define DISTANCE2 4000

int StepCounter = 0;
int Stepping = false;
int currentFloor = 1;
void setup() {
  void UpOne();

  pinMode(8, OUTPUT); //Forward and Reverse (HIGH,LOW)
  pinMode(9, OUTPUT);
  digitalWrite(8, LOW);
  digitalWrite(9, LOW);

  pinMode(1, INPUT); //Floorswitch 1
  pinMode(2, INPUT); //Floorswitch 2
  pinMode(3, INPUT); //Floorswitch 3
  pinMode(4, INPUT); //Photocell Floor 1
  pinMode(5, INPUT); //Photocell Floor 2
  pinMode(6, INPUT); //Photocell Floor 3
}

void loop() {
  setFloorNumber();
  if (currentFloor = 1) {
    fromFloorOne();
  }

  else if (currentFloor = 2 ) {
    fromFloorTwo();
  }
  else if (currentFloor = 3 ) {
    fromFloorThree();
  }
}



void setFloorNumber() {
  //PHOTOSENSOR READING FLOOR INPUTS
  if (digitalRead(4) /*Photocell Floor 1*/ == LOW) { // Check if elevator on first floor
    currentFloor = 1;
  }
  else if (digitalRead(5) /*Photocell Floor 2*/ == LOW) { // Check if elevator on second floor
    currentFloor = 2;
  }
  else if (digitalRead(6) /*Photocell Floor 3*/ == LOW) { // Check if elevator on third floor
    currentFloor = 3;
  }
}


void fromFloorOne() { //IF BUTTON IS PUSHED WHILE ON FLOOR ONE
  if (currentFloor = 1 && digitalRead(1) /*Switch Floor 1*/  == LOW && Stepping == false) { //Floor One Button One
    noMovement(); //Remain in position
  }
  else if (currentFloor = 1 && digitalRead(2) /*Switch Floor 2*/  == LOW && Stepping == false) { //Floor One Button Two
    upOneFloor(); //Move up one floor
  }
  else if (currentFloor = 1 && digitalRead(3) /*Switch Floor 3*/  == LOW && Stepping == false) { //Floor One Button Three
    upTwoFloor(); //Move up two floor
  }
}
void fromFloorTwo() { //IF BUTTON IS PUSHED WHILE ON FLOOR TWO

  if (currentFloor = 2 && digitalRead(1) /*Switch Floor 1*/  == LOW && Stepping == false); {//Floor Two Button One
    downOneFloor(); //Move down one floor
  }
  if (currentFloor = 2 && digitalRead(2) /*Switch Floor 2*/  == LOW && Stepping == false); {//Floor Two Button Two
    noMovement(); //Remain in position
  }
  if (currentFloor = 2 && digitalRead(3) /*Switch Floor 3*/  == LOW && Stepping == false); {//Floor Two Button Three
    upOneFloor(); //Move up one floor
  }
}
void fromFloorThree() { //IF BUTTON IS PUSHED WHILE ON FLOOR THREE
  if (currentFloor = 3 && digitalRead(1) == LOW && Stepping == false); {//Floor Three Button One
    downTwoFloor; //Move down two floor
  }
  if (currentFloor = 3 && digitalRead(2) == LOW && Stepping == false); {//Floor Three Button Two
    downOneFloor(); //Move down one floor
  }
  if (currentFloor = 3 && digitalRead(3) == LOW && Stepping == false); {//Floor Three Button Three
    noMovement(); //Remain in position
  }
}

void noMovement() { //TRAVEL NO DISTANCE, does not turn on the stepper motor
  Stepping = false;
}
void upOneFloor() { //TRAVEL ONE FLOOR DISTANCE UP
  digitalWrite(8, HIGH);
  Stepping = true;
  delay(1);
  digitalWrite(9, HIGH);
  delay(1);
  digitalWrite(9, LOW);
  delayMicroseconds(100);

  StepCounter = StepCounter + 1;

  if (StepCounter == DISTANCE1)
  {
    StepCounter = 0;
    Stepping = false;
  }
}
void upTwoFloor() { //TRAVEL TWO FLOOR DISTANCE UP
  digitalWrite(8, HIGH);
  Stepping = true;
  delay(1);
  digitalWrite(9, HIGH);
  delay(1);
  digitalWrite(9, LOW);
  delayMicroseconds(100);

  StepCounter = StepCounter + 1;

  if (StepCounter == DISTANCE2)
  {
    StepCounter = 0;
    Stepping = false;
  }
}
void downOneFloor() { //TRAVEL ONE FLOOR DISTANCE DOWN
  digitalWrite(8, LOW);
  Stepping = true;
  delay(1);
  digitalWrite(9, HIGH);
  delay(1);
  digitalWrite(9, LOW);
  delayMicroseconds(100);

  StepCounter = StepCounter + 1;

  if (StepCounter == DISTANCE1)
  {
    StepCounter = 0;
    Stepping = false;
  }
}
void downTwoFloor() { //TRAVEL TWO FLOOR DISTANCE DOWN
  digitalWrite(8, LOW);
  Stepping = true;
  delay(1);
  digitalWrite(9, HIGH);
  delay(1);
  digitalWrite(9, LOW);
  delayMicroseconds(100);

  StepCounter = StepCounter + 1;

  if (StepCounter == DISTANCE2)
  {
    StepCounter = 0;
    Stepping = false;
  }
}