Seltsames verhalten von Klassenvariablen

[jacoblange]

Hallo,

ich bin gerade etwas am verzweifeln, weil ich das verhalten von einer Klassenvariablen, die ich nur im Konstruktor setze und ansonsten nur lesend darauf zugreife, nicht nachvollziehen kann. Hintergrund ist ein Programm um drei DC Motoren möglichst synchron zu steuern (für einen selbstgebauten höhenverstellbaren Eckschreibtisch). Und gleich vorweg, der Schreibtisch ist so konstruiert, das z.B. bei einer Blockade die elektronische Abschaltung versagen sollte, durch einen zweiten Sicherheitsmechanismus der Strom komplett unterbrochen wird.

Aber nun zum eigentlichen Problem. Es gibt eine Motor Klasse, von der für jeden Motor eine Instanz erzeugt wird (aktuell teste ich erstmal mit zwei Motoren). Und eine MotorController Klasse, die die unterschiede zwischen den Motoren überwacht. Der langsamste Motor ist immer der Master Motor, und die schnelleren Motoren müssen sich mittels PWM an den langsamsten Motor angleichen.
Da ich aber als Interrupt Rutine (für die Codier Scheibe jedes Motors) nur Statische Routinen verwenden kann, löse ich das setzen der Werte aus den Codier Scheiben per Statischen Arrays. Und jeder Motor hat eine Klassenvariable "id", die den Index enthält und im Konstruktor gesetzt wird. Das funktioniert auch zuerst doch dann plötzlich hat "id" auf einmal einen anderen Wert und ich verstehe nicht warum.

Auch wenn es jetzt etwas viel wird, habe ich der Vollständigkeit halber mal alle Sourcen gepostet, einfach zum besseren Verständnis. Und ich hoffe das einer eine Idee hat woran es liegen könnte.

Hier die Klasse Motor

Motor.h

#ifndef Motor_H
#define Motor_H

#include "Arduino.h"
#include <EEPROM.h>

#define UP    1
#define DOWN -1
#define OFF   0

#define EEPROM_OFF_ABS_MIN 0x00
#define EEPROM_OFF_ABS_MAX 0x04
#define EEPROM_OFF_MIN     0x08
#define EEPROM_OFF_MAX     0x0C
#define EEPROM_OFF_MASTER  0x10
#define EEPROM_OFF_POS     0x11


class Motor
{

  public:
    // Constructor
    Motor(uint8_t _outA, uint8_t _outB, uint8_t _sensorIn);

    uint32_t GetPosition();
    uint8_t  GetId();
    uint8_t  GetCurrentPwm();
    bool     IsMaster();
    void Init();
    void Calculate(uint32_t posOfSlowestMotor);
    void SetMinMaxPosition(uint32_t _min, uint32_t _max);
    void SetMaster(bool _master);
    void SetReverseDirection(bool _reverse);

    void Break();
    void Stop();
    void MoveUp();
    void MoveDown();



  private:
    uint8_t  id;
    bool reverseDirection;     // Flag if the direction reverse
    volatile bool master;      // Flagh if motor is Master
    uint8_t lastPwm;           // Last Pwm value;
    bool breakisOn;            // Flag if motor break is on
    uint32_t breakOnMillis;    // Millis on break start

    uint8_t  a;                // Motor Control A Pin
    uint8_t  b;                // Motor Control B Pin
    uint8_t  s;                // Sensor Pin

    uint32_t minPos;           // Saved minimal down position
    uint32_t maxPos;           // Saved maximal up position
    uint32_t absolutMinPos;    // Absolute minimal down position
    uint32_t absolutMaxPos;    // Absolute maximal up position
    uint32_t centerPos;        // Center position between absolut min and max


    static volatile uint32_t positions[4];
    static volatile int8_t direction[4];   // Direction Up or Down
    static uint8_t idCount;

    static void OnSensor1Changed();
    static void OnSensor2Changed();
    static void OnSensor3Changed();
    static void OnSensor4Changed();

    void MoveMaxPower();
    void MovePID(uint8_t pwm);
    bool CheckMinMaxLimits();
    void ReadFromEEProm();
    void WriteToEEProm();
};

