Help with splitting code out into a library class

Using Arduino Programming Questions
1

Hi, I'm hoping for a little help. i am working with an Adafruit motor shield v2.
I have used one of the example files Accel_MultiStepper. I have amended the code to suit my setup, one motor shield and two stepper motors, and the code behaved as hoped when uploaded to the Arduino (uno). however i want to use this code (or a variant of it) as a library class. I have split it out into a cpp and header file, and call it from a test script, it compiles, but for some reason does not behave as expected (it does nothing). After using some serial print lines at various points, it seems like its the creation of my new class where the problem is.
I'm only just getting back into writing code after a very long break, and seem to have forgotten more than i remember, so any help would be really appreciated.

the working example script:

#include <Wire.h>
#include <AccelStepper.h>
#include <Adafruit_MotorShield.h>

Adafruit_MotorShield AFMStop(0x60); // Default address, no jumpers

// Connect two steppers with 200 steps per revolution (1.8 degree)
// to the top shield
Adafruit_StepperMotor *myrightMotor = AFMStop.getStepper(200, 1);
Adafruit_StepperMotor *myleftMotor = AFMStop.getStepper(200, 2);



// you can change these to SINGLE, DOUBLE, INTERLEAVE or MICROSTEP!
// wrappers for the first motor!
void forwardstep1() {  
  myrightMotor->onestep(FORWARD, DOUBLE);
}
void backwardstep1() {  
  myrightMotor->onestep(BACKWARD, DOUBLE);
}
// wrappers for the second motor!
void forwardstep2() {  
  myleftMotor->onestep(FORWARD, DOUBLE);
}
void backwardstep2() {  
  myleftMotor->onestep(BACKWARD, DOUBLE);
}

// Now we'll wrap the 3 steppers in an AccelStepper object
AccelStepper rightMotor(forwardstep1, backwardstep1);
AccelStepper leftMotor(backwardstep2, forwardstep2);

bool stepsAreSet = 0;

void setup()
{
  AFMStop.begin(); // Start the top shield
   
  rightMotor.setMaxSpeed(200.0);
  rightMotor.setAcceleration(100.0);
  
    
  leftMotor.setMaxSpeed(200.0);
  leftMotor.setAcceleration(100.0);

  Serial.begin(9600);
  Serial.println("setup "); 
  Serial.print("\n");
  
}

void loop()
{
  if(!stepsAreSet)
  {
    rightMotor.moveTo(50000);
    leftMotor.moveTo(50000);
    stepsAreSet = 1;
  }
    // Change direction at the limits
    if (rightMotor.distanceToGo() == 0)
	rightMotor.moveTo(-rightMotor.currentPosition());

    if (leftMotor.distanceToGo() == 0)
	leftMotor.moveTo(-leftMotor.currentPosition());

    rightMotor.run();
    leftMotor.run();
}

script:

#include <Robot_Tracks.h>

Robot_Tracks Tracks = Robot_Tracks();

void setup() 
{
  Tracks.begin(); 
}

void loop() 
{
  Tracks.forwardTo();
}

cpp:

#include <Wire.h>
#include <AccelStepper.h>
#include <Adafruit_MotorShield.h>
#include <Robot_Tracks.h>

Adafruit_MotorShield AFMStop(0x60); // Default address, no jumpers

// Connect two steppers with 200 steps per revolution (1.8 degree)
// to the top shield
Adafruit_StepperMotor *myrightMotor = AFMStop.getStepper(200, 1);
Adafruit_StepperMotor *myleftMotor = AFMStop.getStepper(200, 2);



// you can change these to SINGLE, DOUBLE, INTERLEAVE or MICROSTEP!
// wrappers for the first motor!
void forwardstep1() {
	myrightMotor->onestep(FORWARD, DOUBLE);
}

void backwardstep1() {
	myrightMotor->onestep(BACKWARD, DOUBLE);
}

// wrappers for the second motor!
void forwardstep2() {
	myleftMotor->onestep(FORWARD, DOUBLE);
}

void backwardstep2() {
	myleftMotor->onestep(BACKWARD, DOUBLE);
}

// Now we'll wrap the 3 steppers in an AccelStepper object
AccelStepper rightMotor(forwardstep1, backwardstep1);
AccelStepper leftMotor(backwardstep2, forwardstep2);

