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);
}

• chrisl: Is there a reason, why you are defining the MPU9250_DMP outside of your class and in your main sketch? If you declare it inside of the class as a member variable you can ditch the reference handling
• Mikael Patel: Try passing the imu parameter by-reference (instead of by-copy), i.e. MPU9250_DMP& imu, to the SetupSensors and ReadSensors member functions.
• Hamish_Fernsby: @ChrisL When it was defined just in CDSensors class ithe sensor returned all zeros. I checked with local serialprint, I suppose somehow the connection between settings and reading data is lost.
• Hamish_Fernsby: @Nisse sorry tidied indents & typo, resolved your comment I think.
• chrisl: How die you define it in the class? Did you execute its constructor? If not, that might have been the problem

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);

• Hamish_Fernsby: thanks Nick, I tried this variation, it also compiles ok, but still have the same problem with the accl sub-sensor returning zeros
• Nick Gammon: Add some debugging prints, don't thrash around adding pointers in the hope it will fix things.
• Nick Gammon: Also see my amended answer.
• Hamish_Fernsby: Re if (imu.begin() != INV_SUCCESS) sorryyyyyyy typo in code at the top, which is now corrected.
• Hamish_Fernsby: I wondered if it might be to do with constructors, but if I understood correctly the class is created on startup with CDSensors* LSensors = &CDSensors (0); and is then persistent. (unlike what i'm used to in delphi where you can type create/destroy all over the place).