#endif

Motor.cpp

#include "Arduino.h"
#include "Motor.h"

uint8_t Motor::idCount = 0;
volatile uint32_t Motor::positions[4];
volatile int8_t Motor::direction[4];

Motor::Motor(uint8_t _outA, uint8_t _outB, uint8_t _sensorIn)
{
  id = idCount;

  idCount++;

  positions[id] = 0;
  direction[id] = OFF;
  master = ((id == 0) ? true : false);
  breakisOn = false;
  a = _outA;
  b = _outB;
  s = _sensorIn; 

  lastPwm = 255;
}

uint32_t Motor::GetPosition()
{
  return positions[id];
}

uint8_t Motor::GetId()
{
  return id;
}

uint8_t Motor::GetCurrentPwm()
{
  return ((master) ? 255 : lastPwm);
}

bool Motor::IsMaster()
{
  return master;
}

void Motor::Init()
{
  Serial.print("Init motor with id ");
  Serial.println(id);

  ReadFromEEProm();
  
  pinMode(s, INPUT_PULLUP);
  pinMode(a, OUTPUT);
  pinMode(b, OUTPUT);

  digitalWrite(a, LOW);
  digitalWrite(b, LOW);
  
  switch(id)
  {
    case 0:
    {
      attachInterrupt(digitalPinToInterrupt(s), OnSensor1Changed, CHANGE);
      break;
    }
    case 1:
    {
      attachInterrupt(digitalPinToInterrupt(s), OnSensor2Changed, CHANGE);
      break;
    }
    case 2:
    {
      attachInterrupt(digitalPinToInterrupt(s), OnSensor3Changed, CHANGE);
      break;
    }
    case 3:
    {
      attachInterrupt(digitalPinToInterrupt(s), OnSensor4Changed, CHANGE);
      break;
    }
  }
}

void Motor::Calculate(uint32_t posOfSlowestMotor)
{
  
  if(direction[id] != OFF)
  {
    Serial.print("Calculate motor ");
    Serial.println(id);
    if(CheckMinMaxLimits())
    {
      return;
    }
    if(master)
    {
      MoveMaxPower();
    }
    else
    {
      uint8_t pwm = lastPwm;
      int32_t diff = ((direction[id] == UP) ? (positions[id] - posOfSlowestMotor) : (posOfSlowestMotor - positions[id]));
      
      if(diff > 0)
      {
        pwm = pwm - 1;
      }
      else if(diff < 0)
      {
        pwm = pwm + 1;
      }
      
      MovePID(pwm);

      lastPwm = pwm;
    }
  }
  else
  {
    if(breakisOn)
    {
      int32_t timeDiff = (millis() - breakOnMillis);
      if(timeDiff > 500)
      {
        Stop();
      }
    }
  }
}

void Motor::SetMinMaxPosition(uint32_t _min, uint32_t _max)
{
  minPos = _min;
  maxPos = _max;

  WriteToEEProm();
}

void Motor::SetMaster(bool _master)
{
  master = _master;

  if(master)
  {
    lastPwm = 255;
  }

  WriteToEEProm();
}

void Motor::SetReverseDirection(bool _reverse)
{
  reverseDirection = _reverse;
}

void Motor::Break()
{
  digitalWrite(a, HIGH);
  digitalWrite(b, HIGH);

  direction[id] = OFF;

  breakOnMillis = millis();
  breakisOn = true;
}

void Motor::Stop()
{
  digitalWrite(a, LOW);
  digitalWrite(b, LOW);

  direction[id] = OFF;

  breakisOn = false;
}

void Motor::MoveUp()
{
  direction[id] = UP;
}

void Motor::MoveDown()
{
  direction[id] = DOWN;
}

void Motor::MoveMaxPower()
{
  if(direction[id] == UP)
  {
    digitalWrite(a, ((reverseDirection) ? LOW : HIGH));
    digitalWrite(b, ((reverseDirection) ? HIGH : LOW));
  }
  else
  {
    digitalWrite(a, ((reverseDirection) ? HIGH : LOW));
    digitalWrite(b, ((reverseDirection) ? LOW : HIGH));
  }
}

