ota.cpp restructured; .h file sanitization

include/globals.h

@@ -38,6 +38,14 @@ typedef struct {
   uint8_t Message[PAYLOAD_BUFFER_SIZE];
 } MessageBuffer_t;
 
+typedef struct {
+  uint32_t latitude;
+  uint32_t longitude;
+  uint8_t satellites;
+  uint16_t hdop;
+  uint16_t altitude;
+} gpsStatus_t;
+
 // global variables
 extern configData_t cfg;                      // current device configuration
 extern char display_line6[], display_line7[]; // screen buffers
@@ -52,16 +60,13 @@ extern std::array<uint64_t, 0xff> beacons;
 
 extern TaskHandle_t irqHandlerTask, wifiSwitchTask;
 
+#include "led.h"
+#include "payload.h"
+
 #ifdef HAS_GPS
 #include "gpsread.h"
 #endif
 
-#if (HAS_LED != NOT_A_PIN) || defined(HAS_RGB_LED)
-#include "led.h"
-#endif
-
-#include "payload.h"
-
 #ifdef HAS_LORA
 #include "lorawan.h"
 #endif
@@ -86,8 +91,4 @@ extern TaskHandle_t irqHandlerTask, wifiSwitchTask;
 #include "antenna.h"
 #endif
 
-void reset_counters(void);
-void blink_LED(uint16_t set_color, uint16_t set_blinkduration);
-uint64_t uptime();
-
 #endif

include/gpsread.h

@@ -8,14 +8,6 @@
 #include <Wire.h>
 #endif
 
-typedef struct {
-  uint32_t latitude;
-  uint32_t longitude;
-  uint8_t satellites;
-  uint16_t hdop;
-  uint16_t altitude;
-} gpsStatus_t;
-
 extern TinyGPSPlus gps; // Make TinyGPS++ instance globally availabe
 extern gpsStatus_t
     gps_status; // Make struct for storing gps data globally available

include/led.h

@@ -37,8 +37,6 @@ extern TaskHandle_t ledLoopTask;
 void rgb_set_color(uint16_t hue);
 void blink_LED(uint16_t set_color, uint16_t set_blinkduration);
 void ledLoop(void *parameter);
-#if (HAS_LED != NOT_A_PIN)
 void switch_LED(uint8_t state);
-#endif
 
 #endif

include/ota.h

@@ -11,7 +11,7 @@
 #include <BintrayClient.h>
 #include <string>
 
