Code behaves differently after splitting ino file into cpp and header files

Hi,
I'm working on a 4 legged robot that uses 8 motors and Arduino Uno. When I started out, I had one giant 8_leg_calibration.ino file that did everything and the robot walked great. Now I'm trying to split out the code so that its easier to work with by adding .h and .cpp files to quadraped_robot.zip. I changed it by creating a Robot class that held all the constant values and attaches the motors to the pins on instantiation but now, after the change, the robot walks about 3 steps and then one motor stops moving. I re-uploaded the original 8_leg_calibration.ino file and it walked fine again. I repeated the testing several times and the quadraped_robot.zip causes one motor to stop moving every time while the original 8_leg_calibration.ino file runs fine. Wondering if its something I did wrong? The motor works fine normally so it doesn't seem like a hardware issue.

One more strange thing is if I instantiate the Robot class by doing

Robot robot;
void setup() {
  goToRestPos(robot);
}

above the setup() to create the var globally, all the motors on the robot starts behaving erratically. However, if I do it inside setup() as a local var:

void setup() {
  Robot robot;
  goToRestPos(robot);
}

or I do

Robot *robot;
void setup() {
  robot = new Robot();
  goToRestPos(*robot);
}

Then the motors move normally (minus the above issue where one motor stops working while walking).

*This is the first time I'm writing cpp code so please excuse any style errors. I'll gladly take suggestions!

Thanks for the help!

8_leg_calibration.ino (4.95 KB)

quadraped_robot.zip (2.59 KB)

1.) Put library code in the Arduino/libraries folder and #include them from there, not in the directory of the ino
2.) It's a good idea to host library code on GitHub for ease of cloning/installing and version control - it also makes it easier for us to look at it
3.) Try this:

ino:

#include <Servo.h>
#include <walk.h>

int iteration = 0;

// Instantiate robot class. Default constructor attaches the servos to pins.
Robot robot;

void setup() {
  robot.begin();
  delay(1000);
  // calibrate motors to rest position.
  goToRestPos(robot);
}

void loop() {
  // put your main code here, to run repeatedly:
    if (iteration<10) {
      walkOneStep(robot);
      iteration++;
  }
}

robot.h:

#include <Arduino.h>
#include <Servo.h> 

class Robot {
  public:
    Servo servo_front_right_top;
    Servo servo_front_right_bot;
    Servo servo_front_left_top;
    Servo servo_front_left_bot;
    Servo servo_back_right_top;
    Servo servo_back_right_bot;
    Servo servo_back_left_top;
    Servo servo_back_left_bot;

    const int ANGLE_rest_front_right_topShaft = 125;
    const int ANGLE_rest_front_left_topShaft = 55;
    const int ANGLE_rest_back_left_topShaft = 90;
    const int ANGLE_rest_back_right_topShaft = 90;
    const int ANGLE_rest_botShaft = 45;
    const int SWEEP_ANGLE_front = 20; // size of step
    const int SWEEP_ANGLE_back = 35; // size of step
    const int ANGLE_res = 10;

    void begin() {
      // Assign motor pins
      servo_front_right_top.attach(7);
      servo_front_right_bot.attach(6);
      servo_front_left_top.attach(2);
      servo_front_left_bot.attach(3);
      servo_back_right_top.attach(9);
      servo_back_right_bot.attach(8);
      servo_back_left_top.attach(5);
      servo_back_left_bot.attach(4);
    }
};

walk.h:

#include <Servo.h>
#include <robot.h>

void walkOneLeg(Servo servo, int targetAngle, int restAngle, int stepSize);
void liftLeg(Servo servo);
void restLeg(Servo servo);

void goToRestPos(Robot robot);
void walkOneStep(Robot robot);

void turnRightOneStep();
void turnLeftOneStep();

walk.cpp:

#include <Arduino.h>
#include <walk.h>
#include <Servo.h>

void walkOneLeg(Servo servo, int targetAngle, int restAngle, int stepSize, Robot robot) {
  int counter = 0;
  while (counter < (stepSize/robot.ANGLE_res)) {
    counter++;

    int ANGLE_topShaft = map(counter, 1, (stepSize/robot.ANGLE_res), restAngle, targetAngle);
    servo.write(ANGLE_topShaft);
    delay(50);
  }
  delay(50);
}

void liftLeg(Servo servo, Robot robot) {
  servo.write(robot.ANGLE_rest_botShaft+30);
  delay(50);
}

void restLeg(Servo servo, Robot robot) {
  servo.write(robot.ANGLE_rest_botShaft);
  delay(50);
}