void Motor::MovePID(uint8_t pwm)
{
  if(direction[id] == UP)
  {
    analogWrite(a, ((reverseDirection) ? 0 : pwm));
    analogWrite(b, ((reverseDirection) ? pwm : 0));
  }
  else
  {
    analogWrite(a, ((reverseDirection) ? pwm : 0));
    analogWrite(b, ((reverseDirection) ? 0 : pwm));
  }
}

bool Motor::CheckMinMaxLimits()
{
  if(direction[id] == UP)
  {
    if(positions[id] > max(maxPos, absolutMaxPos))
    {
      Break();
      return true;
    }
  }
  else
  {
    if(positions[id] < min(minPos, absolutMinPos))
    {
      Break();
      return true;
    }
  }
  return false;
}

void Motor::OnSensor1Changed()
{
  Serial.print("Sensor 0 changed ");
  Serial.println(positions[0]);
  if(direction[0] == UP)
  {
    positions[0] = positions[0] + 1;
  }
  else if(direction[0] == DOWN)
  {
    positions[0] = positions[0] - 1;
  }
}

void Motor::OnSensor2Changed()
{
  Serial.print("Sensor 1 changed ");
  Serial.println(positions[1]);
  if(direction[1] == UP)
  {
    positions[1] = positions[1] + 1;
  }
  else if(direction[1] == DOWN)
  {
    positions[1] = positions[1] - 1;
  }
}

void Motor::OnSensor3Changed()
{
  if(direction[2] == UP)
  {
    positions[2] = positions[2] + 1;
  }
  else if(direction[2] == DOWN)
  {
    positions[2] = positions[2] - 1;
  }
}

void Motor::OnSensor4Changed()
{
  if(direction[3] == UP)
  {
    positions[3] = positions[3] + 1;
  }
  else if(direction[3] == DOWN)
  {
    positions[3] = positions[3] - 1;
  }
}

void Motor::ReadFromEEProm()
{
  EEPROM.get(EEPROM_OFF_ABS_MIN, absolutMinPos);
  EEPROM.get(EEPROM_OFF_ABS_MAX, absolutMaxPos);
  EEPROM.get(EEPROM_OFF_MIN, minPos);
  EEPROM.get(EEPROM_OFF_MAX, maxPos);

  uint8_t masterId = 0;

  EEPROM.get(EEPROM_OFF_MASTER, masterId);

  master = (masterId == id);

  centerPos = ((absolutMaxPos - absolutMinPos) / 2);

  uint32_t pos = 0;

  EEPROM.get(EEPROM_OFF_POS + (id * 0x04), pos);

  positions[id] = pos;
}

void Motor::WriteToEEProm()
{
  EEPROM.put(EEPROM_OFF_ABS_MIN, absolutMinPos);
  EEPROM.put(EEPROM_OFF_ABS_MAX, absolutMaxPos);
  EEPROM.put(EEPROM_OFF_MIN, minPos);
  EEPROM.put(EEPROM_OFF_MAX, maxPos);

  if(master)
  {
    EEPROM.get(EEPROM_OFF_MASTER, id);
  }

  uint32_t pos = positions[id];

  EEPROM.put(EEPROM_OFF_POS + (id * 0x04), pos);
}

MotorController.h

#ifndef MotorController_H
#define MotorController_H

#include "Arduino.h"
#include "Motor.h"

class MotorController
{
  public:
    MotorController();

    void Init();
    void AddMotor(Motor* m, int stopSensorPin);
    void MoveUp();
    void MoveDown();
    void MoveTo(uint32_t pos);
    void Stop();

    void Calculate();

  private:
    Motor* motors[4];
    uint8_t count;
    uint8_t direction;
    uint8_t stopSensorPins[4];
    uint32_t stopPos;
    bool doStop;

    int GetMinMotorPos();
    int GetMaxMotorPos();
    void SetMasterMotor(int id);
};

#endif

MotorController.cpp

#include "Arduino.h"
#include "MotorController.h"

MotorController::MotorController()
{
  count = 0;
  direction = OFF;
  stopPos = 0;
  doStop = false;
}

