tonyp7 / esp32-wifi-manager

Captive Portal for ESP32 that can connect to a saved wireless network or start an access point where you can connect to existing wifis.
MIT License
664 stars 217 forks source link

Request to Create the wifi_manager_event_group On wifi_manager_start() #51

Closed qcarver closed 4 years ago

qcarver commented 5 years ago

Hello,

Thank you for your useful contribution.

In my application I do not use the callback feature, instead I read bits from the wifi_manager_event_group.

There is a race condition where if I read from the group before wifi_manager's task is started I will crash trying to read a group that hasn't been created yet.

Why not just create it in the very first method, the start method?

` void wifi_manager_start(){

/* disable the default wifi logging */
esp_log_level_set("wifi", ESP_LOG_NONE);

//move to here
wifi_manager_event_group = xEventGroupCreate();

`

Also, recommend that you initialize it to zero at the top of the file :)

Thanks Again, QC

tonyp7 commented 5 years ago

Good catch. You are right all objects should be created in wifi_manager_start to avoid threading related issues later on. Flagging as bug for fixing later!

tonyp7 commented 5 years ago

fixed in branch https://github.com/tonyp7/esp32-wifi-manager/tree/issue%2351

haven't tested it yet but should be fairly straightforward. Will merge after testing.

qcarver commented 5 years ago

Okay, thanks for the heads up. V/R, QC

On Mon, Sep 2, 2019 at 8:36 AM tonyp7 notifications@github.com wrote:

fixed in branch https://github.com/tonyp7/esp32-wifi-manager/tree/issue%2351

haven't tested it yet but should be fairly straightforward. Will merge after testing.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tonyp7/esp32-wifi-manager/issues/51?email_source=notifications&email_token=AAK5TK7MLDKL6SUSMCLUXPTQHUCFLA5CNFSM4IOYHDZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5VWNXQ#issuecomment-527132382, or mute the thread https://github.com/notifications/unsubscribe-auth/AAK5TK57VL5HTF5WSWO2RN3QHUCFLANCNFSM4IOYHDZA .

tonyp7 commented 4 years ago

https://github.com/tonyp7/esp32-wifi-manager/pull/59 fixes the issue -- closing