Property values are lost (after each loop) when nesting libraries

Question

I created 2 libraries to use in my Arduino code. One is a HwSwitch library, the other is a HwServo library which uses the HwSwitch library.

HwSwitch Library:

HwSwitch::HwSwitch(String switchName, int switchPort, int inputType, int pressedState)
{ 
    Name = switchName;
    SwitchPort = switchPort;
    _pressedState = pressedState;
    _lastCheckMillis = 0;

    pinMode(switchPort, inputType);
    _lastPinState = digitalRead(SwitchPort);
}

bool HwSwitch::IsPressed()
{
    int currentPinState = GetPinState();
    return currentPinState == _pressedState;
}

bool HwSwitch::SwitchStateChanged()
{
    int currentPinState = GetPinState();
    if (_lastPinState != currentPinState)
    {
        Serial.println("---");
        Serial.println("1. Now: " + String(currentPinState) + " - Prev: " + String(_lastPinState));

        _lastPinState = currentPinState;
        Serial.println("2. Now: " + String(currentPinState) + " - Prev: " + String(_lastPinState));

        return true;
    }

    return false;
}

int HwSwitch::GetPinState()
{
    unsigned long ms = millis();
    if ((ms - _lastCheckMillis) < 50)
    {
        return _lastPinState;
    }

    _lastCheckMillis = ms;
    return digitalRead(SwitchPort);
}

HwServo Library:

HwServo::HwServo(int servoPort, int zeroPoint, HwSwitch limitSwitch)
{
    _servo.attach(servoPort);
    _servo.write(zeroPoint);

    ServoPort = servoPort;
    ZeroPoint = zeroPoint;

    LimitSwitch = limitSwitch;
}

void HwServo::RotateUp()
{
    _servo.write(ZeroPoint + UP);
}

void HwServo::RotateDown()
{
    if (!LimitSwitch.IsPressed())
    {
        _servo.write(ZeroPoint + DOWN);
    }
}

void HwServo::Stop()
{
    _servo.write(ZeroPoint);
}

And this is how I initialized it in the Arduino code:

HwServo HwServos[] = {
    HwServo(9, 94, HwSwitch("S1", 14, INPUT_PULLUP, HIGH)),
    HwServo(5, 90, HwSwitch("S2", 8, INPUT_PULLUP, HIGH)),
};

void setup() { }

void loop() {

    for(int i = 0; i < 2; i++)
    {
        HwServo hwServo = HwServos[i];

        if (hwServo.LimitSwitch.SwitchStateChanged())
        {
            SendSwitchStateUpdate(hwServo.LimitSwitch);

            if (hwServo.LimitSwitch.IsPressed())
            {
                hwServo.Stop();
            }
        }
    }
}

Now finally to the problem! As you can see in the HwSwitch library I output some data using Serial.println. Here I can see that _lastPinState is successfully updated, but gets reset after every loop. However, when I create a HwSwitch directly and use it, _lastPinState is not reset. In other words, the resetting of the value only seems to occur when the HwSwitch library is used inside the HwServo library.

Appearently this has something to do with the pointers? I am probably initializing my classes incorrectly, but I have no idea how to fix it. Anyone that can help with (and preferably explain) this issue?

Answer 1

I don't have my Arduino on me right now, but I took look and re-wrote your code, added the omitted constructors at my best guess, and got it to compile. There were some things which needed corrected. I'm sure there are other ways, but this is what I did.

For complete code, go here.

First, I created some pointers to objects I'd like to stick around, like so:

HwServo *HwServos[2];
HwSwitch *s1;
HwSwitch *s2; 
HwServo *sv1; 
HwServo *sv2;

Now each is reserved in memory on the Arduino.

Now, construct the objects in setup():

void setup() {
    s1 = new HwSwitch("S1", 14, INPUT_PULLUP, HIGH);
    s2 = new HwSwitch("S2", 8, INPUT_PULLUP, HIGH);
    sv1 = new HwServo(9, 94, *s1);
    sv2 = new HwServo(5, 90, *s2);


    //Now, since you're going through an array:
    HwServos[0] = sv1;
    HwServos[1] = sv2;
}

Use that setup function!!! Maybe not always necessary, or in some cases even recommended, but it's nice to collect things which only need created once there, especially is this case.

Note that new was not used inside the scope of either object, but rather in the scope of the program... So no fancy destructors in your objects are required. Normally, you'd worry about deleting them all before program termination (or whenever best suited), but in Arduino's case, it'll just lose power and kill everything anyway.

You should change your class definitions to this:

class HwSwitch {
public:
    String Name;
    int SwitchPort;
    int _pressedState;
    int _lastCheckMillis;
    int _lastPinState;
    HwSwitch(String, int, int, int);
    bool IsPressed();
    bool SwitchStateChanged();
    int GetPinState();
};

class HwServo {
public:
    HwServo();
    HwServo(int, int, HwSwitch &);
    int ServoPort;
    int ZeroPoint;
    HwSwitch & LimitSwitch;
    void RotateUp();
    void RotateDown();
    void Stop();
    Servo _servo;
};

Note: I made everything public, feel free to move private stuff back to private if you wish.

I changed the constructors to:

HwSwitch::HwSwitch(String switchName, int switchPort, int inputType, int pressedState)
{ 
    Name = switchName;
    SwitchPort = switchPort;
    _pressedState = pressedState;
    _lastCheckMillis = 0;

    pinMode(switchPort, inputType);
    _lastPinState = digitalRead(SwitchPort);
}

HwServo::HwServo(int servoPort, int zeroPoint, HwSwitch &limitSwitch)
{
    _servo.attach(servoPort);
    _servo.write(zeroPoint);

    ServoPort = servoPort;
    ZeroPoint = zeroPoint;

    LimitSwitch = limitSwitch;
}

And I modified loop() like so:

void loop() {
// put your main code here, to run repeatedly:
    for(int i = 0; i < 2; i++)
    {
        if (HwServos[i]->LimitSwitch.SwitchStateChanged())
        {
            SendSwitchStateUpdate(HwServos[i]->LimitSwitch);
            if (HwServos[i]->LimitSwitch.IsPressed())
            {
                HwServos[i]->Stop();
            }
        }
    }
}