void MotorController::AddMotor(Motor* m, int stopSensorPin)
{
  motors[count] = m;
  stopSensorPins[count] = stopSensorPin;

  count++;

}

void MotorController::Init()
{
  for(int i=0; i<count; i++)
  {
    pinMode(stopSensorPins[i], INPUT_PULLUP);
    motors[i]->Init();
  }

}

void MotorController::MoveUp()
{
  direction = UP;

  for(int i=0; i<count; i++)
  {
    motors[i]->MoveUp();
  }

  doStop = false;
}

void MotorController::MoveDown()
{
  direction = DOWN;

  for(int i=0; i<count; i++)
  {
    motors[i]->MoveDown();
  }

  doStop = false;
}

void MotorController::MoveTo(uint32_t pos)
{

}

void MotorController::Stop()
{
  if(direction != OFF)
  {
    int idOfFastestMotor = ((direction == UP) ? GetMaxMotorPos() : GetMinMotorPos());
    stopPos = motors[idOfFastestMotor]->GetPosition();
    doStop = true;
  }
}

void MotorController::Calculate()
{
  uint32_t maxPos = 0;
  int isStop = LOW;
  for(int i=0; i<count; i++)
  {
    isStop = (isStop | digitalRead(stopSensorPins[i]));
  }
  // Stop all motors if sensor detect colision
  if(isStop == HIGH)
  {
    for(int i=0; i<count; i++)
    {
      motors[i]->Break();
    }
  }

  if(direction != OFF)
  {
    int idOfSlowestMotor = ((direction == UP) ? GetMinMotorPos() : GetMaxMotorPos());
    int idOfFastestMotor = ((direction == UP) ? GetMaxMotorPos() : GetMinMotorPos());

    maxPos = motors[idOfFastestMotor]->GetPosition();
    
    if((!motors[idOfSlowestMotor]->IsMaster()) && motors[idOfSlowestMotor]->GetCurrentPwm() == 255)
    {
      SetMasterMotor(idOfSlowestMotor);
    }

    if(doStop)
    {
      for(int i=0; i<count; i++)
      {
        if(direction == UP)
        {
          if(motors[i]->GetPosition() >= stopPos)
          {
            motors[i]->Stop();
          }
        }
        else
        {
          if(motors[i]->GetPosition() <= stopPos)
          {
            motors[i]->Stop();
          }
        }
      }
    }
  }

  for(int i=0; i<count; i++)
  {
    motors[i]->Calculate(maxPos);
  }
}

int MotorController::GetMinMotorPos()
{
  uint32_t min = 0xFFFFFFFF;
  int motorId = 0;
  for(int i=0; i<count; i++)
  {
    if(motors[i]->GetPosition() < min)
    {
      min = motors[i]->GetPosition();
      motorId = i;
    }
  }
  return motorId;
}

int MotorController::GetMaxMotorPos()
{
  uint32_t max = 0x00;
  int motorId = 0;
  for(int i=0; i<count; i++)
  {
    if(motors[i]->GetPosition() > max)
    {
      max = motors[i]->GetPosition();
      motorId = i;
    }
  }
  return motorId;
}

void MotorController::SetMasterMotor(int id)
{
  for(int i=0; i<count; i++)
  {
    motors[i]->SetMaster((id == i));
  }
}

Schreibtisch.ino

#include "MotorController.h"
#include "Motor.h"


const int motorForwardR  = 10;
const int motorBackwardR =  9;
const int motorForwardL  =  5;
const int motorBackwardL =  6;
const int motorSensorR   =  3;
const int motorSensorL   =  2;
const int btnDown        =  0;
const int btnUp          =  1;
const int speedCtrl      = A0;

MotorController motorController;

Motor m1(motorForwardR, motorBackwardR, motorSensorR);
Motor m2(motorForwardL, motorBackwardL, motorSensorL);

bool motorUp = false;
bool motorDown = false;
 
