fix REST repsonse issues

cmd/cmd.h

@@ -55,7 +55,7 @@ typedef enum {
   CMD_REST_REQUEST,
   CMD_REST_SETHEADER,
   CMD_REST_EVENTS,
-  CMD_ADD_SENSOR, // 15
+  CMD_ADD_CALLBACK,   // 15
   CMD_SENSOR_EVENTS
 } CmdName;
 
@@ -71,19 +71,23 @@ typedef struct {
   uint32_t callback;
 } cmdCallback;
 
-void ICACHE_FLASH_ATTR CMD_parse_packet(uint8_t *buf, short len);
-cmdCallback* ICACHE_FLASH_ATTR CMD_GetCbByName(char* name);
+// Used by slip protocol to cause parsing of a received packet
+void CMD_parse_packet(uint8_t *buf, short len);
+
+// Return the info about a callback to the attached uC by name, these are callbacks that the
+// attached uC registers using the ADD_SENSOR command
+cmdCallback* CMD_GetCbByName(char* name);
 
 // Responses
 
 // Start a response, returns the partial CRC
-uint16_t ICACHE_FLASH_ATTR CMD_ResponseStart(uint16_t cmd, uint32_t callback, uint32_t _return, uint16_t argc);
+uint16_t CMD_ResponseStart(uint16_t cmd, uint32_t callback, uint32_t _return, uint16_t argc);
 // Adds data to a response, returns the partial CRC
-uint16_t ICACHE_FLASH_ATTR CMD_ResponseBody(uint16_t crc_in, uint8_t* data, short len);
+uint16_t CMD_ResponseBody(uint16_t crc_in, uint8_t* data, short len);
 // Ends a response
-void ICACHE_FLASH_ATTR CMD_ResponseEnd(uint16_t crc);
+void CMD_ResponseEnd(uint16_t crc);
 
-//void ICACHE_FLASH_ATTR CMD_Response(uint16_t cmd, uint32_t callback, uint32_t _return, uint16_t argc, CmdArg* args[]);
+//void CMD_Response(uint16_t cmd, uint32_t callback, uint32_t _return, uint16_t argc, CmdArg* args[]);
 
 // Requests
 

cmd/handlers.c

@@ -10,17 +10,19 @@
 #include "uart.h"
 #include "cgiwifi.h"
 
-static uint32_t ICACHE_FLASH_ATTR CMD_Null(CmdPacket *cmd);
-static uint32_t ICACHE_FLASH_ATTR CMD_IsReady(CmdPacket *cmd);
-static uint32_t ICACHE_FLASH_ATTR CMD_WifiConnect(CmdPacket *cmd);
-static uint32_t ICACHE_FLASH_ATTR CMD_AddSensor(CmdPacket *cmd);
+static uint32_t CMD_Null(CmdPacket *cmd);
+static uint32_t CMD_IsReady(CmdPacket *cmd);
+static uint32_t CMD_Reset(CmdPacket *cmd);
+static uint32_t CMD_WifiConnect(CmdPacket *cmd);
+static uint32_t CMD_AddCallback(CmdPacket *cmd);
 
