Look into the scheduling of lamba function with captures as another option

[davetcc]

We need to look at how lambda captures work and how we can pass a lambda with a captured argument to task manager.

There are two phases here:

1. understand how lambda's with captured arguments are implemented in C++ and what performance, memory visibility and memory overhead is involved, based on this the support may only be enabled on larger boards.
2. build out an implementation that provides this support.

[afoeder]

Stumbled upon this one; is this why my code at https://github.com/afoeder/Somfy_Remote/blob/2d0a154fd3dbc646dfd8dcc6693c7388357a66cf/src/main.cpp#L113-L123 doesn't work? I am very new to c++ and therefore not very proficient, so any input is appreciated. Thank you!

[maxgerhardt]

If change

https://github.com/davetcc/TaskManagerIO/blob/af2568f942636515a2765aa9cd9082fd4be172cd/src/TaskTypes.h#L119

to

#include <functional>
typedef std::function<void()> TimerFn;

and change

https://github.com/davetcc/TaskManagerIO/blob/82ec77f87c0d86e119ea0e4e45decb6ca797db00/src/TaskTypes.h#L165-L169

to

TimerFn callback;
Executable *taskRef;
BaseEvent *eventRef;

(otherwise there's an error that std::function has a non-trivial destructor and cannot be used in that union), that gives the possibility of using arbitrary lambdas in as the scheduling expression.

Test code

#include <Arduino.h>
#include <TaskManagerIO.h>

class MyClass {
private:
    String m_name;
public:
    MyClass(String name){
        this->m_name = name;
    }
    void doSomethingWithParam(String param) {
        Serial.println(this->m_name + " says: " + param);
    }

    void doSchedule() {
        String myParam = "Hello, World!";
        /* schedule execution of member function with some param */
        taskManager.scheduleFixedRate(1000, [this, myParam]() {
            this->doSomethingWithParam(myParam);
        }, TIME_MILLIS);
    }
};

MyClass myObj("test");

void setup() {
    Serial.begin(115200);
    myObj.doSchedule();
}

void loop() {
  taskManager.runLoop();
}

gives output

test says: Hello, World!
test says: Hello, World!

every second.

So that works nicely.

Without that, one can just use pure function pointers to void x() functions, since that is what is defined as TimerFn. If objects are involved some sort of mechanism has to be used there to call the right object then, with those restrictions.

This of course only works for those compilers supporting std::function. The ARM, ESP8266 and ESP32 compilers do, but not e.g. the AVR one.

You can e.g. try the forked library version by using

lib_deps = 
   https://github.com/maxgerhardt/TaskManagerIO/archive/refs/heads/master.zip

instead of the old TaskManagerIO directive in the platformio.ini.

[davetcc]

Stumbled upon this one; is this why my code at https://github.com/afoeder/Somfy_Remote/blob/2d0a154fd3dbc646dfd8dcc6693c7388357a66cf/src/main.cpp#L113-L123 doesn't work?

Yes, this is the issue, you could try the fix suggested by Max above as long as you're on a platform that supports it. I'll look to integrate something shortly into the library on a board-by-board basis, with the possibility of an override for where we can't guess if std::function is available.

This of course only works for those compilers supporting std::function. The ARM, ESP8266 and ESP32 compilers do, but not e.g. the AVR one.

Agreed, we already have platform determination anyway that does much of this to handle the spin-waiting and CAS. We'll try and guess if std::function is available based on that with an override definition in case we get it wrong.

[davetcc]

Many thanks go to @maxgerhardt for doing the testing on this. It did turn out to be a small task as you suspected. For now, I will enable it for ESP*, mbed, and SAMD21. As these seem to be the boards that support it.

I'm in the middle of testing tcMenu 2.0 and an EepromAbstraction for STM32 battery-backed RAM anyway, so it's a good time to integrate this. I've now tested on STM32 mbed, once I've tested on ESP, AVR, and SAMD then I'll commit it.

[maxgerhardt]

Commit looks very good. The new 1.2.0 version should soon be picked up in https://platformio.org/lib/show/11063/TaskManagerIO/installation for normal usage :)

[davetcc]

Yes, these days I've moved off the legacy auto-deploy, but I'll release it shortly, I just want to do a bit more testing of it first.

It would be good if you can double-check the way I clear down the callback in void TimerTask::clear() because I'm fairly sure it will run the destructor before copy-assignment of the empty function, but a second opinion would be good. The rationale for this clear down is that without it, the task may hold on to a quite heavy function object (with captures etc).

[maxgerhardt]

It would be good if you can double-check the way I clear down the callback in void TimerTask::clear() because I'm fairly sure it will run the destructor before copy-assignment of the empty function, but a second opinion would be good.

I've compiled your commit and looked at the disassembly with debug info and the function, for a ARM Bluepill board, and it compiles and decompiles to

void __fastcall TimerTask::clear(TimerTask *const this)
{
  TimerTask *pThis; // r4@1
  Executable *v2; // r0@2
  int p_temp_callback; // [sp+0h] [bp-30h]@4
  int v4; // [sp+8h] [bp-28h]@4
  std::function<void()> empty_callback; // [sp+10h] [bp-20h]@4

  pThis = this;
  if ( this->executeMode & 8 )
  {
    v2 = this->_anon_0.taskRef;
    if ( v2 )
      (*((void (**)(void))v2->_vptr.Executable + 2))();
  }
  pThis->_anon_0.taskRef = 0;
  v4 = 0;
  empty_callback.baseclass_1._M_manager = 0;
  std::function<void ()(void)>::swap((std::function<void()> *const )&p_temp_callback, &empty_callback);
  std::function<void ()(void)>::swap(&empty_callback, &pThis->callback);
  std::_Function_base::~_Function_base(&empty_callback.baseclass_1);
  std::_Function_base::~_Function_base((std::_Function_base *const )&p_temp_callback);
  pThis->scheduledAt = 0;
  pThis->timingInformation = 2;
  pThis->next = 0;
  pThis->taskInUse = 0;
}

so the compiler optimizes this to a std::swap with two temporary variables, basically this->callback is exchanged with clean new std::function<void()> object and then the destructor is called on the old object. So it looks okay.

A nitpick is though that i've noticed that it didn't allow me to use lambdas with a configuration of

[env:bluepill]
platform = ststm32
board = bluepill_f103c8
framework = arduino
lib_deps = https://github.com/davetcc/TaskManagerIO.git

unless I specified

build_flags = -DTM_ALLOW_CAPTURED_LAMBDA

the detection code seems to be a bit too strict here. I'd recommend using this feature to detect if <functional> is available.

#if __has_include(<functional>)
# define TM_ALLOW_CAPTURED_LAMBDA
#endif

works in TaskPlatformDeps.h in place of the other checks for it pretty well.

[davetcc]

If this is universal it would be great. I'll try it on a few boards later to confirm. However, it doesn't look like ESP8266 gcc is new enough though (as I think GCC 5 is needed from the docs):

% ./xtensa-lx106-elf-c++ -v gcc version 4.8.2 (GCC)

Otherwise, for ARM, maybe I could pick up on a CMSIS level flag/definition instead of the one I'm using now just for SAMD.

[maxgerhardt]

Ahh indeed, 4.8.2 may be too low. Hm, this could have been nicely universal if it wasn't for that.

Per this one can check __GNUC__ for >= 5 and do other checks in the else branch otherwise. I think the ESP8266 is also the only one using this low of a GCC version though.

But for the ESP8266 it worked with also the non-modified version, so maybe a

#if __has_include(<functional>)
# define TM_ALLOW_CAPTURED_LAMBDA
#endif

is always good and then keep the old enable checks (while checking for #ifndef TM_ALLOW_CAPTURED_LAMBDA) and we get the superposition of it.

[davetcc]

Thanks again for looking into this, I'll use the GNUC check as that seems universal. There's just so many board possibilities it's difficult to know if another will be stuck on an old version.

[davetcc]

Just pushed a new version now, I think this is the most flexible pretty much as described above. Could you test it with the same board and see if it works now.

[davetcc]

Will be in next release being generated shortly

[davetcc]

I think we may need to look at putting this behind a feature switch, to ensure it is not enabled by default, even on larger boards. Many in the embedded community do not like using the new operator, and as far as I can see, this is using new to allocate the space for the captures so it's not even something that can be easily controlled or understood.

Many users will not use the capture support, and they are paying the cost of it anyway, I think we should go the same way as mbed and enable it behind a flag: TM_ENABLE_CAPTURED_LAMBDAS. Which will only enable it if the board can support it. I don't like changing existing logic, but this an exceptional case where we are putting a weight onto everyone who's using it.

In a future version, we may also by default go to using mbed's callback if it can be done without any effect on the compilation of existing projects. as this seems to be a good compromise.

[davetcc]

To enable capture just define TM_ENABLE_CAPTURED_LAMBDAS in your compiler options. This has a runtime footprint.

Ensure you fully understand the memory semantics associated with the capture and that the capture is not overly large or frequently re-evaluated.