bool stepsAreSet = 0;

Robot_Tracks::Robot_Tracks() {
	
}

void Robot_Tracks::begin() {
	AFMStop.begin(); // Start the top shield

	rightMotor.setMaxSpeed(200.0);
	rightMotor.setAcceleration(100.0);


	leftMotor.setMaxSpeed(200.0);
	leftMotor.setAcceleration(100.0);
}

void Robot_Tracks::forwardTo() {
	if (!stepsAreSet) {
		rightMotor.moveTo(50000);
		leftMotor.moveTo(50000);
		stepsAreSet = 1;
	}
	// Change direction at the limits
	if (rightMotor.distanceToGo() == 0)
		rightMotor.moveTo(-rightMotor.currentPosition());

	if (leftMotor.distanceToGo() == 0)
		leftMotor.moveTo(-leftMotor.currentPosition());

	rightMotor.run();
	leftMotor.run();
}

header:

#ifndef Robot_Tracks_h
#define Robot_Tracks_h

#include <Wire.h>
#include <AccelStepper.h>
#include <Adafruit_MotorShield.h>
#include <Robot_Tracks.h>

class Robot_Tracks
{
	public:
		Robot_Tracks();
		void begin();
		void forwardstep1();
		void backwardstep1();
		void forwardstep2();
		void backwardstep2();
		void forwardTo();
		
	private:
		bool stepsAreSet;
		Adafruit_MotorShield AFMStop;
		Adafruit_StepperMotor *myrightMotor;
		Adafruit_StepperMotor *myleftMotor;
		AccelStepper rightMotor;
		AccelStepper lefttMotor;
		
};
#endif
2

Those are instance variables

those are global variables

when you are in the method of your class like

you are not referring to the global variables but the instance variables and they are not initialized

--

given you depend on a global unique Adafruit_MotorShield instance, making a class has limited value.

3

Hi, thanks for your reply.
I’m kind of trying to recreate my final year project I did at uni. I had to create an API for use with a robot, the point of the API was to make it easier for first year students to write code to control the robot.
Given I’ve not written a line of code in a long time I thought this would be a good project to refresh/relearn. I’d also like my grandkids to be able to use it, see if I can spark their interest in it.

4

Worth every effort to get them interested for sure.

If you want a class it would have to either be a subclass of the adafruit stuff or a completely separated class and you pass the motor reference to its constructor for example

5

Again, thanks for that. I’ll do a bit of Googling and reading based on what you’ve said.

6

I am still going around in circles with this. I have restructured my code various ways now. I have also pleased a some 'Serial.print's in the Adafruit classes to see where I'm hitting a problem. I seem to be creating and successfully calling their 'begin' methods. I'm even able to see 'distanceToGo' in 'AccelSteppetinger' counting down as I call 'run', but still no action from the motors. From what I can see when using the working example code, I'm getting the same data spat out, but the motors do work.
I really would appreciate some help here.
This is what my code currently looks like:
Script:

#include <Wire.h>
#include <AccelStepper.h>
#include <Adafruit_MotorShield.h>
#include <Tracks.h>

Tracks myTracks;

void setup()
{ 
  delay(2000);
  Serial.begin(9600);
  Serial.println("setup");
  Serial.print("\n");
  myTracks.begin();
}

void loop()
{
  delay(100);
  myTracks.forward();
}

My class cpp:

#include <Wire.h>
#include <AccelStepper.h>
#include <Adafruit_MotorShield.h>
#include <Tracks.h>

bool stepsAreSet;

Tracks::Tracks() : _AFMS(0x60), _myrightMotor(_AFMS.getStepper(200, 1)), _myleftMotor(_AFMS.getStepper(200, 2)),
_rightMotor(_myrightMotor->onestep(FORWARD, DOUBLE), _myrightMotor->onestep(FORWARD, DOUBLE)),
_leftMotor(_myleftMotor->onestep(BACKWARD, DOUBLE), _myleftMotor->onestep(FORWARD, DOUBLE))
{	
}

void Tracks::begin()
{
	Serial.print("Tracks begin: ");
  	Serial.print("\n");
  
	delay(2000);
	stepsAreSet = 0;
	_AFMS.begin();
	delay(200);
	_rightMotor.setMaxSpeed(200.0);
  	_rightMotor.setAcceleration(100.0);
  	_leftMotor.setMaxSpeed(200.0);
  	_leftMotor.setAcceleration(100.0);
  	
}

