Approach for implementing Device and Protocol layers in C++?

Question

I'm writing a program that will interface with an external device. It will support numerous devices that may use different communication interfaces like USB, serial, etc.

This is what I have so far:

An abstract Device class with some generic methods and some pure virtual methods:

class Device
{
private:
    bool initialised = false;

public:
    bool isInitialised() const {
        return this->initialised;
    }

    void setInitialised(bool initialised) {
        this->initialised = initialised;
    }

    virtual void open() = 0;

    virtual void close() = 0;

    // These will be overridden by derived classes that implement a specific communication interface (USB, serial, etc)
    virtual void read(unsigned char* buffer, int length, unsigned int timeout) = 0;

    virtual void write(unsigned char* buffer, int length) = 0;
};

And numerous classes that extend the Device class and implement a specific communication interface. Here is a simplified version of the USBDevice class:

class USBDevice : public Device
{
private:
    libusb_device* libUsbDevice = nullptr;
    libusb_device_handle* libUsbDeviceHandle = nullptr;
    uint16_t vendorId;
    uint16_t productId;

public:
    // Attempts to obtain a device handle and interface via libusb.
    void open() override;

    void close() override;

    // Communication via libusb
    void read(unsigned char* buffer, int length, unsigned int timeout) override;

    // Communication via libusb
    void write(unsigned char* buffer, int length) override;
};

Some of the devices employ their own communication protocol, and others (say, from a specific manufacturer) use the same protocol.

So, in order to avoid code duplication, I'd like to introduce a protocol layer, where I can define the protocol once and use it on multiple devices.

class Protocol1
{
};

class Protocol2
{
};

The problem is, I need to expose my device to my protocol class, so that I can communicate with the device via the protocol, like this:

Protocol1::doSomething()
{
    // Send data to the device
    device->write(blah blah);
}

And I'm not sure what the best way to do this would be.

I can have the Protocol classes inherit from the device class:

class Protocol1 : public Device
{
};

class Protocol2 : public Device
{
};

And then, for my actual device:

class SomeDevice : public USBDevice : public Protocol1
{

};

And for devices that support multiple protocols:

class SomeOtherDevice : public USBDevice : public Protocol1 : public Protocol2
{

};

The problem with this approach is that a) it results in a "Diamond of Dread" case, and b) it seems wrong in the sense that a protocol is not a type of device, so it shouldn't really inherit from the device class.

The other approach I thought of was to have the device compose of the protocol layer, because after all, a device does contain one or more protocols:

class SomeOtherDevice : public USBDevice
{
private:
    Protocol1 protocol1;
    Protocol2 protocol2;
};

And with this approach, I would expose the device to the protocol by injecting it at point of init:

Protocol1::Protocol1(Device dev)
{
    this->device = dev;
}

But with this approach, it just feels hacky. I can't really think of a reason for why it feels like that, but it just seems wrong. I don't think it's as wrong as the first approach, where I'd effectively be abusing multiple inheritance, but I'm sure there's a better way that I haven't thought of.

Are any of the approaches I've described acceptable? How would you do it?

Answer 1

Main problem with your first approach, as you discovered correctly already, is that you create a diamond pattern:

class A { };
class B : public A { };
class C : public A { };
class D : public B, public C { };

D will now inherit two instances of A, one from B, one from C, and you will run into ambiguities as soon as you try to access A directly from D (which A???).

To get just one base instance of A, you need virtual inheritance instead:

class B : public virtual A { };
class C : public virtual A { };

Apart from this problem, I do not consider it a good approach. Instead of inheritance, I'd prefer aggregation; so your second approach. I don't see a better way at the moment; the protocol classes might be templates, though, this way, you get around the need of virtual helper functions in your device base class (where you might have to declare multiple different helpers for different protocols, as these possibly are not always compatible):

template <typename Device>
class ProtocolX
{
    Device& d;
public:
    ProtocolX(Device& d) : d(d) { }
    void write(unsigned char const* buffer, size_t length)
    // write is just reading, so buffer should be const!
    {
        while(length)
        {
            d.write(*buffer++); // write single character
            --length;
        }
    }
};

class SomeDevice
{
public:
    SomeDevice() : p(*this) { }
    void write(unsigned char const* buffer, size_t length) override
    // (see ProtocolX::write)
    {
        p.write(buffer, length);
    }
private:
    friend class ProtocolX;
    ProtocolX<SomeDevice> p;
    void write(unsigned char c) { /* ... */ }
};

Writing a single character might not be appropriate for any protocol (e. g. UDP or IP, both relying on packets/datagramms) – but it should illustrate clearly enough the basic idea...

Sidenote 1: Overloading, as I did, might clash if you need same parameters as public write function – you might prefer inventing up a better name (which I just didn't due to lack of phantasy...).

Sidenote 2: Did you notice that I changed signature to size_t? Negative values are meaningless for lengths, so you might prefer an unsigned type, and size_t is defined such that it is large enough to hold any length that could be allocated on your architecture, provided sufficient memory installed.