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.

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

Fix the race condition that happens when in LOAD_AND_RESTORE mode and a user tries to configure a new network. #135

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ if(IDF_VERSION_MAJOR GREATER_EQUAL 4)
idf_component_register(SRC_DIRS src
REQUIRES log nvs_flash mdns wpa_supplicant lwip esp_http_server
INCLUDE_DIRS src
EMBED_FILES src/style.css src/code.js src/index.html)
EMBED_FILES src/web/style.css src/web/code.js src/web/index.html)
else()
set(COMPONENT_SRCDIRS src)
set(COMPONENT_ADD_INCLUDEDIRS src)
set(COMPONENT_REQUIRES log nvs_flash mdns wpa_supplicant lwip esp_http_server)
set(COMPONENT_EMBED_FILES src/style.css src/code.js src/index.html)
set(COMPONENT_EMBED_FILES src/web/style.css src/web/code.js src/web/index.html)
register_component()
endif()
6 changes: 6 additions & 0 deletions Kconfig
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
menu "Wifi Manager Configuration"

config WIFI_MANAGER_DNS_SERVER_ENABLE
bool "Enable the DNS server if set"
default y
help
Enables the DNS Server when set to true.

config WIFI_MANAGER_TASK_PRIORITY
int "RTOS Task Priority for the wifi_manager"
default 5
Expand Down
2 changes: 1 addition & 1 deletion component.mk
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
COMPONENT_ADD_INCLUDEDIRS = src
COMPONENT_SRCDIRS = src
COMPONENT_DEPENDS = log esp_http_server
COMPONENT_EMBED_FILES := src/style.css src/code.js src/index.html
COMPONENT_EMBED_FILES := src/web/style.css src/web/code.js src/web/index.html
File renamed without changes
File renamed without changes
File renamed without changes
File renamed without changes
File renamed without changes
File renamed without changes
2 changes: 0 additions & 2 deletions src/connect

This file was deleted.

File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
43 changes: 38 additions & 5 deletions src/wifi_manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,19 @@ Contains the freeRTOS task and all necessary support
#include "lwip/err.h"
#include "lwip/netdb.h"
#include "lwip/ip4_addr.h"
#include "sdkconfig.h"


#include "json.h"
#include "dns_server.h"
#include "nvs_sync.h"
#include "wifi_manager.h"


Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too sure about this. It adds nothing useful. The build system will always define CONFIG_WIFI_MANAGER_DNS_SERVER_ENABLE

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should define the build time variables as per wifi_manager.h which is best practice. No need to define a new global byte.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have to validate, from what I remember when set to 'n' `CONFIG_WIFI_MANAGER_DNS_SERVER_ENABLE' was simply undefined. That why I check if it's defined or not.

You're right though, this should be in wifi_manager.h.

#ifdef CONFIG_WIFI_MANAGER_DNS_SERVER_ENABLE
uint8_t dns_server_enable = 1;
#else
uint8_t dns_server_enable = 0;
#endif

/* objects used to manipulate the main queue of events */
QueueHandle_t wifi_manager_queue;
Expand All @@ -77,6 +82,7 @@ SemaphoreHandle_t wifi_manager_json_mutex = NULL;
SemaphoreHandle_t wifi_manager_sta_ip_mutex = NULL;
char *wifi_manager_sta_ip = NULL;
uint16_t ap_num = MAX_AP_NUM;
uint16_t ap_users = 0;
wifi_ap_record_t *accessp_records;
char *accessp_json = NULL;
char *ip_info_json = NULL;
Expand Down Expand Up @@ -685,6 +691,14 @@ static void wifi_manager_event_handler(void* arg, esp_event_base_t event_base, i
* to do something, for example, to get the info of the connected STA, etc. */
case WIFI_EVENT_AP_STACONNECTED:
ESP_LOGI(TAG, "WIFI_EVENT_AP_STACONNECTED");
xEventGroupSetBits(wifi_manager_event_group, WIFI_MANAGER_AP_STA_CONNECTED_BIT);

if(xTimerIsTimerActive(wifi_manager_retry_timer) == pdTRUE)
xTimerStop(wifi_manager_retry_timer, (TickType_t)0);

xEventGroupClearBits(wifi_manager_event_group, WIFI_MANAGER_REQUEST_RESTORE_STA_BIT);
ap_users += 1;
ESP_LOGI(TAG, "WIFI_EVENT_AP_STACONNECTED, connected users: %d", ap_users);
break;

/* This event can happen in the following scenarios:
Expand All @@ -695,6 +709,17 @@ static void wifi_manager_event_handler(void* arg, esp_event_base_t event_base, i
* something, e.g., close the socket which is related to this station, etc. */
case WIFI_EVENT_AP_STADISCONNECTED:
ESP_LOGI(TAG, "WIFI_EVENT_AP_STADISCONNECTED");
if (ap_users > 0){
ap_users -= 1;
}
if (ap_users == 0) {
xEventGroupClearBits(wifi_manager_event_group, WIFI_MANAGER_AP_STA_CONNECTED_BIT);
EventBits_t uxBits = xEventGroupGetBits(wifi_manager_event_group);
if(! (uxBits & WIFI_MANAGER_WIFI_CONNECTED_BIT) ) {
wifi_manager_send_message(WM_ORDER_LOAD_AND_RESTORE_STA, NULL);
}
}
ESP_LOGI(TAG, "WIFI_EVENT_AP_STADISCONNECTED: Remaining connected users: %d", ap_users);
break;

/* This event is disabled by default. The application can enable it via API esp_wifi_set_event_mask().
Expand Down Expand Up @@ -1171,7 +1196,7 @@ void wifi_manager( void * pvParameters ){
/* start SoftAP */
wifi_manager_send_message(WM_ORDER_START_AP, NULL);
}
else{
else if (! (uxBits & WIFI_MANAGER_AP_STA_CONNECTED_BIT)){
/* lost connection ? */
if(wifi_manager_lock_json_buffer( portMAX_DELAY )){
wifi_manager_generate_ip_info_json( UPDATE_LOST_CONNECTION );
Expand Down Expand Up @@ -1200,6 +1225,8 @@ void wifi_manager( void * pvParameters ){
wifi_manager_send_message(WM_ORDER_START_AP, NULL);
}
}
} else {
ESP_LOGI(TAG, "Disconnected");
}

/* callback */
Expand All @@ -1218,7 +1245,9 @@ void wifi_manager( void * pvParameters ){
http_app_start(true);

/* start DNS */
dns_server_start();
if(dns_server_enable) {
dns_server_start();
}

/* callback */
if(cb_ptr_arr[msg.code]) (*cb_ptr_arr[msg.code])(NULL);
Expand All @@ -1240,7 +1269,9 @@ void wifi_manager( void * pvParameters ){
esp_wifi_set_mode(WIFI_MODE_STA);

/* stop DNS */
dns_server_stop();
if(dns_server_enable) {
dns_server_stop();
}

/* restart HTTP daemon */
http_app_stop();
Expand Down Expand Up @@ -1283,7 +1314,9 @@ void wifi_manager( void * pvParameters ){
else { abort(); }

/* bring down DNS hijack */
dns_server_stop();
if(dns_server_enable) {
dns_server_stop();
}

/* start the timer that will eventually shutdown the access point
* We check first that it's actually running because in case of a boot and restore connection
Expand Down