From 89b9182a6161d2ab2b0d436e7496f38250124edc Mon Sep 17 00:00:00 2001 From: philip Date: Mon, 26 Dec 2016 16:19:34 -0500 Subject: [PATCH 1/6] Fix the error callback from not being called sometimes --- app/modules/mqtt.c | 41 ++++++++++++++++++++++++++++++----------- docs/en/modules/mqtt.md | 5 +++++ 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/app/modules/mqtt.c b/app/modules/mqtt.c index c9813af2b3..b8bd5363aa 100644 --- a/app/modules/mqtt.c +++ b/app/modules/mqtt.c @@ -42,10 +42,14 @@ typedef struct mqtt_event_data_t uint16_t data_offset; } mqtt_event_data_t; +#define RECONNECT_OFF 0 +#define RECONNECT_POSSIBLE 1 +#define RECONNECT_ON 2 + typedef struct mqtt_state_t { uint16_t port; - int auto_reconnect; + uint8_t auto_reconnect; // 0 is not auto_reconnect. 1 is auto reconnect, but never connected. 2 is auto reconnect, but once connected mqtt_connect_info_t* connect_info; uint16_t message_length; uint16_t message_length_read; @@ -106,7 +110,7 @@ static void mqtt_socket_disconnected(void *arg) // tcp only } } - if(mud->mqtt_state.auto_reconnect){ + if(mud->mqtt_state.auto_reconnect == RECONNECT_ON) { mud->pesp_conn->reverse = mud; mud->pesp_conn->type = ESPCONN_TCP; mud->pesp_conn->state = ESPCONN_NONE; @@ -153,7 +157,7 @@ static void mqtt_socket_reconnected(void *arg, sint8_t err) mud->event_timeout = 0; // no need to count anymore - if(mud->mqtt_state.auto_reconnect){ + if(mud->mqtt_state.auto_reconnect == RECONNECT_ON) { pesp_conn->proto.tcp->remote_port = mud->mqtt_state.port; pesp_conn->proto.tcp->local_port = espconn_port(); socket_connect(pesp_conn); @@ -166,6 +170,8 @@ static void mqtt_socket_reconnected(void *arg, sint8_t err) { espconn_disconnect(pesp_conn); } + mqtt_connack_fail(mud, MQTT_CONN_FAIL_SERVER_NOT_FOUND); + mqtt_socket_disconnected(arg); } NODE_DBG("leave mqtt_socket_reconnected.\n"); @@ -543,6 +549,9 @@ static void mqtt_socket_connected(void *arg) if(mud == NULL) return; mud->connected = true; + if (mud->mqtt_state.auto_reconnect == RECONNECT_POSSIBLE) { + mud->mqtt_state.auto_reconnect == RECONNECT_ON; + } espconn_regist_recvcb(pesp_conn, mqtt_socket_received); espconn_regist_sentcb(pesp_conn, mqtt_socket_sent); espconn_regist_disconcb(pesp_conn, mqtt_socket_disconnected); @@ -603,6 +612,16 @@ void mqtt_socket_timer(void *arg) NODE_DBG("Can not connect to broker.\n"); os_timer_disarm(&mud->mqttTimer); mqtt_connack_fail(mud, MQTT_CONN_FAIL_SERVER_NOT_FOUND); +#ifdef CLIENT_SSL_ENABLE + if(mud->secure) + { + espconn_secure_disconnect(mud->pesp_conn); + } + else +#endif + { + espconn_disconnect(mud->pesp_conn); + } } else if(mud->connState == MQTT_CONNECT_SENDING){ // MQTT_CONNECT send time out. NODE_DBG("sSend MQTT_CONNECT failed.\n"); mud->connState = MQTT_INIT; @@ -779,7 +798,7 @@ static int mqtt_socket_client( lua_State* L ) mud->connect_info.keepalive = keepalive; mud->mqtt_state.pending_msg_q = NULL; - mud->mqtt_state.auto_reconnect = 0; + mud->mqtt_state.auto_reconnect = RECONNECT_OFF; mud->mqtt_state.port = 1883; mud->mqtt_state.connect_info = &mud->connect_info; @@ -924,7 +943,7 @@ static sint8 socket_dns_found(const char *name, ip_addr_t *ipaddr, void *arg) { dns_reconn_count++; if( dns_reconn_count >= 5 ){ - NODE_ERR( "DNS Fail!\n" ); + NODE_DBG( "DNS Fail!\n" ); // Note: should delete the pesp_conn or unref self_ref here. struct espconn *pesp_conn = arg; @@ -938,7 +957,7 @@ static sint8 socket_dns_found(const char *name, ip_addr_t *ipaddr, void *arg) mqtt_socket_disconnected(arg); // although not connected, but fire disconnect callback to release every thing. return -1; } - NODE_ERR( "DNS retry %d!\n", dns_reconn_count ); + NODE_DBG( "DNS retry %d!\n", dns_reconn_count ); host_ip.addr = 0; return espconn_gethostbyname(pesp_conn, name, &host_ip, socket_dns_foundcb); } @@ -968,7 +987,7 @@ static int mqtt_socket_connect( lua_State* L ) ip_addr_t ipaddr; const char *domain; int stack = 1; - unsigned secure = 0, auto_reconnect = 0; + unsigned secure = 0, auto_reconnect = RECONNECT_OFF; int top = lua_gettop(L); sint8 espconn_status; @@ -1054,11 +1073,11 @@ static int mqtt_socket_connect( lua_State* L ) { auto_reconnect = lua_tointeger(L, stack); stack++; - if ( auto_reconnect != 0 && auto_reconnect != 1 ){ - auto_reconnect = 0; // default to 0 + if ( auto_reconnect != RECONNECT_OFF && auto_reconnect != RECONNECT_POSSIBLE ){ + auto_reconnect = RECONNECT_OFF; // default to 0 } } else { - auto_reconnect = 0; // default to 0 + auto_reconnect = RECONNECT_OFF; // default to 0 } mud->mqtt_state.auto_reconnect = auto_reconnect; @@ -1128,7 +1147,7 @@ static int mqtt_socket_close( lua_State* L ) return 1; } - mud->mqtt_state.auto_reconnect = 0; // stop auto reconnect. + mud->mqtt_state.auto_reconnect = RECONNECT_OFF; // stop auto reconnect. sint8 espconn_status = ESPCONN_CONN; if (mud->connected) { diff --git a/docs/en/modules/mqtt.md b/docs/en/modules/mqtt.md index 403777fd8d..3c543e1dad 100644 --- a/docs/en/modules/mqtt.md +++ b/docs/en/modules/mqtt.md @@ -96,6 +96,11 @@ Connects to the broker specified by the given host, port, and secure options. #### Returns `true` on success, `false` otherwise +#### Notes + +When `autoreconnect` is set, then the connection will be re-established when it breaks. No error indication will be given. However, if the +very first connection fails, then no reconnect attempt is made, and the error is signalled through the callback (if any). + #### Connection failure callback reason codes: | Constant | Value | Description | From b8a4132ab3393e3a7ad58f3f81e9b0e0a72ca80e Mon Sep 17 00:00:00 2001 From: philip Date: Tue, 27 Dec 2016 09:23:39 -0500 Subject: [PATCH 2/6] Moved the setting of the reconnect status to after the connack is recevied --- app/modules/mqtt.c | 6 +++--- docs/en/modules/mqtt.md | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/modules/mqtt.c b/app/modules/mqtt.c index b8bd5363aa..c2833ede2a 100644 --- a/app/modules/mqtt.c +++ b/app/modules/mqtt.c @@ -336,6 +336,9 @@ static void mqtt_socket_received(void *arg, char *pdata, unsigned short len) } else { mud->connState = MQTT_DATA; NODE_DBG("MQTT: Connected\r\n"); + if (mud->mqtt_state.auto_reconnect == RECONNECT_POSSIBLE) { + mud->mqtt_state.auto_reconnect = RECONNECT_ON; + } if(mud->cb_connect_ref == LUA_NOREF) break; if(mud->self_ref == LUA_NOREF) @@ -549,9 +552,6 @@ static void mqtt_socket_connected(void *arg) if(mud == NULL) return; mud->connected = true; - if (mud->mqtt_state.auto_reconnect == RECONNECT_POSSIBLE) { - mud->mqtt_state.auto_reconnect == RECONNECT_ON; - } espconn_regist_recvcb(pesp_conn, mqtt_socket_received); espconn_regist_sentcb(pesp_conn, mqtt_socket_sent); espconn_regist_disconcb(pesp_conn, mqtt_socket_disconnected); diff --git a/docs/en/modules/mqtt.md b/docs/en/modules/mqtt.md index 3c543e1dad..67d2b06529 100644 --- a/docs/en/modules/mqtt.md +++ b/docs/en/modules/mqtt.md @@ -91,7 +91,7 @@ Connects to the broker specified by the given host, port, and secure options. - `secure` 0/1 for `false`/`true`, default 0. Take note of constraints documented in the [net module](net.md). - `autoreconnect` 0/1 for `false`/`true`, default 0 - `function(client)` callback function for when the connection was established -- `function(client, reason)` callback function for when the connection could not be established +- `function(client, reason)` callback function for when the connection could not be established. No further callbacks should be called. #### Returns `true` on success, `false` otherwise @@ -99,7 +99,9 @@ Connects to the broker specified by the given host, port, and secure options. #### Notes When `autoreconnect` is set, then the connection will be re-established when it breaks. No error indication will be given. However, if the -very first connection fails, then no reconnect attempt is made, and the error is signalled through the callback (if any). +very first connection fails, then no reconnect attempt is made, and the error is signalled through the callback (if any). The first connection +is considered a success if the client connects to a server and gets back a good response packet in response to its MQTT connection request. +This implies (for example) that the username and password are correct. #### Connection failure callback reason codes: From 61a1ec867ec4dc66b99021aa054f0eca7676efe6 Mon Sep 17 00:00:00 2001 From: philip Date: Sun, 22 Jan 2017 15:01:09 -0500 Subject: [PATCH 3/6] Increase the irom0_seg size --- ld/nodemcu.ld | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ld/nodemcu.ld b/ld/nodemcu.ld index a27affa9a0..d2136862b3 100644 --- a/ld/nodemcu.ld +++ b/ld/nodemcu.ld @@ -5,7 +5,7 @@ MEMORY dport0_0_seg : org = 0x3FF00000, len = 0x10 dram0_0_seg : org = 0x3FFE8000, len = 0x14000 iram1_0_seg : org = 0x40100000, len = 0x8000 - irom0_0_seg : org = 0x40210000, len = 0xD0000 + irom0_0_seg : org = 0x40210000, len = 0xE0000 } PHDRS From afcb257e8916949d67b740d48a8cfbd817a1df4e Mon Sep 17 00:00:00 2001 From: philip Date: Mon, 23 Jan 2017 08:38:55 -0500 Subject: [PATCH 4/6] Formatting changes --- app/modules/mqtt.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/modules/mqtt.c b/app/modules/mqtt.c index c2833ede2a..579a65538d 100644 --- a/app/modules/mqtt.c +++ b/app/modules/mqtt.c @@ -1,5 +1,5 @@ // Module for mqtt - +// #include "module.h" #include "lauxlib.h" #include "platform.h" @@ -170,6 +170,7 @@ static void mqtt_socket_reconnected(void *arg, sint8_t err) { espconn_disconnect(pesp_conn); } + mqtt_connack_fail(mud, MQTT_CONN_FAIL_SERVER_NOT_FOUND); mqtt_socket_disconnected(arg); @@ -293,7 +294,7 @@ static void mqtt_socket_received(void *arg, char *pdata, unsigned short len) switch(mud->connState){ case MQTT_CONNECT_SENDING: case MQTT_CONNECT_SENT: - mud->event_timeout = 0; + mud->event_timeout = 0; if(mqtt_get_type(in_buffer) != MQTT_MSG_TYPE_CONNACK){ NODE_DBG("MQTT: Invalid packet\r\n"); From ff27f2e82e40bef49a827762db933d25ea6dd505 Mon Sep 17 00:00:00 2001 From: philip Date: Mon, 23 Jan 2017 20:57:11 -0500 Subject: [PATCH 5/6] Updated the documentation --- docs/en/modules/mqtt.md | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/docs/en/modules/mqtt.md b/docs/en/modules/mqtt.md index ba2c34c400..4c98104258 100644 --- a/docs/en/modules/mqtt.md +++ b/docs/en/modules/mqtt.md @@ -18,7 +18,7 @@ Creates a MQTT client. - `keepalive` keepalive seconds - `username` user name - `password` user password -- `cleansession` 0/1 for `false`/`true` +- `cleansession` 0/1 for `false`/`true`. Default is 1 (`true`). #### Returns MQTT client @@ -101,10 +101,31 @@ Connects to the broker specified by the given host, port, and secure options. #### Notes -When `autoreconnect` is set, then the connection will be re-established when it breaks. No error indication will be given. However, if the -very first connection fails, then no reconnect attempt is made, and the error is signalled through the callback (if any). The first connection -is considered a success if the client connects to a server and gets back a good response packet in response to its MQTT connection request. -This implies (for example) that the username and password are correct. +Don't use `autoreconnect`. Let me repeat that, don't use `autoreconnect`. You should handle the errors explicitly and appropriately for +your application. In particular, the default for `cleansession` above is `true`, so all subscriptions are destroyed when the connection +is lost for any reason. + +In order to acheive a consistent connection, handle errors in the error callback. For example: + +``` +function handle_mqtt_error(client, reason) + tmr.create():alarm(10 * 1000, tmr.ALARM_SINGLE, do_mqtt_connect) +end + +function do_mqtt_connect() + mqtt:connect("server", function(client) print("connected") end, handle_mqtt_error) +end +``` + +In reality, the connected function should do something useful! + +This is the description of how the `autoreconnect` functionality may (or may not) work. + +> When `autoreconnect` is set, then the connection will be re-established when it breaks. No error indication will be given (but all the +> subscriptions may be lost if `cleansession` is true). However, if the +> very first connection fails, then no reconnect attempt is made, and the error is signalled through the callback (if any). The first connection +> is considered a success if the client connects to a server and gets back a good response packet in response to its MQTT connection request. +> This implies (for example) that the username and password are correct. #### Connection failure callback reason codes: From bd7ef86e21ba2367fb286a567959a0c0345b1f96 Mon Sep 17 00:00:00 2001 From: philip Date: Mon, 23 Jan 2017 20:58:50 -0500 Subject: [PATCH 6/6] Make it clearer that autoreconnect is deprecated --- docs/en/modules/mqtt.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/modules/mqtt.md b/docs/en/modules/mqtt.md index 4c98104258..e7635c1773 100644 --- a/docs/en/modules/mqtt.md +++ b/docs/en/modules/mqtt.md @@ -92,7 +92,7 @@ Connects to the broker specified by the given host, port, and secure options. - `host` host, domain or IP (string) - `port` broker port (number), default 1883 - `secure` 0/1 for `false`/`true`, default 0. Take note of constraints documented in the [net module](net.md). -- `autoreconnect` 0/1 for `false`/`true`, default 0 +- `autoreconnect` 0/1 for `false`/`true`, default 0. This option is *deprecated*. - `function(client)` callback function for when the connection was established - `function(client, reason)` callback function for when the connection could not be established. No further callbacks should be called.