Merge pull request #133 from sarfata/bugfix/esp-crashes

Bugfix/esp crashes

README.md

@@ -192,6 +192,11 @@ been possible!**
 
 ## Changelog
 
+ * 2018 04 25 - v1.2.4
+   * Improved WiFi connection stability
+     This required the latest version of ESP firmware.
+     And also some changes to the TCP/NMEA server.
+   * #96: make wifi led go green when we have a websocket client connected
  * 2018 04 23 - v1.2.3
    * Repeat all NMEA and NMEA2000 data to the computer when the serial port is
      opened at 38400 bauds.

esp-crash.py

@@ -0,0 +1,138 @@
+#!/usr/bin/env python
+
+import argparse
+import re
+import os
+import subprocess
+import sys
+
+
+class ESPCrashParser(object):
+    ESP_EXCEPTIONS = [
+        "Illegal instruction",
+        "SYSCALL instruction",
+        "InstructionFetchError: Processor internal physical address or data error during instruction fetch",
+        "LoadStoreError: Processor internal physical address or data error during load or store",
+        "Level1Interrupt: Level-1 interrupt as indicated by set level-1 bits in the INTERRUPT register",
+        "Alloca: MOVSP instruction, if caller's registers are not in the register file",
+        "IntegerDivideByZero: QUOS, QUOU, REMS, or REMU divisor operand is zero",
+        "reserved",
+        "Privileged: Attempt to execute a privileged operation when CRING ? 0",
+        "LoadStoreAlignmentCause: Load or store to an unaligned address",
+        "reserved",
+        "reserved",
+        "InstrPIFDataError: PIF data error during instruction fetch",
+        "LoadStorePIFDataError: Synchronous PIF data error during LoadStore access",
+        "InstrPIFAddrError: PIF address error during instruction fetch",
+        "LoadStorePIFAddrError: Synchronous PIF address error during LoadStore access",
+        "InstTLBMiss: Error during Instruction TLB refill",
+        "InstTLBMultiHit: Multiple instruction TLB entries matched",
+        "InstFetchPrivilege: An instruction fetch referenced a virtual address at a ring level less than CRING",
+        "reserved",
+        "InstFetchProhibited: An instruction fetch referenced a page mapped with an attribute that does not permit instruction fetch",
+        "reserved",
+        "reserved",
+        "reserved",
+        "LoadStoreTLBMiss: Error during TLB refill for a load or store",
+        "LoadStoreTLBMultiHit: Multiple TLB entries matched for a load or store",
+        "LoadStorePrivilege: A load or store referenced a virtual address at a ring level less than CRING",
+        "reserved",
+        "LoadProhibited: A load referenced a page mapped with an attribute that does not permit loads",
+        "StoreProhibited: A store referenced a page mapped with an attribute that does not permit stores"
+    ]
+
+    def __init__(self, toolchain_path, elf_path):
+        self.toolchain_path = toolchain_path
+        self.gdb_path = os.path.join(toolchain_path, "bin", "xtensa-lx106-elf-gdb")
+        self.addr2line_path = os.path.join(toolchain_path, "bin", "xtensa-lx106-elf-addr2line")
+
+        if not os.path.exists(self.gdb_path):
+            raise Exception("GDB for ESP not found in {} - {} does not exist.\nUse --toolchain to point to "
+                            "your toolchain folder.".format(self.toolchain_path, self.gdb_path))
+
+        if not os.path.exists(self.addr2line_path):
+            raise Exception("addr2line for ESP not found in {} - {} does not exist.\nUse --toolchain to point to "
+                            "your toolchain folder.".format(self.toolchain_path, self.addr2line_path))
+
+        self.elf_path = elf_path
+        if not os.path.exists(self.elf_path):
+            raise Exception("ELF file not found: '{}'".format(self.elf_path))
+
+    def parse_text(self, text):
+        print self.parse_exception(text)
+
+        m = re.search('stack(.*)stack', text, flags = re.MULTILINE | re.DOTALL)
+        if m:
+            print "Stack trace:"
+            for l in self.parse_stack(m.group(1)):
+                print "  " + l
+        else:
+            print "No stack trace found."
+
+    def parse_exception(self, text):
+        m = re.search('Exception \(([0-9]*)\):', text)
+        if m:
+            exception_id = int(m.group(1))
+            if 0 <= exception_id <= 29:
+                return "Exception {}: {}".format(exception_id, ESPCrashParser.ESP_EXCEPTIONS[exception_id])
+            else:
+                return "Unknown exception: {}".format(exception_id)
+
+    '''
+    Decode one stack or backtrace.
+    
+    See: https://github.com/me-no-dev/EspExceptionDecoder/blob/master/src/EspExceptionDecoder.java#L402
+    '''
+    def parse_stack(self, text):
+        r = re.compile('40[0-2][0-9a-fA-F]{5}\s')
+        m = r.findall(text)
+        return self.decode_function_addresses(m)
+
+    def decode_function_address(self, address):
+        args = [self.addr2line_path, "-e", self.elf_path, "-aipfC", address]
+        return subprocess.check_output(args).strip()
+
+    def decode_function_addresses(self, addresses):
+        out = []
+        for a in addresses:
+            out.append(self.decode_function_address(a))
+        return out
+
+    '''
+    GDB Should produce line number: https://github.com/me-no-dev/EspExceptionDecoder/commit/a78672da204151cc93979a96ed9f89139a73893f
+    However it does not produce anything for me. So not using it for now.
+    '''
+    def decode_function_addresses_with_gdb(self, addresses):
+        args = [self.gdb_path, "--batch"]
+
+        # Disable user config file which might interfere here
+        args.extend(["-iex", "set auto-load local-gdbinit off"])
+
+        args.append(self.elf_path)
+
+        args.extend(["-ex", "set listsize 1"])
+        for address in addresses:
+            args.append("-ex")
+            args.append("l *0x{}".format(address))
+        args.extend(["-ex", "q"])
+
+        print "Running: {}".format(args)
+        out = subprocess.check_output(args)
+        print out
+
+def main():
+    parser = argparse.ArgumentParser()
+    parser.add_argument("--toolchain", help="Path to the Xtensa toolchain",
+                        default=os.path.join(os.environ.get("HOME"), ".platformio/packages/toolchain-xtensa"))
+    parser.add_argument("--elf", help="Path to the ELF file of the firmware",
+                        default=".pioenvs/esp/firmware.elf")
+    parser.add_argument("input", type=argparse.FileType('r'), default=sys.stdin)
+
+    args = parser.parse_args()
+
+    crash_parser = ESPCrashParser(args.toolchain, args.elf)
+    crash_parser.parse_text(args.input.read())
+
+
+if __name__ == '__main__':
+    main()

