Passing reference to hardware between methods in custom class

Question

I'm a C++ / Arduino newbie, trying to build a custom library for the first time. I created a unit called CDSensors.cpp, which has two methods, SetupSensor and ReadSensors. The sensor is a SparkFun accelerometer/magnetometer.

If there are any errors in the settings, the sensor returns all zeros for compass and accelerometer. If there is no pointer or other connection between the SetupSensor and ReadSensor methods, the sensor returns all zeros as if it hasn't been setup.

So I'm trying to pass a reference to the sensor library between the methods as shown with the Limu object in the code below. I now get mag data but the accelerometer returns all zeros so something in my object references is messing with the sensor setup? (the accelerometer works ok in the SparkFun demo code, on which my code is based).

Main.ino

#include <CDSensors.h>
#include <Wire.h>
#include <SPI.h>
#include <SparkFunMPU9250-DMP.h>

CDSensors* LSensors = &CDSensors     (0);
CDSerialPrint* LSerialPrint = &CDSerialPrint (0);
MPU9250_DMP Limu = MPU9250_DMP ();
unsigned long LTimeNow = 0;

struct meas {
  int BUx;
  int acc_x
};

struct meas r;

void setup() {
  LSensors->SetupSensors (Limu);
}

void loop() {
  LSensors->ReadSensors  (Limu, LTimeNow, r.BUx, r.acc_x);
  Serial.print((int) (r.BUx) );
  Serial.print((int) (r.acc_x) );
}

CDSensors.h

#ifndef CDSensors_h_h
#define CDSensors_h
#include <Wire.h>
#include <SPI.h>
#include <SparkFunMPU9250-DMP.h>

class CDSensors {
  public:
    CDSensors(int);
    static void SetupSensors(MPU9250_DMP *imu);
    void ReadSensors (MPU9250_DMP *imu, unsigned long& TimeNow, int&     BUx,int& BUy,int& BUz,int& acc_x,int& acc_y,int& acc_z,int& a0,int& a1,int& a2,int& a3 );
  private:
    int value;
};
#endif

CDSensors.cpp

#include <Wire.h>
#include <SPI.h>
#include <SparkFunMPU9250-DMP.h>
#include "CDSensors.h"

//MPU9250_DMP imu;
boolean       MPU9250_found = false;
unsigned long TimeNow = 0;
CDSensors::CDSensors(int givenValue) {
  value = givenValue;
}

static void CDSensors::SetupSensors(MPU9250_DMP imu) {
  if (imu.begin() != INV_SUCCESS) {
    while (1) {
      Serial.println("No MPU-9250 found, Check connections");
      delay(5000);
    }
  }
  imu.setSensors(INV_XYZ_ACCEL | INV_XYZ_GYRO | INV_XYZ_COMPASS);
  imu.setGyroFSR(2000);
  imu.setAccelFSR(2);
  imu.setLPF(188);
  imu.setSampleRate(100);
  imu.setCompassSampleRate(100);
}

void CDSensors::ReadSensors(MPU9250_DMP imu, unsigned long& TimeNow, int& BUx, int& acc_x) {
  TimeNow = millis();
  if (imu.dataReady()) {
    imu.update(UPDATE_ACCEL | UPDATE_GYRO | UPDATE_COMPASS);
  }
  BUx = imu.calcMag(imu.mx);
  acc_x = imu.calcAccel(imu.ax);
}

Answer 1

I don't think you are creating instances of these classes correctly. Instead of:

CDSensors*     LSensors      = &CDSensors     (0);
CDSerialPrint* LSerialPrint  = &CDSerialPrint (0);            
MPU9250_DMP   Limu = MPU9250_DMP ();

How about:

CDSensors LSensors (0);
CDSerialPrint LSerialPrint (0);
MPU9250_DMP Limu;

I can't get your code to compile, but what I suggest looks a lot simpler.

---

Is your intention here:

if (imu.begin() != INV_SUCCESS)
  imu.setSensors(INV_XYZ_ACCEL | INV_XYZ_GYRO | INV_XYZ_COMPASS); 
  imu.setGyroFSR(2000); 
  imu.setAccelFSR(2); 
  imu.setLPF(188); 
  imu.setSampleRate(100); 
  imu.setCompassSampleRate(100); 

.. to really be:

if (imu.begin() != INV_SUCCESS)
  {
  imu.setSensors(INV_XYZ_ACCEL | INV_XYZ_GYRO | INV_XYZ_COMPASS); 
  imu.setGyroFSR(2000); 
  imu.setAccelFSR(2); 
  imu.setLPF(188); 
  imu.setSampleRate(100); 
  imu.setCompassSampleRate(100);
  }

The indentation suggests so.

---

if I understood correctly the class is created on startup with

       CDSensors* LSensors = &CDSensors (0); 

and is then persistent.

LSensors is a persistent pointer but CDSensors (0) is a temporary object created for the purpose of doing an assignment. For example:

int foo = 2 + 3;

Now the compiler calculates 2 + 3 (giving 5), assigns that to foo, and then discards the temporarily-calculated 5 used to do the assignment.

In your case CDSensors (0) calls the constructor for CDSensors, creating a temporary object. Then the "&" operator takes its address (somewhere on the stack I would guess). This address is copied into LSensors. Then the object is destroyed because its lifetime is over. The compiler even warns you this is happening:

 CDSensors*     LSensors      = &CDSensors     (0);
                                                 ^
sketch_may07a:5: error: taking address of temporary [-fpermissive]

That means the pointer is now invalid because the underlying object (that it is pointing to) has now been discarded.

---

I've even seen people do something like this quite a bit:

     CDSensors LSensors = CDSensors (0);

This isn't so bad because pointers aren't involved, but it still involves making a temporary instance of the class, then copying that into LSensors, and discarding the original one.

This is how to make a new instance of a class:

 CDSensors LSensors (0);   // the actual class, not a pointer

Or if you want a pointer:

 CDSensors * pLSensors;  // a pointer to the class (initially NULL)

Then in setup you create the instance:

pLSensors = new CDSensors (0);