0

I'm fairly new to C++, this is also my first post on here. I'm trying to use C++ in an embedded systems project so I can take the OOP approach. I'm using the AVR crosspack toolchain (AVR G++ compiler)

My problem is this:

From what i've read, the heap should not be used for dynamic memory allocation in embedded systems. In any case, there is no implementation for "new" in AVR G++ anyway. I'm using composition, starting with a USART driver (lets call it a service), and a logger (singleton pattern, and also a service).

It's my understanding that services should have their dependancies passed in on instantiation using constructor parameters, however when I try to compose the objects needed in this way I get the following error:

Main/main.cpp: In function 'int main()':
Main/main.cpp:21:13: error: request for member 'log' in 'logSystem', which is of non-class type 'LogSystem(Usart)'
   21 |   logSystem.log("Hello");
      |             ^~~
make: *** [Main/main.o] Error 1

My sense is that my syntax for passing in an object as a constructor parameter is wrong, but I'm not sure what it should be as all the examples i can find use the "new" keyword in the constructor definition to create the object on the free store. Can anyone help?

The Code:

In "usart.h":

#include <avr/io.h>
#include <util/setbaud.h>

class Usart
{
public:
  // Constructor and destructor
  Usart();
  ~Usart();

  // Initialisation routine
  static void const init(void);

  // Utility function to transmit a string
  static void const print(const char myString[]);
};

In "logger.h":

#include "usart.h"

class LogSystem
{
public:

  LogSystem(Usart usart);
  ~LogSystem();

  Usart usart;

  static void const log(char *msg);

};

In "logger.cpp"

#include "logger.h"

LogSystem::LogSystem(Usart usart)
{
  Usart usart;
  usart.init();
}

LogSystem::~LogSystem()
{
}

LogSystem::log(char *msg)
{
  usart.print(msg);
}

In "main.cpp":

#include "logger.h"


int main()
{
    LogSystem logSystem(Usart usart);

    while(1)
    {
        logSystem.log("Hello");
    }

    return 0;
}
KamilCuk
  • 120,984
  • 8
  • 59
  • 111
Jason
  • 19
  • 4
  • 1
    You have a bunch of extra uses of the class name `Usart` littered around the code. `LogSystem logSystem(Usart usart);` doesn't create an instance of a `LogSystem` object named `logSystem`, it declares a _function_ named `logSystem` that takes a single `Usart` instance as its argument and returns a `LogSystem`. In your `LogSystem` constructor, the line `Usart usart;` is declaring a variable local to the constructor, shadowing both the `usart` parameter _and_ the `LogSystem::usart` member variable. – Nathan Pierson Oct 16 '21 at 17:07
  • Thanks for your comment @NathanPierson - I see what you are saying, it felt to me like that's what was wrong, however I'm still unclear as to how these instantiations should be carried out correctly, could you elaborate for me please? The part I don't understand is what should I be doing **instead** of: `new Usart = usart;` inside the constructor. – Jason Oct 16 '21 at 17:44
  • 1
    You probably want to look at [initializer lists](https://stackoverflow.com/questions/1711990/what-is-this-weird-colon-member-syntax-in-the-constructor) to see how to initialize values. You'd also pass a specific `Usart` instance to the constructor. However as currently designed I'm not sure what exactly it should be doing because `Usart` only has static methods anyway so I'm not sure exactly how it should be composed into `LogSystem`. – Nathan Pierson Oct 16 '21 at 17:49
  • There are some basic errors in your code. Would be great to review some early chapters of some C++ book, on how to define a class and member functions. – KamilCuk Oct 17 '21 at 13:24
  • Thanks for that @NathanPierson - You're right, I had the wrong idea about static declarations to begin with. – Jason Oct 18 '21 at 18:03

1 Answers1

3

[...] the heap should not be used for dynamic memory allocation in embedded systems.

It depends. I'm currently in an embedded project with maximum safety-related requirements, and we use new, but not delete. So we have a heap, but don't allocate "dynamically", because all allocated objects are kept during the runtime.

In any case, there is no implementation for "new" in AVR G++ anyway.

Is this true? I never checked... It might be necessary to provide a heap before being able to use new.

It's my understanding that services should have their dependancies passed in on instantiation using constructor parameters, [...]

This is a good idea. ;-) It helps unit-testing.

For your syntactical and design problems: This is how I would write your sources.

"usart.h":

All methods are non-static to have access to member variables.

The const attribute on a return type is doing nothing. Did you mean to declare the method constant? Then const belongs after the parameter list. However, this attribute might be wrong if such a method changes any member variable.

#include <avr/io.h>
#include <util/setbaud.h>

class Usart
{
public:
  Usart();
  ~Usart();

  void init(void);

  void print(const char myString[]);
};

"logger.h":

Just give and store a reference to the USART to avoid a copy.

#include "usart.h"

class LogSystem
{
public:
  LogSystem(Usart& usart);
  ~LogSystem();

  void log(const char *msg);

private:
  Usart& _usart;
};

"logger.cpp"

The member variable _usart is directly initialized in the constructor, before any statement is executed.

#include "logger.h"

LogSystem::LogSystem(Usart& usart) : _usart(usart)
{
  _usart.init();
}

LogSystem::~LogSystem()
{
}

void LogSystem::log(const char *msg)
{
  _usart.print(msg);
}

"main.cpp":

Provide the USART object on the stack, as the logger.

#include "logger.h"

int main()
{
    Usart usart;
    LogSystem logSystem(usart);

    while(1)
    {
        logSystem.log("Hello");
    }

    return 0;
}

The singleton design pattern is deprecated since it was invented, because it is so hard to test. Simply use just one object or a limiting object factory.

the busybee
  • 10,755
  • 3
  • 13
  • 30
  • Thats brilliant, I just re-wrote the source as per your suggestions and it works great, thanks so much! Your suggestion for allocating heap space to objects which last the length of the program is interesting, I'll take a look at that. I was referring to what I read here [https://www.avrfreaks.net/forum/c-new-delete-operator-confusion] which seemed like you have to write your own implementation for new and delete. Clearly I need to go back and re-read the chapter on passing by reference etc! Also your comment about the deprecation of singleton pattern is noted. Cheers! – Jason Oct 18 '21 at 17:17