More pointer safety

SignalK · Oct 10, 2024 · 28cd3cf · 28cd3cf
1 parent ae6f58a
commit 28cd3cf
Show file tree

Hide file tree

Showing 13 changed files with 103 additions and 80 deletions.
diff --git a/src/sensesp/net/http_server.h b/src/sensesp/net/http_server.h
@@ -139,11 +139,7 @@ class HTTPServer : public FileSystemSaveable {
     return true;
   }
 
-  void add_handler(HTTPRequestHandler* handler) {
-    handlers_.push_back(std::make_shared<HTTPRequestHandler>(*handler));
-  }
-
-  void add_handler(std::shared_ptr<HTTPRequestHandler> handler) {
+  void add_handler(std::shared_ptr<HTTPRequestHandler>& handler) {
     handlers_.push_back(handler);
   }
 

diff --git a/src/sensesp/net/networking.h b/src/sensesp/net/networking.h
@@ -277,12 +277,12 @@ class Networking : public FileSystemSaveable,
 
   std::unique_ptr<DNSServer> dns_server_;
 
-  std::shared_ptr<WiFiStateProducer> wifi_state_producer_{new WiFiStateProducer()};
+  std::shared_ptr<WiFiStateProducer> wifi_state_producer_ =
+      std::make_shared<WiFiStateProducer>();
 
   std::shared_ptr<LambdaConsumer<WiFiState>> wifi_state_emitter_;
 };
 
-
 inline bool ConfigRequiresRestart(const Networking& obj) { return true; }
 
 }  // namespace sensesp

