From f42889781058b3f10c37a07becd70a3c85193ad0 Mon Sep 17 00:00:00 2001
From: Johny Mattsson <johny.mattsson+github@gmail.com>
Date: Fri, 15 Apr 2016 20:49:18 +1000
Subject: [PATCH] SNTP module enhancements (#1243)

* Provide an error code to SNTP error callback.

* Switch SNTP to use ephemeral port.

In case we're being hit by ISP-level thou-shall-not-run-NTP silliness.
---
 app/modules/sntp.c      | 27 ++++++++++++++++++---------
 docs/en/modules/sntp.md |  6 +++++-
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/app/modules/sntp.c b/app/modules/sntp.c
index 6b9cabd4..68b78577 100644
--- a/app/modules/sntp.c
+++ b/app/modules/sntp.c
@@ -58,6 +58,14 @@
 # define sntp_dbg(...)
 #endif
 
+typedef enum {
+  NTP_NO_ERR = 0,
+  NTP_DNS_ERR,
+  NTP_MEM_ERR,
+  NTP_SEND_ERR,
+  NTP_TIMEOUT_ERR
+} ntp_err_t;
+
 typedef struct
 {
   uint32_t sec;
@@ -107,14 +115,15 @@ static void cleanup (lua_State *L)
 }
 
 
-static void handle_error (lua_State *L)
+static void handle_error (lua_State *L, ntp_err_t err)
 {
   sntp_dbg("sntp: handle_error\n");
   if (state->err_cb_ref != LUA_NOREF)
   {
     lua_rawgeti (L, LUA_REGISTRYINDEX, state->err_cb_ref);
+    lua_pushinteger (L, err);
     cleanup (L);
-    lua_call (L, 0, 0);
+    lua_call (L, 1, 0);
   }
   else
     cleanup (L);
@@ -135,7 +144,7 @@ static void sntp_dosend (lua_State *L)
 
   struct pbuf *p = pbuf_alloc (PBUF_TRANSPORT, sizeof (ntp_frame_t), PBUF_RAM);
   if (!p)
-    handle_error (L);
+    handle_error (L, NTP_MEM_ERR);
 
   ntp_frame_t req;
   os_memset (&req, 0, sizeof (req));
@@ -156,7 +165,7 @@ static void sntp_dosend (lua_State *L)
   sntp_dbg("sntp: send: %d\n", ret);
   pbuf_free (p);
   if (ret != ERR_OK)
-    handle_error (L);
+    handle_error (L, NTP_SEND_ERR);
 }
 
 
@@ -167,8 +176,8 @@ static void sntp_dns_found(const char *name, ip_addr_t *ipaddr, void *arg)
   lua_State *L = lua_getstate ();
   if (ipaddr == NULL)
   {
-    NODE_ERR("DNS Fail!\n");
-    handle_error(L);
+    sntp_dbg("DNS Fail!\n");
+    handle_error(L, NTP_DNS_ERR);
   }
   else
   {
@@ -184,7 +193,7 @@ static void on_timeout (void *arg)
   sntp_dbg("sntp: timer\n");
   lua_State *L = lua_getstate ();
   if (state->attempts >= MAX_ATTEMPTS)
-    handle_error (L);
+    handle_error (L, NTP_TIMEOUT_ERR);
   else
     sntp_dosend (L);
 }
@@ -332,8 +341,8 @@ static int sntp_sync (lua_State *L)
   if (!state->pcb)
     sync_err ("out of memory");
 
-  if (udp_bind (state->pcb, IP_ADDR_ANY, NTP_PORT) != ERR_OK)
-    sync_err ("ntp port in use");
+  if (udp_bind (state->pcb, IP_ADDR_ANY, 0) != ERR_OK)
+    sync_err ("no port available");
 
   udp_recv (state->pcb, on_recv, L);
 
diff --git a/docs/en/modules/sntp.md b/docs/en/modules/sntp.md
index f944961e..9b495961 100644
--- a/docs/en/modules/sntp.md
+++ b/docs/en/modules/sntp.md
@@ -18,7 +18,11 @@ Attempts to obtain time synchronization.
 #### Parameters
 - `server_ip` if non-`nil`, that server is used. If `nil`, then the last contacted server is used. This ties in with the NTP anycast mode, where the first responding server is remembered for future synchronization requests. The easiest way to use anycast is to always pass nil for the server argument.
 - `callback` Iif provided it will be invoked on a successful synchronization, with three parameters: seconds, microseconds, and server. Note that when the [rtctime](rtctime.md) module is available, there is no need to explicitly call [`rtctime.set()`](rtctime.md#rtctimeset) - this module takes care of doing so internally automatically, for best accuracy.
-- `errcallback` failure callback with no parameters. The module automatically performs a number of retries before giving up and reporting the error.
+- `errcallback` failure callback with a single integer parameter describing the type of error. The module automatically performs a number of retries before giving up and reporting the error. Error codes:
+  - 1: DNS lookup failed
+  - 2: Memory allocation failure
+  - 3: UDP send failed
+  - 4: Timeout, no NTP response received
 
 #### Returns
 `nil`