platformio.ini

@@ -60,9 +60,10 @@ lib_deps =
 [env:esp]
 src_filter = +<common/*>,+<esp/*>
 # no-strict-aliasing required here due to ESP+NMEA2000 incompatibilities
-build_flags = -Wall -Wno-strict-aliasing
+build_flags = -Wall -Werror -fno-strict-aliasing
     -Isrc/common -Isrc/esp
     -DHAVE_STRLCPY -DHAVE_STRLCAT
+#platform=espressif8266
 platform=https://github.com/platformio/platform-espressif8266.git#feature/stage
 framework = arduino
 board = esp_wroom_02

src/esp/NetServer.cpp

@@ -34,29 +34,26 @@ NetServer::NetServer(int port) : server(port) {
 }
 
 void NetServer::handleDisconnect(int clientIndex) {
-  clients[clientIndex]->free();
-  delete(clients[clientIndex]);
-  clients[clientIndex] = 0;
-  queues[clientIndex].clear();
+  DEBUG("Disconnect for client %i", clientIndex);
+  // AsyncPrinter will delete the client object in the client->onDisconnect handler.
 }
 
 void NetServer::handleNewClient(AsyncClient *client) {
   DEBUG("New connection");
   int i;
   for (i = 0; i < maxClients; i++) {
-    if (!clients[i] || !clients[i]->connected()) {
-      clients[i] = client;
+    if (!clients[i]) {
+      clients[i] = AsyncPrinter(client, 2048);
 
-      client->onAck([this, i](void *s, AsyncClient *c, size_t len, uint32_t time) {
-          //DEBUG("Got ack for client %i len=%u time=%u", i, len, time);
-        }, 0);
-      client->onData([this, i](void *s, AsyncClient *c, void *data, size_t len) {
+      clients[i].onData([this, i](void *s, AsyncPrinter *c, uint8_t *data, size_t len) {
           DEBUG("Got data from client %i len=%i", i, len);
         }, 0);
-      client->onDisconnect([this, i](void *s, AsyncClient *c) {
-          DEBUG("Disconnect for client %i", i);
+      clients[i].onClose([this, i](void *s, AsyncPrinter *c) {
           handleDisconnect(i);
         }, 0);
+
+      // Attach some debug messages directly to the client for information.
+      // These handlers are not set by AsyncPrinter.
       client->onError([this, i](void *s, AsyncClient *c, int8_t error) {
           DEBUG("Error %s (%i) on client %i", c->errorToString(error), error, i);
         }, 0);
@@ -69,7 +66,8 @@ void NetServer::handleNewClient(AsyncClient *client) {
     }
   }
   DEBUG("Rejecting client - Too many connections already.");
-  // We cannot accept this connection at the moment
+  // We cannot accept this connection at the moment. Set a handler to free the object when disconnected
+  // and start disconnection.
   client->onDisconnect([](void *s, AsyncClient *c) {
       delete(c);
     });
@@ -78,50 +76,24 @@ void NetServer::handleNewClient(AsyncClient *client) {
 
 void NetServer::writeAll(const uint8_t *bytes, int len) {
   for (int i = 0; i < maxClients; i++) {
-    if (clients[i] && clients[i]->connected()) {
-      queues[i].add(NetMessage(bytes, len));
+    if (clients[i]) {
+      clients[i].write(bytes, len);
     }
   }
 }
 
 int NetServer::clientsCount() {
   int count = 0;
   for (int i = 0; i < maxClients; i++) {
-    //DEBUG("counting - i=%i count=%i clients[i]=%p clients[i]->connected()=%s", 
-        //i, count, clients[i], clients[i] ? (clients[i]->connected() ? "connected" : "not connected") : "n/a");
-    if (clients[i] && clients[i]->connected()) {
+    if (clients[i]) {
       count++;
     }
   }
   return count;
 }
 
 void NetServer::loop() {
-  for (int i = 0; i < maxClients; i++) {
-    if (clients[i] && clients[i]->connected()) {
-      if (clients[i]->canSend() && queues[i].size() > 0) {
-        //DEBUG("Sending for clients[%i] queue len=%i", i, queues[i].size());
-
-        LinkedList<NetMessage>::iterator it = queues[i].begin();
-        if (clients[i]->write((const char*)it->bytes(), it->len()) > 0) {
-          queues[i].removeFirst();
-        }
-      }
-    }
-    else {
-      queues[i].clear();
-    }
-  }
-  /*
-    if (!clients[i]->canSend()) {
-      DEBUG("client[%i]: BUSY in state %s", i, clients[i]->stateToString());
-      continue;
-    }
-    size_t sent = clients[i]->write((char*)bytes, len);
-    if (sent != len) {
-      DEBUG("client[%i]: sent %i of %i bytes", i, sent, len);
-    }
-    */
+  // Writes are performed asynchronously.
 }
 
 

