Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
espconn: De-orbit espconn_gethostbyname
Browse files Browse the repository at this point in the history
Further work on nodemcu#3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.
nwf committed Feb 23, 2020
1 parent deceb3a commit 47258e9
Showing 6 changed files with 37 additions and 96 deletions.
9 changes: 4 additions & 5 deletions app/http/httpclient.c
Original file line number Diff line number Diff line change
@@ -563,21 +563,20 @@ void ICACHE_FLASH_ATTR http_raw_request( const char * hostname, int port, bool s
req->redirect_follow_count = redirect_follow_count;

ip_addr_t addr;
err_t error = espconn_gethostbyname( (struct espconn *) req, /* It seems we don't need a real espconn pointer here. */
hostname, &addr, http_dns_callback );
err_t error = dns_gethostbyname( hostname, &addr, http_dns_callback, req );

if ( error == ESPCONN_INPROGRESS )
if ( error == ERR_INPROGRESS )
{
HTTPCLIENT_DEBUG( "DNS pending" );
}
else if ( error == ESPCONN_OK )
else if ( error == ERR_OK )
{
/* Already in the local names table (or hostname was an IP address), execute the callback ourselves. */
http_dns_callback( hostname, &addr, req );
}
else
{
if ( error == ESPCONN_ARG )
if ( error == ERR_ARG )
{
HTTPCLIENT_ERR( "DNS arg error %s", hostname );
}else {
20 changes: 0 additions & 20 deletions app/include/lwip/app/espconn.h
Original file line number Diff line number Diff line change
@@ -475,26 +475,6 @@ extern sint8 espconn_regist_disconcb(struct espconn *espconn, espconn_connect_ca

extern uint32 espconn_port(void);

/******************************************************************************
* FunctionName : espconn_gethostbyname
* Description : Resolve a hostname (string) into an IP address.
* Parameters : pespconn -- espconn to resolve a hostname
* hostname -- the hostname that is to be queried
* addr -- pointer to a ip_addr_t where to store the address if
* it is already cached in the dns_table (only valid if
* ESPCONN_OK is returned!)
* found -- a callback function to be called on success, failure
* or timeout (only if ERR_INPROGRESS is returned!)
* Returns : err_t return code
* - ESPCONN_OK if hostname is a valid IP address string or the host
* name is already in the local names table.
* - ESPCONN_INPROGRESS enqueue a request to be sent to the DNS server
* for resolution if no errors are present.
* - ESPCONN_ARG: dns client not initialized or invalid hostname
*******************************************************************************/

extern err_t espconn_gethostbyname(struct espconn *pespconn, const char *name, ip_addr_t *addr, dns_found_callback found);

/******************************************************************************
* FunctionName : espconn_encry_connect
* Description : The function given as connection
23 changes: 0 additions & 23 deletions app/lwip/app/espconn.c
Original file line number Diff line number Diff line change
@@ -887,26 +887,3 @@ espconn_port(void)

return port;
}

/******************************************************************************
* FunctionName : espconn_gethostbyname
* Description : Resolve a hostname (string) into an IP address.
* Parameters : pespconn -- espconn to resolve a hostname
* hostname -- the hostname that is to be queried
* addr -- pointer to a ip_addr_t where to store the address if
* it is already cached in the dns_table (only valid if
* ESPCONN_OK is returned!)
* found -- a callback function to be called on success, failure
* or timeout (only if ERR_INPROGRESS is returned!)
* Returns : err_t return code
* - ESPCONN_OK if hostname is a valid IP address string or the host
* name is already in the local names table.
* - ESPCONN_INPROGRESS enqueue a request to be sent to the DNS server
* for resolution if no errors are present.
* - ESPCONN_ARG: dns client not initialized or invalid hostname
*******************************************************************************/
err_t ICACHE_FLASH_ATTR
espconn_gethostbyname(struct espconn *pespconn, const char *hostname, ip_addr_t *addr, dns_found_callback found)
{
return dns_gethostbyname(hostname, addr, found, pespconn);
}
73 changes: 29 additions & 44 deletions app/modules/mqtt.c
Original file line number Diff line number Diff line change
@@ -1156,64 +1156,48 @@ static sint8 socket_connect(struct espconn *pesp_conn)
return espconn_status;
}

static sint8 socket_dns_found(const char *name, ip_addr_t *ipaddr, void *arg);
static int dns_reconn_count = 0;
static ip_addr_t host_ip; // for dns

/* wrapper for using socket_dns_found() as callback function */
static void socket_dns_foundcb(const char *name, ip_addr_t *ipaddr, void *arg)
{
socket_dns_found(name, ipaddr, arg);
}

static sint8 socket_dns_found(const char *name, ip_addr_t *ipaddr, void *arg)
{
lmqtt_userdata *mud = arg;

NODE_DBG("enter socket_dns_found.\n");
sint8 espconn_status = ESPCONN_OK;
struct espconn *pesp_conn = arg;
struct espconn *pesp_conn = mud->pesp_conn;
if(pesp_conn == NULL){
NODE_DBG("pesp_conn null.\n");
return -1;
}

if(ipaddr == NULL)
{
dns_reconn_count++;
if( dns_reconn_count >= 5 ){
NODE_DBG( "DNS Fail!\n" );
// Note: should delete the pesp_conn or unref self_ref here.

struct espconn *pesp_conn = arg;
if(pesp_conn != NULL) {
lmqtt_userdata *mud = (lmqtt_userdata *)pesp_conn->reverse;
if(mud != NULL) {
mqtt_connack_fail(mud, MQTT_CONN_FAIL_DNS);
}
}
mqtt_connack_fail(mud, MQTT_CONN_FAIL_DNS);

mqtt_socket_disconnected(arg); // although not connected, but fire disconnect callback to release every thing.
return -1;
}
NODE_DBG( "DNS retry %d!\n", dns_reconn_count );
host_ip.addr = 0;
return espconn_gethostbyname(pesp_conn, name, &host_ip, socket_dns_foundcb);
// although not connected, but fire disconnect callback to release every thing.
mqtt_socket_disconnected(arg);
return -1;
}

// ipaddr->addr is a uint32_t ip
if(ipaddr->addr != 0)
{
dns_reconn_count = 0;
memcpy(pesp_conn->proto.tcp->remote_ip, &(ipaddr->addr), 4);
NODE_DBG("TCP ip is set: ");
NODE_DBG(IPSTR, IP2STR(&(ipaddr->addr)));
NODE_DBG("\n");
espconn_status = socket_connect(pesp_conn);
}

NODE_DBG("leave socket_dns_found.\n");

return espconn_status;
}

/* wrapper for using socket_dns_found() as callback function */
static void socket_dns_foundcb(const char *name, ip_addr_t *ipaddr, void *arg)
{
socket_dns_found(name, ipaddr, arg);
}

#include "pm/swtimer.h"
// Lua: mqtt:connect( host, port, secure, function(client), function(client, connect_return_code) )
static int mqtt_socket_connect( lua_State* L )
@@ -1227,7 +1211,6 @@ static int mqtt_socket_connect( lua_State* L )
int stack = 1;
unsigned secure = 0;
int top = lua_gettop(L);
sint8 espconn_status;

mud = (lmqtt_userdata *)luaL_checkudata(L, stack, "mqtt.socket");
luaL_argcheck(L, mud, stack, "mqtt.socket expected");
@@ -1330,8 +1313,8 @@ static int mqtt_socket_connect( lua_State* L )
luaL_unref(L, LUA_REGISTRYINDEX, mud->self_ref);
mud->self_ref = luaL_ref(L, LUA_REGISTRYINDEX);

espconn_status = espconn_regist_connectcb(pesp_conn, mqtt_socket_connected);
espconn_status |= espconn_regist_reconcb(pesp_conn, mqtt_socket_reconnected);
espconn_regist_connectcb(pesp_conn, mqtt_socket_connected);
espconn_regist_reconcb(pesp_conn, mqtt_socket_reconnected);

os_timer_disarm(&mud->mqttTimer);
os_timer_setfn(&mud->mqttTimer, (os_timer_func_t *)mqtt_socket_timer, mud);
@@ -1342,25 +1325,27 @@ static int mqtt_socket_connect( lua_State* L )

if((ipaddr.addr == IPADDR_NONE) && (memcmp(domain,"255.255.255.255",16) != 0))
{
host_ip.addr = 0;
dns_reconn_count = 0;
if(ESPCONN_OK == espconn_gethostbyname(pesp_conn, domain, &host_ip, socket_dns_foundcb)){
espconn_status |= socket_dns_found(domain, &host_ip, pesp_conn); // ip is returned in host_ip.
ip_addr_t host_ip;
switch (dns_gethostbyname(domain, &host_ip, socket_dns_foundcb, mud))
{
case ERR_OK:
socket_dns_found(domain, &host_ip, mud); // ip is returned in host_ip.
break;
case ERR_INPROGRESS:
break;
default:
// Something has gone wrong; bail out?
mqtt_connack_fail(mud, MQTT_CONN_FAIL_DNS);
}
}
else
{
espconn_status |= socket_connect(pesp_conn);
socket_connect(pesp_conn);
}

NODE_DBG("leave mqtt_socket_connect.\n");

if (espconn_status == ESPCONN_OK) {
lua_pushboolean(L, 1);
} else {
lua_pushboolean(L, 0);
}
return 1;
return 0;
}

// Lua: mqtt:close()
4 changes: 2 additions & 2 deletions app/websocket/websocketclient.c
Original file line number Diff line number Diff line change
@@ -824,9 +824,9 @@ void ws_connect(ws_info *ws, const char *url) {

// Attempt to resolve hostname address
ip_addr_t addr;
err_t result = espconn_gethostbyname(conn, hostname, &addr, dns_callback);
err_t result = dns_gethostbyname(hostname, &addr, dns_callback, conn);

if (result == ESPCONN_INPROGRESS) {
if (result == ERR_INPROGRESS) {
NODE_DBG("DNS pending\n");
} else {
dns_callback(hostname, &addr, conn);
4 changes: 2 additions & 2 deletions docs/modules/mqtt.md
Original file line number Diff line number Diff line change
@@ -134,11 +134,11 @@ Connects to the broker specified by the given host, port, and secure options.

!!! attention

Secure (`https`) connections come with quite a few limitations. Please see
Secure (`mqtts`) connections come with quite a few limitations. Please see
the warnings in the [tls module](tls.md)'s documentation.

#### Returns
`true` on success, `false` otherwise
`nil`; use callbacks to observe the outcome.

#### Notes

0 comments on commit 47258e9

Please sign in to comment.