drivers: wifi: esp: Improvements to ESP driver

- Fix passive mode protocol selection depending on AT versions
- Use Kconfig value for reset timeout
- Fix bug with parsing security from scan result
- Re-order some AT commands during init due to some commands having
  dependency on other commands

Signed-off-by: Tobias Svehagen <tobias.svehagen@gmail.com>

drivers/wifi/esp/Kconfig.esp

@@ -42,13 +42,21 @@ config WIFI_ESP_WORKQ_THREAD_PRIORITY
 	help
 	  Priority of thread used for processing driver work queue items.
 
-config WIFI_ESP_PASSIVE_TCP
-	bool "Use passive TCP"
+config WIFI_ESP_PASSIVE_MODE
+	bool "Use passive mode"
 	help
 	  This lets the ESP handle the TCP window so that data can flow
 	  at a rate that the driver can handle. Without this, data might get
 	  lost if the driver cannot empty the device buffer quickly enough.
 
+config WIFI_ESP_RESET_TIMEOUT
+	int "Reset timeout"
+	default 3000
+	help
+	  How long to wait for device to become ready after AT+RST has been
+	  sent. This can vary with chipset (ESP8266/ESP32) and firmware
+	  version. This is ignored if a reset pin is configured.
+
 choice WIFI_ESP_AT_VERSION
 	prompt "AT version"
 	default WIFI_ESP_AT_VERSION_2_0

drivers/wifi/esp/esp.c

@@ -132,6 +132,7 @@ MODEM_CMD_DEFINE(on_cmd_cipstamac)
 	return 0;
 }
 
