olikraus / u8g2

U8glib library for monochrome displays, version 2
Other
5.1k stars 1.05k forks source link

Allow use of C++ semantics #2258

Open PhilippMolitor opened 1 year ago

PhilippMolitor commented 1 year ago

Background: https://github.com/olikraus/u8g2/discussions/2252

Current Status

The U8g2 library is based around a C core library, that implements the actual functionality of U8g2 in object-oriented C. Additionally, the library provides a thin C++ wrapper around that C core library for convenience.

Some of the C++ semantics do not work with this C++ layer though. Consider this example, which showcases what works and what doesn't.

uint8_t cb_test_a(mui_t* ui, uint8_t msg) {
  // ...
}

class UIManager {
 private:
  // LCD
  U8G2Class m_lcd = U8G2Class(U8G2_R0);
  MUIU8G2 m_menuConfig;

  fds_t m_menuContent[256] =
      // clang-format off
      MUI_FORM(0)
      // ... more U8g2 MUI stuff
      // clang-format on
      ;
  muif_t m_menuData[256] = {
      // A callback function just like from the manual. Works!
      MUIF_RO("TA", cb_test_a),

      // A lambda function, no scoped variables. Works!
      MUIF_RO("TB",
              [](mui_t* ui, uint8_t msg) {
                // do something...
                return (uint8_t)0;
              }),

      // A class method. Does not work!
      MUIF_RO("TC", m_menuConfig.leaveForm),

      // A lambda function, passes `this` as reference. Does not work!
      MUIF_RO("TD",
              [&](mui_t* ui, uint8_t msg) {
                this->m_menuConfigRedraw = true;
                return (uint8_t)0;
              }),
      //
  };

  // state
  bool m_menuConfigRedraw = true;

 public:
  void begin() {
    // LCD init
    m_lcd.begin();
    m_menuConfig.begin(m_lcd, m_menuContent, m_menuData,
                       sizeof(m_menuData) / sizeof(muif_t));
    m_menuConfig.gotoForm(1, 0);
  }

  void updateDisplayMenu() {
    if (!m_menuConfigRedraw)
      return;

    m_lcd.firstPage();
    do {
      m_lcd.setFont(u8g2_font_helvR08_tr);
      m_menuConfig.draw();
    } while (m_lcd.nextPage());

    m_menuConfigRedraw = false;
  }

  void updateDisplayIdleScreen() {
    m_lcd.firstPage();
    do {
      m_lcd.clearBuffer();
      m_lcd.drawRFrame(2, 2, 124, 60, 2);

      m_lcd.setFont(u8g2_font_helvR08_tr);
      m_lcd.setCursor(20, 50);
      m_lcd.print(millis());

    } while (m_lcd.nextPage());
  }

