Skip to main content
Added more explanations.
Source Link
Nick Gammon
  • 38.9k
  • 13
  • 70
  • 126

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

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);
Added more explanations.
Source Link
Nick Gammon
  • 38.9k
  • 13
  • 70
  • 126

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.

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.

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.

Source Link
Nick Gammon
  • 38.9k
  • 13
  • 70
  • 126

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.