diff --git a/src/sensesp/net/web/base_command_handler.cpp b/src/sensesp/net/web/base_command_handler.cpp
@@ -6,7 +6,7 @@
 namespace sensesp {
 
 void add_http_reset_handler(std::shared_ptr<HTTPServer>& server) {
-  HTTPRequestHandler* reset_handler = new HTTPRequestHandler(
+  auto reset_handler = std::make_shared<HTTPRequestHandler>(
       1 << HTTP_POST, "/api/device/reset", [](httpd_req_t* req) {
         httpd_resp_send(req,
                         "Resetting device back to factory defaults. "
@@ -19,7 +19,7 @@ void add_http_reset_handler(std::shared_ptr<HTTPServer>& server) {
 }
 
 void add_http_restart_handler(std::shared_ptr<HTTPServer>& server) {
-  HTTPRequestHandler* restart_handler = new HTTPRequestHandler(
+  auto restart_handler = std::make_shared<HTTPRequestHandler>(
       1 << HTTP_POST, "/api/device/restart", [](httpd_req_t* req) {
         httpd_resp_send(req, "Restarting device", 0);
         event_loop()->onDelay(500, []() { ESP.restart(); });
@@ -29,8 +29,8 @@ void add_http_restart_handler(std::shared_ptr<HTTPServer>& server) {
 }
 
 void add_http_info_handler(std::shared_ptr<HTTPServer>& server) {
-  HTTPRequestHandler* info_handler =
-      new HTTPRequestHandler(1 << HTTP_GET, "/api/info", [](httpd_req_t* req) {
+  auto info_handler = std::make_shared<HTTPRequestHandler>(
+      1 << HTTP_GET, "/api/info", [](httpd_req_t* req) {
         auto status_page_items = StatusPageItemBase::get_status_page_items();
 
         JsonDocument json_doc;
@@ -74,7 +74,7 @@ void add_routes_handlers(std::shared_ptr<HTTPServer>& server) {
 
   serializeJson(routes_json, response);
 
-  HTTPRequestHandler* routes_handler = new HTTPRequestHandler(
+  auto routes_handler = std::make_shared<HTTPRequestHandler>(
       1 << HTTP_GET, "/api/routes", [response](httpd_req_t* req) {
         httpd_resp_set_type(req, "application/json");
         httpd_resp_sendstr(req, response.c_str());
@@ -100,7 +100,7 @@ void add_routes_handlers(std::shared_ptr<HTTPServer>& server) {
 
   for (auto it = routes.begin(); it != routes.end(); ++it) {
     String path = it->get_path();
-    HTTPRequestHandler* route_handler = new HTTPRequestHandler(
+    auto route_handler = std::make_shared<HTTPRequestHandler>(
         1 << HTTP_GET, path.c_str(), [root_page](httpd_req_t* req) {
           httpd_resp_set_type(req, root_page->content_type);
           if (root_page->content_encoding != nullptr) {

diff --git a/src/sensesp/net/web/config_handler.cpp b/src/sensesp/net/web/config_handler.cpp
@@ -80,12 +80,13 @@ esp_err_t handle_config_item_list(httpd_req_t* req) {
 }
 
 void add_config_list_handler(std::shared_ptr<HTTPServer>& server) {
-  server->add_handler(new HTTPRequestHandler(1 << HTTP_GET, "/api/config",
-                                             handle_config_item_list));
+  auto handler = std::make_shared<HTTPRequestHandler>(
+      1 << HTTP_GET, "/api/config", handle_config_item_list);
+  server->add_handler(handler);
 }
 
 void add_config_get_handler(std::shared_ptr<HTTPServer>& server) {
-  server->add_handler(new HTTPRequestHandler(
+  auto handler = std::make_shared<HTTPRequestHandler>(
       1 << HTTP_GET, "/api/config/*", [](httpd_req_t* req) {
         ESP_LOGD("ConfigHandler", "GET request to URL %s", req->uri);
         String url_tail = String(req->uri).substring(11);
@@ -123,11 +124,12 @@ void add_config_get_handler(std::shared_ptr<HTTPServer>& server) {
         httpd_resp_set_type(req, "application/json");
         httpd_resp_sendstr(req, response.c_str());
         return ESP_OK;
-      }));
+      });
+  server->add_handler(handler);
 }
 
 void add_config_put_handler(std::shared_ptr<HTTPServer>& server) {
-  server->add_handler(new HTTPRequestHandler(
+  auto handler = std::make_shared<HTTPRequestHandler>(
       1 << HTTP_PUT, "/api/config/*",
       [](httpd_req_t* req) {  // check that the content type is JSON
         ESP_LOGI(__FILENAME__, "PUT request to URL %s", req->uri);
@@ -198,7 +200,8 @@ void add_config_put_handler(std::shared_ptr<HTTPServer>& server) {
         httpd_resp_sendstr(req, response.c_str());
         return ESP_OK;
 
-      }));
+      });
+  server->add_handler(handler);
 }
 
 void add_config_handlers(std::shared_ptr<HTTPServer>& server) {

diff --git a/src/sensesp/net/web/static_file_handler.cpp b/src/sensesp/net/web/static_file_handler.cpp
@@ -7,7 +7,7 @@ namespace sensesp {
 void add_static_file_handlers(std::shared_ptr<HTTPServer> server) {
   for (int i = 0; kFrontendFiles[i].url != nullptr; i++) {
     const StaticFileData& data = kFrontendFiles[i];
-    HTTPRequestHandler* handler = new HTTPRequestHandler(
+    auto handler = std::make_shared<HTTPRequestHandler>(
         1 << HTTP_GET, kFrontendFiles[i].url, [data](httpd_req_t* req) {
           httpd_resp_set_type(req, data.content_type);
           if (data.content_encoding != nullptr) {

diff --git a/src/sensesp/sensors/system_info.h b/src/sensesp/sensors/system_info.h
@@ -1,9 +1,9 @@
 #ifndef SENSESP_SENSORS_SYSTEM_INFO_H_
 #define SENSESP_SENSORS_SYSTEM_INFO_H_
 
-#include <memory>
 #include <Arduino.h>
 #include <elapsedMillis.h>
+#include <memory>
 
 #include "sensesp/signalk/signalk_output.h"
 #include "sensesp_base_app.h"
@@ -15,7 +15,8 @@ namespace sensesp {
  * @brief Connect a system information sensor to SKOutput
  **/
 template <typename T>
-void connect_system_info_sensor(ValueProducer<T>* sensor, String prefix, String name) {
+void connect_system_info_sensor(std::shared_ptr<ValueProducer<T>>& sensor,
+                                const String& prefix, const String& name) {
   auto hostname_obs = SensESPBaseApp::get()->get_hostname_observable();
   String hostname = hostname_obs->get();
   String path = prefix + hostname + "." + name;
@@ -49,8 +50,7 @@ class SystemHz : public ValueProducer<float> {
     elapsed_millis_ = 0;
 
     event_loop()->onTick([this]() { this->tick(); });
-    event_loop()->onRepeat(1000,
-                                               [this]() { this->update(); });
+    event_loop()->onRepeat(1000, [this]() { this->update(); });
   }
   String get_value_name() { return "systemhz"; }
 
@@ -71,8 +71,7 @@ class SystemHz : public ValueProducer<float> {
 class FreeMem : public ValueProducer<uint32_t> {
  public:
   FreeMem() {
-    event_loop()->onRepeat(1000,
-                                               [this]() { this->update(); });
+    event_loop()->onRepeat(1000, [this]() { this->update(); });
   }
   String get_value_name() { return "freemem"; }
 
@@ -91,8 +90,7 @@ class FreeMem : public ValueProducer<uint32_t> {
 class Uptime : public ValueProducer<float> {
  public:
   Uptime() {
-    event_loop()->onRepeat(1000,
-                                               [this]() { this->update(); });
+    event_loop()->onRepeat(1000, [this]() { this->update(); });
   }
   String get_value_name() { return "uptime"; }
 
@@ -111,8 +109,7 @@ class Uptime : public ValueProducer<float> {
 class IPAddrDev : public ValueProducer<String> {
  public:
   IPAddrDev() {
-    event_loop()->onRepeat(10000,
-                                               [this]() { this->update(); });
+    event_loop()->onRepeat(10000, [this]() { this->update(); });
   }
   String get_value_name() { return "ipaddr"; }
 
@@ -131,8 +128,7 @@ class IPAddrDev : public ValueProducer<String> {
 class WiFiSignal : public ValueProducer<int> {
  public:
   WiFiSignal() {
-    event_loop()->onRepeat(3000,
-                                               [this]() { this->update(); });
+    event_loop()->onRepeat(3000, [this]() { this->update(); });
   }
   String get_value_name() { return "wifisignal"; }
 

diff --git a/src/sensesp/signalk/signalk_ws_client.cpp b/src/sensesp/signalk/signalk_ws_client.cpp
@@ -74,7 +74,7 @@ static void websocket_event_handler(void* handler_args, esp_event_base_t base,
   }
 }
 
-SKWSClient::SKWSClient(const String& config_path, SKDeltaQueue* sk_delta_queue,
+SKWSClient::SKWSClient(const String& config_path, std::shared_ptr<SKDeltaQueue> sk_delta_queue,
                        const String& server_address, uint16_t server_port,
                        bool use_mdns)
     : FileSystemSaveable{config_path},

diff --git a/src/sensesp/signalk/signalk_ws_client.h b/src/sensesp/signalk/signalk_ws_client.h
@@ -37,7 +37,7 @@ class SKWSClient : public FileSystemSaveable,
   /////////////////////////////////////////////////////////
   // main task methods
 
-  SKWSClient(const String& config_path, SKDeltaQueue* sk_delta_queue,
+  SKWSClient(const String& config_path, std::shared_ptr<SKDeltaQueue> sk_delta_queue,
              const String& server_address, uint16_t server_port,
              bool use_mdns = true);
 
@@ -113,7 +113,7 @@ class SKWSClient : public FileSystemSaveable,
 
   WiFiClient wifi_client_{};
   esp_websocket_client_handle_t client_{};
-  SKDeltaQueue* sk_delta_queue_;
+  std::shared_ptr<SKDeltaQueue> sk_delta_queue_;
   /// @brief Emits the number of deltas sent since last report
   TaskQueueProducer<int> delta_tx_tick_producer_{0, event_loop(), 990};
   Integrator<int, int> delta_tx_count_producer_{1, 0, ""};

diff --git a/src/sensesp_app.h b/src/sensesp_app.h
@@ -46,15 +46,6 @@ class SensESPApp : public SensESPBaseApp {
    */
   SensESPApp(SensESPApp& other) = delete;
 
-  ~SensESPApp() {
-    delete ota_;
-    delete ws_client_;
-    delete mdns_discovery_;
-    delete sk_delta_queue_;
-    delete system_status_led_;
-    delete button_handler_;
-  }
-
   /**
    * Singletons should not be assignable
    */
@@ -85,12 +76,12 @@ class SensESPApp : public SensESPBaseApp {
   }
 
   // getters for internal members
-  SKDeltaQueue* get_sk_delta() { return this->sk_delta_queue_; }
-  SystemStatusController* get_system_status_controller() {
-    return &(this->system_status_controller_);
+  std::shared_ptr<SKDeltaQueue> get_sk_delta() { return this->sk_delta_queue_; }
+  std::shared_ptr<SystemStatusController> get_system_status_controller() {
+    return this->system_status_controller_;
   }
   std::shared_ptr<Networking>& get_networking() { return this->networking_; }
-  SKWSClient* get_ws_client() { return this->ws_client_; }
+  std::shared_ptr<SKWSClient> get_ws_client() { return this->ws_client_; }
 
  protected:
   /**
@@ -132,7 +123,8 @@ class SensESPApp : public SensESPBaseApp {
     this->sk_server_port_ = sk_server_port;
     return this;
   }
-  const SensESPApp* set_system_status_led(SystemStatusLed* system_status_led) {
+  const SensESPApp* set_system_status_led(
+      std::shared_ptr<SystemStatusLed>& system_status_led) {
     this->system_status_led_ = system_status_led;
     return this;
   }
@@ -171,7 +163,7 @@ class SensESPApp : public SensESPBaseApp {
 
     if (ota_password_ != nullptr) {
       // create the OTA object
-      ota_ = new OTA(ota_password_);
+      ota_ = std::make_shared<OTA>(ota_password_);
     }
 
     bool captive_portal_enabled = networking_->is_captive_portal_enabled();
@@ -189,38 +181,38 @@ class SensESPApp : public SensESPBaseApp {
     ConfigItem(this->http_server_);
 
     // create the SK delta object
-    sk_delta_queue_ = new SKDeltaQueue();
+    sk_delta_queue_ = std::make_shared<SKDeltaQueue>();
 
     // create the websocket client
     bool const use_mdns = sk_server_address_ == "";
-    this->ws_client_ =
-        new SKWSClient("/System/Signal K Settings", sk_delta_queue_,
-                       sk_server_address_, sk_server_port_, use_mdns);
+    this->ws_client_ = std::make_shared<SKWSClient>(
+        "/System/Signal K Settings", sk_delta_queue_, sk_server_address_,
+        sk_server_port_, use_mdns);
 
     ConfigItem(this->ws_client_);
 
     // connect the system status controller
     this->networking_->get_wifi_state_producer()->connect_to(
-        &system_status_controller_.get_wifi_state_consumer());
+        &system_status_controller_->get_wifi_state_consumer());
     this->ws_client_->connect_to(
-        &system_status_controller_.get_ws_connection_state_consumer());
+        &system_status_controller_->get_ws_connection_state_consumer());
 
     // create the MDNS discovery object
-    mdns_discovery_ = new MDNSDiscovery();
+    mdns_discovery_ = std::make_shared<MDNSDiscovery>();
 
     // create a system status led and connect it
 
-    if (system_status_led_ == NULL) {
-      system_status_led_ = new SystemStatusLed(LED_PIN);
+    if (system_status_led_ == nullptr) {
+      system_status_led_ = std::make_shared<SystemStatusLed>(LED_PIN);
     }
-    this->system_status_controller_.connect_to(
+    this->system_status_controller_->connect_to(
         system_status_led_->get_system_status_consumer());
     this->ws_client_->get_delta_tx_count_producer().connect_to(
         system_status_led_->get_delta_tx_count_consumer());
 
     // create the button handler
     if (button_gpio_pin_ != -1) {
-      button_handler_ = new ButtonHandler(button_gpio_pin_);
+      button_handler_ = std::make_shared<ButtonHandler>(button_gpio_pin_);
     }
 
     // connect status page items
@@ -258,19 +250,20 @@ class SensESPApp : public SensESPBaseApp {
   String ap_password_ = "thisisfine";
   const char* ota_password_ = nullptr;
 
-  MDNSDiscovery* mdns_discovery_;
+  std::shared_ptr<MDNSDiscovery> mdns_discovery_;
   std::shared_ptr<HTTPServer> http_server_;
 
-  SystemStatusLed* system_status_led_ = NULL;
-  SystemStatusController system_status_controller_;
+  std::shared_ptr<SystemStatusLed> system_status_led_;
+  std::shared_ptr<SystemStatusController> system_status_controller_ =
+      std::make_shared<SystemStatusController>();
   int button_gpio_pin_ = SENSESP_BUTTON_PIN;
-  ButtonHandler* button_handler_ = nullptr;
+  std::shared_ptr<ButtonHandler> button_handler_;
 
   std::shared_ptr<Networking> networking_;
 
-  OTA* ota_;
-  SKDeltaQueue* sk_delta_queue_;
-  SKWSClient* ws_client_;
+  std::shared_ptr<OTA> ota_;
+  std::shared_ptr<SKDeltaQueue> sk_delta_queue_;
+  std::shared_ptr<SKWSClient> ws_client_;
 
   StatusPageItem<String> sensesp_version_ui_output_{
       "SenseESP version", kSensESPVersion, "Software", 1900};
@@ -295,6 +288,13 @@ class SensESPApp : public SensESPBaseApp {
   StatusPageItem<int> delta_rx_count_ui_output_{"SK Delta RX count", 0,
                                                 "Signal K", 1800};
 
+// Placeholders for system status sensors in case they are created
+std::shared_ptr<ValueProducer<float>> system_hz_sensor_;
+std::shared_ptr<ValueProducer<uint32_t>> free_mem_sensor_;
+std::shared_ptr<ValueProducer<float>> uptime_sensor_;
+std::shared_ptr<ValueProducer<String>> ip_address_sensor_;
+std::shared_ptr<ValueProducer<int>> wifi_signal_sensor_;
+
   friend class WebServer;
   friend class SensESPAppBuilder;
 };