void setup() 
{  
  Serial.begin(9600);
  while(!Serial);
  pinMode(speedCtrl, INPUT);
  pinMode(btnDown, INPUT_PULLUP);
  pinMode(btnUp, INPUT_PULLUP);

  m1.SetReverseDirection(false);
  m2.SetReverseDirection(true);

  motorController.AddMotor(&m1, 7);
  motorController.AddMotor(&m2, 8);

  motorController.Init();
}

void loop()
{
  int speedCtrlVal = analogRead(speedCtrl);
  
  int _up = digitalRead(btnUp);
  int _down = digitalRead(btnDown);

  if(_up == LOW)
  {
    motorController.MoveUp();
  }
  else if(_down == LOW)
  {
    motorController.MoveDown();
  }
  else
  {
    motorController.Stop();
  }

  motorController.Calculate();
  
  delay(5);
}

Hier die Ausgaben etwas gekürzt, wo aber zu sehen ist, das die Variable "id" plötzlich einen komplett anderen Wert hat. Und ich weiß nicht warum?

Init motor with id  0
Init motor with id  1
Sensor 1 changed 0
Sensor 0 changed 0
Sensor 1 changed 1
Sensor 0 changed 1
Sensor 0 changed 2
Sensor 1 changed 2
Sensor 0 changed 3
Sensor 1 changed 3
Sensor 0 changed 4
Sensor 1 changed 4
Sensor 0 changed 5
Sensor 1 changed 5
Sensor 0 changed 6
Sensor 1 changed 6
Sensor 0 changed 7
Sensor 1 changed 7
Sensor 0 changed 8
Sensor 1 changed 8
Sensor 0 changed 9
Sensor 0 changed 10
Sensor 1 changed 9
Sensor 0 changed 11
Sensor 1 changed 10
Calculate motor 69
Calculate motor 69
Calculate motor 69
Calculate motor 69
Calculate motor 69

[noiasca]

@jacoblange

wo aber zu sehen ist, das die Variable "id" plötzlich einen komplett anderen Wert hat. Und ich weiß nicht warum?

wo sieht man das? Ich seh da nur 0 und 1.
Oder meinst das Calculate motor?

Vermutlich überschreibst du dir irgendwelchen Speicher.

Ich meine in einer ISR hat ein Serial print nichts verloren.

Ich würde nicht die switch case Krücke im init machen sondern die entsprechende ISR dem Konstruktor übergeben.

Die statischen Arrays in der Klasse deuten für mich darauf hin, dass das Konzept nicht wirklich dafür geeignet ist. Das sieht für mich nicht gut aus.

Wenn du deine Dateien zusätzlich in ein Zip packst wäre das einfacher zum Nachvollziehen, so mit 8cm Scrollbalken macht das keinen Spaß.

P.S.: member functions auch camelCase mit Kleinbuchstaben beginnen lassen.

[combie]

Ich sehe da auch einige Probleme.

z.B.
Deine Klasse macht zuviel.
Steuert den Motor, macht Berechnungen, und wertet den Sensor aus.
Die 3 kann man gut einzeln bauen.

Dann schmecken mir die statischen Dinge nicht.
Die sind ein ernstes Problem bei der Vererbung und auch bei der Mehrfach Instanziierung

[jacoblange]

Danke für die schnellen Antworten. Bei den Statischen Arrays ist mir halt keine bessere Lösung eingefallen, da ich der ISR keine Klassenmethoden übergeben kann. Aber dann werde ich mal gucken das ich den Teil, der die Sensoren ausließt, komplett aus den Klassen raus nehme.

Hier auch nochmal zur besseren Lesbarkeit der Quelltext als ZIP:

Schreibtisch.zip (4,1 KB)

[noiasca]

die ISR braucht ja nur eine entsprechende Klassen-Funktion triggern ...
setter für up und down ... dann hast deine Variable wieder in deiner Klasse (wenn du sie dort haben willst)

[combie]

Leider verträgt sich eine konstante Interruptsprungleiste im Flash nicht so dolle gut mit flexibler Instanziierung von Klassen.

Zudem kann attachInterrupt() nicht jede Sorte Lambdafunktionen abhandeln. Was wohl an der eher primitiven Implementierung liegt. Und diese liegt am ehesten an der fehlenden Unterstützung durch die libstdc++ bei den AVRs

