From a3f2dd67657a37e495649fe1786e1e7ee954b130 Mon Sep 17 00:00:00 2001 From: Ed Coyne Date: Fri, 9 Dec 2016 13:51:12 -0800 Subject: [PATCH 1/3] Create a ConfigJsonSerializer object to make interfacing with the serialization logic cleaner, fix bug where sometimes the connection would be closed while printing the config. --- src/thing/Config.cpp | 77 ++++++++++++++++++++++++----------------- src/thing/Config.h | 13 +++++-- src/thing/FireThing.cpp | 5 +-- src/thing/Portal.cpp | 12 +++---- 4 files changed, 65 insertions(+), 42 deletions(-) diff --git a/src/thing/Config.cpp b/src/thing/Config.cpp index 559ce508..2c2bd0c7 100644 --- a/src/thing/Config.cpp +++ b/src/thing/Config.cpp @@ -4,44 +4,57 @@ namespace thing { -void Config::SerializeToJson(Stream* output, std::function 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 = json_.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; +} + +int ConfigJsonSerializer::content_length() const { + return root_->measureLength(); } -void Config::ReadFromJson(char* string) { +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. + String buffer; + root_->printTo(buffer); + output->print(buffer); +} + +Config ConfigJsonSerializer::Deserialize(char* string) { + Config config; + 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"]; + 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"]; + + return config; } }; diff --git a/src/thing/Config.h b/src/thing/Config.h index c2dcdac1..7f656889 100644 --- a/src/thing/Config.h +++ b/src/thing/Config.h @@ -4,6 +4,7 @@ #include "Arduino.h" #include #include +#include "third-party/arduino-json-5.6.7/include/ArduinoJson.h" namespace thing { @@ -29,11 +30,19 @@ struct Config { int wifi_connect_attempts; Pins pins; +}; - void SerializeToJson(Stream* output, std::function handle_size) const; +class ConfigJsonSerializer { + public: + ConfigJsonSerializer(const Config& config); + int content_length() const; + void SerializeTo(Stream* output); // We need a mutable char array here, otherwise a copy will be made. - void ReadFromJson(char* string); + static Config Deserialize(char* string); + private: + DynamicJsonBuffer json_; + JsonObject* root_; }; } // namespace thing diff --git a/src/thing/FireThing.cpp b/src/thing/FireThing.cpp index 00696d8b..c1d35e9b 100644 --- a/src/thing/FireThing.cpp +++ b/src/thing/FireThing.cpp @@ -93,7 +93,7 @@ bool FireThing::ReadConfigFromStorage(Config* config) { } char buffer[cfg.size()]; cfg.readBytes(buffer, cfg.size()); - config->ReadFromJson(buffer); + *config = ConfigJsonSerializer::Deserialize(buffer); debug_("Config read from disk."); } @@ -113,7 +113,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; diff --git a/src/thing/Portal.cpp b/src/thing/Portal.cpp index 672873e3..26575b12 100644 --- a/src/thing/Portal.cpp +++ b/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,7 +132,7 @@ void Portal::Start(const Config& config) { buffer = (char*)malloc(config.length()+1); memcpy(buffer, config.c_str(), config.length()+1); } - config_.ReadFromJson(buffer); + config_ = ConfigJsonSerializer::Deserialize(buffer); free(buffer); callback_(config_); From 06441a1ce8d9e6739c60d574d1799a66d12430bd Mon Sep 17 00:00:00 2001 From: Ed Coyne Date: Tue, 13 Dec 2016 10:19:42 -0800 Subject: [PATCH 2/3] * Changed Serializer::Deserialize to a non-static method. * Fixed default config initialization. * Added Firething call to delete stored config. --- src/thing/Config.cpp | 40 +++++++++++++++++++--------------------- src/thing/Config.h | 11 ++++++++--- src/thing/FireThing.cpp | 20 +++++++++++++++++--- src/thing/FireThing.h | 3 +++ src/thing/Portal.cpp | 7 +++++-- 5 files changed, 52 insertions(+), 29 deletions(-) diff --git a/src/thing/Config.cpp b/src/thing/Config.cpp index 2c2bd0c7..25f30b83 100644 --- a/src/thing/Config.cpp +++ b/src/thing/Config.cpp @@ -5,7 +5,7 @@ namespace thing { ConfigJsonSerializer::ConfigJsonSerializer(const Config& config) { - JsonObject& root = json_.createObject(); + JsonObject& root = buffer_.createObject(); root_ = &root; root["host"] = config.host.c_str(); root["auth"] = config.auth.c_str(); @@ -23,6 +23,10 @@ ConfigJsonSerializer::ConfigJsonSerializer(const Config& config) { pins_root["config_mode_button"] = config.pins.config_mode_button; } +ConfigJsonSerializer::ConfigJsonSerializer(char* serialized_config) { + root_ = &(buffer_.parseObject(serialized_config)); +} + int ConfigJsonSerializer::content_length() const { return root_->measureLength(); } @@ -35,26 +39,20 @@ void ConfigJsonSerializer::SerializeTo(Stream* output) { output->print(buffer); } -Config ConfigJsonSerializer::Deserialize(char* string) { - Config config; - - DynamicJsonBuffer jsonBuffer; - JsonObject& root = jsonBuffer.parseObject(string); - 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"]; - - return config; +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"]; } }; diff --git a/src/thing/Config.h b/src/thing/Config.h index 7f656889..3acf3ed1 100644 --- a/src/thing/Config.h +++ b/src/thing/Config.h @@ -35,13 +35,18 @@ struct Config { class ConfigJsonSerializer { public: ConfigJsonSerializer(const Config& config); + + // We need a mutable char array here, otherwise a copy will be made. + ConfigJsonSerializer(char* config); + int content_length() const; void SerializeTo(Stream* output); + void DeserializeTo(Config* config); - // We need a mutable char array here, otherwise a copy will be made. - static Config Deserialize(char* string); private: - DynamicJsonBuffer json_; + JsonObject& root() {return *root_;} + + DynamicJsonBuffer buffer_; JsonObject* root_; }; diff --git a/src/thing/FireThing.cpp b/src/thing/FireThing.cpp index c1d35e9b..d455727c 100644 --- a/src/thing/FireThing.cpp +++ b/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 = ConfigJsonSerializer::Deserialize(buffer); + ConfigJsonSerializer serializer(buffer); + serializer.DeserializeTo(config); debug_("Config read from disk."); } diff --git a/src/thing/FireThing.h b/src/thing/FireThing.h index 594d9acc..c161a07e 100644 --- a/src/thing/FireThing.h +++ b/src/thing/FireThing.h @@ -19,6 +19,9 @@ class FireThing { void SetDebugHandler(std::function debug); + // Called to delete the currently stored config from the filesystem. + bool DeleteStoredConfig(); + private: bool ReadConfigFromStorage(Config* config); bool WriteConfigToStorage(const Config& config); diff --git a/src/thing/Portal.cpp b/src/thing/Portal.cpp index 26575b12..cf70a363 100644 --- a/src/thing/Portal.cpp +++ b/src/thing/Portal.cpp @@ -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_ = ConfigJsonSerializer::Deserialize(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", ""); From f987e3b3a982250130b06db0ef68f0b9facfd2db Mon Sep 17 00:00:00 2001 From: Ed Coyne Date: Tue, 13 Dec 2016 10:22:37 -0800 Subject: [PATCH 3/3] added savings to todo comment --- src/thing/Config.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/thing/Config.cpp b/src/thing/Config.cpp index 25f30b83..d810630e 100644 --- a/src/thing/Config.cpp +++ b/src/thing/Config.cpp @@ -33,7 +33,8 @@ int ConfigJsonSerializer::content_length() const { 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. + // 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);