+// keep track of last status sent to uC so we can notify it when it changes
 static uint8_t lastWifiStatus = wifiIsDisconnected;
 
 // Command dispatch table for serial -> ESP commands
 const CmdList commands[] = {
   {CMD_NULL,            CMD_Null},
-  {CMD_RESET,           CMD_Null},
+  {CMD_RESET,           CMD_Reset},
   {CMD_IS_READY,        CMD_IsReady},
   {CMD_WIFI_CONNECT,    CMD_WifiConnect},
 
@@ -37,25 +39,14 @@ const CmdList commands[] = {
   {CMD_REST_REQUEST,    REST_Request},
   {CMD_REST_SETHEADER,  REST_SetHeader},
 
-  {CMD_ADD_SENSOR,      CMD_AddSensor },
+  {CMD_ADD_CALLBACK,    CMD_AddCallback },
 
   {CMD_NULL,            NULL}
 };
 
 // WifiCb plus 10 for sensors
-cmdCallback callbacks[12] = {
-  { { '\0' }, -1 },
-  { { '\0' }, -1 },
-  { { '\0' }, -1 },
-  { { '\0' }, -1 },
-  { { '\0' }, -1 },
-  { { '\0' }, -1 },
-  { { '\0' }, -1 },
-  { { '\0' }, -1 },
-  { { '\0' }, -1 },
-  { { '\0' }, -1 },
-  { { '\0' }, -1 }
-};
+#define MAX_CALLBACKS 12
+cmdCallback callbacks[MAX_CALLBACKS]; // cleared in CMD_Reset
 
 // Command handler for IsReady (healthcheck) command
 static uint32_t ICACHE_FLASH_ATTR
@@ -71,20 +62,31 @@ CMD_Null(CmdPacket *cmd) {
   return 1;
 }
 
-static void ICACHE_FLASH_ATTR 
+// Command handler for Reset command, this was originally to reset the ESP but we don't want to
+// do that is esp-link. It is still good to clear any information the ESP has about the attached
+// uC.
+static uint32_t ICACHE_FLASH_ATTR
+CMD_Reset(CmdPacket *cmd) {
+  os_printf("CMD_Reset\n");
+  // clear callbacks table
+  os_memset(callbacks, 0, sizeof(callbacks));
+  return 1;
+}
+
+static uint32_t ICACHE_FLASH_ATTR
 CMD_AddCb(char* name, uint32_t cb) {
-  char checkname[16];
-  os_strncpy(checkname, name, sizeof(checkname));
-  for (uint8_t i = 0; i < sizeof(commands); i++) {    
-    os_printf("CMD_AddCb: index %d name=%s cb=%p\n", i, callbacks[i].name, (void *)callbacks[i].callback);
+  for (uint8_t i = 0; i < MAX_CALLBACKS; i++) {
+    //os_printf("CMD_AddCb: index %d name=%s cb=%p\n", i, callbacks[i].name,
+    //  (void *)callbacks[i].callback);
     // find existing callback or add to the end
-    if (os_strcmp(callbacks[i].name, checkname) == 0 || callbacks[i].name[0] == '\0') {
-      os_strncpy(callbacks[i].name, checkname, sizeof(checkname));
+    if (os_strcmp(callbacks[i].name, name) == 0 || callbacks[i].name[0] == '\0') {
+      os_strncpy(callbacks[i].name, name, sizeof(callbacks[i].name));
       callbacks[i].callback = cb;
       os_printf("CMD_AddCb: cb %s added at index %d\n", callbacks[i].name, i);
-      break;
+      return 1;
     }
   }
+  return 0;
 }
 
 cmdCallback* ICACHE_FLASH_ATTR
@@ -92,7 +94,8 @@ CMD_GetCbByName(char* name) {
   char checkname[16];
   os_strncpy(checkname, name, sizeof(checkname));
   for (uint8_t i = 0; i < sizeof(commands); i++) {
-    os_printf("CMD_GetCbByName: index %d name=%s cb=%p\n", i, callbacks[i].name, (void *)callbacks[i].callback);
+    //os_printf("CMD_GetCbByName: index %d name=%s cb=%p\n", i, callbacks[i].name,
+    //  (void *)callbacks[i].callback);
     // if callback doesn't exist or it's null
     if (os_strcmp(callbacks[i].name, checkname) == 0) {
       os_printf("CMD_GetCbByName: cb %s found at index %d\n", callbacks[i].name, i);
@@ -130,33 +133,31 @@ CMD_WifiConnect(CmdPacket *cmd) {
 
   wifiStatusCb = CMD_WifiCb;    // register our callback with wifi subsystem
   CMD_AddCb("wifiCb", (uint32_t)cmd->callback); // save the MCU's callback
-  lastWifiStatus = wifiIsDisconnected;
+  lastWifiStatus = 0xff; // set to invalid value so we immediately send status cb in all cases
   CMD_WifiCb(wifiState);
 
   return 1;
 }
 
-// Command handler for Wifi connect command
+// Command handler to add a callback to the named-callbacks list, this is for a callback to the uC
 static uint32_t ICACHE_FLASH_ATTR
-CMD_AddSensor(CmdPacket *cmd) {
+CMD_AddCallback(CmdPacket *cmd) {
   CmdRequest req;
   CMD_Request(&req, cmd);
-  os_printf("CMD_AddSensor: setup argc=%ld\n", CMD_GetArgc(&req));
+  os_printf("CMD_AddCallback: setup argc=%ld\n", CMD_GetArgc(&req));
   if (cmd->argc != 1 || cmd->callback == 0)
     return 0;
 
-  uint8_t* name;
+  char name[16];
   uint16_t len;
 
   // get the sensor name
   len = CMD_ArgLen(&req);
-  os_printf("CMD_AddSensor: name len=%d\n", len);
+  os_printf("CMD_AddCallback: name len=%d\n", len);
   if (len > 15) return 0; // max size of name is 15 characters
-  name = (uint8_t*)os_zalloc(len + 1);
-  if (CMD_PopArg(&req, name, len)) return 0;
+  if (CMD_PopArg(&req, (uint8_t *)name, len)) return 0;
   name[len] = 0;
-  os_printf("CMD_AddSensor: name=%s\n", name);
+  os_printf("CMD_AddCallback: name=%s\n", name);
 
-  CMD_AddCb((char*)name, (uint32_t)cmd->callback); // save the sensor callback 
-  return 1;
+  return CMD_AddCb(name, (uint32_t)cmd->callback); // save the sensor callback
 }

cmd/rest.c

@@ -27,59 +27,66 @@ tcpclient_discon_cb(void *arg) {
   client->data = 0;
 }
 
+// Receive HTTP response - this hacky function assumes that the full response is received in
+// one go. Sigh...
 static void ICACHE_FLASH_ATTR
 tcpclient_recv(void *arg, char *pdata, unsigned short len) {
-  uint8_t currentLineIsBlank = 0;
-  uint8_t httpBody = 0;
-  uint8_t inStatus = 0;
-  char statusCode[4];
-  int i = 0, j;
-  uint32_t code = 0;
-  uint16_t crc;
-
   struct espconn *pCon = (struct espconn*)arg;
   RestClient *client = (RestClient *)pCon->reverse;
 
-  for(j=0 ;j<len; j++){
-    char c = pdata[j];
-
-    if(c == ' ' && !inStatus){
-      inStatus = 1;
-    }
-    if(inStatus && i < 3 && c != ' '){
-      statusCode[i] = c;
-      i++;
-    }
-    if(i == 3){
-      statusCode[i] = '\0';
-      code = atoi(statusCode);
+  // parse status line
+  int pi = 0;
+  int32_t code = -1;
+  char statusCode[4] = "\0\0\0\0";
+  int statusLen = 0;
+  bool inStatus = false;
+  while (pi < len) {
+    if (pdata[pi] == '\n') {
+      // end of status line
+      if (code == -1) code = 502; // BAD GATEWAY
+      break;
+    } else if (pdata[pi] == ' ') {
+      if (inStatus) code = atoi(statusCode);
+      inStatus = !inStatus;
+    } else if (inStatus) {
+      if (statusLen < 3) statusCode[statusLen] = pdata[pi];
+      statusLen++;
     }
+    pi++;
+  }
 
-    if(httpBody){
-      //only write response if its not null
-      uint32_t body_len = len - j;
-      os_printf("REST: status=%ld, body=%ld\n", code, body_len);
-      if(body_len == 0){
-        crc = CMD_ResponseStart(CMD_REST_EVENTS, client->resp_cb, code, 0);
-      } else {
-        crc = CMD_ResponseStart(CMD_REST_EVENTS, client->resp_cb, code, 1);
-        crc = CMD_ResponseBody(crc, (uint8_t*)(pdata+j), body_len);
-      }
-      CMD_ResponseEnd(crc);
-      break;
-    } else {
-      if (c == '\n' && currentLineIsBlank) {
-        httpBody = true;
-      }
-      if (c == '\n') {
-        // you're starting a new line
-        currentLineIsBlank = true;
-      } else if (c != '\r') {
-        // you've gotten a character on the current line
-        currentLineIsBlank = false;
+  // parse header, all this does is look for the end of the header
+  bool currentLineIsBlank = false;
+  while (pi < len) {
+    if (pdata[pi] == '\n') {
+      if (currentLineIsBlank) {
+        // body is starting
+        pi++;
+        break;
       }
+      currentLineIsBlank = true;
+    } else if (pdata[pi] != '\r') {
+      currentLineIsBlank = false;
     }
+    pi++;
   }
+  //if (pi < len && pdata[pi] == '\r') pi++; // hacky!
+
+  // collect body and send it
+  uint16_t crc;
+  int body_len = len-pi;
+  os_printf("REST: status=%ld, body=%d\n", code, body_len);
+  if (pi == len) {
+    crc = CMD_ResponseStart(CMD_REST_EVENTS, client->resp_cb, code, 0);
+  } else {
+    crc = CMD_ResponseStart(CMD_REST_EVENTS, client->resp_cb, code, 1);
+    crc = CMD_ResponseBody(crc, (uint8_t*)(pdata+pi), body_len);
+    CMD_ResponseEnd(crc);
+    os_printf("REST: body=");
+    for (int j=pi; j<len; j++) os_printf(" %02x", pdata[j]);
+    os_printf("\n");
+  }
+
   //if(client->security)
   //  espconn_secure_disconnect(client->pCon);
   //else
@@ -338,7 +345,8 @@ REST_Request(CmdPacket *cmd) {
   // we need to allocate memory for the header plus the body. First we count the length of the
   // header (including some extra counted "%s" and then we add the body length. We allocate the
   // whole shebang and copy everything into it.
-  char *headerFmt = "%s %s HTTP/1.1\r\n"
+  // BTW, use http/1.0 to avoid responses with transfer-encoding: chunked
+  char *headerFmt = "%s %s HTTP/1.0\r\n"
                     "Host: %s\r\n"
                     "%s"
                     "Content-Length: %d\r\n"
@@ -360,11 +368,12 @@ REST_Request(CmdPacket *cmd) {
     CMD_PopArg(&req, client->data + client->data_len, realLen);
     client->data_len += realLen;
   }
+  os_printf("\n");
 
+  os_printf("REST: pCon state=%d\n", client->pCon->state);
   client->pCon->state = ESPCONN_NONE;
   espconn_regist_connectcb(client->pCon, tcpclient_connect_cb);
   espconn_regist_reconcb(client->pCon, tcpclient_recon_cb);
-  os_printf("\n");
 
   if(UTILS_StrToIP((char *)client->host, &client->pCon->proto.tcp->remote_ip)) {
     os_printf("REST: Connect to ip %s:%ld\n",client->host, client->port);

serial/slip.c

@@ -70,6 +70,7 @@ slip_printable(char c) {
 
 static void ICACHE_FLASH_ATTR
 slip_reset() {
+  //os_printf("SLIP: reset\n");
   slip_inpkt = false;
   slip_escaped = false;
   slip_len = 0;
@@ -83,6 +84,7 @@ slip_parse_char(char c) {
       if (slip_len > 0) console_process(slip_buf, slip_len);
       slip_reset();
       slip_inpkt = true;
+      os_printf("SLIP: start\n");
       return;
     }
   } else if (slip_escaped) {
@@ -101,6 +103,9 @@ slip_parse_char(char c) {
       return;
     case SLIP_START:
       os_printf("SLIP: got SLIP_START while in packet?\n");
+      //os_printf("SLIP: rcv %d:", slip_len);
+      //for (int i=0; i<slip_len; i++) os_printf(" %02x", slip_buf[i]);
+      //os_printf("\n");
       slip_reset();
       return;
     }