src/esp/NetServer.h

@@ -25,44 +25,7 @@
 
 #include <ESP8266WiFi.h>
 #include <ESPAsyncTCP.h>
-#include "common/algo/List.h"
-
-/*
- * Used to build a buffer (with a LinkedList<NetMessage>) of 
- * messages that are waiting for each client.
- * Using a LinkedList here is pretty inefficient.
- * FIXME: Use a circular buffer or dynamic buffer (with a max size) instead of
- * this.
- */
-class NetMessage {
-  private:
-    uint8_t* _bytes;
-    int _len;
-
-  public:
-    NetMessage(const uint8_t *bytes, int len) : _len(len) {
-      _bytes = (uint8_t*)malloc(len);
-      memcpy(_bytes, bytes, len);
-    };
-
-    NetMessage(const NetMessage &m) {
-      _bytes = (uint8_t*)malloc(m._len);
-      memcpy(_bytes, m._bytes, m._len);
-      _len = m._len;
-    };
-
-    ~NetMessage() {
-      free(_bytes);
-    };
-
-    int len() const {
-      return _len;
-    };
-
-    const uint8_t* bytes() const {
-      return _bytes;
-    };
-};
+#include <AsyncPrinter.h>
 
 class NetServer {
   public:
@@ -74,11 +37,9 @@ class NetServer {
 
   private:
     static const int maxClients = 8;
-    AsyncClient *clients[maxClients];
+    AsyncPrinter clients[maxClients];
     AsyncServer server;
 
-    LinkedList<NetMessage> queues[maxClients];
-
     void handleNewClient(AsyncClient *client);
     void handleDisconnect(int clientIndex);
 };

src/esp/main.cpp

@@ -153,7 +153,7 @@ void loop() {
         lastMessageTimer = 0;
       }
 
-      if (server.clientsCount() > 0) {
+      if (server.clientsCount() + webServer.countClients() > 0) {
         rgb.setPixelColor(0, clientsConnectedColor);
       }
       else {

src/host/config/KBoxConfigParser.cpp

@@ -38,8 +38,8 @@
 #define READ_INT_VALUE(name) READ_VALUE_WITH_TYPE(name, int)
 
 #define READ_INT_VALUE_WRANGE(name, min, max) if (json[#name].is<int>() \
-                                                    && json[#name] > min \
-                                                    && json[#name] < max) { \
+                                                    && json[#name] >= min \
+                                                    && json[#name] <= max) { \
                                                 config.name = \
                                                   json[#name] .as<int>(); \
                                               }

src/host/services/WiFiService.cpp

@@ -64,8 +64,24 @@ void WiFiService::loop() {
     else {
       KBoxMetrics.event(KBoxEventWiFiInvalidKommand);
       ERROR("Invalid command received from WiFi module (id=%i size=%i)", kr.getKommandIdentifier(), kr.dataSize());
+
+      // This is most likely a boot or reboot message from the ESP module.
+      // Print it line by line, otherwise the message will be truncated.
+
+      unsigned int index = 0;
+      uint8_t *currentLine = frame;
+      while (index < len) {
+        if (frame[index] == '\n') {
+          frame[index] = '\0';
+          DEBUG("> %s", currentLine);
+
+          currentLine = frame + index + 1;
+        }
+        index++;
+      }
+
       frame[len-1] = 0;
-      DEBUG("> %s", frame);
+      DEBUG("> %s", currentLine);
     }
 
     _slip.readFrame(0, 0);

tools/nmea-tester/nmea.py

@@ -72,6 +72,7 @@ def send_sentence(output, nmea):
 
 def replay(input, output):
     for nmea in nmea_read_with_delay(input):
+        output.reset_input_buffer()
         send_sentence(output, nmea)
 
 def nmea_checksum(data):
@@ -133,7 +134,7 @@ def main():
 
     port = None
     if args.port:
-        port = serial.Serial(port = args.port, baudrate = args.baud, timeout = 1)
+        port = serial.Serial(port = args.port, baudrate = args.baud, timeout = 1, write_timeout = 1, xonxoff=False, rtscts=False, dsrdtr=False)
 
     if args.command == 'replay':
         if port: