Rainmaker doesn't stop/ deinit (MEGH-4882)

michael-drury commented 10 months ago

Answers checklist.

[x] I have read the Rainmaker documentation and the issue is not addressed there.
[X] I have updated my IDF branch (release/vX.Y) to the latest version and checked that the issue is present there. This is not applicable if you are using Rainmaker with Arduino.
[X] I have searched the Rainmaker forum and issue tracker for a similar issue and not found a similar issue.

IDF / ESP32-Arduino version.

v5.1

Operating System used.

Linux

How did you build your project?

Command line with idf.py

Development Kit.

ESP32-S2

What is the expected behavior?

Calling esp_rmaker_stop(), followed by esp_rmaker_node_deinit() stops any rainmaker processes from running.

What is the actual behavior?

Calling esp_rmaker_stop(), followed by esp_rmaker_node_deinit(), and then attempting to run esp_rmaker_node_init() results in the following error message.

 esp_rmaker_core: ESP RainMaker is still running. Please stop it first.

This is particularly frustrating as all our production code is guarded behind a strict set of unit tests. If it's impossible to teardown and deinit rainmaker, then we can't run tests on it, which leaves our production code exposed to bugs.

Steps to reproduce.

esp_rmaker_node_t *node = NULL;

bool Rainmaker_Init(const char *node_name, const char *node_type)
{
    ESP_LOGD(TAG, "Rainmaker init");
    esp_rmaker_config_t rainmaker_cfg = {
        .enable_time_sync = true,
    };
    node = esp_rmaker_node_init(&rainmaker_cfg, node_name, node_type);
    if (!node)
    {
        ESP_LOGE(TAG, "Could not initialise node");
        return false;
    }

    esp_rmaker_timezone_service_enable();

    return true;
}

void Rainmaker_Deinit(void)
{
    ESP_LOGD(TAG, "Rainmaker deinit");
    esp_rmaker_stop();

    app_insights_disable();
    esp_rmaker_node_deinit(node);
    node = NULL;
}

TEST_CASE("Rainmaker teardown success", "[rainmaker]")
{
    Rainmaker_Init("Test-node", "Test type");
    vTaskDelay(pdMS_TO_TICKS(1000);
    Rainmaker_Deinit();
    vTaskDelay(pdMS_TO_TICKS(1000);
    Rainmaker_Init("Test-node", "Test type");
}

Debug Logs.

E (76306) esp_rmaker_core: ESP RainMaker is still running. Please stop it first.

More Information.

I'm aware this issue is mentioned here https://www.esp32.com/viewtopic.php?f=41&t=34987&p=117909, but have not seen anywhere in the repo that is tracking progress on a bug fix for this.

Josh-TapNoa commented 9 months ago

Would be great if someone would pick this up, its currently impossible to deinit rainmaker -- which additionally makes it pretty impossible to write unit tests too!

shahpiyushv commented 9 months ago

@Josh-TapNoa , this has indeed been taken up and as the first step, the local control functions have been slightly changed so that it can be enabled and disabled via APIs. This would have been on GitHub already, but is stuck because of some compatibility issues with latest release/v5.1 branch if esp-idf. Rest of the stuff will also be in place soon.

Josh-TapNoa commented 9 months ago

In terms of deiniting, I have been using the following patch and seems to work (although feel free to tell me how it doesnt) :)

` esp_err_t esp_rmaker_node_deinit(const esp_rmaker_node_t *node) { if (!esp_rmaker_priv_data) { ESP_LOGE(TAG, "ESP RainMaker already de-initialized."); return ESP_ERR_INVALID_ARG; }

// if (esp_rmaker_priv_data->state != ESP_RMAKER_STATE_INIT_DONE) {
//     ESP_LOGE(TAG, "ESP RainMaker is still running. Please stop it first.");
//     return ESP_ERR_INVALID_STATE;
// }
esp_rmaker_node_delete(node);
esp_rmaker_priv_data->node = NULL;
esp_rmaker_deinit_priv_data(esp_rmaker_priv_data);
esp_rmaker_priv_data = NULL;
return ESP_OK;

} `

Given that you can only deinit if the state is ESP_RMAKER_STATE_INIT_DONE, I just commented out that block, and now it behaves as I expect!

shahpiyushv commented 9 months ago

This will de-initialise some components and reclaim memory, but not stop the services like mqtt, mdns, http server, etc.

Josh-TapNoa commented 5 months ago

Did we ever get to end of this?

Would be nice to be able to deinit the core of rainmaker to free up the mqtt, mdns and server services from running to avoid swaps during timing critical sections.

espressif / esp-rainmaker