Reputation: 731
I'm investigating how I can implement a custom C++ HAL which targets multiple microcontrollers, potentially of differing architectures (ARM, AVR, PIC, etc) while keeping things sane.
I've inherited several large, messy code-bases which are un-maintainable in their current state hence the need for something more structured.
Having picked through a number of good articles and design guides, I'm considering a PIMPL
implementation.
Consider the following example for a UART/serial port:
// -----------------------------
// High-level HAL
// -----------------------------
// serialport.h
class SerialPortPrivate;
class SerialPort {
public:
SerialPort(uint8_t portNumber);
~SerialPort();
bool open();
void close();
void setBaudRate(uint32_t baudRate = 115200);
private:
SerialPortPrivate *_impl;
};
// serialport_p.h
class SerialPort;
class SerialPortPrivate {
public:
SerialPortPrivate(uint8_t portNumber, SerialPort *parent) {
// Store the parent (q_ptr)
_parent = parent;
// Store the port number, this is used to access UART
// specific registers UART->D[portNumber] = 0x10;
_portNumber = portNumber;
}
~SerialPortPrivate();
bool open() = 0;
void close() = 0;
void setBaudRate(uint32_t baudRate) = 0;
protected:
uint8_t _portNumber;
private:
SerialPort *_parent;
};
// serialport.cpp
#include "serialport.h"
#include "serialport_p.h"
#include "stm32serialport_p.h"
#include "avr32serialport_p.h"
#include "nrf52serialport_p.h"
#include "kinetisserialport_p.h"
SerialPort::SerialPort(uint8_t portNumber) {
#if MCU_STM32
_impl = new Stm32SerialPortPrivate(portNumber, this);
#elif MCU_AVR32
_impl = new Avr32SerialPortPrivate(portNumber, this);
#elif MCU_NRF52
_impl = new Nrf52SerialPortPrivate(portNumber, this);
#elif MCU_KINETIS
_impl = new KinetisSerialPortPrivate(portNumber, this);
#endif
}
void SerialPort::setBaudRate(uint32_t baudRate) {
_impl->setBaudRate(baudRate);
}
// -----------------------------
// Low-level BSP
// Hardware-specific overrides
// -----------------------------
// stm32serialport_p.h
class Stm32SerialPortPrivate : public SerialPortPrivate {
};
// nrf52serialport_p.h
class Nrf52SerialPortPrivate : public SerialPortPrivate {
};
// kinetisserialport_p.h
class KinetisSerialPortPrivate : public SerialPortPrivate {
};
The above code only has one set of #if/#endif
statements within the constructor of the high-level interface (SerialPort
) and the hardware-specific code (register accesses, etc) is done within the private implementation.
Taking the above further, I can see the above implementation working well for classes like I2cPort
, SpiPort
, UsbSerialPort
but for other non-port related peripheral sets like clocks, hardware timers.
I'm sure there are some holes in the above concept, can anyone suggest from experience things to avoid or if there is a better way of abstracting?
Upvotes: 5
Views: 6373
Reputation: 35580
To provide cross-platform interfaces, I like to use a "platform.h" file which keep all the #defines out of the source code, while also avoiding the code bloat that a big inheritance tree can generate. See this answer or this one for details.
In terms of what the interfaces actually are, I agree with @Sigve that looking at the use cases is the best design tool. Many low level peripheral interfaces may be reduced to init \ read\ write
with just a few parameters exposed. Many higher level "HAL" tasks can often be decoupled from the hardware entirely and operate only a data stream.
Upvotes: 0
Reputation: 1161
Here are some of the concerns I'd have about your approach:
Firstly, let's say the the peripheral on one platform has some configuration option that simply doesn't exist for equivalent peripherals on other platforms. There are some options for how to handle this,e.g.:
SerialPort
to configure the option (extra function? some kind of callback?).The first two are not very flexible (can't change at runtime) and the third breaks abstraction - platforms must provide functions to configure potentially non-existent options, or SerialPort
users must know details of the underlying platform. All of these, in my opinion are ingredients for a messy codebase.
Secondly, let's say a platform has multiple different peripherals that can provide the same functionality. For instance, I'm currently working with an STM32 which has a USART
and LPUART
peripherals, both of which can provide UART functionality. To handle this, you'll need either to instantiate different pimpls at runtime depending on port, or have a single one for the platform that can handle. Doable, but can get messy.
Thirdly, to add support for another platform, you now need to modify a lot of other code to add new #elif
clauses. Also the #if
- #elif
- #endif
makes the code less readable, although good syntax highlighting will shade the inactive sections of code.
As for my advice:
Find the right interfaces. There's a temptation to try creating an interface for what the hardware can do - it's a hardware abstraction layer right?. However, I find it better to look at it from the interface clients' perspective - what are the use cases for the HAL. If you find a simple interface that can meet most or all of your use cases, it's probably a good one.
(This I think, is probably the most relevant to your point about clocks and hardware timers. Ask yourself: what are your use cases?)
A good example here is I2C. In my experience, most of the time a particular I2C peripheral is permanently a master or permanently a slave. I haven't often come across a need to swap at runtime between master and slave. With this in mind is it better to provide an I2CDriver
that tries to encapsulate what a 'typical' I2C peripheral on any platform is capable of, or to provide a pair of interfaces I2CMasterDriver
and I2CSlaveDriver
, each of which provide only the use cases for one end of the I2C transactions.
I consider the latter the best starting point. The typical use case is either master or slave, and the use case is known at compile time.
Limit interfaces to what is 'universally common'. Some platforms may provide a single peripheral that does SPI/I2C, others provide separate peripherals. The same peripherals may, as mentioned above, have different configuration options between platforms.
Provide an abstract interface for the 'universally common' functionality.
Provide platform-specific implementations of that interface. These can also provide any required platform-specific configuration.
I think doing this - separating the 'universally common' and the hardware specific - keeps the interfaces smaller and simpler. That makes it easier to spot when it starts getting messy.
Here's an example of how I would go about this. First, define an abstract interface for the universally common functions.
/* hal/uart.h */
namespace hal
{
struct Uart
{
virtual ~Uart() {};
virtual void configure( baud_rate, framing_spec ) = 0;
/* further universally common functions */
};
}
Next, create implementations of this interface, which can include platform-specific details - configuration options, resource management. Configure your toolchain to only include these for the specific platform
/* hal/avr32/uart.h */
namespace hal::avr
{
struct Uart : public hal::Uart
{
Uart( port_id );
~Uart();
void configure( /*platform-specific options */ );
virtual void configure( baud_rate, framing_spec );
/* the rest of the pure virtual functions required by hal::Uart */
};
}
For completeness, let's add a few higher-level 'clients' of the interface above. Note that they take the abstract interface by reference (could be pointer, but can't be by value as that'll slice the object). I've omitted namespaces and base classes here, as I think they illustrate better without.
/* elsewhere */
struct MaestroA5135Driver : public GPSDriver
{
MaestroA5135Driver( hal::Uart& uart );
}
struct MicrochipRN4871Driver : public BluetoothDriver
{
MicrochipRN4871Driver( hal::Uart& uart );
}
struct ContrivedPositionAdvertiser
{
ContrivedPositionAdvertiser( GPSDriver& gps, BluetoothDriver& bluetooth );
}
Finally let's put it all together in a contrived example. Notice that the hardware -specific configuration is done especially, because the clients can't access it.
/* main.cpp */
void main()
{
hal::avr::Uart gps_uart( Uart1 );
gps_uart.configure(); /* do the hardware-specific config here */
MaestroA5135Driver gps( gps_uart ); /* can do the generic UART config */
hal::avr::Uart bluetooth_uart( Uart2 );
bluetooth_uart.configure(); /* do the hardware-specific config here */
MicrochipRN4871Driver bluetooth( bluetooth_uart ); /* can do the generic UART config */
ContrivedPositionAdvertiser cpa( gps, bluetooth );
for(;;)
{
/* do something */
}
}
There are some drawbacks with this approach as well. For instance, passing instances to the constructor of higher-level classes can grow big fast. So all the instances need to be managed. But in general, I think the drawbacks are outweighed by the advantages - For instance, easy to add another platform, easy to unit test hal clients using test doubles.
Upvotes: 9