-void do_ota_update();
+bool do_ota_update();
 void start_ota_update();
 int version_compare(const String v1, const String v2);
 void display(const uint8_t row, const std::string status,

include/payload.h

@@ -38,12 +38,8 @@ public:
   void addStatus(uint16_t voltage, uint64_t uptime, float cputemp, uint32_t mem,
                  uint8_t reset1, uint8_t reset2);
   void addAlarm(int8_t rssi, uint8_t message);
-#ifdef HAS_GPS
   void addGPS(gpsStatus_t value);
-#endif
-#ifdef HAS_BUTTON
   void addButton(uint8_t value);
-#endif
 
 #if PAYLOAD_ENCODER == 1 // format plain
 

include/rcommand.h

@@ -2,6 +2,7 @@
 #define _RCOMMAND_H
 
 #include "senddata.h"
+#include "cyclic.h"
 #include "configmanager.h"
 #include "lorawan.h"
 #include "macsniff.h"

include/senddata.h

@@ -3,6 +3,7 @@
 
 #include "spislave.h"
 #include "lorawan.h"
+#include "cyclic.h"
 
 void SendData(uint8_t port);
 void sendPayload(void);

src/led.cpp

@@ -87,9 +87,8 @@ void rgb_set_color(uint16_t hue) {}
 
 #endif
 
-#if (HAS_LED != NOT_A_PIN)
-
 void switch_LED(uint8_t state) {
+#if (HAS_LED != NOT_A_PIN)
   if (state == LED_ON) {
     // switch LED on
 #ifdef LED_ACTIVE_LOW
@@ -105,9 +104,8 @@ void switch_LED(uint8_t state) {
     digitalWrite(HAS_LED, LOW);
 #endif
   }
-}
-
 #endif
+}
 
 #if (HAS_LED != NOT_A_PIN) || defined(HAS_RGB_LED)
 

src/ota.cpp

@@ -26,9 +26,6 @@ const BintrayClient bintray(BINTRAY_USER, BINTRAY_REPO, BINTRAY_PACKAGE);
 // Connection port (HTTPS)
 const int port = 443;
 
-// Connection timeout
-const uint32_t RESPONSE_TIMEOUT_MS = 5000;
-
 // Variables to validate firmware content
 int volatile contentLength = 0;
 bool volatile isValidContentType = false;
@@ -43,24 +40,17 @@ inline String getHeaderValue(String header, String headerName) {
 
 void start_ota_update() {
 
-/*
+  /*
-// check battery status if we can before doing ota
+  // check battery status if we can before doing ota
-#ifdef HAS_BATTERY_PROBE
+  #ifdef HAS_BATTERY_PROBE
     if (!batt_sufficient()) {
       ESP_LOGW(TAG, "Battery voltage %dmV too low for OTA", batt_voltage);
       return;
     }
-#endif
+  #endif
-*/
+  */
 
-// turn on LED
+  switch_LED(LED_ON);
-#if (HAS_LED != NOT_A_PIN)
-#ifdef LED_ACTIVE_LOW
-  digitalWrite(HAS_LED, LOW);
-#else
-  digitalWrite(HAS_LED, HIGH);
-#endif
-#endif
 
 #ifdef HAS_DISPLAY
   u8x8.begin();
@@ -84,42 +74,45 @@ void start_ota_update() {
 
   WiFi.begin(WIFI_SSID, WIFI_PASS);
 
-  int i = WIFI_MAX_TRY;
+  int i = WIFI_MAX_TRY, j = OTA_MAX_TRY;
 
   while (i--) {
     ESP_LOGI(TAG, "Trying to connect to %s", WIFI_SSID);
-    if (WiFi.status() == WL_CONNECTED)
+    if (WiFi.status() == WL_CONNECTED) {
-      break;
+      // we now have wifi connection and try to do an OTA over wifi update
-    vTaskDelay(5000 / portTICK_PERIOD_MS);
-  }
-
-  if (i >= 0) {
       ESP_LOGI(TAG, "Connected to %s", WIFI_SSID);
       display(1, "OK", "WiFi connected");
-    do_ota_update(); // gets and flashes new firmware
+      // do a number of tries limited by OTA_MAX_TRY
+      while (j--) {
+        ESP_LOGI(TAG,
+                 "Starting OTA update, attempt %u of %u. This will take some "
+                 "time to complete...",
+                 OTA_MAX_TRY - j, OTA_MAX_TRY);
+        if (do_ota_update())
+          goto end;
+      }
     } else {
-    ESP_LOGI(TAG, "Could not connect to %s, rebooting.", WIFI_SSID);
+      vTaskDelay(5000 / portTICK_PERIOD_MS);
-    display(1, " E", "no WiFi connect");
+    }
   }
 
+  ESP_LOGI(TAG, "Could not connect to %s, rebooting.", WIFI_SSID);
+  display(1, " E", "no WiFi connect");
+
+end:
+
+  switch_LED(LED_OFF);
   display(5, "**", ""); // mark line rebooting
-
-// turn off LED
-#if (HAS_LED != NOT_A_PIN)
-#ifdef LED_ACTIVE_LOW
-  digitalWrite(HAS_LED, HIGH);
-#else
-  digitalWrite(HAS_LED, LOW);
-#endif
-#endif
-
   vTaskDelay(5000 / portTICK_PERIOD_MS);
   ESP.restart();
 
 } // start_ota_update
 
-void do_ota_update() {
+bool do_ota_update() {
+
   char buf[17];
+  bool redirect = true;
+  size_t written = 0;
 
   // Fetch the latest firmware version
   ESP_LOGI(TAG, "Checking latest firmware version on server...");
@@ -131,11 +124,11 @@ void do_ota_update() {
         TAG,
         "Could not load info about the latest firmware. Rebooting to runmode.");
     display(2, " E", "file not found");
-    return;
+    return false;
   } else if (version_compare(latest, cfg.version) <= 0) {
     ESP_LOGI(TAG, "Current firmware is up to date. Rebooting to runmode.");
     display(2, "NO", "no update found");
-    return;
+    return false;
   }
   ESP_LOGI(TAG, "New firmware version v%s available. Downloading...",
            latest.c_str());
@@ -146,7 +139,7 @@ void do_ota_update() {
   if (!firmwarePath.endsWith(".bin")) {
     ESP_LOGI(TAG, "Unsupported binary format, OTA update cancelled.");
     display(3, " E", "file type error");
-    return;
+    return false;
   }
 
   String currentHost = bintray.getStorageHost();
@@ -158,10 +151,9 @@ void do_ota_update() {
   if (!client.connect(currentHost.c_str(), port)) {
     ESP_LOGI(TAG, "Cannot connect to %s", currentHost.c_str());
     display(3, " E", "connection lost");
-    return;
+    goto failure;
   }
 
-  bool redirect = true;
   while (redirect) {
     if (currentHost != prevHost) {
       client.stop();
@@ -170,7 +162,7 @@ void do_ota_update() {
         ESP_LOGI(TAG, "Redirect detected, but cannot connect to %s",
                  currentHost.c_str());
         display(3, " E", "server error");
-        return;
+        goto failure;
       }
     }
 
@@ -186,8 +178,7 @@ void do_ota_update() {
       if (millis() - timeout > RESPONSE_TIMEOUT_MS) {
         ESP_LOGI(TAG, "Client Timeout.");
         display(3, " E", "client timeout");
-        client.stop();
+        goto failure;
-        return;
       }
     }
 
@@ -243,27 +234,29 @@ void do_ota_update() {
         }
       }
     }
-  }
+  } // while (redirect)
 
   display(3, "OK", ""); // line download
 
   // check whether we have everything for OTA update
-  if (contentLength && isValidContentType) {
+  if (!(contentLength && isValidContentType)) {
+    ESP_LOGI(TAG,
+             "There was no valid content in the response from the OTA server!");
+    display(4, " E", "response error");
+    goto failure;
+  }
 
-    size_t written = 0;
+  if (!Update.begin(contentLength)) {
+    ESP_LOGI(TAG, "There isn't enough space to start OTA update");
+    display(4, " E", "disk full");
+    goto failure;
+  }
 
-    if (Update.begin(contentLength)) {
 #ifdef HAS_DISPLAY
   // register callback function for showing progress while streaming data
   Update.onProgress(&show_progress);
 #endif
-      int i = FLASH_MAX_TRY;
-      while ((i--) && (written != contentLength)) {
 
-        ESP_LOGI(TAG,
-                 "Starting OTA update, attempt %u of %u. This will take some "
-                 "time to complete...",
-                 FLASH_MAX_TRY - i, FLASH_MAX_TRY);
   display(4, "**", "writing...");
 
   written = Update.writeStream(client);
@@ -272,46 +265,31 @@ void do_ota_update() {
     ESP_LOGI(TAG, "Written %u bytes successfully", written);
     snprintf(buf, 17, "%ukB Done!", (uint16_t)(written / 1024));
     display(4, "OK", buf);
-          break;
   } else {
-          ESP_LOGI(TAG,
+    ESP_LOGI(TAG, "Written only %u of %u bytes, OTA update attempt cancelled.",
-                   "Written only %u of %u bytes, OTA update attempt cancelled.",
              written, contentLength);
   }
-      }
 
   if (Update.end()) {
-
+    goto finished;
-        if (Update.isFinished()) {
-          ESP_LOGI(
-              TAG,
-              "OTA update completed. Rebooting to runmode with new version.");
-          client.stop();
-          return;
-        } else {
-          ESP_LOGI(TAG, "Something went wrong! OTA update hasn't been finished "
-                        "properly.");
-        }
   } else {
     ESP_LOGI(TAG, "An error occurred. Error #: %d", Update.getError());
     snprintf(buf, 17, "Error #: %d", Update.getError());
     display(4, " E", buf);
+    goto failure;
   }
 
-    } else {
+finished:
-      ESP_LOGI(TAG, "There isn't enough space to start OTA update");
+  client.stop();
-      display(4, " E", "disk full");
+  ESP_LOGI(TAG, "OTA update completed. Rebooting to runmode with new version.");
-      client.flush();
+  return true;
-    }
+
-  } else {
+failure:
-    ESP_LOGI(TAG,
+  client.stop();
-             "There was no valid content in the response from the OTA server!");
-    display(4, " E", "response error");
-    client.flush();
-  }
   ESP_LOGI(TAG,
            "OTA update failed. Rebooting to runmode with current version.");
-  client.stop();
+  return false;
+
 } // do_ota_update
 
 void display(const uint8_t row, const std::string status,
@@ -329,7 +307,7 @@ void display(const uint8_t row, const std::string status,
 
 #ifdef HAS_DISPLAY
 // callback function to show download progress while streaming data
-void show_progress (unsigned long current, unsigned long size) {
+void show_progress(unsigned long current, unsigned long size) {
   char buf[17];
   snprintf(buf, 17, "%-9lu (%3lu%%)", current, current * 100 / size);
   display(4, "**", buf);

src/paxcounter.conf

@@ -68,8 +68,9 @@
 // OTA settings
 #define USE_OTA                         1       // Comment out to disable OTA update
 #define WIFI_MAX_TRY                    5       // maximum number of wifi connect attempts for OTA update [default = 20]
-#define FLASH_MAX_TRY                   3       // maximum number of attempts for writing update binary to flash [default = 3]
+#define OTA_MAX_TRY                     3       // maximum number of attempts for OTA download and write to flash [default = 3]
-#define OTA_MIN_BATT                    3700    // minimum battery level vor OTA [millivolt]
+#define OTA_MIN_BATT                    3700    // minimum battery level for OTA [millivolt]
+#define RESPONSE_TIMEOUT_MS             30000   // firmware binary server connection timeout [milliseconds]
 
 // LMIC settings
 // moved to src/lmic_config.h

src/payload.cpp

@@ -75,8 +75,8 @@ void PayloadConvert::addStatus(uint16_t voltage, uint64_t uptime, float cputemp,
   buffer[cursor++] = (byte)(reset2);
 }
 
-#ifdef HAS_GPS
 void PayloadConvert::addGPS(gpsStatus_t value) {
+#ifdef HAS_GPS
   buffer[cursor++] = (byte)((value.latitude & 0xFF000000) >> 24);
   buffer[cursor++] = (byte)((value.latitude & 0x00FF0000) >> 16);
   buffer[cursor++] = (byte)((value.latitude & 0x0000FF00) >> 8);
@@ -90,14 +90,17 @@ void PayloadConvert::addGPS(gpsStatus_t value) {
   buffer[cursor++] = lowByte(value.hdop);
   buffer[cursor++] = highByte(value.altitude);
   buffer[cursor++] = lowByte(value.altitude);
+#endif
 }
-#endif
 
+void PayloadConvert::addButton(uint8_t value) {
 #ifdef HAS_BUTTON
-void PayloadConvert::addButton(uint8_t value) { buffer[cursor++] = value; }
+  buffer[cursor++] = value;
 #endif
+}
 
-/* ---------------- packed format with LoRa serialization Encoder ---------- */
+/* ---------------- packed format with LoRa serialization Encoder ----------
+ */
 // derived from
 // https://github.com/thesolarnomad/lora-serialization/blob/master/src/LoraEncoder.cpp
 
@@ -138,18 +141,20 @@ void PayloadConvert::addStatus(uint16_t voltage, uint64_t uptime, float cputemp,
   writeUint8(reset2);
 }
 
-#ifdef HAS_GPS
 void PayloadConvert::addGPS(gpsStatus_t value) {
+#ifdef HAS_GPS
   writeLatLng(value.latitude, value.longitude);
   writeUint8(value.satellites);
   writeUint16(value.hdop);
   writeUint16(value.altitude);
+#endif
 }
-#endif
 
+void PayloadConvert::addButton(uint8_t value) {
 #ifdef HAS_BUTTON
-void PayloadConvert::addButton(uint8_t value) { writeUint8(value); }
+  writeUint8(value);
 #endif
+}
 
 void PayloadConvert::intToBytes(uint8_t pos, int32_t i, uint8_t byteSize) {
   for (uint8_t x = 0; x < byteSize; x++) {
@@ -162,7 +167,7 @@ void PayloadConvert::writeUptime(uint64_t uptime) {
   intToBytes(cursor, uptime, 8);
 }
 
-void PayloadConvert::writeVersion(char * version) {
+void PayloadConvert::writeVersion(char *version) {
   memcpy(buffer + cursor, version, 10);
   cursor += 10;
 }
@@ -273,8 +278,8 @@ void PayloadConvert::addStatus(uint16_t voltage, uint64_t uptime, float celsius,
   buffer[cursor++] = lowByte(temp);
 }
 
-#ifdef HAS_GPS
 void PayloadConvert::addGPS(gpsStatus_t value) {
+#ifdef HAS_GPS
   int32_t lat = value.latitude / 100;
   int32_t lon = value.longitude / 100;
   int32_t alt = value.altitude * 100;
@@ -291,18 +296,18 @@ void PayloadConvert::addGPS(gpsStatus_t value) {
   buffer[cursor++] = (byte)((alt & 0xFF0000) >> 16);
   buffer[cursor++] = (byte)((alt & 0x00FF00) >> 8);
   buffer[cursor++] = (byte)((alt & 0x0000FF));
-}
 #endif
+}
 
-#ifdef HAS_BUTTON
 void PayloadConvert::addButton(uint8_t value) {
+#ifdef HAS_BUTTON
 #if (PAYLOAD_ENCODER == 3)
   buffer[cursor++] = LPP_BUTTON_CHANNEL;
 #endif
   buffer[cursor++] = LPP_DIGITAL_INPUT;
   buffer[cursor++] = value;
-}
 #endif
+}
 
 #else
 #error "No valid payload converter defined"