Skip to main content
Added codefence (and syntax highlighting) and backticked functions
Source Link
// main code, pass a pointer rather than the object itself
Arm Arm1(ShoulderUP, ShoulderDW, ElbowUP, ElbowDW, &ElbowEncoder, &ShoulderEncoder);

// header
public:
  Arm(int ShoulderUpPin, int ShoulderDownPin, int ElbowUpPin, int ElbowDownPin, Encoder *ElbowEncoder, Encoder *ShoulderEncoder);


// cpp
Arm::Arm(int ShoulderUpPin, int ShoulderDownPin, int ElbowUpPin, int ElbowDownPin, Encoder *ElbowEncoder, Encoder *ShoulderEncoder) {

  pinMode(ShoulderUpPin, OUTPUT);
  pinMode(ShoulderDownPin, OUTPUT);
  pinMode(ElbowUpPin, OUTPUT);
  pinMode(ElbowDownPin, OUTPUT);

  _ShoulderUpPin = ShoulderUpPin;
  _ShoulderDownPin = ShoulderDownPin;
  _ElbowUpPin = ElbowUpPin;
  _ElbowDownPin = ElbowDownPin;
  this.ElbowEncoder = ElbowEncoder; // use 'this.' to indicate the class private variable since it's the same name as the passed parameter.
  this.ShoulderEncoder = ShoulderEncoder;
}
    // main code, pass a pointer rather than the object itself
    Arm Arm1(ShoulderUP, ShoulderDW, ElbowUP, ElbowDW, &ElbowEncoder, &ShoulderEncoder);

    // header
    public:
      Arm(int ShoulderUpPin, int ShoulderDownPin, int ElbowUpPin, int ElbowDownPin, Encoder *ElbowEncoder, Encoder *ShoulderEncoder);


    // cpp
    Arm::Arm(int ShoulderUpPin, int ShoulderDownPin, int ElbowUpPin, int ElbowDownPin, Encoder *ElbowEncoder, Encoder *ShoulderEncoder) {

      pinMode(ShoulderUpPin, OUTPUT);
      pinMode(ShoulderDownPin, OUTPUT);
      pinMode(ElbowUpPin, OUTPUT);
      pinMode(ElbowDownPin, OUTPUT);

      _ShoulderUpPin = ShoulderUpPin;
      _ShoulderDownPin = ShoulderDownPin;
      _ElbowUpPin = ElbowUpPin;
      _ElbowDownPin = ElbowDownPin;
      this.ElbowEncoder = ElbowEncoder; // use 'this.' to indicate the class private variable since it's the same name as the passed parameter.
      this.ShoulderEncoder = ShoulderEncoder;
    }

And then within your class, since ElbowEncoder is a pointer rather than the encoder itself, use ElbowEncoder->method()ElbowEncoder->method() rather than ElbowEncoder.method()ElbowEncoder.method()

Personally, I'd re-structurerestructure it a little. Why create the encoders at the top level when they are only used within the arm class? Create them in the arm class inseadinstead:

//in .h
public:
  Arm(int ShoulderUpPin, int ShoulderDownPin, int ShoulderEncoderPinA,int ShoulderEncoderPinB,
      int ElbowUpPin, int ElbowDownPin, int ElbowEncoderPinA, int ElbowEncoderPinB);

private:
  Encoder ShoulderEncoder;
  Encoder ElbowEncoder;


// in .cpp
Arm::Arm(int ShoulderUpPin, int ShoulderDownPin, int ShoulderEncoderPinA,int ShoulderEncoderPinB,
         int ElbowUpPin, int ElbowDownPin, int ElbowEncoderPinA, int ElbowEncoderPinB) :
             ShoulderEncoder(ShoulderEncoderPinA,ShoulderEncoderPinB),
             ElbowEncoder(ElbowEncoderPinA,ElbowEncoderPinB) {
   // constructor code
}
    //in .h
    public:
      Arm(int ShoulderUpPin, int ShoulderDownPin, int ShoulderEncoderPinA,int ShoulderEncoderPinB,
          int ElbowUpPin, int ElbowDownPin, int ElbowEncoderPinA, int ElbowEncoderPinB);

    private:
      Encoder ShoulderEncoder;
      Encoder ElbowEncoder;


    // in .cpp
    Arm::Arm(int ShoulderUpPin, int ShoulderDownPin, int ShoulderEncoderPinA,int ShoulderEncoderPinB,
             int ElbowUpPin, int ElbowDownPin, int ElbowEncoderPinA, int ElbowEncoderPinB) :
                 ShoulderEncoder(ShoulderEncoderPinA,ShoulderEncoderPinB),
                 ElbowEncoder(ElbowEncoderPinA,ElbowEncoderPinB) {
       // constructor code
    }
