Inheritance not working as expected

Question

Short version: This is a lighting control project. Some of the clases are Pin and Channel. Channel contains a Pin. Pin is a base class for DigitalOutPin and will be the base class for AnalogOutPin, and I want Channel to be able to use any child of Pin.

Pin has an abstract method setPinValue(), but when I create a DigitalOutPin object and call setPinValue(), Pin.setPinValue() gets called, not DigitalOutPin.setPinValue() and my program doesn't work because Pin.setPinValue() is empty as it's supposed to be overwritten.

Long version: If I leave out the definition of Pin.setPinValue(), I get that most idiotic of error gcc messages, "undefined reference to `vtable for Pin'". If I make it pure virtual I get several errors in the form of

In file included from /devel/arduino/lightbright/LightBrightMain/LightBrightMain.ino:9:0:
/devel/arduino/libraries/LightBrightLib/Channel.h:21:20: error: cannot declare parameter 'pin' to be of abstract type 'Pin'
     void begin(Pin pin, int channelNumber);
                ^
In file included from /devel/arduino/libraries/LightBrightLib/DigitalOutPin.h:10:0,
             from /devel/arduino/lightbright/LightBrightMain/LightBrightMain.ino:8:
/devel/arduino/libraries/LightBrightLib/Pin.h:16:7: note:   because the following virtual functions are pure within 'Pin':
 class Pin
   ^
/devel/arduino/libraries/LightBrightLib/Pin.h:33:18: note:  virtual void Pin::setPinValue(int)
     virtual void setPinValue(int value)=0;
              ^
In file included from /devel/arduino/lightbright/LightBrightMain/LightBrightMain.ino:9:0:
/devel/arduino/libraries/LightBrightLib/Channel.h:23:9: error: invalid abstract return type for member function 'Pin Channel::getPin()'
 Pin getPin();
     ^
In file included from /devel/arduino/libraries/LightBrightLib/DigitalOutPin.h:10:0,
             from /devel/arduino/lightbright/LightBrightMain/LightBrightMain.ino:8:
/devel/arduino/libraries/LightBrightLib/Pin.h:16:7: note:   since type 'Pin' has pure virtual functions
 class Pin
   ^

Full code is at GitHub but here are the relevant parts: Pin is declared like this right now as pure virtual, but I've tried with and without =0

class Pin
{
  public:
   ...
  protected:
   ...
  // This method is child-specific. It's empty in this base class
  virtual void setPinValue(int value)=0;
};

DigitalOutPin is declared as

class DigitalOutPin : public Pin
{
  public:
    void on();
    void off();
  protected:
    void setPinValue(int value);
};

and defined as

void DigitalOutPin::setPinValue(int value) {
....
}

Disclaimer: I'm primarily a Java developer so C++ OO is very different for me, but as far as I can tell this should work as designed.

Thanks for any advice you can offer.

Your DigitalOutPin is protected - it should be public to match the base class definition and allow access outside the class. — Majenko
– Majenko, Commented Jun 19, 2016 at 17:57
Especially in embedded context, you might want to take advantage of final, which can enable the compiler to produce more efficient binary code: en.cppreference.com/w/cpp/language/final — Igor Stoppa
– Igor Stoppa, Commented Jun 19, 2016 at 18:44
From the same POV, if you can avoid having VMT and runtime lookup, it's definitely more efficient. One way to do that could be to turn is_a relationships into has_a ones. — Igor Stoppa
– Igor Stoppa, Commented Jun 19, 2016 at 18:47
@Majenko Sounds like that could be it! Originally I had another public method that called that one, and likely it broke then. I will try that tonight. — dj_segfault
– dj_segfault, Commented Jun 19, 2016 at 19:56
@IgorStoppa Very good suggestion. Thanks. Will chec out that link tonight. — dj_segfault
– dj_segfault, Commented Jun 19, 2016 at 19:57

Code Gorilla · Accepted Answer · 2016-06-20 13:02:12Z

I have taken your code from above and compiled it for a PC in Visual Studio. When I create an object of type DigitalOutPin and call setPinValue() it calls the function I expect. So this means either you have managed to find a bug in the gcc compiler, or there is something else in your code that is coming into play. Can you try writing a VERY simple minimal test program that defines the Pin class and DigitalOutPin class (with just the setPinValue functions that print a string) and the setup and loop functions. Just to see if you have found a gcc bug.

Even if this fixes the inheritance problem, you have other issues with you code because C++ is weird. In Channel.h your Pin variables will need to be pointers, this will allow them to call the functions in the derived classes, not the base class functions.

I think you may get a performance improvement by using "const int& foo" for you parameters. The const means the function can't change it, this allows the compiler to treat it differently and the "&" means use a reference to the variable rather than make a new copy of it.

Functions that don't change member variables within the code should also be declared constant. "int FooBar(const int& value) const;" It makes it clearer to the reader and the compiler probably does some funky stuff to improve performance. (You might want to investigate "inline" if you are writing one line functions.)

Finally I'd recommend using constructors and destructors. In release builds variables are not assigned a value. If these variables are class members this means you don't know what they are to start with. Also derived classes should initialise the base class, so:

// Class constructor for DigitalOutPin
DigitalOutPin::DigitalOutPin(const int& pinNumber)
  : Pin(pinNumber)  // Initialise the base class
  , m_State(false)  // Initialise all the class members (in order declared in .h file)
{
  // Lots of lovely code
}
// Class destructor for DigitalOutPin (Declared as virtual)
DigitalOutPin::~DigitalOutPin()
{}

Thank you Matt. In doing more research I started to think the problem was that it wanted to instantiate Pin somewhere, and I came to the same conclusion that that Channel needed a *Pin not a Pin, and changed all the uses of it to dereference the pointer. At that point it worked as expected. I still need to make most of the parameters const references, but it's working now. Thank you . — dj_segfault
– dj_segfault, Commented Jun 22, 2016 at 6:04
With respect to constructors and destructors, it's my understanding that constructors have very limited value in the Arduino world, and it's best to have a begin() method to initialize the object, because of the order things are initialized, what can happen in loop(), what can happen in setup(),... — dj_segfault
– dj_segfault, Commented Jun 22, 2016 at 6:09

Stack Exchange Network

Inheritance not working as expected

1 Answer 1

Your Answer

Hot Network Questions

Inheritance not working as expected

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

Related

Hot Network Questions