Declaring as a pointer crashes loop after several seconds [solved]

This works:

void loop() {
  if (can.available()) {
    Frame frame;
    can.receive(&frame);
    Serial.println(millis());
  }
  delay(10);
}

This doesn't:

void loop() {
  if (can.available()) {
    Frame *frame = new Frame();
    can.receive(frame);
    Serial.println(millis());
  }
  delay(10);
}

It does compile and runs for about 10 seconds, but then the Arduino freezes / crashes and cannot be reprogrammed without double clicking the reset button. I'm using an MKR 1310, SAMD Architecture. Does anyone have an idea as to why? Is my memory running full when using pointers maybe? Or is this a typical memory leak? How do I free the memory after using the variable in that case? Using the delete keyword?

running out of memory due to memory leak most likely. read about ‘new’ and ‘delete’

Yep using the delete keyword does keep working. Funny that defining it not using a pointer the object somehow get's deleted by itself then.

And I'm reading dynamic allocation of memory like this is frowned upon with microcontrollers because something called "memory fragmantation" can happen rather quickly with little memory and without an operating system.

So back to declaring global variables.

Thanks!

When you declared frame as a local, it is on the stack and goes away when the loop function ends. You need not declare it global as long as you use it in (or pass it from) loop.

Fragmentation is a thing, but it has nothing to do with the lack of O/S. It happens very easily when you allocate memory on a device with little RAM, like some Arduinos.

1 Like

Ah yes that makes sense! So locals are not bad. And in some cases, when making pointers to new instances of classes, just don't forget to delete them after usage. Then it's okay? Or local preferred and pass their address instead?

If you allocate memory, you risk fragmentation so locals are safer. There are some data structures that naturally lend themselves to dynamic allocation, but unless you actually have to do so, it's best avoided.

The really nasty thing about allocation is that it makes testing harder - it's hard to be sure that your code will not fragment memory under some circumstances, so you may find for example that your code ran fine for weeks but then mysteriously crashed.

Do you really need a "fresh" Frame object on every iteration of loop()? How expensive is that class's constructor? Why not use a static local?

Thanks, I'll heed that advice. And this might be the answer to some unexpected behaviour I've seen indeed.

About the need for a fresh frame every time: Yeah good question. The class is just a data container of maybe 12 bytes. What I understand though is that assigning it dynamic is faster, because the memory controller can just assign the address where it currently is. If you assign it as a static it first has to move to the location that was originally assigned. Or is this also true for a normal local variable as it is located in the stack?

None of that is true.

1 Like

Please don't be offended, but you should work through a C++ tutorial.

Okay, please explain then how it works, the benefits of each of the following and which one to use best in this basic scenario:

  1. Dynamic allocation using 'new' and 'delete'
  2. Local instance
  3. Static local instance

Can you recommend a good tutorial?

No.

I learned C++ when it was invented, from Bjarne Stroustrup's Book,
long before free tutorials were a concept.

To look up unknown newer functions or to clarify things, I use cppreference.com.

For now I'll go with Jessie Zhangs explanation:
Static Class:

You cannot create the instance of static class.

Loaded automatically by the .NET Framework common language runtime (CLR) when the program or namespace containing the class is loaded.

We cannot pass the static class to method.

We cannot inherit Static class to another Static class in C#.

A class having all static methods.

Better performance (static methods are bonded on compile time)

Singleton:

You can create one instance of the object and reuse it.

Singleton instance is created for the first time when the user requested.

Singleton class can have constructor.

You can create the object of singleton class and pass it to method.

Singleton class does not say any restriction of Inheritance.

We can dispose the objects of a singleton class but not of static class.

Methods can be overridden.

Can be lazy loaded when need (static classes are always loaded).

We can implement interface(static class can not implement interface).

Source: Singleton instance vs Static in terms of Memory Management - Microsoft Q&A

I don't think the C# .NET view will help you really in understanding C++.

O help, no indeed :man_facepalming:

Global and static (both local and file-scope) variables are assigned storage locations at build-time by the linker. It's typically in the lower part of the memory map.

Non-static local variables exist on the stack and are created when encountered for the first time on every iteration of the function. This technique allows your functions to support recursion and reentrancy.

Creating plain variable types on the stack is very quick since it simply involves applying offsets to the Stack Pointer. However, creating objects could be more expensive if their class's constructor is time-consuming. That's why I asked about the "Frame" class since you didn't show us how it's defined (another reason you should always post full code).

Objects created dynamically ("new" operator) go on the heap. Creating them involves finding and allocating the heap space as well as doing the bookkeeping. And, of course, the (possibly expensive) constructor must be executed. This all happens at run-time whenever the object is created.

Once created, access all of the above variable types is efficient enough that you shouldn't concern yourself with which is faster.

1 Like

Thank you very much, I appreciate the time you took to pen this down. This helps a lot to understand the impact of how to "create" class instances. Sometimes new needs to be used to have control over the order of calling the constructors. But we have to be careful when using it in loops.

So for this Frame case, since there's insignificant impact on accessing a static class instance, and to use as little as memory possible, is it an idea to create a static instance of frame in a base class? Each class that needs it (always temporary) can inherit it:

CanbusDevice.h

#pragma once

#include <Arduino.h>
#include <Frame.h>

class CanbusDevice {
  public:
    CanbusDevice(int nodeId);
    
    static Frame frame;
    
    int _nodeId = -1;
};

CanbusDevice.cpp

#include "CanbusDevice.h"

Frame CanbusDevice::frame;

CanbusDevice::CanbusDevice(int nodeId) {
  _nodeId = nodeId;
}

Motor.h

#pragma once

#include "CanbusDevice.h"

class Motor : public CanbusDevice {
	public:
		Motor(uint8_t nodeId);
}

Motor.cpp

#include "Motor.h"

Motor::Motor(uint8_t nodeId) : CanbusDevice(nodeId) {
	Serial.print("Motor ");
	Serial.print(nodeId);
	Serial.println(" constructed");
}

Finally up, the secret Frame is in from CanBusData_asukiaaa (Thanks Asukia, beautiful lightweight library).

CanBusData_asukiaaa.h

#pragma once

#include <Arduino.h>
namespace CanBusData_asukiaaa {

class Frame {
 public:
  uint32_t id = 0;
  bool ext = false;
  bool rtr = false;  // false -> data frame, true -> remote frame
  uint8_t len = 8;
  union {
    uint64_t data64;     // Caution: subject to endianness
    uint32_t data32[2];  // Caution: subject to endianness
    uint16_t data16[4];  // Caution: subject to endianness
    float dataFloat[2];  // Caution: subject to endianness
    uint8_t data[8] = {0, 0, 0, 0, 0, 0, 0, 0};
  };

  String toString() const;
};

And calling it from the main sketch:

#include "Motor.h"

Motor *motorLeft;
begin() {
	Serial.begin();
	delay(2000);
	motorLeft = new Motor(1);
}

loop() {
	motorLeft.setSpeed(100); // todo: define in Motor.h
}

What evidence do you have to support that assertion?

1 Like