  void updateDisplay() {
    if (m_menuConfig.isFormActive()) {
      updateDisplayMenu();
    } else {
      updateDisplayIdleScreen();
    }
  }

Ideas for solving the issue

The current implementation defines the call back with:

typedef uint8_t (*muif_cb)(mui_t *ui, uint8_t msg);

You suggested to use a friend function, which I think is not very elegant for a few reasons:

A modern C++ approach probably would choose the std::function type:

#include <functional>

typedef std::function<uint8_t(mui_t *ui, uint8_t msg)> muif_cb;

A good practical example of that is Espressif's implementation of attachInterrupt() in their Arduino Core: https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/FunctionalInterrupt.h

olikraus commented 1 year ago

I don't understand how your suggestion with std::function can solve the class member and this arg example. I mean, I am sure that also the Espressif attachIterrupt does not allow to use class member functions, or did I miss something here.

PhilippMolitor commented 1 year ago

For me, using a lambda function works fine in this example:

attachInterrupt(somePin, [&]() { someMemberFunction(); }, CHANGE);

But there also seems to be another solution suggested by maxgerhardt:

attachInterrupt(somePin, std::bind(&MyClassName::someMemberFunction, this), CHANGE);

(see https://community.platformio.org/t/how-to-assign-a-class-method-as-an-interrupt-handler/20501/2)

olikraus commented 1 year ago

The "bind" approach seems to be promising, but you need a valid "this" pointer while building the MUIF array.

https://docs.w3cub.com/cpp/utility/functional/bind

PhilippMolitor commented 1 year ago

I added the MUIF array to the member variables, just like in the example. Doesn't seem like a bad thing to me. In this scope, it should have access to the this operator (or is this just the case for member functions?). Personally, I prefer the lambda function approach for readability (and maybe executing stuff there that should not be executed by a member function), but I guess that is just a personal style perference.

PhilippMolitor commented 1 year ago

I am currently testing the implementation of std::function.

This is what i changed in mui.h:

#include <functional>
typedef std::function<uint8_t(mui_t* ui, uint8_t msg)> muif_cb;
// typedef uint8_t (*muif_cb)(mui_t* ui, uint8_t msg);

Although my project code now accepts a lambda function with a reference to this, the compiler does not like the #include <functional>, as this std libary is not available in the AVR toolchain.

As I am compiling for an ESP32 with the PlatformIO system, it seems like the U8g2 library itself chooses that compiler. I have to investigate further to make this run and then test if my changes work on the running MCU.

If #include <functional> is no option, maybe we could borrow an implementation from the embedded template library: https://github.com/ETLCPP/etl/blob/master/include/etl/function.h

vortigont commented 5 months ago

not being able to use functional callbacks or class members is a bit frustrating :( As alternative solution it is possible to use additional void* arg parameter in callback function type. Then it could be used to pass a pointer to this and typecasting it to required class pointer. For example this way it is used in RTOS C callback prototypes which allows to bind tasks, timers, etc with a member functions of c++ class instances. It's highly compatible and stays within C. But this would require changing types for all of mui_u8g2_*'s, plus whole a lot of macros. Plus expand struct muif_struct, struct mui_u8g2_list_struct with void* arg member. Plus it's a braking change for existing code... ugh...

// mui.h
typedef uint8_t (*muif_cb)(mui_t *ui, uint8_t msg, void* arg);

// then somewhere in the class
class MyClass {
  uint8_t myvar;
  void dosomething(uint8_t msg);

  muif_t muif_main_configuration_menu_list[16] = {
    // ...
    MUIF_RO("HR", [](mui_t* ui, uint8_t msg, void* self){ static_cast<MyClass*>(self)->dosomething(msg); }, this),
};

  fds_t mymenu[10] =
      // A non-capturing lambda function 
      MUIF_RO("AB",
              [](mui_t* ui, uint8_t msg, void* self) {
                static_cast<MyClass*>(self)->myvar = 42;
                return (uint8_t)0;
              }),
    // in case of static function this arg must be NULL'ed
    MUIF_RO("HR", mui_hrule, NULL),                       // draw horizontal line on display
  };
olikraus commented 5 months ago

Well, yes "mui" is truly minimal. I wanted something small... I am sure there are better UI libs out there ;-)

vortigont commented 5 months ago

to my surprise most of them lack functional callbacks either :) Your one is really nice to have bundled with main u8g2 lib. Maybe there could be a compromise solution? For example having typedef std::function for callback function at least for those architectures where it is available. Some #ifdef's would not be much of a problem?

olikraus commented 5 months ago

Any solution should allow standard C compiler so handle mui code. But I am happy to included PRs if this precondition is met.

vortigont commented 4 months ago

I scratched my head for weeks trying to work-around it and somehow pass a reference to this into MUI's callback struct so not to bring std::function dependency. But one way or the the other it has to change the structs with additional void pointers and function arguments. So I guess it would not be a feasible option to keep backward compatibility with all the platforms u8g2 supports. MUI is pretty compact and works just fine for it's purpose. So I gave up the idea and re-implemented MUI-like lib for C++ using pure u8g2's back-end. Looks bulky but allows full dynamic menu elements with func callbacks and capturing lambda's and integrates nicely into user class instances. It's just proof of concept, but works.