void goToRestPos(Robot robot) {
  robot.servo_front_right_top.write(robot.ANGLE_rest_front_right_topShaft);
  robot.servo_front_right_bot.write(robot.ANGLE_rest_botShaft);
  robot.servo_front_left_top.write(robot.ANGLE_rest_front_left_topShaft);
  robot.servo_front_left_bot.write(robot.ANGLE_rest_botShaft);
  robot.servo_back_right_top.write(robot.ANGLE_rest_back_right_topShaft);
  robot.servo_back_right_bot.write(robot.ANGLE_rest_botShaft);
  robot.servo_back_left_top.write(robot.ANGLE_rest_back_left_topShaft);
  robot.servo_back_left_bot.write(robot.ANGLE_rest_botShaft);
  delay(500);
}

void walkOneStep(Robot robot) {
    liftLeg(robot.servo_back_right_bot, robot);
    walkOneLeg(
      robot.servo_back_right_top,
      robot.ANGLE_rest_back_right_topShaft - robot.SWEEP_ANGLE_back,
      robot.ANGLE_rest_back_right_topShaft,
      robot.SWEEP_ANGLE_back, robot);
    restLeg(robot.servo_back_right_bot, robot);

    liftLeg(robot.servo_back_left_bot, robot);
    walkOneLeg(
      robot.servo_back_left_top,
      robot.ANGLE_rest_back_left_topShaft + robot.SWEEP_ANGLE_back,
      robot.ANGLE_rest_back_left_topShaft,
      robot.SWEEP_ANGLE_back, robot);
    restLeg(robot.servo_back_left_bot, robot);

    liftLeg(robot.servo_front_right_bot, robot);
    walkOneLeg(
      robot.servo_front_right_top,
      robot.ANGLE_rest_front_right_topShaft - robot.SWEEP_ANGLE_front,
      robot.ANGLE_rest_front_right_topShaft,
      robot.SWEEP_ANGLE_front, robot);
    restLeg(robot.servo_front_right_bot, robot);

    liftLeg(robot.servo_front_left_bot, robot);
    walkOneLeg(
      robot.servo_front_left_top,
      robot.ANGLE_rest_front_left_topShaft + robot.SWEEP_ANGLE_front,
      robot.ANGLE_rest_front_left_topShaft,
      robot.SWEEP_ANGLE_front, robot);
    restLeg(robot.servo_front_left_bot, robot);

    goToRestPos(robot);
}


void turnRightOneStep(Robot robot) {
  liftLeg(robot.servo_back_right_bot, robot);
  walkOneLeg(
    robot.servo_back_right_top,
    robot.ANGLE_rest_back_right_topShaft + robot.SWEEP_ANGLE_back,
    robot.ANGLE_rest_back_right_topShaft,
    robot.SWEEP_ANGLE_back, robot);
  restLeg(robot.servo_back_right_bot, robot);
  delay(100);

  liftLeg(robot.servo_back_left_bot, robot);
  walkOneLeg(robot.servo_back_left_top,
    robot.ANGLE_rest_back_left_topShaft + robot.SWEEP_ANGLE_back,
    robot.ANGLE_rest_back_left_topShaft,
    robot.SWEEP_ANGLE_back, robot);
  restLeg(robot.servo_back_left_bot, robot);
  delay(100);

  liftLeg(robot.servo_front_right_bot, robot);
  walkOneLeg(
    robot.servo_front_right_top,
    robot.ANGLE_rest_front_right_topShaft + robot.SWEEP_ANGLE_front,
    robot.ANGLE_rest_front_right_topShaft,
    robot.SWEEP_ANGLE_front, robot);
  restLeg(robot.servo_front_right_bot, robot);
  delay(100);

  liftLeg(robot.servo_front_left_bot, robot);
  walkOneLeg(robot.servo_front_left_top,
    robot.ANGLE_rest_front_left_topShaft + robot.SWEEP_ANGLE_front,
    robot.ANGLE_rest_front_left_topShaft,
    robot.SWEEP_ANGLE_front, robot);
  restLeg(robot.servo_front_left_bot, robot);
  delay(100);

  goToRestPos(robot);
}

