espressif / esp-mqtt

ESP32 mqtt component
Apache License 2.0
591 stars 254 forks source link

Not compatible with CPP, IDF 5.0 (IDFGH-9334) #248

Closed DEADSEC-SECURITY closed 1 year ago

DEADSEC-SECURITY commented 1 year ago

So I'm currently developing a program for my ESP32 that uses mqtt but unfortunately the library doesn't seem to integrate well with CPP.

The problem: I created a mqtt event for new data inside my cpp class using a non-class function as a callback passing this as an extra event arg, but if I keep the type esp_event_handler_t defined as:

typedef void         (*esp_event_handler_t)(void*event_handler_arg,
                                        esp_event_base_t event_base,
                                        int32_t event_id,
                                        void* event_data); /**< function called when an event is posted to the queue */

I get an error from CPP compiler saying that esp_mqtt_event_handle_t event = event_data; is permissive with the following error: error: invalid conversion from 'void*' to 'esp_mqtt_event_handle_t' {aka 'esp_mqtt_event_t*'} [-fpermissive].

But it doesn't end here because if I try to set a type in the header then as it requests I get another error: error: invalid conversion from 'void (*)(void*, esp_event_base_t, int32_t, esp_mqtt_event_handle_t)' {aka 'void (*)(void*, const char*, long int, esp_mqtt_event_t*)'} to 'esp_event_handler_t' {aka 'void (*)(void*, const char*, long int, void*)'} [-fpermissive] basically stating that I cannot use types different from the typedef.

Basically this error cannot be fixed unless I restructure the typedef but then I cannot use this cuz C doesn't have class types, which I would need to accept this as an extra arg, and it would trigger a cycle import which would be another error. This being said there is a solution that is to use functional and make it cpp with c linkage but thats more of a bandage then a proper fix so any tips i would appreciate.

Anyway if my idea is ok to use functional so it can be used in CPP, I would be more then welcome to make a PR but since this is a C lib there is maybe a better way to keep it all C.

Function being called to register event: esp_mqtt_client_register_event(client, MQTT_EVENT_DATA, mqtt_publish_handler, this);

Handler function:

static void mqtt_publish_handler(void *config, esp_event_base_t base, int32_t event_id, void *event_data) {
    esp_mqtt_event_handle_t event = event_data;
    esp_mqtt_client_handle_t client = event->client;
}
euripedesrocha commented 1 year ago

Hi @DEADSEC-SECURITY, could you share a bigger snippet? I believe that it's likely that you need to rewrite your typedef. We also have espressif/esp-protocols#189 that adds a C++ version of esp-mqtt API. It's under review now, and your inputs are welcomed. You can take a look on the approach we took there.

DEADSEC-SECURITY commented 1 year ago

Well after testing making a custom type when its CPP doesn't seem to work, at least this way, because esp_event_handler_t is not defined.

#ifdef __cplusplus
/**< function called when an event is posted to the queue */
#include "mqtt_client.h"

typedef void         (*esp_event_handler_t)(void*event_handler_arg,
                                            esp_event_base_t event_base,
                                            int32_t event_id,
                                            esp_event_handler_t *event_data);
#else
/**< function called when an event is posted to the queue */
typedef void         (*esp_event_handler_t)(void*event_handler_arg,
                                        esp_event_base_t event_base,
                                        int32_t event_id,
                                        void* event_data);
#endif
DEADSEC-SECURITY commented 1 year ago

Yes @euripedesrocha

static void mqtt_publish_handler(void *config, esp_event_base_t base, int32_t event_id, esp_event_handler_t event_data) {
    esp_mqtt_event_handle_t event = event_data;
    esp_mqtt_client_handle_t client = event->client;
    char *topic = (char*) malloc(event->topic_len + 1);
    strcpy(topic, event->topic);
    topic[event->topic_len + 1] = *"\0";

    if (strcmp(topic, config->get_set_topic()->c_str()) != 0) {free(topic); return;}
    free(topic);

    char *data = (char*) malloc(event->data_len + 1);
    strcpy(data, event->data);
    data[event->data_len + 1] = *"\0";

    str stringData = data;
    free(data);
    config->callback(data);
}

BaseConfig::BaseConfig(const str& name, const str& uniqueId, str component, esp_mqtt_client_handle_t client, Callback callback) : client(client), callback(std::move(callback)) {
    this->component = std::move(component);

    // Create and populate config
    this->config = cJSON_CreateObject();
    this->add_config("name", name);
    this->add_config("uniq_id", uniqueId);
    this->add_config("obj_id", uniqueId);

    this->gen_config_topic();
    this->gen_state_topic();
    this->gen_set_topic();

    esp_mqtt_client_register_event(client, MQTT_EVENT_DATA, mqtt_publish_handler, this);
}

So thats besically the logic.

Every time the object is created it registers a new data event in the client, when the event callback is triggered it checks if the topics match and if they do it calls the object callback.

euripedesrocha commented 1 year ago

HI @DEADSEC-SECURITY Thanks for the extra info. I was reading your issue again and I understand your problem. You are trying to automatically cast from void*and the compiler is correctly saying that it's an invalid conversion. You need to static_cast it explicitly. I will again invite you to take a look on the c++ version of esp-mqtt and give your inputs about your needs and if that component solves your problem.

DEADSEC-SECURITY commented 1 year ago

Im currently testing the cxx version in my program. Thanks