Retaining C++ static members

I am working on a big project which will have many servos. As a result I've acquired a serial servo controller. I am trying to write a class which will encapsulate the code required to work with this controller and I've hit a roadblock that seems to be rooted in my C++ skill (or lack thereof). The controller is capable of managing 6 or 7 servos so I've written a class of which I can construct instances as I add servos.

At this point I have one error which is causing a reboot. I assume it is something to do with the way I am encapsulating and instantiating the NewSoftSerial library because when I call _serial->print the chip reboots! I've marked this line with a few comments.

Any advice as to the structure of things is welcome but I'd sure like to sort out what I'm doing wrong first.

Thanks for looking.

I would like the end result to look something like this:

SerialServo arm;
SerialServo head;

void setup(){
  SerialServo::init();

  //zero and one are the positions on the servo controller
  arm.attach(0);
  head.attach(1);
}

void loop(){
  //.......do stuff
  head.set(120);
  arm.set(36);
  //.......do more stuff
}

And here is what I have so far:

//main arduino sketch
#include <NewSoftSerial.h>
#include <Math.h>

#include "SerialServo.h"

SerialServo servo;

void setup() {
  Serial.begin(9600);
  Serial.println("Awake!");

  SerialServo::init(7,8);
  Serial.println("post init");

  servo.set(150);
}

void loop () {}
//the serialservo.h file
#ifndef SERIALSERVO_H
#define SERIALSERVO_H

#include "NewSoftSerial.h"

class SerialServo {
  public:
    //                             rx, tx
    static NewSoftSerial *_serial;//(9, 8);

    int _servo_id;

    static void init(char tx, char rx);
    static void send(int servo_id, int angle);
    void attach(int servo_id);
    void set(int spot);
};
#endif
//the serialservo.cpp file
#ifndef SERIALSERVO_CPP
#define SERIALSERVO_CPP

#include "WProgram.h"
#include "SerialServo.h"

//static
void SerialServo::init(char tx, char rx){
  _serial = &NewSoftSerial(rx, tx);
  _serial->begin(9600);
  Serial.println("SerialServo::init");
}

//static
void SerialServo::send(int servo_id, int angle){
  unsigned char buff[6];

  int temp     = angle & 0x1f80;
  char pos_hi  = temp >> 7;
  char pos_low = angle & 0x7f;

  buff[0] = 0x80;        // start byte
  buff[1] = 0x01;        // device id
  buff[2] = 0x04;        // command number
  buff[3] = servo_id;       // servo number
  buff[4] = pos_hi;      // data1
  buff[5] = pos_low;     // data2

  for(int i=0; i<6; i++){
    //
    //this line causes reboot!
   //
    _serial->print(buff[i], BYTE);
    Serial.println("SerialServo::set_angle");
  }
}

void SerialServo::attach(int servo_id){
  _servo_id = servo_id;
}

void SerialServo::set(int angle){
  Serial.println("SerialServo::set_angle");
  SerialServo::send(_servo_id, angle);
  Serial.println("SerialServo::set_angle");
}

NewSoftSerial * SerialServo::_serial;
#endif

Some personal background: I've been doing arduino projects on and off since the ol' NG days. However, recently I decided I need to organize my arduino code in such a way so it can be easily understood by more than just myself. As a result I'm learning C++ classes. I'm no stranger to OOP, but C++ is a language I never had the opportunity to learn.

OK, here's what you started with:

The SerialServo class has a member, _serial, which is a pointer to a NewSoftSerial object. So far, so good.

Now how do you get the vershluggener thing initialized?

Here's one way:

  1. Declare a global NewSoftSerial object with the desired pin numbers, and an "Empty" SerialServo object. (Default constructor).

  2. Make the SerialServo::init() function take a pointer to the NewSoftSerial object and a baud value so that it can initialize the NewSoftSerial object and set the SerialServo::_serial pointer value to the address of the NewSoftSerial object:

//SerialServo.h
#ifndef SERIALSERVO_H
#define SERIALSERVO_H

#include "NewSoftSerial.h"

class SerialServo {
  // Actually you probably won't want the members to be public, but
  // for easier debugging you can make everything public.
  public:
    NewSoftSerial * _serial;
    int _servo_id;
    
    void init(NewSoftSerial * channel, int baud);
    void send(int servo_id, int angle);
    void attach(int servo_id);
    void set(int spot);
};
#endif
//SerialServo.cpp

#include "WProgram.h"
#include "SerialServo.h"

void SerialServo::init(NewSoftSerial * channel, int baud)
{
    _serial = channel;
    _serial->begin(baud);
    Serial.println("Returning from SerialServo::init");
}

