invalid use of non-static member function

[Andrew Z]

hello,
im attempting to simplify the C code for arduino/idf framework.
the setup_task function worked well until i introduced the Zap class. And now it is failing with :
[code]

rc/main.cpp: In function 'void setup()':
src/main.cpp:112:115: error: invalid use of non-static member function
setup_task(outlet.OutletToggleTask, (char*)"Outlet Task", eventGroup, outlet.OutletCallback, outlet.GetCycleTicks);
^
In file included from src/main.cpp:19:0:

[/code]

I appreciate the guidance and any help since I clearly don't understand what is wrong. I found a similar thread here (https://forum.arduino.cc/index.php?topic=524167.0). But im not sure why there is a need to create such a complex hierarchy...

simplified code:
[code]
main.cpp
#include <Arduino.h>
...
#include <zap.h>
#define pdMS_TO_TICKS( xTimeInMs ) ( ( TickType_t ) ( xTimeInMs ) * ( configTICK_RATE_HZ / ( TickType_t ) 1000 ) )
const int TIME_BIT = BIT0 ;
#define DATA_PIN 4
#define CHANNEL 1


EventGroupHandle_t eventGroup;

void setup_task( void (*DeviceTask)(void*), char* TaskName, EventGroupHandle_t eventGroup,
void (*Callback_Fn)(TimerHandle_t), TickType_t cycle_ticks ) {};

void setup(){

eventGroup = xEventGroupCreate(); //check if NULL NOT returned.
Zap outlet(DATA_PIN, CHANNEL , 20000); // make a new outlet instance with
setup_task(outlet.OutletToggleTask, (char*)"Outlet Task", eventGroup, outlet.OutletCallback, outlet.GetCycleTicks); // <-- Failure here

....
}

zap.h
#ifndef _ZAP_H
#define _ZAP_H
#include <Outlet.h>

class Zap : public Outlet {
private:
int channel ; //= 1 ; //CHANNEL 1
const TickType_t t_outlet_cycle ;
const TickType_t t_outlet_cycle_ticks = 10000;
public:
Zap( int pin , int ch=1, TickType_t cycle_ms = 10000 ): Outlet(pin), channel {ch} , t_outlet_cycle {cycle_ms}
{
assert( cycle_ms >= 10000) ;

} ;

void OutletCallback(TimerHandle_t handle) ;
void OutletToggleTask( void *Params) ;
int GetChannel() { return channel ; } ;
TickType_t GetCycle () { return t_outlet_cycle ;} ;
TickType_t GetCycleTicks () { return t_outlet_cycle_ticks ;} ;
};

#endif // _ZAP_H

#include "zap.h"

#define BLUE_PIN GPIO_NUM_2
extern EventGroupHandle_t eventGroup;
extern int TIME_BIT ;

const bool ON = true;
const bool OFF = false;

void Zap::OutletCallback(TimerHandle_t handle) {
xEventGroupSetBits(eventGroup,TIME_BIT);
};

void Zap::OutletToggleTask( void *Params) {

EventBits_t bits;
...
while (1) {
bits = xEventGroupWaitBits(eventGroup,TIME_BIT,pdTRUE, pdFAIL,pdMS_TO_TICKS(15000));
if (bits == 1) {
xEventGroupClearBits(eventGroup,TIME_BIT);
....
}
}
};


outlet.h:

#ifndef Outlet_h
#define Outlet_h
#include "Arduino.h"

class Outlet {
public:
Outlet(int pin);
// toggles all outlets on the specified channel on(true) or off(false)
void toggle(int channel, bool state);
...
private:
int DATA_PIN;
...
};

#endif
[/code]

[Alf P. Steinbach]

Use `const char*` for C string that's not going to be changed.

It's UB to cast a literal to `char*` and the modify it.

