Internal improvements: upgrade to ArduinoJson v6, use upstream HTTP l…

…ib, etc... (#458)

* Use upstream library for webserver

* Use std::vector instead of raw arrays

* Fix memory leak when recreating MQTT client

* Fix memory leak when applying new settings

* Use upstream lib for routes

* Upgrade to ArduinoJson v6

* don't use custom method -- client() was exposed in upstream library

* add tests

* true/flase -> on/off

* use builtin OTA handler

* add passthrough authprovider

* remove unused class def

* stick with SDK 2.4 for now

lib/Helpers/JsonHelpers.h

@@ -9,11 +9,26 @@
 class JsonHelpers {
 public:
   template<typename T>
-  static std::vector<T> jsonArrToVector(JsonArray& arr, std::function<T (const String&)> converter, const bool unique = true) {
+  static void copyFrom(JsonArray arr, std::vector<T> vec) {
+    for (typename std::vector<T>::const_iterator it = vec.begin(); it != vec.end(); ++it) {
+      arr.add(*it);
+    }
+  }
+
+  template<typename T>
+  static void copyTo(JsonArray arr, std::vector<T> vec) {
+    for (size_t i = 0; i < arr.size(); ++i) {
+      JsonVariant val = arr[i];
+      vec.push_back(val.as<T>());
+    }
+  }
+
+  template<typename T, typename StrType>
+  static std::vector<T> jsonArrToVector(JsonArray& arr, std::function<T (const StrType)> converter, const bool unique = true) {
     std::vector<T> vec;
 
     for (size_t i = 0; i < arr.size(); ++i) {
-      String strVal = arr.get<const char*>(i);
+      StrType strVal = arr[i];
       T convertedVal = converter(strVal);
 
       // inefficient, but everything using this is tiny, so doesn't matter
@@ -25,8 +40,8 @@ class JsonHelpers {
     return vec;
   }
 
-  template<typename T>
-  static void vectorToJsonArr(JsonArray& arr, const std::vector<T>& vec, std::function<String (const T&)> converter) {
+  template<typename T, typename StrType>
+  static void vectorToJsonArr(JsonArray& arr, const std::vector<T>& vec, std::function<StrType (const T&)> converter) {
     for (typename std::vector<T>::const_iterator it = vec.begin(); it != vec.end(); ++it) {
       arr.add(converter(*it));
     }

lib/MQTT/BulbStateUpdater.cpp

@@ -39,10 +39,11 @@ void BulbStateUpdater::loop() {
 
 inline void BulbStateUpdater::flushGroup(BulbId bulbId, GroupState& state) {
   char buffer[200];
-  StaticJsonBuffer<200> jsonBuffer;
-  JsonObject& message = jsonBuffer.createObject();
-  state.applyState(message, bulbId, settings.groupStateFields, settings.numGroupStateFields);
-  message.printTo(buffer);
+  StaticJsonDocument<200> json;
+  JsonObject message = json.to<JsonObject>();
+
+  state.applyState(message, bulbId, settings.groupStateFields);
+  serializeJson(json, buffer);
 
   mqttClient.sendState(
     *MiLightRemoteConfig::fromType(bulbId.deviceType),

lib/MQTT/MqttClient.cpp

@@ -13,21 +13,20 @@ static const char* STATUS_DISCONNECTED = "disconnected_clean";
 static const char* STATUS_LWT_DISCONNECTED = "disconnected_unclean";
 
 MqttClient::MqttClient(Settings& settings, MiLightClient*& milightClient)
-  : milightClient(milightClient),
+  : mqttClient(tcpClient),
+    milightClient(milightClient),
     settings(settings),
     lastConnectAttempt(0)
 {
   String strDomain = settings.mqttServer();
   this->domain = new char[strDomain.length() + 1];
   strcpy(this->domain, strDomain.c_str());
-
-  this->mqttClient = new PubSubClient(tcpClient);
 }
 
 MqttClient::~MqttClient() {
   String aboutStr = generateConnectionStatusMessage(STATUS_DISCONNECTED);
-  mqttClient->publish(settings.mqttClientStatusTopic.c_str(), aboutStr.c_str(), true);
-  mqttClient->disconnect();
+  mqttClient.publish(settings.mqttClientStatusTopic.c_str(), aboutStr.c_str(), true);
+  mqttClient.disconnect();
   delete this->domain;
 }
 
@@ -41,8 +40,8 @@ void MqttClient::begin() {
   );
 #endif
 
-  mqttClient->setServer(this->domain, settings.mqttPort());
-  mqttClient->setCallback(
+  mqttClient.setServer(this->domain, settings.mqttPort());
+  mqttClient.setCallback(
     [this](char* topic, byte* payload, int length) {
       this->publishCallback(topic, payload, length);
     }
@@ -59,7 +58,7 @@ bool MqttClient::connect() {
 #endif
 
   if (settings.mqttUsername.length() > 0 && settings.mqttClientStatusTopic.length() > 0) {
-    return mqttClient->connect(
+    return mqttClient.connect(
       nameBuffer,
       settings.mqttUsername.c_str(),
       settings.mqttPassword.c_str(),
@@ -69,28 +68,28 @@ bool MqttClient::connect() {
       generateConnectionStatusMessage(STATUS_LWT_DISCONNECTED).c_str()
     );
   } else if (settings.mqttUsername.length() > 0) {
-    return mqttClient->connect(
+    return mqttClient.connect(
       nameBuffer,
       settings.mqttUsername.c_str(),
       settings.mqttPassword.c_str()
     );
   } else if (settings.mqttClientStatusTopic.length() > 0) {
-    return mqttClient->connect(
+    return mqttClient.connect(
       nameBuffer,
       settings.mqttClientStatusTopic.c_str(),
       2,
       true,
       generateConnectionStatusMessage(STATUS_LWT_DISCONNECTED).c_str()
     );
   } else {
-    return mqttClient->connect(nameBuffer);
+    return mqttClient.connect(nameBuffer);
   }
 }
 
 void MqttClient::sendBirthMessage() {
   if (settings.mqttClientStatusTopic.length() > 0) {
     String aboutStr = generateConnectionStatusMessage(STATUS_CONNECTED);
-    mqttClient->publish(settings.mqttClientStatusTopic.c_str(), aboutStr.c_str(), true);
+    mqttClient.publish(settings.mqttClientStatusTopic.c_str(), aboutStr.c_str(), true);
   }
 }
 
@@ -99,7 +98,7 @@ void MqttClient::reconnect() {
     return;
   }
 
-  if (! mqttClient->connected()) {
+  if (! mqttClient.connected()) {
     if (connect()) {
       subscribe();
       sendBirthMessage();
@@ -117,7 +116,7 @@ void MqttClient::reconnect() {
 
 void MqttClient::handleClient() {
   reconnect();
-  mqttClient->loop();
+  mqttClient.loop();
 }
 
 void MqttClient::sendUpdate(const MiLightRemoteConfig& remoteConfig, uint16_t deviceId, uint16_t groupId, const char* update) {
@@ -141,7 +140,7 @@ void MqttClient::subscribe() {
   printf_P(PSTR("MqttClient - subscribing to topic: %s\n"), topic.c_str());
 #endif
 
-  mqttClient->subscribe(topic.c_str());
+  mqttClient.subscribe(topic.c_str());
 }
 
 void MqttClient::publish(
@@ -163,7 +162,7 @@ void MqttClient::publish(
   printf("MqttClient - publishing update to %s\n", topic.c_str());
 #endif
 
-  mqttClient->publish(topic.c_str(), message, retain);
+  mqttClient.publish(topic.c_str(), message, retain);
 }
 
 void MqttClient::publishCallback(char* topic, byte* payload, int length) {
@@ -208,8 +207,9 @@ void MqttClient::publishCallback(char* topic, byte* payload, int length) {
     Serial.println(F("MqttClient - WARNING: could not find device_type token.  Defaulting to FUT092.\n"));
   }
 
-  StaticJsonBuffer<400> buffer;
-  JsonObject& obj = buffer.parseObject(cstrPayload);
+  StaticJsonDocument<400> buffer;
+  deserializeJson(buffer, cstrPayload);
+  JsonObject obj = buffer.as<JsonObject>();
 
 #ifdef MQTT_DEBUG
   printf("MqttClient - device %04X, group %u\n", deviceId, groupId);
@@ -237,15 +237,14 @@ inline void MqttClient::bindTopicString(
 }
 
 String MqttClient::generateConnectionStatusMessage(const char* connectionStatus) {
-  DynamicJsonBuffer buffer;
-  JsonObject& status = buffer.createObject();
-  status["status"] = connectionStatus;
+  StaticJsonDocument<1024> json;
+  json["status"] = connectionStatus;
 
   // Fill other fields
-  AboutHelper::generateAboutObject(status, true);
+  AboutHelper::generateAboutObject(json, true);
 
   String response;
-  status.printTo(response);
+  serializeJson(json, response);
 
   return response;
 }

lib/MQTT/MqttClient.h

@@ -24,7 +24,7 @@ class MqttClient {
 
 private:
   WiFiClient tcpClient;
-  PubSubClient* mqttClient;
+  PubSubClient mqttClient;
   MiLightClient*& milightClient;
   Settings& settings;
   char* domain;

lib/MiLight/CctPacketFormatter.cpp

@@ -189,7 +189,7 @@ MiLightStatus CctPacketFormatter::cctCommandToStatus(uint8_t command) {
   }
 }
 
-BulbId CctPacketFormatter::parsePacket(const uint8_t* packet, JsonObject& result) {
+BulbId CctPacketFormatter::parsePacket(const uint8_t* packet, JsonObject result) {
   uint8_t command = packet[CCT_COMMAND_INDEX] & 0x7F;
 
   uint8_t onOffGroupId = cctCommandIdToGroup(command);

lib/MiLight/CctPacketFormatter.h

@@ -46,7 +46,7 @@ class CctPacketFormatter : public PacketFormatter {
   virtual void format(uint8_t const* packet, char* buffer);
   virtual void initializePacket(uint8_t* packet);
   virtual void finalizePacket(uint8_t* packet);
-  virtual BulbId parsePacket(const uint8_t* packet, JsonObject& result);
+  virtual BulbId parsePacket(const uint8_t* packet, JsonObject result);
 
   static uint8_t getCctStatusButton(uint8_t groupId, MiLightStatus status);
   static uint8_t cctCommandIdToGroup(uint8_t command);

lib/MiLight/FUT089PacketFormatter.cpp

@@ -91,7 +91,7 @@ void FUT089PacketFormatter::enableNightMode() {
   command(FUT089_ON | 0x80, arg);
 }
 
-BulbId FUT089PacketFormatter::parsePacket(const uint8_t *packet, JsonObject& result) {
+BulbId FUT089PacketFormatter::parsePacket(const uint8_t *packet, JsonObject result) {
   if (stateStore == NULL) {
     Serial.println(F("ERROR: stateStore not set.  Prepare was not called!  **THIS IS A BUG**"));
     BulbId fakeId(0, 0, REMOTE_TYPE_FUT089);

lib/MiLight/FUT089PacketFormatter.h

@@ -39,7 +39,7 @@ class FUT089PacketFormatter : public V2PacketFormatter {
   virtual void modeSpeedUp();
   virtual void updateMode(uint8_t mode);
 
-  virtual BulbId parsePacket(const uint8_t* packet, JsonObject& result);
+  virtual BulbId parsePacket(const uint8_t* packet, JsonObject result);
 };
 
 #endif

lib/MiLight/FUT091PacketFormatter.cpp

@@ -18,7 +18,7 @@ void FUT091PacketFormatter::enableNightMode() {
   command(static_cast<uint8_t>(FUT091Command::ON_OFF) | 0x80, arg);
 }
 
-BulbId FUT091PacketFormatter::parsePacket(const uint8_t *packet, JsonObject& result) {
+BulbId FUT091PacketFormatter::parsePacket(const uint8_t *packet, JsonObject result) {
   uint8_t packetCopy[V2_PACKET_LEN];
   memcpy(packetCopy, packet, V2_PACKET_LEN);
   V2RFEncoding::decodeV2Packet(packetCopy);

lib/MiLight/FUT091PacketFormatter.h

@@ -19,7 +19,7 @@ class FUT091PacketFormatter : public V2PacketFormatter {
   virtual void updateTemperature(uint8_t value);
   virtual void enableNightMode();
 
-  virtual BulbId parsePacket(const uint8_t* packet, JsonObject& result);
+  virtual BulbId parsePacket(const uint8_t* packet, JsonObject result);
 };
 
 #endif

lib/MiLight/MiLightClient.cpp

@@ -6,13 +6,12 @@
 #include <TokenIterator.h>
 
 MiLightClient::MiLightClient(
-  MiLightRadioFactory* radioFactory,
+  std::shared_ptr<MiLightRadioFactory> radioFactory,
   GroupStateStore* stateStore,
   Settings* settings
 )
   : currentRadio(NULL),
     currentRemote(NULL),
-    numRadios(MiLightRadioConfig::NUM_CONFIGS),
     packetSentHandler(NULL),
     updateBeginHandler(NULL),
     updateEndHandler(NULL),
@@ -21,15 +20,13 @@ MiLightClient::MiLightClient(
     lastSend(0),
     baseResendCount(MILIGHT_DEFAULT_RESEND_COUNT)
 {
-  radios = new MiLightRadio*[numRadios];
-
-  for (size_t i = 0; i < numRadios; i++) {
-    radios[i] = radioFactory->create(MiLightRadioConfig::ALL_CONFIGS[i]);
+  for (size_t i = 0; i < MiLightRadioConfig::NUM_CONFIGS; i++) {
+    radios.push_back(radioFactory->create(MiLightRadioConfig::ALL_CONFIGS[i]));
   }
 }
 
 void MiLightClient::begin() {
-  for (size_t i = 0; i < numRadios; i++) {
+  for (size_t i = 0; i < radios.size(); i++) {
     radios[i]->begin();
   }
 
@@ -49,10 +46,10 @@ void MiLightClient::setHeld(bool held) {
 }
 
 size_t MiLightClient::getNumRadios() const {
-  return numRadios;
+  return radios.size();
 }
 
-MiLightRadio* MiLightClient::switchRadio(size_t radioIx) {
+std::shared_ptr<MiLightRadio> MiLightClient::switchRadio(size_t radioIx) {
   if (radioIx >= getNumRadios()) {
     return NULL;
   }
@@ -65,10 +62,10 @@ MiLightRadio* MiLightClient::switchRadio(size_t radioIx) {
   return this->currentRadio;
 }
 
-MiLightRadio* MiLightClient::switchRadio(const MiLightRemoteConfig* remoteConfig) {
-  MiLightRadio* radio = NULL;
+std::shared_ptr<MiLightRadio> MiLightClient::switchRadio(const MiLightRemoteConfig* remoteConfig) {
+  std::shared_ptr<MiLightRadio> radio = NULL;
 
-  for (size_t i = 0; i < numRadios; i++) {
+  for (size_t i = 0; i < radios.size(); i++) {
     if (&this->radios[i]->config() == &remoteConfig->radioConfig) {
       radio = switchRadio(i);
       break;
@@ -330,7 +327,7 @@ void MiLightClient::toggleStatus() {
   flushPacket();
 }
 
-void MiLightClient::update(const JsonObject& request) {
+void MiLightClient::update(JsonObject request) {
   if (this->updateBeginHandler) {
     this->updateBeginHandler();
   }
@@ -347,11 +344,11 @@ void MiLightClient::update(const JsonObject& request) {
   }
 
   if (request.containsKey("commands")) {
-    JsonArray& commands = request["commands"];
+    JsonArray commands = request["commands"];
 
-    if (commands.success()) {
+    if (! commands.isNull()) {
       for (size_t i = 0; i < commands.size(); i++) {
-        this->handleCommand(commands.get<String>(i));
+        this->handleCommand(commands[i].as<const char*>());
       }
     }
   }
@@ -373,7 +370,7 @@ void MiLightClient::update(const JsonObject& request) {
     uint16_t r, g, b;
 
     if (request["color"].is<JsonObject>()) {
-      JsonObject& color = request["color"];
+      JsonObject color = request["color"];
 
       r = color["r"];
       g = color["g"];
@@ -422,7 +419,7 @@ void MiLightClient::update(const JsonObject& request) {
   }
   // HomeAssistant
   if (request.containsKey("brightness")) {
-    uint8_t scaledBrightness = Units::rescale(request.get<uint8_t>("brightness"), 100, 255);
+    uint8_t scaledBrightness = Units::rescale(request["brightness"].as<uint8_t>(), 100, 255);
     this->updateBrightness(scaledBrightness);
   }
 
@@ -498,13 +495,13 @@ void MiLightClient::handleEffect(const String& effect) {
   }
 }
 
-uint8_t MiLightClient::parseStatus(const JsonObject& object) {
+uint8_t MiLightClient::parseStatus(JsonObject object) {
   String strStatus;
 
   if (object.containsKey("status")) {
-    strStatus = object.get<char*>("status");
+    strStatus = object["status"].as<char*>();
   } else if (object.containsKey("state")) {
-    strStatus = object.get<char*>("state");
+    strStatus = object["state"].as<char*>();
   } else {
     return 255;
   }