void Tracks::forward()
{
	if(!stepsAreSet)
	{
    	_rightMotor.moveTo(5000);
    	_leftMotor.moveTo(5000);
    	stepsAreSet = 1;
	}
  	_rightMotor.run();
  	_leftMotor.run();
  
  	Serial.print("DisToGo: ");
  	Serial.print(_leftMotor.distanceToGo());
  	Serial.print("\n");
}

My class header:

#ifndef Tracks_h
#define Tracks_h

#include <Wire.h>
#include <AccelStepper.h>
#include <Adafruit_MotorShield.h>
//#include <Tracks.h>



class Tracks
{
   public:
       Tracks();
       void begin();
       void forward();
       Adafruit_MotorShield _AFMS; 
		Adafruit_StepperMotor *_myrightMotor;
		Adafruit_StepperMotor *_myleftMotor;
		AccelStepper _rightMotor;
		AccelStepper _leftMotor;
       
    private:
    	bool stepsAreSet;
		
};
#endif

As far back up the inherited classes I've checked whats going on:

bool Adafruit_MS_PWMServoDriver::begin(TwoWire *theWire) {
	Serial.print("PWMS begin: ");
	Serial.print(_i2caddr);
  	Serial.print("\n");
  	
  if (i2c_dev)
    delete i2c_dev;
  i2c_dev = new Adafruit_I2CDevice(_i2caddr, theWire);
  if (!i2c_dev->begin())
  {
  	Serial.print("PWMS begin failed");
  	Serial.print("\n");
    return false;
    }
  reset();
  return true;
}
7

I don’t use the Adafruit_MotorShield library. Are you 100% sure that those initializers can be called very early (before the arduino is really set up) ? Shouldn’t some of them be in begin()?

8

I'm far from 100% sure about this to be honest. I have tried initializing in begin(), but I get errors. maybe i should be going in that direction, it's just I 'feel' like i was closer doing it via the constructor as I don't get any errors. I also seem to be creating all the objects I need, and when I check the values I expect them to be initialized with they seem to check out. When i call run and then call distanceToGo, I can see the steps are counting down.
In another forum, I have been told: "You need to define the forwardStep and backwardStep functions and initialize your AccelStepper motor objects with pointers to those function as it is done in the sample code.".
But I get scope errors when I try that within my class. I also can't see the difference between doing it like it is done in the sample code:

void forwardstep1() {  
  myrightMotor->onestep(FORWARD, DOUBLE);
}
void backwardstep1() {  
  myrightMotor->onestep(BACKWARD, DOUBLE);
}
AccelStepper rightMotor(forwardstep1, backwardstep1);

And how I do it:

_rightMotor(_myrightMotor->onestep(FORWARD, DOUBLE), _myrightMotor->onestep(BACKWARD, DOUBLE))

surely either way I am passing two pointers to the Adafruit_StepperMotor method onestep?

9

what you need to do is provide a callback function for the motors like you did here

AccelStepper rightMotor(forwardstep1, backwardstep1);
AccelStepper leftMotor(backwardstep2, forwardstep2);

so in theory you could use bind to attach member functions to the constructor call for your right and left motors, something like this

Tracks::Tracks() : _AFMS(0x60),
                   _myrightMotor(_AFMS.getStepper(200, 1)),
                   _myleftMotor (_AFMS.getStepper(200, 2)),
                   _rightMotor(std::bind(&Tracks::forwardstep1, this), std::bind(&Tracks::backwardstep1, this)),
                   _leftMotor(std::bind(&Tracks::backwardstep2, this), std::bind(&Tracks::forwardstep2, this)) {}

where forwardstep1, forwardstep2, backwardstep2 and forwardstep2 would be private member functions of the class.

but the challenge is that the AccelStepper constructor is expecting 2 function pointers, the prototype is like this

AccelStepper::AccelStepper(void(*)() forward, void(*)() backward);

and you can't use std::bind (nor lambda) to get the right signature in the constructor and so the compiler will bark at you.

solving for this would require either to modify the AccelStepper library or do some ugly workarounds and bind does not even work on UNO and the likes so it would not be great.

