Some OOP design advice, please

Community General Discussion
1

First, disclosure; I am not a C++ programmer - I think in C, and translate.
I don't naturally think in terms of classes, and "is-a" or "has-a" hierarchies or inheritancies.

Statement of problem:
I have a class which is a serial sniffer, for a connection between a sensor and a microcontroller
(it's an IKEA PM25 sensor, with the usual IKEA alpabetti spaghetti name), with a few smarts (well, a simple state machine)
to interpret the data packets presented by the sensor.

The class needs serial receive-only, and because the comms are slow, a software serial object can be used, though the class should
be able to handle hardware serial as well.

The class needs only the following functions from the serial object:
begin () - to set the line speed to the sensor's line speed (9600 bps)
available ()
read ()

Now, the tricky (for me) bit: How best to maintain encapsulation?

The calling sketch needs to provide an instance of a serial object, but SoftwareSerial and HardwareSerial are different types, and the only common class they inherit from is Stream, but Stream does not have the ability to set line speed.

So, I could simply pass the serial object (hardware or software) as an instance of Stream to the constructor, but that would mean that the calling sketch would have to set the line speed, which (in my mind) breaks encapsulation.

The outline scheme I came up with is:

class IKEA_PM25 {
  public:  
    IKEA_PM25 ();
    void     handleSensor (); // called every iteration of loop (), 
                              // calls serialAvailable and (conditionally) serialRead functions,
                              // basic, simple serial handling.
    ...
  private:
    virtual int serialAvailable () = 0;
    virtual int serialRead () = 0;
    ...   
};

class softIKEA_PM25 : public IKEA_PM25 {
  public:
    softIKEA_PM25 (SoftwareSerial& serial_) : serial (serial_) { };
    void begin () {serial.begin (9600);};
  private:
    SoftwareSerial& serial;
    int serialAvailable () {return serial.available ();};
    int serialRead ()      {return serial.read ();};
};

class hardIKEA_PM25 : public IKEA_PM25 {
  public:
    hardIKEA_PM25 (HardwareSerial& serial_) : serial (serial_) { };
    void begin () {serial.begin (9600);};
  private:
    HardwareSerial& serial;    
    int serialAvailable () {return serial.available ();};
    int serialRead ()      {return serial.read ();};
};

So, the two derived classes are the only ones capable of being instantiated . . . but they look so similar,
I feel there must be a better way - a template? (I find template syntax hard to follow and, frankly, ugly)
Make the base class inherit from both HardwareSerial and SoftwareSerial? (just doesn't smell right)

Any thoughts?

2

If I understand the problem correctly, what I'd do is wrap both hardware and software serial in another class, VersatileSerial and pass that to the constructor of this new class.

So VersatileSerial might expose Send(), Receive() and SetSpeed() methods (just guessing here) and be constructed with a parameter that indicates whether it should connect via hardware or software serial port.

At least that's my analysis at a quick glance :slight_smile:

3

Does it work fine? I have attempted passing serial reference to constructor before and ended up creating init method to be called from set up because serial wasn’t ready during class initialisation (even though it appeared like it was ready) but was available during setup.

4

Could always make a template

5

No need for inheritance or OOP.
When you find yourself repeating the same code for different types, you probably need a template:

template <class SerialT>
class IKEA_PM25 {
  public:  
    IKEA_PM25(SerialT &serial) : serial(&serial) {}

    void begin() {
        serial->begin(9600);
    }

    void handleSensor() {
        if (serial->available() > 0) {
            int data = serial->read();
            serial->write(data);
        }
    }
    
  private:
    SerialT *serial;
};

using SoftIKEA_PM25 = IKEA_PM25<SoftwareSerial>;
using HardIKEA_PM25 = IKEA_PM25<HardwareSerial>;

HardIKEA_PM25 pm25 { Serial };

void setup() {
  pm25.begin();
}

void loop() {
  pm25.handleSensor();
}

Granted, the architecture of the HardwareSerial and SoftwareSerial classes is very poorly designed, they should share a common base class with methods relevant to UARTs, but that's not something you can change as a user (unless you create your own polymorphic wrappers).

6

Given that you only need to patch a single method, you could implement some basic type erasure yourself. The advantage here is that you only have a single class, IKEA_PM25 doesn't have to be a template at all:

class IKEA_PM25 {
  public:
    template <class SerialT>
    IKEA_PM25(SerialT &serial) : serial(&serial) {
      begin_fun = [](void *serial, unsigned long baud) {
        auto *true_serial = static_cast<SerialT *>(serial);
        true_serial->begin(baud);
      };
    }

    void begin() {
        begin_fun(serial, 9600);
    }

    void handleSensor() {
        if (serial->available() > 0) {
            int data = serial->read();
            serial->write(data);
        }
    }
    
  private:
    Stream *serial;
    using begin_signature_t = void (void *serial, unsigned long baud);
    begin_signature_t *begin_fun;
};

Just the constructor is generic. It has to be, because you have to know the type of the serial port (HardwareSerial or SoftwareSerial) to know which begin() method to call, there's no way around that.
But once you know that, you can just save a pointer to the function that casts the serial port back to the original type and calls the right begin() function. It's similar to the virtual function tables the compiler might generate for runtime polymorphism, but without the need for inheritance, interfaces or other OOP boilerplate.

The syntax [](void *serial, unsigned long baud) { ... } defines an inline function, see Lambda expressions (since C++11) - cppreference.com.
It is defined inside of the constructor because you need access to SerialT, the true type of the serial port the user passed in.
Since it doesn't capture any data, the inline function behaves just like an ordinary function, so you can store a pointer to it to call it later.

IKEA_PM25 pm25 { Serial };

void setup() {
  pm25.begin();
}

void loop() {
  pm25.handleSensor();
}

For real code, I'd probably move the type erasure to a separate wrapper class, a class for reading sensors has no business dealing with type erasing poorly designed Arduino library types, but you get the idea.

7

Yup, as I suspected, time to put on my big boy's pants, and embrace templates, even though they look like the ugly, angular love child of the C preprocessor and BNF.

The lambda function method looks even less intuitive, but interesting nonetheless.

Thanks everyone!

@PieterP - one further question, if I may?

Why take the address of a reference, and then use the pointer within the class, and not simply use the reference (which seems to me more natural)?

8

I'm in much the same place, so my knowledge of OO design is largely theoretical. I was hoping that @PieterP would weigh in, and since he now has, this may be somewhat passé but patterns that spring to mind, theoretically speaking, are factory and dependency injection.

9

Absolutely.

10

board?

11

Esp8266, (NodeMCU) and Uno.

12

Because references can only be bound once, you cannot re-assign the reference. By extension, the same applies to structs/classes that contain references. If you try to assign to a struct with a reference member, you'll get an error like:

error: object of type 'IKEA_PM25' cannot be assigned because its copy assignment operator is implicitly deleted

note: copy assignment operator of 'IKEA_PM25' is implicitly deleted because field 'serial' is of reference type 'Stream &'
  Stream &serial;
          ^

Pointers don't have this issue, they can be re-assigned without problems after they have been created.

In this case, you probably don't need the copy assignment operator anyway, so it doesn't really matter. You might even want to prevent the user from assigning to an instance of IKEA_PM25 (but in that case you'll want to delete these operators explicitly so you get a more meaningful error message).

13

Thanks for the reply

Then I think that it's fine to use a reference - it's an embedded application, so the sensor is bound to a single pin which will never change for the lifetime of the system, and for the same reason, I rarely even bother with a destructor.

14

A reasonable choice if program memory is a concern. There's even a design pattern for it. Bridge? Something like that.