Interface Layer for mbed-drivers

[tobiasjaster]

Description

While working on a driver for the MCP23S17 I had the intention that both internal standard drivers and external drivers could use the same interface. This could be used, for example, to control a DigitalIn object in the same way as a GPIO expander pin. This in turn allows the control of an external module with mixed GPIOs.

Example:
Commit: https://github.com/tobiasjaster/mbed-os/#597c90613b2b81d2047b97d334d114303e1c6617

main.cpp:
#include "mbed.h"
#include "DeviceExpander.h"
#include "GPIOExpansionInterface.h"
#include "MCP/MCP23S17.h"

SPI spi(SPI1_MOSI, SPI1_MISO, SPI1_SCK);
MCP23S17 mcp(&spi, SPI1_CS2, 0x40, TIM1_CH2N);
DigitalOut led1(TIM4_CH4);
DigitalOut reset(TIM1_CH1N);
ExpDigitalOut led2(&mcp, EXPPORT_B, EXPPIN_5, PullUp);
ExpDigitalIn expb4(&mcp, EXPPORT_B, EXPPIN_4, PullUp);
ExpInterruptIn expb3(&mcp, EXPPORT_B, EXPPIN_3, PullUp);
ExpDigitalOut expb2(&mcp, EXPPORT_B, EXPPIN_2, 0);
// main() runs in its own thread in the OS

void toggleLED(void){
led2 = !led2;
}
int main() {

reset = 1;
expb3.rise(toggleLED);

while (true) {
    led1 = !led1;
    expb2 = expb4;
    wait(0.5);
}

}

Issue request type

[x] Question
[ ] Enhancement
[ ] Bug

[40Grit]

I like this concept.

The approach I have wanted to take with it is to have a base class representing an "ExternalCircuit".

All external circuits have some mode of communication/control. i2c/spi/gpio/usb/etc.

The appropriate API libraries can then be applied on top of the external circuit to create something similar to what you have here but more abstracted.

@kjbracey-arm thoughts? Who runs the hal these days?

[tobiasjaster]

Since the interface layer of the internal drivers is a minimally invasive change that only extends the current architecture and should not do any harm, this change could be considered on its own.

I think the idea with the abstract class for external circuits is great, I would like to open another issue to discuss a possible architecture of the "components/drivers_expansion".

[40Grit]

Being more experienced in C I have no idea how best to architect all this from a C++ standpoint.
But I'll be happy to continue discussion in the related issue.

[kjbracey]

Using inheritance for run-time polymorphism has a significant image size cost, and it hits everyone using the polymorphic classes, whether they need it or not. I'd be wary about introducing more of it.

My current inclination would be to reserve abstract classes for some pretty core "big" stuff, like FileHandle, Socket and NetworkInterface.

For this case, I would be inclined to keep separate classes like you have here - the only reason you would need a base interface class is if you're intending to define functions that are prototyped like BlinkLED(DigitalOutInterface &gpio).

I would suggest you don't need that usually, and can instead write such things as a template:

 template <class Out>
 void BlinkLED(Out &gpio) {
     gpio = 1;
     ThisThread::sleep_for(100);
     gpio = 0;
 }

 BlinkLED(my_led); // doesn't matter what type `my_led` is, as long as it works like a `DigitalOut`

For the vast majority of platforms, they will only ever instantiate one copy of that function, for DigitalOut, and it will be the same size as if it was not a template. No run-time polymorphism cost either.

If a platform has two types of "DigitalOut", then maybe you get two copies of the BlinkLED, so you may start paying more cost than the run-time polymorphism (vtables etc). But that cost is only hitting the bigger platforms that have multiple implementations of one device type

And there is far more scope for optimisation for each copy, when it knows the actual type, than there would be for a polymorphic type. At the moment a DigitalOut write is totally inline all the way to the HAL - just a hardware poke. Start adding polymorphism, and that goes. (DigitalOutInterface::write could never be inline, and even DigitalOut::write inlining would be lost unless care was taken to stick final keywords on).

Now that we're using C++11, there's more scope for wrapping teeny bits of stuff in lambdas and similar and passing them to templates. I think C++11 can help push away from the 1999-era "always use base classes and inheritance" model, which is not ideal for the tiniest devices.

[kjbracey]

If you think in a C point of view about what the C++ compiler is doing behind the scenes:

• abstract base classes and virtual mean every "digital out" operation becomes a call via a function pointer in the vtable. Major optimisation loss at every call site (no inlining of trivial functions), the vtable takes space, and GCC and ARMC6 linkers cannot eliminate unused virtual functions (ARMC5 and IAR can), meaning lots of dead code may go into the image.
• the template means making a custom BlinkLED(type) for every possible DigitalOut-like type. Potentially lots of copies of code being written, but each one can be fully optimised, and they're only generated when used.

There's a trade-off depending how many times you expect to instantiate each template, and how much redundancy there would be doing so. If you template a large function that only varies in one spot on the template parameter, then that's clearly inefficient compared to just making it use a function pointer at one spot. (Once you do call it with 2 or more types - 1 type it's fine).

[40Grit]

@kjbracey-arm

Somehow I have gotten by without having to care much for resource consumption with my C code.
Even on junk PIC16 I got away with function pointers calling function pointers calling function pointers. (Baremetal custom scheduler though)

I need to catch up on C++ badly; especially templating.

Are you saying the "ExternalCircuit" concept is heavyweight no matter what, and has no place in MCU's or that templating will generally be the way to go?

[kjbracey]

"No place in MCUs" is strong, but we are repeatedly hitting an issue in multiple platforms where we're struggling to squeeze into the space available.

Anything that makes Mbed OS unconditionally bigger, making those problems worse, is going to be a hard sell.

Indeed, there's been a push in the opposite direction to devirtualize interfaces - eg #9219

Now, saying "just use templates" is glib, you do sometimes want polymorphism. It would be nice to be able to get it when you need it only.

A bit of Googling found this library where someone has tried to tackle it. Matches what I've been thinking, and looks quite neat:

https://github.com/ldionne/dyno

I see he has some CppCon talks - going to have to watch those.

[kjbracey]

And here's another approach: https://github.com/facebook/folly/blob/master/folly/docs/Poly.md

[tobiasjaster]

OK, I understand the problem.

The solution with the inheritance was actually easier for me to understand in the first place and I would have been happy about a quasi equality of external and internal drivers, but with the additional problems it makes no sense.

My intention was that libraries could be used with these pointers on previously instantiated external or internal drivers in necessary cases.

Does it still make sense to open a new issue regarding external drivers in the components folder (which would then be implemented without inheritance)?

[40Grit]

There is debate as to what "components" are and whether external circuit drivers should be part of the os.

My feeling is that if a decent package management option for c++ can be found then that should be used to pull in external drivers written for MbedOS.

The "ExternalCircuit" concept is how I envisioned standardizing interfaces for such driver libraries.

CMSIS-Packs and CMSIS-Driver are supposed to solve some of these problems but progress is really really slow.

[kjbracey]

I would have been happy about a quasi equality of external and internal drivers, but with the additional problems it makes no sense.

So would I, and we have achieved it for a few bits - eg BlockDevice and NetworkInterface, along with the get_default_instance model. But we are paying the virtualisation cost, and I wouldn't want to apply that to the entire low-level API.

Part of the reason for upgrading to C++11/C++14 was to investigate alternatives to that model.
Those poly libraries look promising, but obvious anything like that is potentially quite intimidatingly scary compared to the more familiar C++ native inheritance thing.

I'm going to investigate further.

[tobiasjaster]

OK, I will close this Issue. Thank you for your feedback @kjbracey-arm & @40Grit