+/* +CWLAP:(sec,ssid,rssi,channel) */
 MODEM_CMD_DEFINE(on_cmd_cwlap)
 {
 	struct esp_data *dev = CONTAINER_OF(data, struct esp_data,
@@ -139,7 +140,7 @@ MODEM_CMD_DEFINE(on_cmd_cwlap)
 	struct wifi_scan_result res = { 0 };
 	int i;
 
-	i = strtol(argv[0], NULL, 10);
+	i = strtol(&argv[0][1], NULL, 10);
 	if (i == 0) {
 		res.security = WIFI_SECURITY_TYPE_NONE;
 	} else {
@@ -341,8 +342,8 @@ struct net_pkt *esp_prepare_pkt(struct esp_data *dev, struct net_buf *src,
 }
 
 /*
- * Passive TCP: "+IPD,<id>,<len>\r\n"
- * Other:       "+IPD,<id>,<len>:<data>"
+ * Passive mode: "+IPD,<id>,<len>\r\n"
+ * Other:        "+IPD,<id>,<len>:<data>"
  */
 #define MIN_IPD_LEN (sizeof("+IPD,I,LE") - 1)
 #define MAX_IPD_LEN (sizeof("+IPD,I,LLLLE") - 1)
@@ -384,9 +385,8 @@ MODEM_CMD_DIRECT_DEFINE(on_cmd_ipd)
 		goto out;
 	}
 
-	/* When using passive TCP, the +IPD command ends with \r\n */
-	if (IS_ENABLED(CONFIG_WIFI_ESP_PASSIVE_TCP) &&
-	    sock->ip_proto == IPPROTO_TCP) {
+	/* When using passive mode, the +IPD command ends with \r\n */
+	if (ESP_PROTO_PASSIVE(sock->ip_proto)) {
 		end = '\r';
 	} else {
 		end = ':';
@@ -416,8 +416,7 @@ MODEM_CMD_DIRECT_DEFINE(on_cmd_ipd)
 	 * When using passive TCP, the data itself is not included in the +IPD
 	 * command but must be polled with AT+CIPRECVDATA.
 	 */
-	if (IS_ENABLED(CONFIG_WIFI_ESP_PASSIVE_TCP) &&
-	    sock->ip_proto == IPPROTO_TCP) {
+	if (ESP_PROTO_PASSIVE(sock->ip_proto)) {
 		sock->bytes_avail = data_len;
 		k_work_submit_to_queue(&dev->workq, &sock->recvdata_work);
 		ret = data_offset;
@@ -466,6 +465,7 @@ MODEM_CMD_DEFINE(on_cmd_ready)
 {
 	struct esp_data *dev = CONTAINER_OF(data, struct esp_data,
 					    cmd_handler_data);
+	k_sem_give(&dev->sem_if_ready);
 
 	if (net_if_is_up(dev->net_iface)) {
 		net_if_down(dev->net_iface);
@@ -698,16 +698,16 @@ static void esp_init_work(struct k_work *work)
 		SETUP_CMD_NOHANDLE("AT"),
 		/* turn off echo */
 		SETUP_CMD_NOHANDLE("ATE0"),
+		SETUP_CMD_NOHANDLE("AT+UART_CUR="_UART_CUR),
+		SETUP_CMD_NOHANDLE("AT+"_CWMODE"=1"),
+		/* enable multiple socket support */
+		SETUP_CMD_NOHANDLE("AT+CIPMUX=1"),
+		/* only need ecn,ssid,rssi,channel */
+		SETUP_CMD_NOHANDLE("AT+CWLAPOPT=0,23"),
 #if defined(CONFIG_WIFI_ESP_AT_VERSION_2_0)
 		SETUP_CMD_NOHANDLE("AT+CWAUTOCONN=0"),
 #endif
-		SETUP_CMD_NOHANDLE("AT+UART_CUR="_UART_CUR),
-		/* enable multiple socket support */
-		SETUP_CMD_NOHANDLE("AT+CIPMUX=1"),
-		SETUP_CMD_NOHANDLE("AT+"_CWMODE"=1"),
-		/* only need ecn,ssid,rssi,channel */
-		SETUP_CMD_NOHANDLE("AT+CWLAPOPT=0,23"),
-#if defined(CONFIG_WIFI_ESP_PASSIVE_TCP)
+#if defined(CONFIG_WIFI_ESP_PASSIVE_MODE)
 		SETUP_CMD_NOHANDLE("AT+CIPRECVMODE=1"),
 #endif
 		SETUP_CMD("AT+"_CIPSTAMAC"?", "+"_CIPSTAMAC":",
@@ -753,8 +753,8 @@ static void esp_reset(struct esp_data *dev)
 
 	while (retries--) {
 		ret = modem_cmd_send(&dev->mctx.iface, &dev->mctx.cmd_handler,
-				     NULL, 0, "AT+RST", &dev->sem_response,
-				     K_MSEC(100));
+				     NULL, 0, "AT+RST", &dev->sem_if_ready,
+				     K_MSEC(CONFIG_WIFI_ESP_RESET_TIMEOUT));
 		if (ret == 0 || ret != -ETIMEDOUT) {
 			break;
 		}
@@ -801,6 +801,7 @@ static int esp_init(struct device *dev)
 
 	k_sem_init(&data->sem_tx_ready, 0, 1);
 	k_sem_init(&data->sem_response, 0, 1);
+	k_sem_init(&data->sem_if_ready, 0, 1);
 	k_sem_init(&data->sem_if_up, 0, 1);
 
 	k_work_init(&data->init_work, esp_init_work);

drivers/wifi/esp/esp.h

@@ -29,14 +29,35 @@ extern "C" {
 #define _CWJAP  "CWJAP_CUR"
 #define _CIPSTA "CIPSTA_CUR"
 #define _CIPSTAMAC "CIPSTAMAC_CUR"
+#define _CIPRECVDATA "+CIPRECVDATA,"
+#define _CIPRECVDATA_END ':'
 #else
 #define _CWMODE "CWMODE"
 #define _CWSAP  "CWSAP"
 #define _CWJAP  "CWJAP"
 #define _CIPSTA "CIPSTA"
 #define _CIPSTAMAC "CIPSTAMAC"
+#define _CIPRECVDATA "+CIPRECVDATA:"
+#define _CIPRECVDATA_END ','
 #endif
 
+/*
+ * Passive mode differs a bit between firmware versions and the macro
+ * ESP_PROTO_PASSIVE is therefore used to determine what protocol operates in
+ * passive mode. For AT version 1.7 passive mode only affects TCP but in AT
+ * version 2.0 it affects both TCP and UDP.
+ */
+#if defined(CONFIG_WIFI_ESP_PASSIVE_MODE)
+#if defined(CONFIG_WIFI_ESP_AT_VERSION_1_7)
+#define ESP_PROTO_PASSIVE(proto) (proto == IPPROTO_TCP)
+#else
+#define ESP_PROTO_PASSIVE(proto) \
+	(proto == IPPROTO_TCP || proto == IPPROTO_UDP)
+#endif /* CONFIG_WIFI_ESP_AT_VERSION_1_7 */
+#else
+#define ESP_PROTO_PASSIVE(proto) 0
+#endif /* CONFIG_WIFI_ESP_PASSIVE_MODE */
+
 #define ESP_BUS DT_BUS(DT_DRV_INST(0))
 
 #if DT_PROP(ESP_BUS, hw_flow_control) == 1
@@ -163,9 +184,10 @@ struct esp_data {
 
 	scan_result_cb_t scan_cb;
 
-	/* response semaphore */
+	/* semaphores */
 	struct k_sem sem_tx_ready;
 	struct k_sem sem_response;
+	struct k_sem sem_if_ready;
 	struct k_sem sem_if_up;
 };
 

drivers/wifi/esp/esp_offload.c

@@ -452,8 +452,9 @@ MODEM_CMD_DIRECT_DEFINE(on_cmd_ciprecvdata)
 	} else if (*endptr == 0) {
 		ret = -EAGAIN;
 		goto out;
-	} else if (*endptr != ':') {
-		LOG_ERR("Invalid end of cmd: 0x%02x != 0x%02x", *endptr, ':');
+	} else if (*endptr != _CIPRECVDATA_END) {
+		LOG_ERR("Invalid end of cmd: 0x%02x != 0x%02x", *endptr,
+			_CIPRECVDATA_END);
 		ret = len;
 		goto out;
 	}
@@ -493,7 +494,7 @@ static void esp_recvdata_work(struct k_work *work)
 	int len = CIPRECVDATA_MAX_LEN, ret;
 	char cmd[32];
 	struct modem_cmd cmds[] = {
-		MODEM_CMD_DIRECT("+CIPRECVDATA,", on_cmd_ciprecvdata),
+		MODEM_CMD_DIRECT(_CIPRECVDATA, on_cmd_ciprecvdata),
 	};
 
 	sock = CONTAINER_OF(work, struct esp_socket, recvdata_work);