// main code, pass a pointer rather than the object itself
Arm Arm1(ShoulderUP, ShoulderDW, ElbowUP, ElbowDW, &ElbowEncoder, &ShoulderEncoder);

// header
public:
  Arm(int ShoulderUpPin, int ShoulderDownPin, int ElbowUpPin, int ElbowDownPin, Encoder *ElbowEncoder, Encoder *ShoulderEncoder);


// cpp
Arm::Arm(int ShoulderUpPin, int ShoulderDownPin, int ElbowUpPin, int ElbowDownPin, Encoder *ElbowEncoder, Encoder *ShoulderEncoder) {

  pinMode(ShoulderUpPin, OUTPUT);
  pinMode(ShoulderDownPin, OUTPUT);
  pinMode(ElbowUpPin, OUTPUT);
  pinMode(ElbowDownPin, OUTPUT);

  _ShoulderUpPin = ShoulderUpPin;
  _ShoulderDownPin = ShoulderDownPin;
  _ElbowUpPin = ElbowUpPin;
  _ElbowDownPin = ElbowDownPin;
  this.ElbowEncoder = ElbowEncoder; // use 'this.' to indicate the class private variable since it's the same name as the passed parameter.
  this.ShoulderEncoder = ShoulderEncoder;
}

And then within your class since ElbowEncoder is a pointer rather than the encoder itself use ElbowEncoder->method() rather than ElbowEncoder.method()

Personally I'd re-structure it a little. Why create the encoders at the top level when they are only used within the arm class? Create them in the arm class insead:

//in .h
public:
  Arm(int ShoulderUpPin, int ShoulderDownPin, int ShoulderEncoderPinA,int ShoulderEncoderPinB,
      int ElbowUpPin, int ElbowDownPin, int ElbowEncoderPinA, int ElbowEncoderPinB);

private:
  Encoder ShoulderEncoder;
  Encoder ElbowEncoder;


// in .cpp
Arm::Arm(int ShoulderUpPin, int ShoulderDownPin, int ShoulderEncoderPinA,int ShoulderEncoderPinB,
         int ElbowUpPin, int ElbowDownPin, int ElbowEncoderPinA, int ElbowEncoderPinB) :
             ShoulderEncoder(ShoulderEncoderPinA,ShoulderEncoderPinB),
             ElbowEncoder(ElbowEncoderPinA,ElbowEncoderPinB) {
   // constructor code
}
    // main code, pass a pointer rather than the object itself
    Arm Arm1(ShoulderUP, ShoulderDW, ElbowUP, ElbowDW, &ElbowEncoder, &ShoulderEncoder);

    // header
    public:
      Arm(int ShoulderUpPin, int ShoulderDownPin, int ElbowUpPin, int ElbowDownPin, Encoder *ElbowEncoder, Encoder *ShoulderEncoder);


    // cpp
    Arm::Arm(int ShoulderUpPin, int ShoulderDownPin, int ElbowUpPin, int ElbowDownPin, Encoder *ElbowEncoder, Encoder *ShoulderEncoder) {

      pinMode(ShoulderUpPin, OUTPUT);
      pinMode(ShoulderDownPin, OUTPUT);
      pinMode(ElbowUpPin, OUTPUT);
      pinMode(ElbowDownPin, OUTPUT);

      _ShoulderUpPin = ShoulderUpPin;
      _ShoulderDownPin = ShoulderDownPin;
      _ElbowUpPin = ElbowUpPin;
      _ElbowDownPin = ElbowDownPin;
      this.ElbowEncoder = ElbowEncoder; // use 'this.' to indicate the class private variable since it's the same name as the passed parameter.
      this.ShoulderEncoder = ShoulderEncoder;
    }

