Makuna / NeoPixelBus

An Arduino NeoPixel support library supporting a large variety of individually addressable LEDs. Please refer to the Wiki for more details. Please use the GitHub Discussions to ask questions as the GitHub Issues feature is used for bug tracking.
GNU Lesser General Public License v3.0
1.17k stars 257 forks source link

Base class for the NeoPixelBus template #338

Closed moritz89 closed 4 years ago

moritz89 commented 4 years ago

Is your feature request related to a problem? Please describe. In order to support polymorphism of the templated NeoPixelBus class, it would be useful if they all inherited from the same base class. This would allow the selection of the exact template parameters to be chosen during run time, while the function calls can remain the same.

Describe the solution you'd like I would like to make the NeoPixelBus library inherit from a base class NeoPixelBusBase that has all the functions as pure virtual methods. Effectively creating an interface class.

Describe alternatives you've considered Alternatively I would create a member pointer for each template (10 if only regarding the T_COLOR_FEATURE, many more if also respecting the T_METHOD options).

Another alternative is to use a void pointer, but this then requires using reinterpret casts.

Additional context I am using a TaskScheduler task to control NeoPixel strips and would like to have a unified control logic for the various types of NeoPixels.

moritz89 commented 4 years ago

In an initial implementation, the following class could act as the interface:

class NeoPixelBusBase {
 public:
  virtual ~NeoPixelBusBase() = default;

  virtual void Begin() = 0;

  virtual void Show(bool maintainBufferConsistency = true) = 0;
  virtual bool CanShow() const = 0;

  virtual bool IsDirty() const = 0;
  virtual void Dirty() = 0;
  virtual void ResetDirty() = 0;
  virtual uint8_t* Pixels() = 0;
  virtual size_t PixelsSize() const = 0;
  virtual size_t PixelSize() const = 0;
  virtual uint16_t PixelCount() const = 0;
};

To create a comprehensive interface, template agnostic function calls would be required, such as Set/GetPixelColor() using RGB / HSL color objects instead of the T_COLOR_FEATURE::ColorObject objects.

Is there any interest in accepting this type of functionality or are there any blockers that have been encountered?

Makuna commented 4 years ago

Absolutely not. This library was based on modern template designs which specifically does not use "interface" classes and virtual functions for many good reasons.

Selecting at runtime is not something most users of Arduino sketches want or need. The only valid design case I have heard is for a "testing" sketch; but in this very rare case there are ways to work around it (heavy in code, but its the exception). What is your case?

Virtual methods create RAM memory usage. Every virtual method would require a pointer in memory (VTABLE) for it for EVERY derived class. For what you show, that is 11 pointers, and lets just say its only a 16 bit system for pointers, that's 22 bytes of ram. This library works on Arduinos platforms that often have only 256 bytes of ram, that's a pretty big chunk of memory you just required on these small platforms.