Improve OTA speed, fix status JSON buffer overflow

components/squeezelite-ota/component.mk

@@ -7,5 +7,5 @@
 COMPONENT_ADD_INCLUDEDIRS := .
 COMPONENT_ADD_INCLUDEDIRS += include
 COMPONENT_EXTRA_INCLUDES += $(PROJECT_PATH)/main/
-CFLAGS += -DLOG_LOCAL_LEVEL=ESP_LOG_INFO -DCONFIG_OTA_ALLOW_HTTP=1
+CFLAGS += -DLOG_LOCAL_LEVEL=ESP_LOG_DEBUG -DCONFIG_OTA_ALLOW_HTTP=1
 COMPONENT_EMBED_TXTFILES :=  ${PROJECT_PATH}/server_certs/github.pem

components/squeezelite-ota/squeezelite-ota.c

@@ -79,11 +79,12 @@ void triggerStatusJsonRefresh(bool bDelay,const char * status, ...){
 	wifi_manager_refresh_ota_json();
 	if(bDelay){
 		ESP_LOGD(TAG,"Holding task...");
-	    vTaskDelay(700 / portTICK_PERIOD_MS);  // wait here for a short amount of time.  This will help with refreshing the UI status
+	    vTaskDelay(200 / portTICK_PERIOD_MS);  // wait here for a short amount of time.  This will help with refreshing the UI status
 		ESP_LOGD(TAG,"Done holding task...");
 	}
 	else
 	{
+		ESP_LOGI(TAG,"%s",ota_status.status_text);
 		taskYIELD();
 	}
 }
@@ -161,19 +162,17 @@ esp_err_t _http_event_handler(esp_http_client_event_t *evt)
         }
         break;
     case HTTP_EVENT_ON_DATA:
-
-    	vTaskDelay(5/ portTICK_RATE_MS);
     	if(!ota_status.bOTAStarted)  {
     		ESP_LOGD(TAG, "HTTP_EVENT_ON_DATA, status_code=%d, len=%d",esp_http_client_get_status_code(evt->client), evt->data_len);
     	}
     	else if(ota_status.bOTAStarted && esp_http_client_get_status_code(evt->client) == 200 ){
 			ota_status.ota_actual_len+=evt->data_len;
-			if(ota_get_pct_complete()%2 == 0) ota_status.newpct = ota_get_pct_complete();
+			if(ota_get_pct_complete()%5 == 0) ota_status.newpct = ota_get_pct_complete();
 			if(ota_status.lastpct!=ota_status.newpct )
 			{
 				gettimeofday(&tv, NULL);
 				uint32_t elapsed_ms= (tv.tv_sec-ota_status.OTA_start.tv_sec )*1000+(tv.tv_usec-ota_status.OTA_start.tv_usec)/1000;
-				ESP_LOGI(TAG,"OTA chunk : %d bytes (%d of %d) (%d pct), %d KB/s", evt->data_len, ota_status.ota_actual_len, ota_status.ota_total_len, ota_status.newpct, elapsed_ms>0?ota_status.ota_actual_len*1000/elapsed_ms/1024:0);
+				ESP_LOGI(TAG,"OTA progress : %d/%d (%d pct), %d KB/s", ota_status.ota_actual_len, ota_status.ota_total_len, ota_status.newpct, elapsed_ms>0?ota_status.ota_actual_len*1000/elapsed_ms/1024:0);
 				wifi_manager_refresh_ota_json();
 				ota_status.lastpct=ota_status.newpct;
 			}
@@ -194,7 +193,7 @@ esp_err_t init_config(esp_http_client_config_t * conf, const char * url){
 
 	conf->cert_pem =cert==NULL?(char *)server_cert_pem_start:cert;
 	conf->event_handler = _http_event_handler;
-	conf->buffer_size = 2048;
+	conf->buffer_size = 2048*2;
 	conf->disable_auto_redirect=true;
 	conf->skip_cert_common_name_check = false;
 	conf->url = strdup(url);
@@ -275,6 +274,7 @@ void ota_task(void *pvParameter)
 	ota_status.bRedirectFound=false;
 	if(passedURL==NULL || strlen(passedURL)==0){
 		ESP_LOGE(TAG,"HTTP OTA called without a url");
+		triggerStatusJsonRefresh(true,"Updating needs a URL!");
 		ota_status.bOTAThreadStarted=false;
 		vTaskDelete(NULL);
 		return ;
@@ -349,7 +349,7 @@ esp_err_t process_recovery_ota(const char * bin_url){
 #endif
     ESP_LOGI(TAG, "Starting ota on core %u for : %s", OTA_CORE,urlPtr);
 
-    ret=xTaskCreatePinnedToCore(&ota_task, "ota_task", 1024*20, (void *)urlPtr, ESP_TASK_MAIN_PRIO-1, NULL, OTA_CORE);
+    ret=xTaskCreatePinnedToCore(&ota_task, "ota_task", 1024*20, (void *)urlPtr, ESP_TASK_MAIN_PRIO+1, NULL, OTA_CORE);
     if (ret != pdPASS)  {
             ESP_LOGI(TAG, "create thread %s failed", "ota_task");
             return ESP_FAIL;

components/wifi-manager/wifi_manager.c

@@ -58,6 +58,11 @@ Contains the freeRTOS task and all necessary support
 #include "esp_app_format.h"
 #include "driver/gpio.h"
 #include "driver/adc.h"
+#include "CJson.h"
+
+#ifndef RECOVERY_APPLICATION
+#define RECOVERY_APPLICATION 0
+#endif
 
 #ifndef SQUEEZELITE_ESP32_RELEASE_URL
 #define SQUEEZELITE_ESP32_RELEASE_URL "https://github.com/sle118/squeezelite-esp32/releases"
@@ -79,6 +84,7 @@ uint16_t ap_num = MAX_AP_NUM;
 wifi_ap_record_t *accessp_records;
 char *accessp_json = NULL;
 char *ip_info_json = NULL;
+cJSON * ip_info_cjson=NULL;
 wifi_config_t* wifi_manager_config_sta = NULL;
 static update_reason_code_t last_update_reason_code=0;
 
@@ -166,8 +172,8 @@ void wifi_manager_start(){
 	accessp_records = (wifi_ap_record_t*)malloc(sizeof(wifi_ap_record_t) * MAX_AP_NUM);
 	accessp_json = (char*)malloc(MAX_AP_NUM * JSON_ONE_APP_SIZE + 4); /* 4 bytes for json encapsulation of "[\n" and "]\0" */
 	wifi_manager_clear_access_points_json();
-	ip_info_json = (char*)malloc(sizeof(char) * JSON_IP_INFO_SIZE);
-	wifi_manager_clear_ip_info_json();
+	ip_info_json = NULL;
+	ip_info_cjson = wifi_manager_clear_ip_info_json(&ip_info_cjson);
 	wifi_manager_config_sta = (wifi_config_t*)malloc(sizeof(wifi_config_t));
 	memset(wifi_manager_config_sta, 0x00, sizeof(wifi_config_t));
 	memset(&wifi_settings.sta_static_ip_config, 0x00, sizeof(tcpip_adapter_ip_info_t));
@@ -370,102 +376,58 @@ bool wifi_manager_fetch_wifi_sta_config(){
 }
 
 
-void wifi_manager_clear_ip_info_json(){
-	strcpy(ip_info_json, "{}\n");
+cJSON * wifi_manager_get_new_json(cJSON **old){
+	cJSON * root=*old;
+	if(root!=NULL){
+	    cJSON_Delete(root);
+	    *old=NULL;
+	}
+	 return cJSON_CreateObject();
+}
+cJSON * wifi_manager_clear_ip_info_json(cJSON **old){
+	 cJSON *root = wifi_manager_get_new_json(old);
+ 	 cJSON_AddItemToObject(root, "message", cJSON_CreateString("Initializing"));
+ 	 return root;
 }
 
 
 void wifi_manager_generate_ip_info_json(update_reason_code_t update_reason_code){
 	wifi_config_t *config = wifi_manager_get_wifi_sta_config();
+	ip_info_cjson = wifi_manager_get_new_json(&ip_info_cjson);
 	if(update_reason_code == UPDATE_OTA) {
 		update_reason_code = last_update_reason_code;
 	}
-	else
-	{
+	else {
 		last_update_reason_code = update_reason_code;
 	}
+	const esp_app_desc_t* desc = esp_ota_get_app_description();
+	cJSON_AddItemToObject(ip_info_cjson, "project_name", cJSON_CreateString(desc->project_name));
+	cJSON_AddItemToObject(ip_info_cjson, "version", cJSON_CreateString(desc->version));
+	cJSON_AddNumberToObject(ip_info_cjson,"recovery",	RECOVERY_APPLICATION	);
+	cJSON_AddNumberToObject(ip_info_cjson, "urc", update_reason_code);
 	if(config){
-#if RECOVERY_APPLICATION
-				const char ip_info_json_format[] = ",\"ip\":\"%s\",\"netmask\":\"%s\",\"gw\":\"%s\",\"urc\":%d,\"project_name\":\"%s\",\"version\":\"%s\", \"ota_dsc\":\"%s\", \"ota_pct\":%u,\"recovery\": 1,\"Jack\" : \"%s\",  \"Voltage\" : %.2f }\n";
-#else
-				const char ip_info_json_format[] = ",\"ip\":\"%s\",\"netmask\":\"%s\",\"gw\":\"%s\",\"urc\":%d,\"project_name\":\"%s\",\"version\":\"%s\",\"recovery\": 0,\"Jack\" : \"%s\",  \"Voltage\" : %.2f }\n";
-#endif
-		memset(ip_info_json, 0x00, JSON_IP_INFO_SIZE);
-		const esp_app_desc_t* desc = esp_ota_get_app_description();
-		/* to avoid declaring a new buffer we copy the data directly into the buffer at its correct address */
-		strcpy(ip_info_json, "{\"ssid\":");
-		json_print_string(config->sta.ssid,  (unsigned char*)(ip_info_json+strlen(ip_info_json)) );
+		cJSON_AddItemToObject(ip_info_cjson, "ssid", cJSON_CreateString((char *)config->sta.ssid));
+
 		if(update_reason_code == UPDATE_CONNECTION_OK){
 			/* rest of the information is copied after the ssid */
 			tcpip_adapter_ip_info_t ip_info;
 			ESP_ERROR_CHECK(tcpip_adapter_get_ip_info(TCPIP_ADAPTER_IF_STA, &ip_info));
-			char ip[IP4ADDR_STRLEN_MAX]; /* note: IP4ADDR_STRLEN_MAX is defined in lwip */
-			char gw[IP4ADDR_STRLEN_MAX];
-			char netmask[IP4ADDR_STRLEN_MAX];
-			strcpy(ip, ip4addr_ntoa(&ip_info.ip));
-			strcpy(netmask, ip4addr_ntoa(&ip_info.netmask));
-			strcpy(gw, ip4addr_ntoa(&ip_info.gw));
-
-			snprintf( (ip_info_json + strlen(ip_info_json)), JSON_IP_INFO_SIZE, ip_info_json_format,
-					ip,
-					netmask,
-					gw,
-					(int)update_reason_code,
-					desc->project_name,
-					desc->version,
-
-
-#if RECOVERY_APPLICATION
-					ota_get_status(),
-					 ota_get_pct_complete(),
-#endif
-					 JACK_LEVEL,
-					 adc1_get_raw(ADC1_CHANNEL_7) / 4095. * (10+174)/10. * 1.1
-			);
-		}
-		else{
-			/* notify in the json output the reason code why this was updated without a connection */
-			snprintf( (ip_info_json + strlen(ip_info_json)), JSON_IP_INFO_SIZE, ip_info_json_format,
-								"0",
-								"0",
-								"0",
-								(int)update_reason_code,
-								desc->project_name,
-								desc->version,
-
-#if RECOVERY_APPLICATION
-								"",
-								0,
-#endif
-					 JACK_LEVEL,
-					 adc1_get_raw(ADC1_CHANNEL_7) / 4095. * (10+174)/10. * 1.1
-);
+			cJSON_AddItemToObject(ip_info_cjson, "ip", cJSON_CreateString(ip4addr_ntoa(&ip_info.ip)));
+			cJSON_AddItemToObject(ip_info_cjson, "netmask", cJSON_CreateString(ip4addr_ntoa(&ip_info.netmask)));
+			cJSON_AddItemToObject(ip_info_cjson, "gw", cJSON_CreateString(ip4addr_ntoa(&ip_info.gw)));
 		}
 	}
-	else{
-
-#if RECOVERY_APPLICATION
-				const char ip_info_json_format[] = ",\"project_name\":\"%s\",\"version\":\"%s\", \"ota_dsc\":\"%s\", \"ota_pct\":%d,\"Jack\" : \"%s\",  \"Voltage\" : %.2f }\n";
 
-#else
-				const char ip_info_json_format[] = ",\"project_name\":\"%s\",\"version\":\"%s\",\"Jack\" : \"%s\",  \"Voltage\" : %.2f }\n";
-#endif
 
 
-		memset(ip_info_json, 0x00, JSON_IP_INFO_SIZE);
-		const esp_app_desc_t* desc = esp_ota_get_app_description();
-		/* to avoid declaring a new buffer we copy the data directly into the buffer at its correct address */
-		snprintf( (ip_info_json + strlen(ip_info_json)), JSON_IP_INFO_SIZE, ip_info_json_format,
-		desc->project_name,
-		desc->version,
 #if RECOVERY_APPLICATION
-				ota_get_status(),
-					 ota_get_pct_complete(),
+	cJSON_AddItemToObject(ip_info_cjson, "ota_dsc", cJSON_CreateString(ota_get_status()));
+	cJSON_AddNumberToObject(ip_info_cjson,"ota_pct",	ota_get_pct_complete()	);
 #endif
-					 JACK_LEVEL,
-					 adc1_get_raw(ADC1_CHANNEL_7) / 4095. * (10+174)/10. * 1.1
-			);
-	}
+
+	cJSON_AddItemToObject(ip_info_cjson, "Jack", cJSON_CreateString(JACK_LEVEL));
+	cJSON_AddNumberToObject(ip_info_cjson,"Voltage",	adc1_get_raw(ADC1_CHANNEL_7) / 4095. * (10+174)/10. * 1.1);
+
 }
 
 
@@ -653,7 +615,7 @@ void wifi_manager_connect_async(){
 	 * it's a remnant from a previous connection
 	 */
 	if(wifi_manager_lock_json_buffer( portMAX_DELAY )){
-		wifi_manager_clear_ip_info_json();
+		wifi_manager_clear_ip_info_json(&ip_info_cjson);
 		wifi_manager_unlock_json_buffer();
 	}
 	wifi_manager_send_message(ORDER_CONNECT_STA, (void*)CONNECTION_REQUEST_USER);
@@ -661,7 +623,7 @@ void wifi_manager_connect_async(){
 
 
 char* wifi_manager_get_ip_info_json(){
-	return ip_info_json;
+	return cJSON_Print(ip_info_cjson);
 }
 
 void wifi_manager_destroy(){
@@ -674,7 +636,8 @@ void wifi_manager_destroy(){
 	free(accessp_json);
 	accessp_json = NULL;
 	free(ip_info_json);
-	ip_info_json = NULL;
+	cJSON_Delete(ip_info_cjson);
+	ip_info_cjson=NULL;
 	free(wifi_manager_sta_ip);
 	wifi_manager_sta_ip = NULL;
 	if(wifi_manager_config_sta){
@@ -889,12 +852,6 @@ void wifi_manager( void * pvParameters ){
 				if(wifi_manager_lock_json_buffer( portMAX_DELAY )){
 					wifi_manager_generate_ip_info_json( UPDATE_OTA );
 					wifi_manager_unlock_json_buffer();
-#if RECOVERY_APPLICATION
-					ESP_LOGI(TAG,"Refresh from OTA status: %s - flash percent: %d%% ",
-					ota_get_status(),
-					 ota_get_pct_complete());
-#endif
-
 				}
 				break;
 

components/wifi-manager/wifi_manager.h

@@ -40,6 +40,7 @@ extern "C" {
 #include "esp_wifi.h"
 #include "esp_wifi_types.h"
 #include "squeezelite-ota.h"
+#include "cJSON.h"
 
 #ifndef RECOVERY_APPLICATION
 #error "RECOVERY_APPLICATION not defined. Defaulting to squeezelite"
@@ -172,18 +173,7 @@ extern "C" {
  */
 #define JSON_ONE_APP_SIZE					99
 
-/**
- * @brief Defines the maximum length in bytes of a JSON representation of the IP information
- * assuming all ips are 4*3 digits, and all characters in the ssid require to be escaped.
- * example: {"ssid":"abcdefghijklmnopqrstuvwxyz012345","ip":"192.168.1.119","netmask":"255.255.255.0","gw":"192.168.1.1","urc":0, "ota_dsc":"Installing...", "ota_pct":100}
- */
-#if RECOVERY_APPLICATION
-// recovery has more resources available. Let's use them to include more details about the OTA process
-#define JSON_IP_INFO_SIZE 					150+255
-#else
-// 40 chars for appname and version
-#define JSON_IP_INFO_SIZE 					150+40
-#endif
+
 
 
 /**
@@ -363,8 +353,8 @@ void wifi_manager_generate_ip_info_json(update_reason_code_t update_reason_code)
  * @brief Clears the connection status json.
  * @note This is not thread-safe and should be called only if wifi_manager_lock_json_buffer call is successful.
  */
-void wifi_manager_clear_ip_info_json();
-
+cJSON * wifi_manager_clear_ip_info_json(cJSON **old);
+cJSON * wifi_manager_get_new_json(cJSON **old);
 /**
  * @brief Generates the list of access points after a wifi scan.
  * @note This is not thread-safe and should be called only if wifi_manager_lock_json_buffer call is successful.