And then within your class, since ElbowEncoder is a pointer rather than the encoder itself, use ElbowEncoder->method() rather than ElbowEncoder.method()

Personally, I'd restructure it a little. Why create the encoders at the top level when they are only used within the arm class? Create them in the arm class instead:

    //in .h
    public:
      Arm(int ShoulderUpPin, int ShoulderDownPin, int ShoulderEncoderPinA,int ShoulderEncoderPinB,
          int ElbowUpPin, int ElbowDownPin, int ElbowEncoderPinA, int ElbowEncoderPinB);

    private:
      Encoder ShoulderEncoder;
      Encoder ElbowEncoder;


    // in .cpp
    Arm::Arm(int ShoulderUpPin, int ShoulderDownPin, int ShoulderEncoderPinA,int ShoulderEncoderPinB,
             int ElbowUpPin, int ElbowDownPin, int ElbowEncoderPinA, int ElbowEncoderPinB) :
                 ShoulderEncoder(ShoulderEncoderPinA,ShoulderEncoderPinB),
                 ElbowEncoder(ElbowEncoderPinA,ElbowEncoderPinB) {
       // constructor code
    }
Source Link
Andrew
  • 1.1k
  • 5
  • 8

You're jumping backwards and forwards between pointers and the objects themselves. Change it to pass pointers to the arm constructor and consistently use pointers (or pass a reference and always use that, just be consistent). This would be something like:

// main code, pass a pointer rather than the object itself
Arm Arm1(ShoulderUP, ShoulderDW, ElbowUP, ElbowDW, &ElbowEncoder, &ShoulderEncoder);

// header
public:
  Arm(int ShoulderUpPin, int ShoulderDownPin, int ElbowUpPin, int ElbowDownPin, Encoder *ElbowEncoder, Encoder *ShoulderEncoder);


// cpp
Arm::Arm(int ShoulderUpPin, int ShoulderDownPin, int ElbowUpPin, int ElbowDownPin, Encoder *ElbowEncoder, Encoder *ShoulderEncoder) {

  pinMode(ShoulderUpPin, OUTPUT);
  pinMode(ShoulderDownPin, OUTPUT);
  pinMode(ElbowUpPin, OUTPUT);
  pinMode(ElbowDownPin, OUTPUT);

  _ShoulderUpPin = ShoulderUpPin;
  _ShoulderDownPin = ShoulderDownPin;
  _ElbowUpPin = ElbowUpPin;
  _ElbowDownPin = ElbowDownPin;
  this.ElbowEncoder = ElbowEncoder; // use 'this.' to indicate the class private variable since it's the same name as the passed parameter.
  this.ShoulderEncoder = ShoulderEncoder;
}

And then within your class since ElbowEncoder is a pointer rather than the encoder itself use ElbowEncoder->method() rather than ElbowEncoder.method()

Personally I'd re-structure it a little. Why create the encoders at the top level when they are only used within the arm class? Create them in the arm class insead:

//in .h
public:
  Arm(int ShoulderUpPin, int ShoulderDownPin, int ShoulderEncoderPinA,int ShoulderEncoderPinB,
      int ElbowUpPin, int ElbowDownPin, int ElbowEncoderPinA, int ElbowEncoderPinB);

private:
  Encoder ShoulderEncoder;
  Encoder ElbowEncoder;


// in .cpp
Arm::Arm(int ShoulderUpPin, int ShoulderDownPin, int ShoulderEncoderPinA,int ShoulderEncoderPinB,
         int ElbowUpPin, int ElbowDownPin, int ElbowEncoderPinA, int ElbowEncoderPinB) :
             ShoulderEncoder(ShoulderEncoderPinA,ShoulderEncoderPinB),
             ElbowEncoder(ElbowEncoderPinA,ElbowEncoderPinB) {
   // constructor code
}

This will create the encoders as private to the arm as soon as you call the arm constructor. Especially handy if you want to have more than one arm.