void turnLeftOneStep(Robot robot) {
  liftLeg(robot.servo_back_right_bot, robot);
  walkOneLeg(
    robot.servo_back_right_top,
    robot.ANGLE_rest_back_right_topShaft - robot.SWEEP_ANGLE_back,
    robot.ANGLE_rest_back_right_topShaft,
    robot.SWEEP_ANGLE_back, robot);
  restLeg(robot.servo_back_right_bot, robot);
  delay(100);

  liftLeg(robot.servo_back_left_bot, robot);
  walkOneLeg(
    robot.servo_back_left_top,
    robot.ANGLE_rest_back_left_topShaft - robot.SWEEP_ANGLE_back,
    robot.ANGLE_rest_back_left_topShaft,
    robot.SWEEP_ANGLE_back, robot);
  restLeg(robot.servo_back_left_bot, robot);
  delay(100);

  liftLeg(robot.servo_front_right_bot, robot);
  walkOneLeg(
    robot.servo_front_right_top,
    robot.ANGLE_rest_front_right_topShaft - robot.SWEEP_ANGLE_front,
    robot.ANGLE_rest_front_right_topShaft,
    robot.SWEEP_ANGLE_front, robot);
  restLeg(robot.servo_front_right_bot, robot);
  delay(100);

  liftLeg(robot.servo_front_left_bot, robot);
  walkOneLeg(
    robot.servo_front_left_top,
    robot.ANGLE_rest_front_left_topShaft - robot.SWEEP_ANGLE_front,
    robot.ANGLE_rest_front_left_topShaft,
    robot.SWEEP_ANGLE_front, robot);
  restLeg(robot.servo_front_left_bot, robot);
  delay(100);

  goToRestPos(robot);
}

Note:
Power_Broker did beat me to it.

One possible cause of the problem is that manipulation of pins should not be done in the constructor; your constructor in robot.h calls Servo.attach() which calls pinMode. If you look at original examples and your calibration code, it's done in setup().

Place the Servo.attach() calls in e.g. a begin() method in the Robot class and call that method in the setup().

Do not use absolute paths (as indicated by Power_Broker); I tried to compile the code and first had to modify a number of files to get rid of include errors.

Thanks for the help! I will give that a try. One question, I have the header and cpp files in the same directory as I read that I should use tabs when adding supporting files. Tabs put the files in the same directory as the .ino file. When should I use tabs vs a separate directory (library)? The absolute paths were because otherwise, the compiler throws a not found error when I try to include the header files.

You create libraries when you want to re-use code in a different project.

You add files in the project directory if they are part of the project.

There is a third way but i'm not behind a computer to verify the exact detailt. It involves a subdirectory (I think called src) in the project directory under which you can put 'kind of' libraries. Usefull once you have debugged a part of your code and don't want to end up with tens of tabs.

sterretje:
There is a third way but i'm not behind a computer to verify the exact detailt. It involves a subdirectory (I think called src) in the project directory under which you can put 'kind of' libraries. Usefull once you have debugged a part of your code and don't want to end up with tens of tabs.

You don't actually need a src subdirectory, but the library code does need to be in it's own folder within the Arduino/libraries folder.

This is the full directory to the Arduino "custom libraries" directory: "C:\Users<<>>\Documents\Arduino\libraries"

Thank you both for all the help and clarification! The motors behave as expected now!

Power_Broker:
You don't actually need a src subdirectory, but the library code does need to be in it's own folder within the Arduino/libraries folder.

This is the full directory to the Arduino "custom libraries" directory: "C:\Users<<>>\Documents\Arduino\libraries"

If you have no need to share .h and .cpp files with other projects, why would you put it in the libraries directory?

C:\USERS\STERRETJE\DOCUMENTS\ARDUINO\DEMO\TESTSKETCH_WITH_LOCALSRCLIBRARY
\---testsketch_with_localsrclibrary
    |   testsketch_with_localsrclibrary.ino
    |
    \---src
        \---myLib
                myLib.cpp
                myLib.h

And in your ino file, use #include "src/myLib/myLib.h".

sterretje:
If you have no need to share .h and .cpp files with other projects, why would you put it in the libraries directory?

Why not? It's so much easier/more generalized, and, who knows, maybe one day you'll want to apply that code to another project.

What everyone else said: don't call anything that invokes pinMode or any of the other arduino functions in your constructor. Instead, provide a separate initializtion function. I like using setup() as the name of that methd, because I think it makes clear what is going on.

class MyClass {
  const byte inPin;
  const byte outPin;

  byte previousRead;

public:
  MyClass(byte attachA, byte attachB) :
    inPin(attachA), outPin(attachB) {
  }

  void setup() {
    pinMode(inPin, INPUT);
    pinMode(outPin, OUTPUT);

    previousRead = digitalRead(inPin);
    digitalWrite(outPin, HIGH); // initial state
  }

};


MyClass myVar(7,8);

void setup() {
  myVar.setup();
}

While we're on the subject of constructors..

If there is -any- chance at all that a class will be created as a global, do NOT do anything in the consrtuctor that relies on the outside world beyond the bits you get passed in. Because there is no way to tell the state of the outside world before your code starts. Setup what you can without help and leave the rest to something like a begin() call.

I've been bit by this a few times and learned the hard way.

-jim lee