void SerialServo::send(int servo_id, int angle)
{
    unsigned char buff[6];

    int temp     = angle & 0x1f80;
    char pos_hi  = temp >> 7;
    char pos_low = angle & 0x7f;

    buff[0] = 0x80;        // start byte
    buff[1] = 0x01;        // device id
    buff[2] = 0x04;        // command number
    buff[3] = servo_id;    // servo number
    buff[4] = pos_hi;      // data1
    buff[5] = pos_low;     // data2

    for(int i = 0; i < 6; i++) {
        _serial->print(buff[i], BYTE);
    }
    // For debugging, show on the screen the values of the bytes
    // that are going out the NewSoftSerial data line
    for (int i = 0; i < 6; i++) {
        Serial.print("0x");
        Serial.print(buff[i], HEX);
        Serial.print("  ");
    }
    Serial.println();
}

void SerialServo::attach(int servo_id)
{
    _servo_id = servo_id;
}


void SerialServo::set(int angle)
{
    Serial.println("1: SerialServo::set_angle");
    send(_servo_id, angle);
    Serial.println("2: SerialServo::set_angle");
}
// TestSerialServo.pde

#include <NewSoftSerial.h>
#include "SerialServo.h"

// Declare the NewSoftSerial object
// and the SerialServo object
NewSoftSerial ch78(7, 8);
SerialServo servo;

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

    Serial.println("Before init");
    servo.init(&ch78, 9600);
    Serial.println("After init");

    servo.set(150);
}

void loop ()
{
    // Whatever
}

Output:


Before init
Returning from SerialServo::init
After init
1: SerialServo::set_angle
0x80  0x1  0x4  0x0  0x1  0x16  
2: SerialServo::set_angle

Regards,

Dave

While I was researching this, Dave replied. :slight_smile:

The gist of your problem is that this line:

 _serial = &NewSoftSerial(rx, tx);

... take the address of a temporary object which goes out of scope at the end of init*, hence the crash when you use that address later on.

It is discussed here somewhat:

http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1218306702

Dave's suggestion (I was trying to invent something along those lines) works around that by creating the object as a static variable in advance, and then you just use the address of it. That way the address stays valid.

(edit) * Actually it goes out of scope at the end of that line so you were a little lucky that this line, even, worked:

_serial->begin(9600);

I was trying this BTW:

//static
void SerialServo::init(char tx, char rx){
  static SoftwareSerial temp = SoftwareSerial(rx, tx);
  _serial = &temp;
  _serial->begin(9600);
  Serial.println("SerialServo::init");
}

That way the variable is static and not temporary. But I got the error message:

