Defining hardware components structure

Question

I'm writing firmware for my Arduino project, and I'm struggling with a clean, scalable hardware mapping structure. Initially, I had the following namespace:

namespace Motherboard {
extern DRV8825 xAxisMotor;
extern MP6500 yAxisMotor;
extern DRV8876 zAxisMotor;
extern ClickEncoder encoder;
extern SSD1331 display;
extern RGBLed rgbIndicator;
extern Relay heaterRelay;
void performMainSequence();
void begin(const EepromData &initialData);
}; // namespace Motherboard

Which I initialized like this:

namespace Motherboard {
//=================== Public ====================
DRV8825 xAxisMotor = {
 23, // ENABLE PIN
 25, // MS0 PIN
 27, // MS1 PIN
 29, // MS2 PIN
 31, // RESET PIN
 33, // SLEEP PIN
 35, // STEP PIN
 37, // DIRECTION PIN
 39, // FAULT PIN
 MOTHERBOARD_X_AXIS_MAX_SPEED,
 MOTHERBOARD_X_AXIS_MIN_POSITION,
 MOTHERBOARD_X_AXIS_MAX_POSITION,
};
MP6500 yAxisMotor = {
 22, // ENABLE PIN
 24, // MS1 PIN
 26, // MS2 PIN
 28, // SLEEP PIN
 30, // STEP PIN
 32, // DIRECTION PIN
 34, // FAULT PIN
 MOTHERBOARD_Y_AXIS_MAX_SPEED,
 MOTHERBOARD_Y_AXIS_MIN_POSITION,
 MOTHERBOARD_Y_AXIS_MAX_POSITION,
};
DRV8876 zAxisMotor = {
 44, // ENABLE PIN
 36, // PHASE PIN
 38, // PMODE PIN
 40, // SLEEP PIN
 42, // FAULT PIN
 MOTHERBOARD_Z_AXIS_MAX_SPEED};
ClickEncoder encoder = {
 45, // A PIN
 43, // B PIN
 41, // SW PIN
 2, // STEPS PER NOTCH
};
SSD1331 display = {
 53, // CS PIN
 48, // DC PIN
 49, // RESET PIN
};
RGBLed rgbIndicator = {
 13, // R PIN
 12, // G PIN
 11, // B PIN
 RGBLed::COMMON_CATHODE, // COMMON
};
Relay heaterRelay = {
 47 // SWITCH PIN
};
void performMainSequence() {
 //
}
void begin(const EepromData &initialData) {
 //
}
//===============================================
}; // namespace Motherboard

A senior in my company saw this piece of code, suggested moving this to a class and create interfaces for hardware related classes. Therefore, I've created this:

struct MotherboardDevices {
 IStepperDriver* xAxisMotor;
 IStepperDriver* yAxisMotor;
 IDcDriver* zAxisMotor;
 ISwitch* heaterRelay;
 RGBLed* rgbIndicator;
 ClickEncoder* encoder;
 IDisplay* display;
};
class Motherboard {
public:
 MotherboardDevices devices;
 Motherboard();
 Motherboard(const MotherboardDevices _devices);
 void motherboardIsr();
 void performMainSequence();
 void begin(const EepromData &initialData);
};

I was struggling with the way of initializing this class. I eventualy created separate file:

namespace Hardware {
namespace {
DRV8825 initXAxisMotor() {
 DRV8825Pinout xAxisPinout = {
 23, // ENABLE PIN
 25, // MS0 PIN
 27, // MS1 PIN
 29, // MS2 PIN
 31, // RESET PIN
 33, // SLEEP PIN
 35, // STEP PIN
 37, // DIRECTION PIN
 39, // FAULT PIN
 };
 DRV8825 xAxisMotor = {
 xAxisPinout,
 MOTHERBOARD_X_AXIS_MAX_SPEED,
 MOTHERBOARD_X_AXIS_MIN_POSITION,
 MOTHERBOARD_X_AXIS_MAX_POSITION,
 };
 return xAxisMotor;
}
MP6500 initYAxisMotor() {
 MP6500Pinout yAxisPinout = {
 22, // ENABLE PIN
 24, // MS1 PIN
 26, // MS2 PIN
 28, // SLEEP PIN
 30, // STEP PIN
 32, // DIRECTION PIN
 34, // FAULT PIN
 };
 MP6500 yAxisMotor = {
 yAxisPinout,
 MOTHERBOARD_Y_AXIS_MAX_SPEED,
 MOTHERBOARD_Y_AXIS_MIN_POSITION,
 MOTHERBOARD_Y_AXIS_MAX_POSITION,
 };
 return yAxisMotor;
}
DRV8876 initZAxisMotor() {
 DRV8876Pinout zAxisPinout = {
 44, // ENABLE PIN
 36, // PHASE PIN
 38, // PMODE PIN
 40, // SLEEP PIN
 42, // FAULT PIN
 };
 DRV8876 zAxisMotor = {
 zAxisPinout,
 MOTHERBOARD_Z_AXIS_MAX_SPEED,
 };
 return zAxisMotor;
}
Relay initHeaterRelay() {
 Relay heaterRelay = {
 47 // SWITCH PIN
 };
 return heaterRelay;
}
SSD1331 initDisplay() {
 SSD1331 display = {
 53, // CS PIN
 48, // DC PIN
 49, // RESET PIN
 };
 return display;
}
ClickEncoder initEncoder() {
 ClickEncoder encoder = {
 45, // A PIN
 43, // B PIN
 41, // SW PIN
 2, // STEPS PER NOTCH
 };
 return encoder;
}
RGBLed initRgbIndicator() {
 RGBLed rgbIndicator = {
 13, // R PIN
 12, // G PIN
 11, // B PIN
 RGBLed::COMMON_CATHODE, // COMMON
 };
 return rgbIndicator;
}
MotherboardDevices createDevices() {
 static auto xAxisMotor = initXAxisMotor();
 static auto yAxisMotor = initYAxisMotor();
 static auto zAxisMotor = initZAxisMotor();
 static auto heaterRelay = initHeaterRelay();
 static auto rgbIndicator = initRgbIndicator();
 static auto encoder = initEncoder();
 static auto display = initDisplay();
 return {
 &xAxisMotor,
 &yAxisMotor,
 &zAxisMotor,
 &heaterRelay,
 &rgbIndicator,
 &encoder,
 &display,
 };
}
MotherboardDevices devices = createDevices();
} // namespace
Motherboard motherboard = Motherboard(devices);
} // namespace Hardware

I feel like now it's messier than before and I could've done that better. It also creates a problem with my UI layer. I'm using ArduinoMenu library for creating UI menu system. This UI is built using macros and needs static, compile time access to hardware related methods. That means I can't use dependency injection and pass created Motherboard object to UI class. I would have to pass global Motherboard instance via extern which is ugly. It also doesn't make sense to create multiple instance of this class.

Would keeping this structure as namespace with hardware interfaces better? It would be statically available for the UI, there wouldn't be a way of creating multiple instances. I could initialize all the hardware in the related to header, cpp file. However, what I would essentially do here is create a bunch of global variables. I also don't like the idea of the possibility to modify this namespace from every place.

Edit, adding more context

Motherboard::devices would be accessed from non members, UI event handler example:

result updateZAxisSpeed(eventMask event) {
 if (event == enterEvent) {
 Hardware::motherboard.devices.zAxisMotor->start();
 } else if (event == updateEvent) {
 uint8_t updatedValue = EepromManager::currentSettings.zSpeed;
 Hardware::motherboard.devices.zAxisMotor->setSpeed(updatedValue);
 } else if (event == exitEvent) {
 EepromManager::syncEeprom();
 Hardware::motherboard.devices.zAxisMotor->stop();
 }
 return proceed;
}

Motherboard::begin is a late initialization method. It configures provided devices, like:

devices.encoder->setAccelerationEnabled(false);
devices.encoder->setButtonHeldEnabled(false);
devices.encoder->setDoubleClickEnabled(false);

Motherboard::performMainSequence method:

void Motherboard::performMainSequence() {
 devices.heaterRelay->toggleOn();
 delay(5000);
 long xHalfWay = devices.xAxisMotor->getEndPosition() / 2;
 devices.yAxisMotor->moveToEnd();
 devices.xAxisMotor->moveTo(xHalfWay);
 devices.zAxisMotor->start();
 devices.xAxisMotor->moveToEnd();
 delay(2000);
 devices.xAxisMotor->moveTo(xHalfWay);
 devices.zAxisMotor->stop();
 devices.heaterRelay->toggleOff();
 devices.xAxisMotor->moveToStart();
 devices.yAxisMotor->moveToStart();
}

Motherboard::motherboardIsr() is an interrupt routine:

void Motherboard::motherboardIsr() {
 devices.encoder->service();
}

When Motherboard was a namespace I could just directly asign service() method of my encoder to microcontroller's timer interrupt. Since interrupts needs global context and Motherboard now is a class I had to expose isr routine method and asign it globally:

Timer1.attachInterrupt([]() { Hardware::motherboard.motherboardIsr(); });

• G. Sliepen: It's hard to review this without some more context. Is Motherboard::devices going to be accessed directly by non-member functions? What do motherboardIsr(), performMainSequence() and begin() do?
• Dead1nside: Thanks for the interest @G.Sliepen I've updated my post providing more context.

