From 77be5b98670ca22a8fd7c972fea124c535d1f1b5 Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Mon, 7 Oct 2024 10:39:26 +0300 Subject: [PATCH 1/6] Use underscore names for member variables --- src/sensesp/sensors/analog_input.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sensesp/sensors/analog_input.cpp b/src/sensesp/sensors/analog_input.cpp index f079b8ace..cac18e5d3 100644 --- a/src/sensesp/sensors/analog_input.cpp +++ b/src/sensesp/sensors/analog_input.cpp @@ -13,7 +13,7 @@ AnalogInput::AnalogInput(uint8_t pin, unsigned int read_delay, pin{pin}, read_delay{read_delay}, output_scale{output_scale} { - analog_reader_ = new AnalogReader(pin); + analog_reader_ = std::unique_ptr(new AnalogReader(pin)); load(); if (this->analog_reader_->configure()) { From 41699dd791fef4ea571fccd40d4522c6d5213222 Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Sun, 6 Oct 2024 19:45:32 +0300 Subject: [PATCH 2/6] Implement some initial unit tests --- platformio.ini | 1 + test/system/test_minimal_app/minimal_app.cpp | 44 ++++++ .../test_serialization/serializable.cpp | 145 ++++++++++++++++++ 3 files changed, 190 insertions(+) create mode 100644 test/system/test_minimal_app/minimal_app.cpp create mode 100644 test/system/test_serialization/serializable.cpp diff --git a/platformio.ini b/platformio.ini index 330223d54..fac59fec7 100644 --- a/platformio.ini +++ b/platformio.ini @@ -66,6 +66,7 @@ build_flags = ;debug_tool = esp-prog ;debug_init_break = tbreak setup +test_build_src = true check_tool = clangtidy check_flags = clangtidy: --fix --format-style=file --config-file=.clang-tidy diff --git a/test/system/test_minimal_app/minimal_app.cpp b/test/system/test_minimal_app/minimal_app.cpp new file mode 100644 index 000000000..7ebb6a442 --- /dev/null +++ b/test/system/test_minimal_app/minimal_app.cpp @@ -0,0 +1,44 @@ +#include "sensesp.h" + +#include "elapsedMillis.h" +#include "sensesp/sensors/sensor.h" +#include "sensesp/system/observablevalue.h" +#include "sensesp/system/lambda_consumer.h" +#include "sensesp_minimal_app_builder.h" +#include "unity.h" + +using namespace sensesp; + +void test_sensesp() { + SensESPMinimalAppBuilder builder; + SensESPMinimalApp* sensesp_app = builder.get_app(); + TEST_ASSERT_NOT_NULL(sensesp_app); + + auto sensor = RepeatSensor(10, []() { + static int value = 42; + return value++; + }); + + ObservableValue* observable = new ObservableValue(); + sensor.connect_to(observable); + + auto test_consumer = new LambdaConsumer([](int value) { + static int target_value = 42; + TEST_ASSERT_EQUAL(target_value, value); + TEST_ASSERT_TRUE(42 <= target_value && target_value < 47); + target_value++; + }); + + elapsedMillis elapsed = 0; + while (elapsed < 100) { + event_loop()->tick(); + } +} + +void setup() { + UNITY_BEGIN(); + RUN_TEST(test_sensesp); + UNITY_END(); +} + +void loop() {} diff --git a/test/system/test_serialization/serializable.cpp b/test/system/test_serialization/serializable.cpp new file mode 100644 index 000000000..e7f2380fe --- /dev/null +++ b/test/system/test_serialization/serializable.cpp @@ -0,0 +1,145 @@ +#include "sensesp.h" + +#include "sensesp/system/serializable.h" +#include "sensesp/system/valueproducer.h" +#include "sensesp/transforms/angle_correction.h" +#include "sensesp_base_app.h" +#include "sensesp_minimal_app_builder.h" +#include "unity.h" + +using namespace sensesp; + +class TestProducer : public ValueProducer, public Serializable { + public: + TestProducer() : ValueProducer() {} + + bool to_json(JsonObject& root) override { + root["a"] = a_; + root["b"] = b_; + JsonArray float_array = root["c"].to(); + for (int i = 0; i < 3; i++) { + float_array.add(c_[i]); + } + JsonObject map = root["map"].to(); + map[map_key_] = map_value_; + return true; + } + + // Normally these would be protected, but for testing purposes, we make them + // public. + int a_ = 567; + String b_ = "hello"; + float c_[3] = {1.1, 2.2, 3.3}; + String map_key_ = "key"; + String map_value_ = "value"; + + bool from_json(const JsonObject& root) override { + a_ = root["a"]; + b_ = root["b"].as(); + JsonArray float_array = root["c"]; + for (int i = 0; i < 3; i++) { + c_[i] = float_array[i]; + } + JsonObject map = root["map"]; + map_value_ = map["key"].as(); + return true; + } +}; + +void test_to_json() { + TestProducer producer; + JsonDocument doc; + JsonObject root = doc.to(); + + // This is not particularly useful as a test because we are simply testing + // the implementation written above. + + producer.to_json(root); + + TEST_ASSERT_EQUAL(567, root["a"]); + TEST_ASSERT_EQUAL_STRING("hello", root["b"]); + TEST_ASSERT_EQUAL_FLOAT(1.1, root["c"][0]); + TEST_ASSERT_EQUAL_FLOAT(2.2, root["c"][1]); + TEST_ASSERT_EQUAL_FLOAT(3.3, root["c"][2]); + TEST_ASSERT_EQUAL_STRING("value", root["map"]["key"]); + + String json_str; + serializeJson(doc, json_str); + ESP_LOGD("test_to_json", "json_str: %s", json_str.c_str()); + + String test_str = + R"...({"a":567,"b":"hello","c":[1.1,2.2,3.3],"map":{"key":"value"}})..."; + + TEST_ASSERT_EQUAL_STRING(test_str.c_str(), json_str.c_str()); +} + +void test_from_json() { + String test_str = + R"...({"a":123,"b":"goodbye","c":[4.4,5.5,6.6],"map":{"key":"new_value"}})..."; + + TestProducer producer; + JsonDocument doc; + DeserializationError error = deserializeJson(doc, test_str); + TEST_ASSERT_FALSE(error); + + JsonObject root = doc.as(); + producer.from_json(root); + + TEST_ASSERT_EQUAL(123, producer.a_); + TEST_ASSERT_EQUAL_STRING("goodbye", producer.b_.c_str()); + TEST_ASSERT_EQUAL_FLOAT(4.4, producer.c_[0]); + TEST_ASSERT_EQUAL_FLOAT(5.5, producer.c_[1]); + TEST_ASSERT_EQUAL_FLOAT(6.6, producer.c_[2]); + TEST_ASSERT_EQUAL_STRING("new_value", producer.map_value_.c_str()); +} + +// To access protected members, we need to create a child class +class AngleCorrection_ : public AngleCorrection { + public: + AngleCorrection_(float offset, float min_angle, const String& config_path = "") + : AngleCorrection(offset, min_angle, config_path) {} + + float get_offset() { return offset_; } + float get_min_angle() { return min_angle_; } +}; + +void test_angle_correction_to_json() { + AngleCorrection_ angle_correction(1.1, 2.2); + JsonDocument doc; + JsonObject root = doc.to(); + + angle_correction.to_json(root); + + TEST_ASSERT_EQUAL_FLOAT(1.1, root["offset"]); + TEST_ASSERT_EQUAL_FLOAT(2.2, root["min_angle"]); +} + +void test_angle_correction_from_json() { + String test_str = R"...({"offset":3.3,"min_angle":4.4})..."; + + AngleCorrection_ angle_correction(1.1, 2.2); + JsonDocument doc; + DeserializationError error = deserializeJson(doc, test_str); + TEST_ASSERT_FALSE(error); + + JsonObject root = doc.as(); + angle_correction.from_json(root); + + TEST_ASSERT_EQUAL_FLOAT(3.3, angle_correction.get_offset()); + TEST_ASSERT_EQUAL_FLOAT(4.4, angle_correction.get_min_angle()); +} + +void setup() { + esp_log_level_set("*", ESP_LOG_VERBOSE); + + UNITY_BEGIN(); + + RUN_TEST(test_to_json); + RUN_TEST(test_from_json); + RUN_TEST(test_angle_correction_to_json); + RUN_TEST(test_angle_correction_from_json); + + UNITY_END(); +} + +void loop() {} From 4abee8f75f761313ebea7d4c7d6a89ff2d2e8b0a Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Mon, 7 Oct 2024 19:34:09 +0300 Subject: [PATCH 3/6] Improve memory safety with shared and unique ptrs --- .../controllers/smart_switch_controller.cpp | 2 +- .../controllers/smart_switch_controller.h | 4 +- src/sensesp/net/http_server.cpp | 4 +- src/sensesp/net/http_server.h | 22 +++++--- src/sensesp/net/networking.cpp | 18 ++++--- src/sensesp/net/networking.h | 6 +-- src/sensesp/net/web/app_command_handler.cpp | 6 ++- src/sensesp/net/web/config_handler.cpp | 1 + src/sensesp/sensors/analog_input.h | 5 +- src/sensesp/sensors/system_info.h | 3 +- src/sensesp/signalk/signalk_emitter.h | 2 +- src/sensesp/signalk/signalk_output.h | 17 +++++-- src/sensesp/system/base_button.h | 5 +- src/sensesp/system/filesystem.cpp | 4 ++ src/sensesp/system/filesystem.h | 1 + src/sensesp/system/system_status_led.cpp | 3 +- src/sensesp/system/system_status_led.h | 4 +- src/sensesp/system/valueproducer.h | 25 ++++++++++ src/sensesp/ui/ui_button.cpp | 2 +- src/sensesp/ui/ui_button.h | 9 ++-- src/sensesp_app.cpp | 29 +++++++---- src/sensesp_app.h | 50 +++++++++---------- src/sensesp_app_builder.h | 2 + src/sensesp_base_app.cpp | 7 +++ src/sensesp_base_app.h | 1 + 25 files changed, 154 insertions(+), 78 deletions(-) diff --git a/src/sensesp/controllers/smart_switch_controller.cpp b/src/sensesp/controllers/smart_switch_controller.cpp index 6b32f7591..338b44487 100644 --- a/src/sensesp/controllers/smart_switch_controller.cpp +++ b/src/sensesp/controllers/smart_switch_controller.cpp @@ -95,7 +95,7 @@ SmartSwitchController::SyncPath::SyncPath() {} SmartSwitchController::SyncPath::SyncPath(String sk_sync_path) : sk_sync_path_{sk_sync_path} { ESP_LOGD(__FILENAME__, "DoubleClick will also sync %s", sk_sync_path.c_str()); - this->put_request_ = new BoolSKPutRequest(sk_sync_path, "", false); + this->put_request_ = std::make_shared(sk_sync_path, "", false); } } // namespace sensesp diff --git a/src/sensesp/controllers/smart_switch_controller.h b/src/sensesp/controllers/smart_switch_controller.h index 765bfcfbf..919c7c070 100644 --- a/src/sensesp/controllers/smart_switch_controller.h +++ b/src/sensesp/controllers/smart_switch_controller.h @@ -1,10 +1,10 @@ #ifndef _smart_switch_controller_h #define _smart_switch_controller_h -#include "sensesp/ui/config_item.h" #include "sensesp/signalk/signalk_put_request.h" #include "sensesp/system/valueconsumer.h" #include "sensesp/transforms/click_type.h" +#include "sensesp/ui/config_item.h" namespace sensesp { @@ -74,8 +74,8 @@ class SmartSwitchController : public BooleanTransform, /// Used to store configuration internally. class SyncPath { public: - BoolSKPutRequest* put_request_; String sk_sync_path_; + std::shared_ptr put_request_; SyncPath(); SyncPath(String sk_sync_path); diff --git a/src/sensesp/net/http_server.cpp b/src/sensesp/net/http_server.cpp index 715dfbe26..01e5f6686 100644 --- a/src/sensesp/net/http_server.cpp +++ b/src/sensesp/net/http_server.cpp @@ -74,8 +74,8 @@ esp_err_t HTTPServer::dispatch_request(httpd_req_t* req) { if (auth_required) { bool success; - success = - authenticate_request(authenticator_, call_request_dispatcher, req); + success = authenticate_request(authenticator_.get(), + call_request_dispatcher, req); if (!success) { // Authentication failed; do not continue but return success. // The client already received a 401 response. diff --git a/src/sensesp/net/http_server.h b/src/sensesp/net/http_server.h index 4f76491ef..47ec45681 100644 --- a/src/sensesp/net/http_server.h +++ b/src/sensesp/net/http_server.h @@ -7,6 +7,7 @@ #include #include #include +#include #include "WiFi.h" #include "sensesp/net/http_authenticator.h" @@ -61,8 +62,7 @@ class HTTPServer : virtual public FileSystemSaveable { public: HTTPServer(int port = HTTP_DEFAULT_PORT, const String& config_path = "/system/httpserver") - : FileSystemSaveable(config_path), - config_(HTTPD_DEFAULT_CONFIG()) { + : FileSystemSaveable(config_path), config_(HTTPD_DEFAULT_CONFIG()) { config_.server_port = port; config_.stack_size = HTTP_SERVER_STACK_SIZE; config_.max_uri_handlers = 20; @@ -70,8 +70,8 @@ class HTTPServer : virtual public FileSystemSaveable { String auth_realm_ = "Login required for " + SensESPBaseApp::get_hostname(); load(); if (auth_required_) { - authenticator_ = - new HTTPDigestAuthenticator(username_, password_, auth_realm_); + authenticator_ = std::unique_ptr( + new HTTPDigestAuthenticator(username_, password_, auth_realm_)); } if (singleton_ == nullptr) { singleton_ = this; @@ -111,7 +111,11 @@ class HTTPServer : virtual public FileSystemSaveable { MDNS.addService("http", "tcp", 80); }); }; - ~HTTPServer() {} + ~HTTPServer() { + if (singleton_ == this) { + singleton_ = nullptr; + } + } void stop() { httpd_stop(server_); } @@ -148,6 +152,10 @@ class HTTPServer : virtual public FileSystemSaveable { } void add_handler(HTTPRequestHandler* handler) { + handlers_.push_back(std::make_shared(*handler)); + } + + void add_handler(std::shared_ptr handler) { handlers_.push_back(handler); } @@ -160,7 +168,7 @@ class HTTPServer : virtual public FileSystemSaveable { String username_; String password_; bool auth_required_ = false; - HTTPAuthenticator* authenticator_ = nullptr; + std::unique_ptr authenticator_; bool authenticate_request(HTTPAuthenticator* auth, std::function handler, @@ -184,7 +192,7 @@ class HTTPServer : virtual public FileSystemSaveable { */ bool handle_captive_portal(httpd_req_t* req); - std::list handlers_; + std::list> handlers_; friend esp_err_t call_request_dispatcher(httpd_req_t* req); }; diff --git a/src/sensesp/net/networking.cpp b/src/sensesp/net/networking.cpp index 5dd10c9df..d0f9c88ff 100644 --- a/src/sensesp/net/networking.cpp +++ b/src/sensesp/net/networking.cpp @@ -92,8 +92,14 @@ Networking::Networking(String config_path, String client_ssid, dns_server_->setErrorReplyCode(DNSReplyCode::NoError); dns_server_->start(53, "*", WiFi.softAPIP()); - event_loop()->onRepeat( - 1, [this]() { dns_server_->processNextRequest(); }); + event_loop()->onRepeat(1, [this]() { dns_server_->processNextRequest(); }); + } +} + +Networking::~Networking() { + if (dns_server_) { + dns_server_->stop(); + delete dns_server_; } } @@ -280,13 +286,9 @@ void Networking::reset() { WiFi.begin("0", "0", 0, nullptr, false); } -WiFiStateProducer* WiFiStateProducer::instance_ = nullptr; - WiFiStateProducer* WiFiStateProducer::get_singleton() { - if (instance_ == nullptr) { - instance_ = new WiFiStateProducer(); - } - return instance_; + static WiFiStateProducer instance; + return &instance; } void Networking::start_wifi_scan() { diff --git a/src/sensesp/net/networking.h b/src/sensesp/net/networking.h index dcb33f171..a90d11012 100644 --- a/src/sensesp/net/networking.h +++ b/src/sensesp/net/networking.h @@ -6,9 +6,9 @@ #include #include "sensesp/net/wifi_state.h" -#include "sensesp/system/serializable.h" #include "sensesp/system/observablevalue.h" #include "sensesp/system/resettable.h" +#include "sensesp/system/serializable.h" #include "sensesp/system/valueproducer.h" #include "sensesp_base_app.h" @@ -97,8 +97,6 @@ class WiFiStateProducer : public ValueProducer { ESP_LOGI(__FILENAME__, "Disconnected from wifi."); this->emit(WiFiState::kWifiDisconnected); } - - static WiFiStateProducer* instance_; }; /** @@ -241,6 +239,8 @@ class Networking : virtual public FileSystemSaveable, public: Networking(String config_path, String client_ssid = "", String client_password = ""); + ~Networking(); + virtual void reset() override; virtual bool to_json(JsonObject& doc) override; diff --git a/src/sensesp/net/web/app_command_handler.cpp b/src/sensesp/net/web/app_command_handler.cpp index 8dd8ab1f2..936addd2f 100644 --- a/src/sensesp/net/web/app_command_handler.cpp +++ b/src/sensesp/net/web/app_command_handler.cpp @@ -1,11 +1,13 @@ #include "app_command_handler.h" +#include + #include namespace sensesp { void add_scan_wifi_networks_handlers(HTTPServer* server) { - HTTPRequestHandler* scan_wifi_networks_handler = new HTTPRequestHandler( + auto scan_wifi_networks_handler = std::make_shared( 1 << HTTP_POST, "/api/wifi/scan", [](httpd_req_t* req) { auto networking = SensESPApp::get()->get_networking(); networking->start_wifi_scan(); @@ -17,7 +19,7 @@ void add_scan_wifi_networks_handlers(HTTPServer* server) { server->add_handler(scan_wifi_networks_handler); - HTTPRequestHandler* scan_results_handler = new HTTPRequestHandler( + auto scan_results_handler = std::make_shared( 1 << HTTP_GET, "/api/wifi/scan-results", [](httpd_req_t* req) { auto networking = SensESPApp::get()->get_networking(); std::vector ssid_list; diff --git a/src/sensesp/net/web/config_handler.cpp b/src/sensesp/net/web/config_handler.cpp index fde0cdf67..85601d9ef 100644 --- a/src/sensesp/net/web/config_handler.cpp +++ b/src/sensesp/net/web/config_handler.cpp @@ -171,6 +171,7 @@ void add_config_put_handler(HTTPServer* server) { // parse the content as JSON JsonDocument doc; DeserializationError error = deserializeJson(doc, payload); + delete[] payload; if (error) { ESP_LOGE("ConfigHandler", "Error parsing JSON payload: %s", error.c_str()); diff --git a/src/sensesp/sensors/analog_input.h b/src/sensesp/sensors/analog_input.h index 5855a48fb..eb802aae8 100644 --- a/src/sensesp/sensors/analog_input.h +++ b/src/sensesp/sensors/analog_input.h @@ -1,6 +1,7 @@ #ifndef SENSESP_SENSORS_ANALOG_INPUT_H_ #define SENSESP_SENSORS_ANALOG_INPUT_H_ +#include #include "analog_reader.h" #include "sensesp/ui/config_item.h" #include "sensor.h" @@ -47,11 +48,11 @@ class AnalogInput : public FloatSensor { virtual bool to_json(JsonObject& root) override; virtual bool from_json(const JsonObject& config) override; - private: + protected: uint8_t pin{}; unsigned int read_delay; float output_scale; - BaseAnalogReader* analog_reader_; + std::unique_ptr analog_reader_{}; void update(); }; diff --git a/src/sensesp/sensors/system_info.h b/src/sensesp/sensors/system_info.h index 3ebddd9a5..0f11528e1 100644 --- a/src/sensesp/sensors/system_info.h +++ b/src/sensesp/sensors/system_info.h @@ -1,6 +1,7 @@ #ifndef SENSESP_SENSORS_SYSTEM_INFO_H_ #define SENSESP_SENSORS_SYSTEM_INFO_H_ +#include #include #include @@ -19,7 +20,7 @@ void connect_system_info_sensor(ValueProducer* sensor, String prefix, String String hostname = hostname_obs->get(); String path = prefix + hostname + "." + name; - auto* sk_output = new SKOutput(path); + auto sk_output = std::make_shared>(path); // connect an observer to hostname to change the output path // if the hostname is changed diff --git a/src/sensesp/signalk/signalk_emitter.h b/src/sensesp/signalk/signalk_emitter.h index cf97cbb77..15259d661 100644 --- a/src/sensesp/signalk/signalk_emitter.h +++ b/src/sensesp/signalk/signalk_emitter.h @@ -42,7 +42,7 @@ class SKEmitter : virtual public Observable { * returned. * @see add_metadata() */ - virtual SKMetadata* get_metadata() { return NULL; } + virtual SKMetadata* get_metadata() { return nullptr; } /** * Adds this emitter's Signal K meta data to the specified diff --git a/src/sensesp/signalk/signalk_output.h b/src/sensesp/signalk/signalk_output.h index 56271c492..274bfaad7 100644 --- a/src/sensesp/signalk/signalk_output.h +++ b/src/sensesp/signalk/signalk_output.h @@ -1,8 +1,10 @@ #ifndef SENSESP_SIGNALK_SIGNALK_OUTPUT_H_ #define SENSESP_SIGNALK_SIGNALK_OUTPUT_H_ -#include "sensesp/ui/config_item.h" +#include + #include "sensesp/transforms/transform.h" +#include "sensesp/ui/config_item.h" #include "signalk_emitter.h" namespace sensesp { @@ -29,7 +31,10 @@ class SKOutput : public SKEmitter, public SymmetricTransform { * metadata to report (or if the path is already an official part of the * Signal K specification) */ - SKOutput(String sk_path, String config_path = "", SKMetadata* meta = NULL) + SKOutput(String sk_path, String config_path = "", SKMetadata* meta = nullptr) + : SKOutput(sk_path, config_path, std::make_shared(*meta)) {} + + SKOutput(String sk_path, String config_path, std::shared_ptr meta) : SKEmitter(sk_path), SymmetricTransform(config_path), meta_{meta} { this->load(); } @@ -64,12 +69,14 @@ class SKOutput : public SKEmitter, public SymmetricTransform { * method of setting the metadata (the first being a parameter * to the constructor). */ - virtual void set_metadata(SKMetadata* meta) { this->meta_ = meta; } + virtual void set_metadata(SKMetadata* meta) { + this->meta_ = std::make_shared(*meta); + } - virtual SKMetadata* get_metadata() override { return meta_; } + virtual SKMetadata* get_metadata() override { return meta_.get(); } protected: - SKMetadata* meta_; + std::shared_ptr meta_; }; template diff --git a/src/sensesp/system/base_button.h b/src/sensesp/system/base_button.h index b3f3b6d5f..d3321ecd7 100644 --- a/src/sensesp/system/base_button.h +++ b/src/sensesp/system/base_button.h @@ -1,6 +1,7 @@ #ifndef SENSESP_SRC_SENSESP_SYSTEM_BASE_BUTTON_H_ #define SENSESP_SRC_SENSESP_SYSTEM_BASE_BUTTON_H_ +#include #include "sensesp.h" #include "AceButton.h" @@ -20,7 +21,7 @@ using namespace ace_button; class BaseButtonHandler : public IEventHandler { public: BaseButtonHandler(int pin, String config_path = "") { - button_ = new AceButton(pin); + button_ = std::unique_ptr(new AceButton(pin)); pinMode(pin, INPUT_PULLUP); ButtonConfig* button_config = button_->getButtonConfig(); @@ -35,7 +36,7 @@ class BaseButtonHandler : public IEventHandler { } protected: - AceButton* button_; + std::unique_ptr button_; }; } // namespace sensesp diff --git a/src/sensesp/system/filesystem.cpp b/src/sensesp/system/filesystem.cpp index 83e7f9a2d..ee9bd8aa3 100644 --- a/src/sensesp/system/filesystem.cpp +++ b/src/sensesp/system/filesystem.cpp @@ -13,6 +13,10 @@ Filesystem::Filesystem() : Resettable(-100) { } } +Filesystem::~Filesystem() { + SPIFFS.end(); +} + void Filesystem::reset() { ESP_LOGI(__FILENAME__, "Formatting filesystem"); SPIFFS.format(); diff --git a/src/sensesp/system/filesystem.h b/src/sensesp/system/filesystem.h index da000234c..b3a1d3543 100644 --- a/src/sensesp/system/filesystem.h +++ b/src/sensesp/system/filesystem.h @@ -8,6 +8,7 @@ namespace sensesp { class Filesystem : public Resettable { public: Filesystem(); + ~Filesystem(); virtual void reset() override; }; diff --git a/src/sensesp/system/system_status_led.cpp b/src/sensesp/system/system_status_led.cpp index c1dbeaee9..6d061bf00 100644 --- a/src/sensesp/system/system_status_led.cpp +++ b/src/sensesp/system/system_status_led.cpp @@ -33,7 +33,8 @@ int ws_disconnected_pattern[] = {50, 50, 50, 50, 50, 50, 50, 650, PATTERN_END}; int ws_authorizing_pattern[] = {200, 200, PATTERN_END}; SystemStatusLed::SystemStatusLed(int pin) - : blinker_(new PatternBlinker(pin, no_ap_pattern)) {} + : blinker_(std::unique_ptr( + new PatternBlinker(pin, no_ap_pattern))) {} void SystemStatusLed::set_wifi_no_ap() { blinker_->set_pattern(no_ap_pattern); } void SystemStatusLed::set_wifi_disconnected() { diff --git a/src/sensesp/system/system_status_led.h b/src/sensesp/system/system_status_led.h index f5b186d8d..766dec23d 100644 --- a/src/sensesp/system/system_status_led.h +++ b/src/sensesp/system/system_status_led.h @@ -1,6 +1,8 @@ #ifndef SENSESP_SRC_SENSESP_SYSTEM_SYSTEM_STATUS_LED_H_ #define SENSESP_SRC_SENSESP_SYSTEM_SYSTEM_STATUS_LED_H_ +#include + #include "lambda_consumer.h" #include "led_blinker.h" #include "sensesp/controllers/system_status_controller.h" @@ -15,7 +17,7 @@ namespace sensesp { class SystemStatusLed : public ValueConsumer, public ValueConsumer { protected: - PatternBlinker* blinker_; + std::unique_ptr blinker_; virtual void set_wifi_no_ap(); virtual void set_wifi_disconnected(); diff --git a/src/sensesp/system/valueproducer.h b/src/sensesp/system/valueproducer.h index 0257ff1d2..a435c3801 100644 --- a/src/sensesp/system/valueproducer.h +++ b/src/sensesp/system/valueproducer.h @@ -2,6 +2,7 @@ #define SENSESP_SYSTEM_VALUE_PRODUCER_H_ #include +#include #include "observable.h" #include "valueconsumer.h" @@ -38,6 +39,9 @@ class ValueProducer : virtual public Observable { void connect_to(ValueConsumer* consumer) { this->attach([this, consumer]() { consumer->set(this->get()); }); } + void connect_to(std::shared_ptr> consumer) { + this->attach([this, consumer]() { consumer->set(this->get()); }); + } void connect_to(ValueConsumer& consumer) { this->attach([this, consumer]() { consumer.set(this->get()); }); } @@ -55,6 +59,11 @@ class ValueProducer : virtual public Observable { void connect_to(ValueConsumer* consumer) { this->attach([this, consumer]() { consumer->set(CT(this->get())); }); } + template + void connect_to(std::shared_ptr> consumer) const { + this->attach([this, consumer]() { consumer->set(CT(this->get())); }); + } + template void connect_to(ValueConsumer& consumer) { this->attach([this, consumer]() { consumer.set(CT(this->get())); }); @@ -74,6 +83,14 @@ class ValueProducer : virtual public Observable { return consumer_producer; } template + Transform* connect_to( + std::shared_ptr> consumer_producer) { + this->attach([this, consumer_producer]() { + consumer_producer->set(T(this->get())); + }); + return consumer_producer.get(); + } + template Transform* connect_to(Transform& consumer_producer) { this->attach( [this, consumer_producer]() { consumer_producer.set(T(this->get())); }); @@ -99,6 +116,14 @@ class ValueProducer : virtual public Observable { return consumer_producer; } template + Transform* connect_to( + std::shared_ptr> consumer_producer) { + this->attach([this, consumer_producer]() { + consumer_producer->set(TT(this->get())); + }); + return consumer_producer.get(); + } + template Transform* connect_to(Transform& consumer_producer) { this->attach([this, consumer_producer]() { consumer_producer->set(TT(this->get())); diff --git a/src/sensesp/ui/ui_button.cpp b/src/sensesp/ui/ui_button.cpp index dd0bd2b84..5088f5925 100644 --- a/src/sensesp/ui/ui_button.cpp +++ b/src/sensesp/ui/ui_button.cpp @@ -2,6 +2,6 @@ namespace sensesp { -std::map UIButton::ui_buttons_; +std::map> UIButton::ui_buttons_; } // namespace sensesp diff --git a/src/sensesp/ui/ui_button.h b/src/sensesp/ui/ui_button.h index aa64da3c7..a9fee7889 100644 --- a/src/sensesp/ui/ui_button.h +++ b/src/sensesp/ui/ui_button.h @@ -2,6 +2,7 @@ #define SENSESP_UI_UI_BUTTON_H #include +#include #include "Arduino.h" #include "sensesp/system/observable.h" @@ -23,15 +24,15 @@ class UIButton : public Observable { const String get_title() { return title_; } const String get_name() { return name_; } - static const std::map& get_ui_buttons() { + static const std::map>& get_ui_buttons() { return ui_buttons_; } static UIButton* add(String name, String title, bool must_confirm = true) { - UIButton* new_cmd = new UIButton(title, name, must_confirm); + auto new_cmd = std::make_shared(title, name, must_confirm); ui_buttons_[name] = new_cmd; - return new_cmd; + return new_cmd.get(); } protected: @@ -39,7 +40,7 @@ class UIButton : public Observable { String name_; bool must_confirm_; - static std::map ui_buttons_; + static std::map> ui_buttons_; }; } // namespace sensesp diff --git a/src/sensesp_app.cpp b/src/sensesp_app.cpp index f5bde5939..6bd8fafc4 100644 --- a/src/sensesp_app.cpp +++ b/src/sensesp_app.cpp @@ -11,6 +11,17 @@ namespace sensesp { +SensESPApp::~SensESPApp() { + delete networking_; + delete ota_; + delete ws_client_; + delete mdns_discovery_; + delete http_server_; + delete sk_delta_queue_; + delete system_status_led_; + delete button_handler_; +} + SensESPApp* SensESPApp::get() { if (instance_ == nullptr) { instance_ = new SensESPApp(); @@ -91,26 +102,26 @@ void SensESPApp::setup() { } void SensESPApp::connect_status_page_items() { - this->hostname_->connect_to(this->hostname_ui_output_); + this->hostname_->connect_to(&this->hostname_ui_output_); this->event_loop_.onRepeat( - 4999, [this]() { wifi_ssid_ui_output_->set(WiFi.SSID()); }); + 4999, [this]() { wifi_ssid_ui_output_.set(WiFi.SSID()); }); this->event_loop_.onRepeat( - 4998, [this]() { free_memory_ui_output_->set(ESP.getFreeHeap()); }); + 4998, [this]() { free_memory_ui_output_.set(ESP.getFreeHeap()); }); this->event_loop_.onRepeat( - 4997, [this]() { wifi_rssi_ui_output_->set(WiFi.RSSI()); }); + 4997, [this]() { wifi_rssi_ui_output_.set(WiFi.RSSI()); }); this->event_loop_.onRepeat(4996, [this]() { - sk_server_address_ui_output_->set(ws_client_->get_server_address()); + sk_server_address_ui_output_.set(ws_client_->get_server_address()); }); this->event_loop_.onRepeat(4995, [this]() { - sk_server_port_ui_output_->set(ws_client_->get_server_port()); + sk_server_port_ui_output_.set(ws_client_->get_server_port()); }); this->event_loop_.onRepeat(4994, [this]() { - sk_server_connection_ui_output_->set(ws_client_->get_connection_status()); + sk_server_connection_ui_output_.set(ws_client_->get_connection_status()); }); ws_client_->get_delta_tx_count_producer().connect_to( - delta_tx_count_ui_output_); + &delta_tx_count_ui_output_); ws_client_->get_delta_rx_count_producer().connect_to( - delta_rx_count_ui_output_); + &delta_rx_count_ui_output_); } ObservableValue* SensESPApp::get_hostname_observable() { diff --git a/src/sensesp_app.h b/src/sensesp_app.h index f2afed995..b005c7abf 100644 --- a/src/sensesp_app.h +++ b/src/sensesp_app.h @@ -71,6 +71,8 @@ class SensESPApp : public SensESPBaseApp { */ SensESPApp() : SensESPBaseApp() {} + ~SensESPApp(); + // setters for all constructor arguments const SensESPApp* set_hostname(String hostname) { @@ -138,32 +140,28 @@ class SensESPApp : public SensESPBaseApp { SKDeltaQueue* sk_delta_queue_; SKWSClient* ws_client_; - StatusPageItem* sensesp_version_ui_output_ = - new StatusPageItem("SenseESP version", kSensESPVersion, - "Software", 1900); - StatusPageItem* build_info_ui_output_ = new StatusPageItem( - "Build date", __DATE__ " " __TIME__, "Software", 2000); - StatusPageItem* hostname_ui_output_ = - new StatusPageItem("Hostname", "", "Network", 500); - StatusPageItem* mac_address_ui_output_ = new StatusPageItem( - "MAC Address", WiFi.macAddress(), "Network", 1100); - StatusPageItem* wifi_ssid_ui_output_ = - new StatusPageItem("SSID", "", "Network", 1200); - StatusPageItem* free_memory_ui_output_ = - new StatusPageItem("Free memory (bytes)", 0, "System", 1250); - StatusPageItem* wifi_rssi_ui_output_ = new StatusPageItem( - "WiFi signal strength (dB)", -128, "Network", 1300); - StatusPageItem* sk_server_address_ui_output_ = - new StatusPageItem("Signal K server address", "", "Signal K", - 1400); - StatusPageItem* sk_server_port_ui_output_ = - new StatusPageItem("Signal K server port", 0, "Signal K", 1500); - StatusPageItem* sk_server_connection_ui_output_ = - new StatusPageItem("SK connection status", "", "Signal K", 1600); - StatusPageItem* delta_tx_count_ui_output_ = - new StatusPageItem("SK Delta TX count", 0, "Signal K", 1700); - StatusPageItem* delta_rx_count_ui_output_ = - new StatusPageItem("SK Delta RX count", 0, "Signal K", 1800); + StatusPageItem sensesp_version_ui_output_{ + "SenseESP version", kSensESPVersion, "Software", 1900}; + StatusPageItem build_info_ui_output_{ + "Build date", __DATE__ " " __TIME__, "Software", 2000}; + StatusPageItem hostname_ui_output_{"Hostname", "", "Network", 500}; + StatusPageItem mac_address_ui_output_{ + "MAC Address", WiFi.macAddress(), "Network", 1100}; + StatusPageItem wifi_ssid_ui_output_{"SSID", "", "Network", 1200}; + StatusPageItem free_memory_ui_output_{"Free memory (bytes)", 0, "System", + 1250}; + StatusPageItem wifi_rssi_ui_output_{"WiFi signal strength (dB)", -128, + "Network", 1300}; + StatusPageItem sk_server_address_ui_output_{"Signal K server address", + "", "Signal K", 1400}; + StatusPageItem sk_server_port_ui_output_{"Signal K server port", 0, + "Signal K", 1500}; + StatusPageItem sk_server_connection_ui_output_{"SK connection status", + "", "Signal K", 1600}; + StatusPageItem delta_tx_count_ui_output_{"SK Delta TX count", 0, + "Signal K", 1700}; + StatusPageItem delta_rx_count_ui_output_{"SK Delta RX count", 0, + "Signal K", 1800}; friend class WebServer; friend class SensESPAppBuilder; diff --git a/src/sensesp_app_builder.h b/src/sensesp_app_builder.h index f05753538..d6704f146 100644 --- a/src/sensesp_app_builder.h +++ b/src/sensesp_app_builder.h @@ -1,6 +1,8 @@ #ifndef SENSESP_APP_BUILDER_H #define SENSESP_APP_BUILDER_H +#include + #include "sensesp/sensors/system_info.h" #include "sensesp/transforms/debounce.h" #include "sensesp_app.h" diff --git a/src/sensesp_base_app.cpp b/src/sensesp_base_app.cpp index ece1ff540..dcc0d9fe3 100644 --- a/src/sensesp_base_app.cpp +++ b/src/sensesp_base_app.cpp @@ -25,6 +25,13 @@ SensESPBaseApp::SensESPBaseApp() : filesystem_(new Filesystem()) { ConfigItem(hostname_); // Make hostname configurable } +SensESPBaseApp::~SensESPBaseApp() { + delete filesystem_; + delete hostname_; + + instance_ = nullptr; +} + /** * Get the singleton SensESPBaseApp singleton instance. * The instance must be set by the builder. diff --git a/src/sensesp_base_app.h b/src/sensesp_base_app.h index 78055b780..4f2a63545 100644 --- a/src/sensesp_base_app.h +++ b/src/sensesp_base_app.h @@ -70,6 +70,7 @@ class SensESPBaseApp { * refactored into a singleton. */ SensESPBaseApp(); + ~SensESPBaseApp(); virtual void setup(); From ed672731cf9ea625d351f2ae0cc6a3d3bc431cff Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Mon, 7 Oct 2024 22:19:03 +0300 Subject: [PATCH 4/6] Unit test ConfigItem --- test/system/test_config_item/config_item.cpp | 50 ++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 test/system/test_config_item/config_item.cpp diff --git a/test/system/test_config_item/config_item.cpp b/test/system/test_config_item/config_item.cpp new file mode 100644 index 000000000..3bfd20ca2 --- /dev/null +++ b/test/system/test_config_item/config_item.cpp @@ -0,0 +1,50 @@ +#include "sensesp.h" + +#include "sensesp/ui/config_item.h" + +#include "sensesp/system/filesystem.h" +#include "sensesp/system/observablevalue.h" +#include "sensesp/system/serializable.h" +#include "sensesp/system/valueproducer.h" +#include "sensesp/transforms/angle_correction.h" +#include "sensesp_base_app.h" +#include "sensesp_minimal_app_builder.h" +#include "unity.h" + +using namespace sensesp; + +void test_persisting_observable_value_schema() { + SensESPMinimalAppBuilder builder; + SensESPMinimalApp* sensesp_app = builder.get_app(); + TEST_ASSERT_NOT_NULL(sensesp_app); + + PersistingObservableValue value{2, "/test"}; + + auto config_item = ConfigItem(&value); + + value.set(3); + + String ref_schema = R"###({"type":"object","properties":{"value":{"title":"Value","type":"number"}}})###"; + String schema = config_item->get_config_schema(); + ESP_LOGD("test_schema", "schema: %s", schema.c_str()); + + TEST_ASSERT_EQUAL_STRING(ref_schema.c_str(), schema.c_str()); + + value.save(); + value.set(4); + value.load(); + + TEST_ASSERT_EQUAL(3, value.get()); +} + +void setup() { + esp_log_level_set("*", ESP_LOG_VERBOSE); + + UNITY_BEGIN(); + + RUN_TEST(test_persisting_observable_value_schema); + + UNITY_END(); +} + +void loop() {} From ce38e9513f52f93b4d4533b7ae67344dc59670d9 Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Tue, 8 Oct 2024 10:52:19 +0300 Subject: [PATCH 5/6] Test that the app can be torn down and reinitialized --- test/system/test_minimal_app/minimal_app.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/system/test_minimal_app/minimal_app.cpp b/test/system/test_minimal_app/minimal_app.cpp index 7ebb6a442..863886d6a 100644 --- a/test/system/test_minimal_app/minimal_app.cpp +++ b/test/system/test_minimal_app/minimal_app.cpp @@ -38,6 +38,9 @@ void test_sensesp() { void setup() { UNITY_BEGIN(); RUN_TEST(test_sensesp); + // Run the same test again to ensure that the app can be torn down and + // reinitialized. + RUN_TEST(test_sensesp); UNITY_END(); } From 174119241d46313c11e97f558fb3d6eb7efe283c Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Tue, 8 Oct 2024 11:38:05 +0300 Subject: [PATCH 6/6] Test serialization and deserialization --- test/system/test_config_item/config_item.cpp | 35 ++++++++++++++++++-- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/test/system/test_config_item/config_item.cpp b/test/system/test_config_item/config_item.cpp index 3bfd20ca2..d806a9411 100644 --- a/test/system/test_config_item/config_item.cpp +++ b/test/system/test_config_item/config_item.cpp @@ -13,28 +13,57 @@ using namespace sensesp; +// PersistingObservableValue saves itself on change void test_persisting_observable_value_schema() { SensESPMinimalAppBuilder builder; SensESPMinimalApp* sensesp_app = builder.get_app(); TEST_ASSERT_NOT_NULL(sensesp_app); + PersistingObservableValue value{2, "/test"}; auto config_item = ConfigItem(&value); - value.set(3); - String ref_schema = R"###({"type":"object","properties":{"value":{"title":"Value","type":"number"}}})###"; String schema = config_item->get_config_schema(); ESP_LOGD("test_schema", "schema: %s", schema.c_str()); TEST_ASSERT_EQUAL_STRING(ref_schema.c_str(), schema.c_str()); + value.set(0); value.save(); - value.set(4); + + value.set(3); value.load(); TEST_ASSERT_EQUAL(3, value.get()); + + value.set(4); + value.load(); + + TEST_ASSERT_EQUAL(4, value.get()); + + JsonDocument doc; + JsonObject root = doc.to(); + bool result = value.to_json(root); + TEST_ASSERT_TRUE(result); + + String json_str; + serializeJson(doc, json_str); + + String ref_json = R"###({"value":4})###"; + + TEST_ASSERT_EQUAL_STRING(ref_json.c_str(), json_str.c_str()); + + String json_input = R"###({"value":5})###"; + + DeserializationError error = deserializeJson(doc, json_input); + TEST_ASSERT_TRUE(error == DeserializationError::Ok); + + result = value.from_json(doc.as()); + TEST_ASSERT_TRUE(result); + + TEST_ASSERT_EQUAL(5, value.get()); } void setup() {