serialservo.cpp.o: In function `SerialServo::init(char, char)':
serialservo.cpp:10: undefined reference to `__cxa_guard_acquire'
serialservo.cpp:10: undefined reference to `__cxa_guard_release'

lol.

Okay. I need to get this project rolling, so I think I will use something similar to Nick's suggestion.

But I feel like there is probably a better solution.

Gentlemen, thanks for your input.

-vox

You're welcome. :slight_smile:

I think the fundamental problem is a design one here. I am uncomfortable with having a single-instance thing (the serial port) bundled up with a multiple-instance thing (the servo gadget). You are trying to work around it by having it as static, but this is causing other problems.

I suggest either just having the serial port separated out (and simply requiring its existence, and passing it down to each servo), or possibly better, make a single "servo owner" object (which owns the serial port), and the servo owner maintains a list of servos.

Well, that is entirely possible.

So here is the logic behind the design.

The board which I'm interacting with via this class is a serial servo controller. It takes one serial port and needs to have exclusive ownership of that port. So, outside of this class, there is no other reason to use those two pins...and choosing to do so would likely corrupt things. Therefore I elected to have the Serial Servo class have a static aspect which would control the serial port.

The dynamic (non-static) part of the class is just the "syntactic sugar" for dealing with each servo individually and merely interfaces with the static functions to get the requests fulfilled.

Is my design logic flawed somehow? It makes sense to me to encapsulate everything so that as little as possible remains for the main() to do to get things up and running.

I thought that was probably your aim, and so far so good. It is nice to have things encapsulated.

But you really have two things:

  • The group of servos, all attached to one serial port
  • Individual servos

So it makes sense for them to be two classes. The "group" class, when created, grabs the serial port, and initializes its list of attached servos to be empty. Then you might do "group.addservo" which adds a new servo to the list (passing down the pointer to the single serial port in the process). That returns an instance of an individual servo which you then play with. But the group can still be used for "group-like" things (eg. turning all servos off).

And if you ever finish with all servos (which might not happen in your case) then destroying or deleting the group class could also remove all owned servos, and release the serial port.

This design even lets you have two groups of servos (if you ever needed to), connected to two different sets of serial ports.

That was the other design I considered.

I had not considered attaching more than one servo array, and that is a design benefit.

However, how would having two discrete classes avail main() of the responsibility of instantiating the serial port? You would still have to call the constructor for NewSoftSerial in the constructor for the serialservocontroller, right?

thanks

-vox

Well for one thing you now only have one serial member (in the controller), so it doesn't have to be static. So that problem goes away.

Here's one way of doing it. This compiles, I'm not totally happy with it for various reasons, but it shows how you might have the serial port initialized in the constructor of the controller:

#include <NewSoftSerial.h>

#define MAX_SERVOS 10

// individual servo
class SerialServo {
    friend class servoController;
    
    NewSoftSerial * _serial;
    int _servo_id;

  public:
    
    SerialServo () : _serial (NULL), _servo_id (0) {};   // default constructor
    
    void init(NewSoftSerial * channel, int baud);
    void send(int servo_id, int angle);
    void attach(int servo_id);
    void set(int spot);
};

void SerialServo::attach (int servo_id)
{
  _servo_id = servo_id;
}


// controlling class
class servoController
{
  
  NewSoftSerial _serial;  // private copy of serial port
  int _servo_count;
  
  // array/list of servo here, eg.
  
   SerialServo _servos [MAX_SERVOS]; 
  
  public:
  
    servoController (const uint8_t receivePin, const uint8_t transmitPin);  // constructor
      
    SerialServo & addServo (const int servo_id);
  
  
};

// constructor
servoController::servoController (const uint8_t receivePin, const uint8_t transmitPin) :
      _serial (receivePin, transmitPin), _servo_count (0)
      {
      // other initialization
      }
     
SerialServo & servoController::addServo (const int servo_id)
{
   
  int which = _servo_count++;
  
  _servos [which].attach (servo_id);
  _servos [which]._serial = &_serial;  // serial port
  
  return _servos [which];
  
}


// ------------------------------- main stuff starts here ------------------------

// make the controller object 
servoController controller (7, 8);

// individual servos
SerialServo & servo1 = controller.addServo (5);
SerialServo & servo2 = controller.addServo (8);
  
void setup ()
{ }

void loop () { }

:astonished:

You clearly have a better syntax-fu than I! I will likely spend many hours learning what you have demonstrated here.

Thanks, and thanks again.

mind-blown

Hello again. Thanks again for your help. I thought I would report back with what I've developed. I've learned a ton about C++ in the last week. I found this really helpful website http://www.learncpp.com to explain what I needed to understand about your code example and implement it.

The project is far from done but I'll be using the techniques you demonstrated throughout the rest of the classes.

If you have any comments I'll take them, but everything seems to be in working order now.

Thanks again,

--vox

//SerialServo.h

#ifndef SERIALSERVO_H
#define SERIALSERVO_H

#include "NewSoftSerial.h"

class SerialServoController;

class SerialServo {
  private:
    SerialServoController * _controller;
    int _servo_id;

  public:
    SerialServo(){};

    void send(int angle);
    void attach(int servo_id);
    void set(int spot);

    friend class SerialServoController;
};


class SerialServoController {
  NewSoftSerial _serial;
  int _servo_count;

  SerialServo _servos[8];

  public:
    SerialServoController(const uint8_t rx, const uint8_t tx);
    SerialServo & add_servo(const uint8_t servo_id);

    void init();
    void write(const uint8_t servo_id, int value);
};

#endif
//SerialServo.cpp

#ifndef SERIALSERVO_CPP
#define SERIALSERVO_CPP

#include "WProgram.h"
#include "SerialServo.h"

void SerialServo::attach(int servo_id){
  _servo_id = servo_id;
}

void SerialServo::set(int angle){
  Serial.println("SerialServo.set");
  _controller->write(_servo_id, angle);
}

////////////////////////////////////////////////////////////////////////////////////////

SerialServoController::SerialServoController(const uint8_t rx, const uint8_t tx)
  : _serial(rx, tx), _servo_count(0)
{ }

void SerialServoController::init(){
  Serial.println("SerialServoController::init");
  _serial.begin(9600);
}

SerialServo & SerialServoController::add_servo(const uint8_t servo_id){
  int i = _servo_count ++;
  _servos[i] = SerialServo();
  _servos[i]._controller = this;
  _servos[i].attach(servo_id);

  return _servos[i];
}

void SerialServoController::write(const uint8_t servo_id, int value){
  Serial.println("ServoController::set_angle");
  return;
  unsigned char buff[6];

  int temp     = value & 0x1f80;
  char pos_hi  = temp >> 7;
  char pos_low = value & 0x7f;

  buff[0] = 0x80;        // start byte
  buff[1] = 0x01;        // device id
  buff[2] = 0x04;        // command number
  buff[3] = servo_id;    // servo number
  buff[4] = pos_hi;      // data1
  buff[5] = pos_low;     // data2

  for(int i=0; i<6; i++){
    //_serial->print(buff[i], BYTE);
  }
}

#endif