Answer 1

Namespace vs. struct

It's great that you created a namespace, that already avoids some of the problems global variables have. However, a struct has some advantages, and it's rather trivial to go from a namespace to a struct:

extern struct {
 DRV8825 xAxisMotor;
 MP6500 yAxisMotor;
 DRV8876 zAxisMotor;
 ...
 void performMainSequence();
 void begin(const EepromData &initialData);
} Motherboard;

This declares a global variable Motherboard whose type is a struct holding all the actuator objects. Instead of Motherboard::zAxisMotor.stop(), you then just have to write Motherboard.zAxisMotor.stop().

However, now we can give the struct a name, pass it around, have multiple instances of it, add access control (public/private), and so on. These are all benefits that are not available with a namespace.

In particular, you can mostly avoid needing global variables this way, but since you have the interrupt service routine, you really do need something global; either the object itself, or a pointer to the object which can then be allocated elsewhere. For an embedded device, I would just make a global object, to avoid the overhead of pointer dereferencing.

Interfaces and inheritance

It's great if each device has a consistent interface, however if you go the route of abstract base classes, then you need to use pointers which have to be dereferenced, and those pointers point to vtables which contain the pointers to the actual functions. So that's more dereferencing, which adds some overhead. For an embedded project where you know exactly which type each actuator is, I think that is unnecessary.

You can write code so the compiler can check that all your classes follow a given interface (see this StackOverflow post).

Having a well-defined interface is handy if you want to write generic functions that work with multiple types of actuators. For example, if you want something that moves to start, then moves to end, then moves back to start (maybe for some initialization sequence where it finds the end stops and records encoder positions). You want to write this once so it works for all tyeps of motors. It would then either pass a pointer to an abstract base class:

void doInitSequence(IStepperMotor* motor) {
 motor->moveToStart();
 ...
 motor->moveToEnd();
 ...
 motor->moveToStart();
}

Or you can make it a template:

template<typename Motor>
void doInitSequence(Motor* motor) {
 motor->moveToStart();
 ...
 motor->moveToEnd();
 ...
 motor->moveToStart();
}

In the latter case, you can directly pass it a pointer to a DRV8825 without needing inheritance.

• Dead1nside: Thank you, thats really helpfull. Is it common to use compile time polymorphism in embedded while building hardware structure?
• Ajinkya Kamat: @Dead1nside Yes, it is. Many embedded platforms avoid heap allocations. So, dynamic polymorphism becomes extremely difficult. It is doable using pool allocators and such. But, when it comes to tiny devices like an AVR microcontroller, sometimes one simply does not have enough space.

Answer 2

I'll expand a bit on your design by introducing an adapter.

Implementation

Firstly, you have somewhere an implementation. This could be a library from the internet that you don't want or can't change. Lets say this looks like this:

class Relay
{
public:
 void Set(bool value);
};

Interface

Now in our application we have defined an interface for working with relays. So there is one single way to work with relays throughout the application.

class IRelay
{
public:
 virtual void TurnOn() = 0;
 virtual void TurnOff() = 0;
};

Adapter

As you can see, the interface is different from the relay implementation. We can't change the implementation because this was a library from the internet. And who knows, we may get another relay in the future that has different methods. (Relay may not be the best example. But what about an IC that measures current?)

Also, we don't want to change the interface, because this would require us to change code all over the place. Besides, if we support 2 different relays with different implementations this isn't possible at all.

This is where the adapter comes in. It forms a link between the library and the interface. Now every time we need to support another relay. We just make another adapter and the relay can be used everywhere in our code.

class RelayAdapter : IRelay
{
 Relay& relay;
public:
 RelayAdapter(Relay& relay) 
 : relay(relay)
 {
 }
 void TurnOn() override {
 relay.Set(true);
 
 }
 void TurnOff()override {
 relay.Set(false);
 }
};

MyDevice

So, how would this look in your code? First, we need to instantiate the relay and the adapter, so it can be used.

class MyDevice
{
 Relay actualRelay1;
 Relay actualRelay2;
 RelayAdapter adapter1 {actualRelay1};
 RelayAdapter adapter2 {actualRelay2};
 
public:
 IRelay relay1 = adapter1;
 IRelay relay2 = adapter2;
}

This class can be used:

int main()
{
 MyDevice device;
 
 device.relay1.TurnOn();
 device.relay2.TurnOff();
 return 0;
}

---

Btw, like G. Sliepen mentioned, keep overhead in mind! For us the overhead is acceptable especially when keeping the advantages in mind. For the ISR, this is probably not preferred. I would do something with an semaphore and a different task to handle isr. Ofcourse, this is dependant on your situation.