Also, leicht ist das Thema nicht!
Und (unschöne?) Umwege sind nötig.

#include <util/atomic.h> 
#define CriticalSection ATOMIC_BLOCK(ATOMIC_RESTORESTATE)

#include <Streaming.h> // die Lib findest du selber ;-)
Print &cout = Serial; // cout Emulation für "Arme"

constexpr byte isrPin {2};

class Test
{
  private:
    uint32_t count;

  public:
    Test(): count{0} {}
    operator uint32_t() const
    {
      uint32_t retVal;
      CriticalSection retVal = count;
      return retVal;
    }
    uint32_t operator ++() // prefix increment operator
    {
      return ++count;
    }
    uint32_t operator ++(int) // postfix increment operator
    {
      return count++;
    }
};

Test test;

void setup()
{
  Serial.begin(9600);
  cout << F("Start: ") << F(__FILE__) << endl;
  
  pinMode(isrPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(isrPin), [](){++test;}, CHANGE);
  //attachInterrupt(digitalPinToInterrupt(isrPin), [](){test++;}, CHANGE);
}

void loop()
{
  delay(1000);
  cout << test << endl;
}

[Tommy56]

@combie

uint32_t operator ++() // prefix decrement operator

Eher increment operator?

Gruß Tommy

[combie]

@Tommy56

Eher

Jau!
Ein einzelner Tippfehler kann ganze Sinnzusammenhänge urinieren.

[jacoblange]

Vielen Dank für guten Hinweise. Hier jetzt ein Update von mir, das seltsame Verhalten habe ich selber verursacht:

void Motor::WriteToEEProm()
{
  EEPROM.put(EEPROM_OFF_ABS_MIN, absolutMinPos);
  EEPROM.put(EEPROM_OFF_ABS_MAX, absolutMaxPos);
  EEPROM.put(EEPROM_OFF_MIN, minPos);
  EEPROM.put(EEPROM_OFF_MAX, maxPos);

  if(master)
  {
    // Hier stand ein EEPROM.get, was natürlich unsinnig war.
    EEPROM.put(EEPROM_OFF_MASTER, id);
  }

  uint32_t pos = position;

  EEPROM.put(EEPROM_OFF_POS + (id * 0x04), pos);
}

Zusätzlich habe ich jetzt aber die ISR aus den Klassen raus genommen, so das es jetzt so aussieht:

void setup() 
{  
  ...
  attachInterrupt(digitalPinToInterrupt(m1.GetSensorPin()), OnSensor1Changed, CHANGE);
  ...
}

void OnSensor1Changed()
{
  m1.ChangePosition();
}

Was auch den Vorteil hat, das ich keine statischen Variablen mehr in der Klasse benötige.

Nur zur Info noch hier die ChangePosition() Funktion:

void Motor::ChangePosition()
{
  if(direction == UP && position < 0xFFFFFFFF)
  {
    position++;
  }
  else if(direction == DOWN && position > 0)
  {
    position--;
  }
}

Und jetzt funktioniert auch alles so wie es sein soll.

[combie]

@jacoblange

  EEPROM.put(EEPROM_OFF_ABS_MIN, absolutMinPos);
  EEPROM.put(EEPROM_OFF_ABS_MAX, absolutMaxPos);
  EEPROM.put(EEPROM_OFF_MIN, minPos);
  EEPROM.put(EEPROM_OFF_MAX, maxPos);

Hier möchte ich dir raten, dem Compiler die Adressberechnung zu überlassen.

[Tutorial] Umgang mit dem AVR EEPROM

Umgang mit dem AVR EEPROM in der Arduino Umgebung. (meine Art damit umzugehen) Gerne wird das EEPROM für irgendwelche Konfigurationswerte verwendet, welche man nicht im Flash unterbringen möchte. Und genau darum soll es hier gehen. Doku zur Library: EEPROM Bitte jedes einzelne Kapitel sorgfältig lesen, sowohl die Beispiele, als auch die Funktionsbeschreibungen. Ich werde es nicht hier ins deutsche übersetzen. Auch nicht weiter erläutern. Aber darauf aufbauen. Den Fokus möchte ich hier au…