Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
3 tasks done
michael-drury opened this issue Aug 16, 2023 · 5 comments · May be fixed by #341
Open
3 tasks done

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

michael-drury opened this issue Aug 16, 2023 · 5 comments · May be fixed by #341

Comments

@michael-drury
Copy link

michael-drury commented Aug 16, 2023 •
edited
Loading

Answers checklist.

  • I have read the Rainmaker documentation and the issue is not addressed there.
  • 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.
  • 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.
@github-actions github-actions bot changed the title Rainmaker doen't stop/ deinit Rainmaker doen't stop/ deinit (MEGH-4882) Aug 16, 2023
@michael-drury michael-drury changed the title Rainmaker doen't stop/ deinit (MEGH-4882) Rainmaker doesn't stop/ deinit Aug 16, 2023
@michael-drury michael-drury changed the title Rainmaker doesn't stop/ deinit Rainmaker doesn't stop/ deinit (MEGH-4882) Aug 18, 2023
@Josh-TapNoa
Copy link

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
Copy link
Collaborator

@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
Copy link

Josh-TapNoa commented Sep 14, 2023 •
edited
Loading

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
Copy link
Collaborator

shahpiyushv commented Sep 20, 2023 •
edited
Loading

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

@Josh-TapNoa
Copy link

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.

@Asc91 Asc91 linked a pull request Nov 8, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Feature/rmaker init deinit (MEGH-6105) Asc91/esp-rainmaker
3 participants
@shahpiyushv @michael-drury @Josh-TapNoa