> void setup(){
>
> eventGroup = xEventGroupCreate(); //check if NULL NOT returned.
> Zap outlet(DATA_PIN, CHANNEL , 20000); // make a new outlet instance with
> setup_task(outlet.OutletToggleTask, (char*)"Outlet Task", eventGroup, outlet.OutletCallback,

OutletToggleTask is a non-static member function.

As a non-static member function it takes a hidden `this` parameter,
which might not be passed in the same way as other parameters. Therefore
you can't convert it to pointer-to-free-function in any portable way.

Additionally the construct `outlet.OutletToggleTask` is a callable
expression that in itself has no type. It's silly in a very serious way:
the lack of a type for it is a core language deficiency that stands in
the way of easily using C++ for e.g. GUI code, or other code that
requires object oriented callbacks. C# has delegates for this, and the
C++ standard library has `std::function` and `std::bind`, but it really
requires core language support for ease of use.

One easy practical solution to your problem is to simply declare the
function `static` in the class, or make it a free function.


[snip]

- Alf

[Manfred]

There's a second problem, which is might be the one detected by the
compiler, although the above is definitely an error too.
The last argument of setup_task is expected to be an object of type
TickType_t, but a pointer to member function is passed instead:
"outlet.GetCycleTicks" should be "outlet.GetCycleTicks()"

[Andrew Z]

Manfred,
Alf,
thank you for the reply.

void setup_task(
void (*DeviceTask)(void*),
char* TaskName,
EventGroupHandle_t eventGroup,
void (*Callback_Fn)(TimerHandle_t),

TickType_t cycle_ticks ) { .. }
setup_task(

outlet.OutletToggleTask,
(char*)"Outlet Task",
eventGroup,
outlet.OutletCallback,
outlet.GetCycleTicks );

why is the outlet.GetCycleTicks is considered to be a pointer, but the eventGroup is not?

changing outlet.GetCycleTicks to outlet.GetCycleTicks() or a number doesn't change the compile time error. I'm still digesting what Alf said above...

[Andrew Z]

Alf,
thank you. im not sure i can use the "Static" for Toggle.
Toggle resets the bit in the eventgroup ( freertos ) and switches an instance of the outlet on/off.
<butchered>

void Zap::OutletToggleTask( void *Params) {

EventBits_t bits;

while (1) {
bits = xEventGroupWaitBits(eventGroup,TIME_BIT,pdTRUE, pdFAIL,pdMS_TO_TICKS(15000));
if (bits == 1) {

toggle(channel,_switch);
...
}
....
}
}

i started writing this code with free functions, when i realized that i can wrap all these functions into an objects and have the concise and clean code... well, best plans of mice and men...

not sure what to do....

[Manfred]

eventGroup is declared as

> EventGroupHandle_t eventGroup;

So, it is an object, not a pointer.

>
> changing outlet.GetCycleTicks to outlet.GetCycleTicks() or a number doesn't change the compile time error. I'm still digesting what Alf said above...

This is not surprising, all of OutletToggleTask, OutletCallback and
GetCycleTicks are non-static member functions that are used invalidly,
although for different reasons.

[Alf P. Steinbach]

It depends on whether the callback mechanism is your own or provided by
the API you're using (I'm not familiar with the Arduino environment).

If it's not your own then presumably the `void*` parameter of the
callback is intended to pass an object pointer. In that case you provide
that object pointer when you install the callback. And so you'll need to
pass that down to the code that installs the callback.


--------------------------------------------------------------------------
template< class T > using Type_ = T;

namespace c_style_api {
extern "C" using Callback = void( void* p_state, const char*
something );

extern "C"
void enumerate_something( const Type_<void*> p_state, const
Type_<Callback*> f )
{
f( p_state, "Look, it's " );
f( p_state, "calling back!" );
}
}

#include <string>
using std::string;

struct Something
{
int m_i;
string m_s;

void callback( Type_<const char*> s )
{
m_s += s;
}
};

extern "C"
void something_cb( const Type_<void*> p_state, const Type_<const char*> s )
{
reinterpret_cast<Something*>( p_state )->callback( s );
}

#include <iostream>
using std::cout, std::endl;
auto main()
-> int
{
Something smth = {};
c_style_api::enumerate_something( &smth, something_cb );
cout << smth.m_s << endl;
}
--------------------------------------------------------------------------


If it's your own callback, however, then you can simply switch from C
function pointer to C++ `function<void()>` (from the <functional>
header), and use a lambda that captures a reference to your object:


--------------------------------------------------------------------------
#include <functional>
using std::function;

template< class T > using Type_ = T;

namespace api {
using Callback = void( const char* something );

void enumerate_something( const function<Callback> f )
{
f( "Look, it's " );
f( "calling back!" );
}
}

#include <string>
using std::string;

struct Something
{
int m_i;
string m_s;

void callback( Type_<const char*> s )
{
m_s += s;
}
};

#include <iostream>
using std::cout, std::endl;
auto main()
-> int
{
Something smth = {};
api::enumerate_something( [&](auto s){ smth.callback( s ); } );
cout << smth.m_s << endl;
}


--------------------------------------------------------------------------


Or alternatively, in C++03 style you can use an interface with a known
virtual function. In that case your object's class needs to implement
the relevant interface.


--------------------------------------------------------------------------
template< class T > using Type_ = T;

namespace api {
// The pure interface class, C++03 style callback:
struct Event_handler{ virtual void on_some_event( const char*
something ) = 0; };

void enumerate_something( Event_handler& o )
{
o.on_some_event( "Look, it's " );
o.on_some_event( "calling back!" );
}
}

#include <string>
using std::string;

struct Something:
api::Event_handler
{
int m_i;
string m_s;

void on_some_event( Type_<const char*> s ) override
{
m_s += s;
}
};

#include <iostream>
using std::cout, std::endl;
auto main()
-> int
{
Something smth = {};
api::enumerate_something( smth );
cout << smth.m_s << endl;
}
--------------------------------------------------------------------------


- Alf

[Andrew Z]

On Wednesday, January 1, 2020 at 12:15:54 PM UTC-5, Manfred wrote:
> On 1/1/2020 7:32 AM, Andrew Z wrote:
> >>> OutletToggleTask is a non-static member function.

.....

> > Manfred,
> > Alf,
> > thank you for the reply.
> > void setup_task(
> > void (*DeviceTask)(void*),
> > char* TaskName,
> > EventGroupHandle_t eventGroup,
> > void (*Callback_Fn)(TimerHandle_t),
> > TickType_t cycle_ticks ) { .. }
> > setup_task(
> > outlet.OutletToggleTask,
> > (char*)"Outlet Task",
> > eventGroup,
> > outlet.OutletCallback,
> > outlet.GetCycleTicks );
> > why is the outlet.GetCycleTicks is considered to be a pointer, but the eventGroup is not?
>
> eventGroup is declared as
>
> > EventGroupHandle_t eventGroup;
>
> So, it is an object, not a pointer.
>
> >
> > changing outlet.GetCycleTicks to outlet.GetCycleTicks() or a number doesn't change the compile time error. I'm still digesting what Alf said above...
>
> This is not surprising, all of OutletToggleTask, OutletCallback and
> GetCycleTicks are non-static member functions that are used invalidly,
> although for different reasons.

Oh, i see now. The outlet.GetCycleTicks is the pointer to the function, the outlet.GetCycleTicks() is the value of the function.

thank you ery much!

[Andrew Z]

Alf,
thank you very much for the reply. It took me some time to learn all the concepts you used in it. And i'm thankful for that.

If i may step back for a moment and describe what im trying to achieve, maybe it will be better, than me asking to "fix my code" .

I'm dealing with a number of devices ( outlet, humidity, temp instruments) attached to "Arduino" board.

Each device wowuld use 2 routines from RTOS:
Task - https://www.freertos.org/a00125.html
Timer - https://www.freertos.org/FreeRTOS-timers-xTimerCreate.html

Timer is used to (re)set a flag;
Task is used to constantly pull the flag value and (once flag changed) activate the device - take a temp/hum measurement etc.

I certainly can add both routines ( task and timer) as part (member function) of the device's class.

My current idea is to have a routine to create a task and a timer per device - setup_task();

Two issues i have:

xTaskCreate( ... void *pvParameters... ) - thus i can pass "this" and you already explained how to go about that.

TimerHandle_t xTimerCreate
( ..., void * const pvTimerID, TimerCallbackFunction_t pxCallbackFunction );

has pvTimerID, that theoretically can be used to pass "this".

BUT the callback function (TimerCallbackFunction_t pxCallbackFunction) has a predetermine signature:
void vCallbackFunction( TimerHandle_t xTimer );

and this is where i'm uncertain how to proceed.