the real question for me is why do you need a class when only one instance is needed since everything is fully set in the constructor with

_AFMS(0x60),
_myrightMotor(_AFMS.getStepper(200, 1)),
_myleftMotor(_AFMS.getStepper(200, 2)),

you won't have two instances of your Tracks class..

C++ is no Java or those programming langages where everything needs to be a class.

You could define for example everything in a .hpp file as functions and variables without doing a class.

10

Thank you for that. I will spend more time this evening considering it.
My only reasons for the Tracks class is to provide a set of methods that control the robots movements in a simplified manner, such as goForward, goBackward, turnLeft180, turnLeft90...
and hide some of the complexity from the user code. If there is another way to achieve this, I'm happy to explore that. I'm not familar with hpp files, I'll do some reading.
Thank you again.

11

Maintaining encapsulation and abstraction is definitely a good idea. I'd recommend a Singleton. See My Post #16 In this Thread.

12

I don't dispute that a class is a good way to provide encapsulation and abstraction, it is in general and indeed a Singleton is an option

In this case I find that it makes it more difficult than necessary (IMHO) to use function pointers where the function is a member function.

I would do something simple like

sketch.ino

#include "Tracks.h"

void setup() {
  configureMotors();
}

void loop() {
 runMotors();
}

Tracks.h

#ifndef _Tracks_h
#define _Tracks_h

void configureMotors();
void runMotors(bool reset = false);

#endif

Tracks.cpp

#include <Wire.h>
#include <AccelStepper.h>
#include <Adafruit_MotorShield.h>
#include "Tracks.h"

static Adafruit_MotorShield AFMStop(0x60); // Default address, no jumpers
static Adafruit_StepperMotor *myrightMotor = AFMStop.getStepper(200, 1);
static Adafruit_StepperMotor *myleftMotor = AFMStop.getStepper(200, 2);

static void forwardstep1() {
  myrightMotor->onestep(FORWARD, DOUBLE);
}

static void backwardstep1() {
  myrightMotor->onestep(BACKWARD, DOUBLE);
}

static void forwardstep2() {
  myleftMotor->onestep(FORWARD, DOUBLE);
}

static void backwardstep2() {
  myleftMotor->onestep(BACKWARD, DOUBLE);
}

static AccelStepper rightMotor(forwardstep1, backwardstep1);
static AccelStepper leftMotor(backwardstep2, forwardstep2);

void configureMotors() {
  AFMStop.begin(); // Start the top shield
  rightMotor.setMaxSpeed(200.0);
  rightMotor.setAcceleration(100.0);
  leftMotor.setMaxSpeed(200.0);
  leftMotor.setAcceleration(100.0);
}

void runMotors(bool reset) {
  static bool stepsAreSet = false;

  if (reset) {
    stepsAreSet = false;
    rightMotor.setCurrentPosition(0);
    leftMotor.setCurrentPosition(0);
  }

  if (!stepsAreSet) {
    rightMotor.moveTo(50000);
    leftMotor.moveTo(50000);
    stepsAreSet = true;
  }

  // Change direction at the limits
  if (rightMotor.distanceToGo() == 0)
    rightMotor.moveTo(-rightMotor.currentPosition());

  if (leftMotor.distanceToGo() == 0)
    leftMotor.moveTo(-leftMotor.currentPosition());

  rightMotor.run();
  leftMotor.run();
}

(typed here, so not 100% sure I did not leave any typo)

13

True. Getting off the AVR platform and on to something with std::function opens up vast new possibilities.

14

indeed, it makes life much easier

With AccelStepper there would still be an issue because the AccelStepper constructor is expecting 2 function pointers, the prototype is like this

AccelStepper::AccelStepper(void(*)() forward, void(*)() backward);

and you can't use std::bind (nor lambda) to get the right signature in the constructor and so the compiler will bark at you.

15

Thank you so much! :slight_smile: compiles and motors run.
I had actually tried something very similar before, though I'd not used configureMotors(), I had done what you do there in begin(), not that I think that should cause an issue. But I also had a lot more declared in my header file which I now suspect must have been the problem.
Thank you again, I really do appreciate your help.
I do need to go away and do a bit more reading though, I need to gain (regain) a better understanding.

16

Glad if it helped !
Have fun

17

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.