Merge pull request #235 from ed7coyne/firethings

Clean up api for serialization, fix bug with config fetching from webserver.

src/thing/Config.cpp

@@ -4,44 +4,56 @@
 
 namespace thing {
 
-void Config::SerializeToJson(Stream* output, std::function<void(int size)> handle_size) const {
-  DynamicJsonBuffer jsonBuffer;
-  JsonObject& root = jsonBuffer.createObject();
-  root["host"] = host.c_str();
-  root["auth"] = auth.c_str();
-  root["path"] = path.c_str();
-  root["wifi_ssid"] = wifi_ssid.c_str();
-  root["wifi_key"] = wifi_key.c_str();
-  root["analog_activation"] = analog_activation_threshold;
-  root["wifi_connect_attempts"] = wifi_connect_attempts;
+ConfigJsonSerializer::ConfigJsonSerializer(const Config& config) {
+  JsonObject& root = buffer_.createObject();
+  root_ = &root;
+  root["host"] = config.host.c_str();
+  root["auth"] = config.auth.c_str();
+  root["path"] = config.path.c_str();
+  root["wifi_ssid"] = config.wifi_ssid.c_str();
+  root["wifi_key"] = config.wifi_key.c_str();
+  root["analog_activation"] = config.analog_activation_threshold;
+  root["wifi_connect_attempts"] = config.wifi_connect_attempts;
 
   JsonObject& pins_root = root.createNestedObject("pins");
-  pins_root["digital_in"] = pins.digital_in;
-  pins_root["digital_out"] = pins.digital_out;
-  pins_root["analog_in"] = pins.analog_in;
-  pins_root["analog_out"] = pins.analog_out;
-  pins_root["config_mode_button"] = pins.config_mode_button;
-
-  handle_size(root.measureLength());
-  root.printTo(*output);
+  pins_root["digital_in"] = config.pins.digital_in;
+  pins_root["digital_out"] = config.pins.digital_out;
+  pins_root["analog_in"] = config.pins.analog_in;
+  pins_root["analog_out"] = config.pins.analog_out;
+  pins_root["config_mode_button"] = config.pins.config_mode_button;
 }
 
-void Config::ReadFromJson(char* string) {
-  DynamicJsonBuffer jsonBuffer;
-  JsonObject& root = jsonBuffer.parseObject(string);
-  host = root["host"].asString();
-  auth = root["auth"].asString();
-  path = root["path"].asString();
-  wifi_ssid = root["wifi_ssid"].asString();
-  wifi_key = root["wifi_key"].asString();
-  analog_activation_threshold = root["activation_threshold"];
-  wifi_connect_attempts = root["wifi_connect_attempts"];
-
-  pins.digital_in = root["pins"]["digital_in"];
-  pins.digital_out = root["pins"]["digital_out"];
-  pins.analog_in = root["pins"]["analog_in"];
-  pins.analog_out = root["pins"]["analog_out"];
-  pins.config_mode_button = root["pins"]["config_mode_button"];
+ConfigJsonSerializer::ConfigJsonSerializer(char* serialized_config) {
+  root_ = &(buffer_.parseObject(serialized_config));
+}
+
+int ConfigJsonSerializer::content_length() const {
+  return root_->measureLength();
+}
+
+void ConfigJsonSerializer::SerializeTo(Stream* output) {
+  // TODO: We "should" be able to have the root_ print directly to the stream
+  // however it currently closes the connection half way through. Fixing this
+  // would save ~250B of memory during serialization.
+  String buffer;
+  root_->printTo(buffer);
+  output->print(buffer);
+}
+
+void ConfigJsonSerializer::DeserializeTo(Config* config) {
+  config->host = root()["host"].asString();
+  config->auth = root()["auth"].asString();
+  config->path = root()["path"].asString();
+  config->wifi_ssid = root()["wifi_ssid"].asString();
+  config->wifi_key = root()["wifi_key"].asString();
+  config->analog_activation_threshold = root()["activation_threshold"];
+  config->wifi_connect_attempts = root()["wifi_connect_attempts"];
+
+  config->pins.digital_in = root()["pins"]["digital_in"];
+  config->pins.digital_out = root()["pins"]["digital_out"];
+  config->pins.analog_in = root()["pins"]["analog_in"];
+  config->pins.analog_out = root()["pins"]["analog_out"];
+  config->pins.config_mode_button = root()["pins"]["config_mode_button"];
 }
 
 };

src/thing/Config.h

@@ -4,6 +4,7 @@
 #include "Arduino.h"
 #include <string>
 #include <functional>
+#include "third-party/arduino-json-5.6.7/include/ArduinoJson.h"
 
 namespace thing {
 
@@ -29,11 +30,24 @@ struct Config {
   int wifi_connect_attempts;
 
   Pins pins;
+};
 
-  void SerializeToJson(Stream* output, std::function<void(int size)> handle_size) const;
+class ConfigJsonSerializer {
+ public:
+  ConfigJsonSerializer(const Config& config);
 
   // We need a mutable char array here, otherwise a copy will be made.
-  void ReadFromJson(char* string);
+  ConfigJsonSerializer(char* config);
+
+  int content_length() const;
+  void SerializeTo(Stream* output);
+  void DeserializeTo(Config* config);
+
+ private:
+  JsonObject& root() {return *root_;}
+
+  DynamicJsonBuffer buffer_;
+  JsonObject* root_;
 };
 
 }  // namespace thing

src/thing/FireThing.cpp

@@ -12,11 +12,14 @@ Config kDefaultConfig = {
   "",  // wifi ssid
   "",  // wifi key
   0.1,  // analog activation threshold
+  2, // wifi connect attempts
+  {
   D1,  // digital in
   BUILTIN_LED,  // digital out
   A0,  // analog in
   D1, // analog out
   D0, // config mode button
+  }
 };
 
 const char kStorageFilename[] = "fthing.cfg";
@@ -32,8 +35,7 @@ bool FireThing::Setup() {
     return false;
   }
   SetPinModes(config);
-
-  if (digitalRead(config.pins.config_mode_button) || !ConnectToWiFi(config)) {
+  if (digitalRead(config.pins.config_mode_button) == HIGH || !ConnectToWiFi(config)) {
     wifi_.StartAP();
   }
 
@@ -54,9 +56,20 @@ void FireThing::Loop() {
   transcriber_.Loop();
 }
 
+bool FireThing::DeleteStoredConfig() {
+  if (!SPIFFS.begin()) {
+    debug_("Failed to mount FS.");
+    return false;
+  }
+  bool success = SPIFFS.remove(kStorageFilename);
+  SPIFFS.end();
+  return success;
+}
+
 bool FireThing::ConnectToWiFi(const Config& config) {
   debug_("Connecting to wifi:");
   debug_(config.wifi_ssid.c_str());
+  // TODO we should probably not print the key to serial.
   debug_(config.wifi_key.c_str());
   if (wifi_.Connect(config.wifi_ssid, config.wifi_key)) {
     debug_("Connected");
@@ -93,7 +106,8 @@ bool FireThing::ReadConfigFromStorage(Config* config) {
     }
     char buffer[cfg.size()];
     cfg.readBytes(buffer, cfg.size());
-    config->ReadFromJson(buffer);
+    ConfigJsonSerializer serializer(buffer);
+    serializer.DeserializeTo(config);
     debug_("Config read from disk.");
   }
 
@@ -113,7 +127,8 @@ bool FireThing::WriteConfigToStorage(const Config& config) {
     SPIFFS.end();
     return false;
   }
-  config.SerializeToJson(&cfg, [](int){});
+  ConfigJsonSerializer serializer(config);
+  serializer.SerializeTo(&cfg);
 
   SPIFFS.end();
   return true;

src/thing/FireThing.h

@@ -19,6 +19,9 @@ class FireThing {
 
   void SetDebugHandler(std::function<void(const char* message)> debug);
 
+  // Called to delete the currently stored config from the filesystem.
+  bool DeleteStoredConfig();
+
  private:
   bool ReadConfigFromStorage(Config* config);
   bool WriteConfigToStorage(const Config& config);

src/thing/Portal.cpp

@@ -111,11 +111,11 @@ void Portal::Start(const Config& config) {
   server_.on("/config", [&] () {
     if (server_.method() == HTTP_GET) {
       auto client = server_.client();
-      config_.SerializeToJson(&client,
-                              [this](int size) {
-                                server_.setContentLength(size);
-                                server_.send(200, "application/json");
-                              });
+
+      ConfigJsonSerializer serializer(config_);
+      server_.setContentLength(serializer.content_length());
+      server_.send(200, "application/json");
+      serializer.SerializeTo(&client);
 
       debug_("config retrieved");
     } else if (server_.method() == HTTP_POST) {
@@ -132,8 +132,11 @@ void Portal::Start(const Config& config) {
         buffer = (char*)malloc(config.length()+1);
         memcpy(buffer, config.c_str(), config.length()+1);
       }
-      config_.ReadFromJson(buffer);
-      free(buffer);
+      { // Scoped because serializer is invalid after free().
+        ConfigJsonSerializer serializer(buffer);
+        serializer.DeserializeTo(&config_);
+        free(buffer);
+      }
 
       callback_(config_);
       server_.send(200, "text/plain", "");