From 1488f56574ecf4bb78288680067176aa28ea0bcc Mon Sep 17 00:00:00 2001
From: Raul Radu <raul.radu@mail.polimi.it>
Date: Tue, 9 Jan 2024 02:03:36 +0100
Subject: [PATCH] [Module] Changed raw pointers to std pointers

Modules are now created as shared pointers to avoid deletition
while still in use by other components
---
 src/shared/Modules/Module.cpp      | 45 ++++++++++++++------
 src/shared/Modules/Module.h        |  5 ++-
 src/shared/Modules/ModuleInfo.cpp  |  5 ++-
 src/shared/Modules/ModuleInfo.h    |  7 +--
 src/shared/Modules/ModulesList.cpp | 68 ++++++++++++++++++------------
 src/shared/Modules/ModulesList.h   |  9 ++--
 6 files changed, 87 insertions(+), 52 deletions(-)

diff --git a/src/shared/Modules/Module.cpp b/src/shared/Modules/Module.cpp
index 1e759785..57d452ab 100644
--- a/src/shared/Modules/Module.cpp
+++ b/src/shared/Modules/Module.cpp
@@ -37,7 +37,8 @@ Module::Module(ModuleId id) : id(id)
 
                     QAction *close = new QAction("Close");
                     connect(close, &QAction::triggered, this,
-                            [this]() { emit closeMe(this); });
+                            [this]()
+                            { emit closeMe(std::shared_ptr<Module>(this)); });
                     menu.addAction(close);
 
                     QAction *replace = new QAction("Replace");
@@ -80,13 +81,25 @@ void Module::fromXmlObject(const XmlObject &xmlObject) {}
 
 void Module::onReplaceMe()
 {
-    ModulesPicker *picker = new ModulesPicker();
+    using ModuleDeleter = std::function<void(ModulesPicker *)>;
+
+    std::unique_ptr<ModulesPicker, ModuleDeleter> picker(
+        new ModulesPicker(),
+        [&](ModulesPicker *deletablePicker) -> void
+        {
+            disconnect(deletablePicker, &ModulesPicker::onModuleSelected, this,
+                       nullptr);
+            delete deletablePicker;
+        });
+
+    auto NewModuleEventEmitter = [this](ModuleId id)
+    {
+        emit replaceMe(std::shared_ptr<Module>(this),
+                       ModulesList::getInstance().instantiateModule(id));
+    };
 
-    connect(picker, &ModulesPicker::onModuleSelected, this,
-            [this](ModuleId id) {
-                emit replaceMe(
-                    this, ModulesList::getInstance().instantiateModule(id));
-            });
+    connect(picker.get(), &ModulesPicker::onModuleSelected, this,
+            NewModuleEventEmitter);
 
     // Launch the modules picker
     picker->exec();
@@ -104,18 +117,24 @@ Window *Module::getWindow()
 
 void Module::error(QString title, QString description, int msecDuration)
 {
-    getWindow()->errorDisplayer->createError(title, description,
-                                             ErrorType::ET_ERROR, msecDuration);
+    std::shared_ptr<ErrorDisplayer> errorDisplayer =
+        getWindow()->errorDisplayer;
+    errorDisplayer->createError(title, description, ErrorType::ET_ERROR,
+                                msecDuration);
 }
 
 void Module::warning(QString title, QString description, int msecDuration)
 {
-    getWindow()->errorDisplayer->createError(
-        title, description, ErrorType::ET_WARNING, msecDuration);
+    std::shared_ptr<ErrorDisplayer> errorDisplayer =
+        getWindow()->errorDisplayer;
+    errorDisplayer->createError(title, description, ErrorType::ET_WARNING,
+                                msecDuration);
 }
 
 void Module::info(QString title, QString description, int msecDuration)
 {
-    getWindow()->errorDisplayer->createError(title, description,
-                                             ErrorType::ET_INFO, msecDuration);
+    std::shared_ptr<ErrorDisplayer> errorDisplayer =
+        getWindow()->errorDisplayer;
+    errorDisplayer->createError(title, description, ErrorType::ET_INFO,
+                                msecDuration);
 }
diff --git a/src/shared/Modules/Module.h b/src/shared/Modules/Module.h
index 279383b9..f6cdab3d 100644
--- a/src/shared/Modules/Module.h
+++ b/src/shared/Modules/Module.h
@@ -57,8 +57,9 @@ public slots:
     // void onCloseMe();
 
 signals:
-    void replaceMe(Module* oldModule, Module* newModule);
-    void closeMe(Module* thisModule);
+    void replaceMe(std::shared_ptr<Module> oldModule,
+                   std::shared_ptr<Module> newModule);
+    void closeMe(std::shared_ptr<Module> thisModule);
 
 protected:
     ModuleId id;
diff --git a/src/shared/Modules/ModuleInfo.cpp b/src/shared/Modules/ModuleInfo.cpp
index 147f9917..a9c37fd4 100644
--- a/src/shared/Modules/ModuleInfo.cpp
+++ b/src/shared/Modules/ModuleInfo.cpp
@@ -41,12 +41,13 @@ QString ModuleInfo::getModuleName() const { return name; }
 
 void ModuleInfo::setName(const QString &value) { name = value; }
 
-std::function<Module *()> ModuleInfo::getFactoryMethod() const
+std::function<std::shared_ptr<Module>()> ModuleInfo::getFactoryMethod() const
 {
     return factoryMethod;
 }
 
-void ModuleInfo::setFactoryMethod(const std::function<Module *()> &value)
+void ModuleInfo::setFactoryMethod(
+    const std::function<std::shared_ptr<Module>()> &value)
 {
     factoryMethod = value;
 }
diff --git a/src/shared/Modules/ModuleInfo.h b/src/shared/Modules/ModuleInfo.h
index 08362667..8d49ac14 100644
--- a/src/shared/Modules/ModuleInfo.h
+++ b/src/shared/Modules/ModuleInfo.h
@@ -80,8 +80,9 @@ public:
     QString getModuleName() const;
     void setName(const QString& value);
 
-    std::function<Module*()> getFactoryMethod() const;
-    void setFactoryMethod(const std::function<Module*()>& value);
+    std::function<std::shared_ptr<Module>()> getFactoryMethod() const;
+    void setFactoryMethod(
+        const std::function<std::shared_ptr<Module>()>& value);
 
     ModuleCategory getModuleCategory() const;
     void setCategory(const ModuleCategory& value);
@@ -90,5 +91,5 @@ private:
     ModuleId id;
     ModuleCategory category = ModuleCategory::HIDDEN;
     QString name;
-    std::function<Module*()> factoryMethod;
+    std::function<std::shared_ptr<Module>()> factoryMethod;
 };
diff --git a/src/shared/Modules/ModulesList.cpp b/src/shared/Modules/ModulesList.cpp
index 1c692843..7eaa0ce0 100644
--- a/src/shared/Modules/ModulesList.cpp
+++ b/src/shared/Modules/ModulesList.cpp
@@ -55,7 +55,8 @@ ModulesList::ModulesList()
     {
         ModuleInfo emptyModule(ModuleId::EMPTY, "Empty",
                                ModuleCategory::HIDDEN);
-        emptyModule.setFactoryMethod([]() { return new EmptyModule(); });
+        emptyModule.setFactoryMethod(
+            []() { return std::make_shared<EmptyModule>(); });
         modulesInfo.append(emptyModule);
     }
 
@@ -63,47 +64,49 @@ ModulesList::ModulesList()
     {
         ModuleInfo graphModule(ModuleId::GRAPH, "Graph",
                                ModuleCategory::DATA_VISUALIZATION);
-        graphModule.setFactoryMethod([]() { return new Graph(); });
+        graphModule.setFactoryMethod([]()
+                                     { return std::make_shared<Graph>(); });
         modulesInfo.append(graphModule);
 
         ModuleInfo outMsgViewer(ModuleId::OUTGOING_MESSAGES_VIEWER,
                                 "Outgoing Messages Viewer",
                                 ModuleCategory::DATA_VISUALIZATION);
         outMsgViewer.setFactoryMethod(
-            []() { return new OutgoingMessagesViewerModule(); });
+            []() { return std::make_shared<OutgoingMessagesViewerModule>(); });
         modulesInfo.append(outMsgViewer);
 
         ModuleInfo inMsgViewer(ModuleId::INCOMING_MESSAGES_VIEWER,
                                "Incoming Messages Viewer",
                                ModuleCategory::DATA_VISUALIZATION);
         inMsgViewer.setFactoryMethod(
-            []() { return new IncomingMessagesViewerModule(); });
+            []() { return std::make_shared<IncomingMessagesViewerModule>(); });
         modulesInfo.append(inMsgViewer);
 
         ModuleInfo mainStateViewer(ModuleId::MAIN_STATE_VIEWER,
                                    "Main State Viewer",
                                    ModuleCategory::DATA_VISUALIZATION);
         mainStateViewer.setFactoryMethod(
-            []() { return new MainStateViewerModule(); });
+            []() { return std::make_shared<MainStateViewerModule>(); });
         modulesInfo.append(mainStateViewer);
 
         ModuleInfo payloadStateViewer(ModuleId::PAYLOAD_STATE_VIEWER,
                                       "Payload State Viewer",
                                       ModuleCategory::DATA_VISUALIZATION);
         payloadStateViewer.setFactoryMethod(
-            []() { return new PayloadStateViewerModule(); });
+            []() { return std::make_shared<PayloadStateViewerModule>(); });
         modulesInfo.append(payloadStateViewer);
 
         ModuleInfo orientationVisualizer(ModuleId::ORIENTATION_VISUALIZER,
                                          "Orientation Visualizer",
                                          ModuleCategory::DATA_VISUALIZATION);
         orientationVisualizer.setFactoryMethod(
-            []() { return new OrientationVisualizer(); });
+            []() { return std::make_shared<OrientationVisualizer>(); });
         modulesInfo.append(orientationVisualizer);
 
         ModuleInfo valvesViewer(ModuleId::VALVES_VIEWER, "Valves Viewer",
                                 ModuleCategory::DATA_VISUALIZATION);
-        valvesViewer.setFactoryMethod([]() { return new ValvesViewer(); });
+        valvesViewer.setFactoryMethod(
+            []() { return std::make_shared<ValvesViewer>(); });
         modulesInfo.append(valvesViewer);
     }
 
@@ -111,18 +114,20 @@ ModulesList::ModulesList()
     {
         ModuleInfo serialMavlink(ModuleId::SERIAL_MAVLINK, "Serial Mavlink",
                                  ModuleCategory::DATA_SOURCES);
-        serialMavlink.setFactoryMethod([]()
-                                       { return new SerialMavlinkModule(); });
+        serialMavlink.setFactoryMethod(
+            []() { return std::make_shared<SerialMavlinkModule>(); });
         modulesInfo.append(serialMavlink);
 
         ModuleInfo udpMavlink(ModuleId::UDP_MAVLINK, "Udp Mavlink",
                               ModuleCategory::DATA_SOURCES);
-        udpMavlink.setFactoryMethod([]() { return new UdpMavlinkModule(); });
+        udpMavlink.setFactoryMethod(
+            []() { return std::make_shared<UdpMavlinkModule>(); });
         modulesInfo.append(udpMavlink);
 
         ModuleInfo fileStream(ModuleId::FILE_STREAM, "File Stream",
                               ModuleCategory::DATA_SOURCES);
-        fileStream.setFactoryMethod([]() { return new FileStreamModule(); });
+        fileStream.setFactoryMethod(
+            []() { return std::make_shared<FileStreamModule>(); });
         modulesInfo.append(fileStream);
     }
 
@@ -130,43 +135,46 @@ ModulesList::ModulesList()
     {
         ModuleInfo commandPad(ModuleId::COMMAND_PAD, "Command Pad",
                               ModuleCategory::UTILITIES);
-        commandPad.setFactoryMethod([]() { return new CommandPad(); });
+        commandPad.setFactoryMethod([]()
+                                    { return std::make_shared<CommandPad>(); });
         modulesInfo.append(commandPad);
 
         ModuleInfo compactCommandPad(ModuleId::COMPACT_COMMAND_PAD,
                                      "Compact Command Pad",
                                      ModuleCategory::UTILITIES);
-        compactCommandPad.setFactoryMethod([]()
-                                           { return new CompactCommandPad(); });
+        compactCommandPad.setFactoryMethod(
+            []() { return std::make_shared<CompactCommandPad>(); });
         modulesInfo.append(compactCommandPad);
 
         ModuleInfo valuesConverterViewer(ModuleId::VALUES_CONVERTER_VIEWER,
                                          "Values Converter Viewer",
                                          ModuleCategory::UTILITIES);
         valuesConverterViewer.setFactoryMethod(
-            []() { return new ValuesConverterViewerModule(); });
+            []() { return std::make_shared<ValuesConverterViewerModule>(); });
         modulesInfo.append(valuesConverterViewer);
 
         ModuleInfo timerController(ModuleId::TIMER_CONTROLLER,
                                    "Timer Controller",
                                    ModuleCategory::UTILITIES);
         timerController.setFactoryMethod(
-            []() { return new TimerControllerModule(); });
+            []() { return std::make_shared<TimerControllerModule>(); });
         modulesInfo.append(timerController);
     }
 
     // Organization
     {
         ModuleInfo tabs(ModuleId::TABS, "Tabs", ModuleCategory::ORGANIZATION);
-        tabs.setFactoryMethod([]() { return new TabsModule(); });
+        tabs.setFactoryMethod([]() { return std::make_shared<TabsModule>(); });
         modulesInfo.append(tabs);
 
         ModuleInfo verticalSplitter(ModuleId::VERTICAL_SPLITTER,
                                     "Vertical Splitter",
                                     ModuleCategory::ORGANIZATION);
         verticalSplitter.setFactoryMethod(
-            []() {
-                return new Splitter(Qt::Vertical, ModuleId::VERTICAL_SPLITTER);
+            []()
+            {
+                return std::make_shared<Splitter>(Qt::Vertical,
+                                                  ModuleId::VERTICAL_SPLITTER);
             });
         modulesInfo.append(verticalSplitter);
 
@@ -174,9 +182,10 @@ ModulesList::ModulesList()
                                       "Horizontal Splitter",
                                       ModuleCategory::ORGANIZATION);
         horizontalSplitter.setFactoryMethod(
-            []() {
-                return new Splitter(Qt::Horizontal,
-                                    ModuleId::HORIZONTAL_SPLITTER);
+            []()
+            {
+                return std::make_shared<Splitter>(
+                    Qt::Horizontal, ModuleId::HORIZONTAL_SPLITTER);
             });
         modulesInfo.append(horizontalSplitter);
     }
@@ -185,7 +194,8 @@ ModulesList::ModulesList()
     {
         ModuleInfo testModule(ModuleId::BROKER_TEST, "Test Broker",
                               ModuleCategory::TESTING);
-        testModule.setFactoryMethod([]() { return new TestModule(); });
+        testModule.setFactoryMethod([]()
+                                    { return std::make_shared<TestModule>(); });
         modulesInfo.append(testModule);
     }
 }
@@ -223,7 +233,8 @@ QString ModulesList::getXmlName(ModuleId id)
     return name;
 }
 
-std::function<Module*()> ModulesList::getFactoryMethod(ModuleId id)
+std::function<std::shared_ptr<Module>()> ModulesList::getFactoryMethod(
+    ModuleId id)
 {
     for (int i = 0; i < modulesInfo.count(); i++)
     {
@@ -233,7 +244,8 @@ std::function<Module*()> ModulesList::getFactoryMethod(ModuleId id)
     return []() { return nullptr; };
 }
 
-std::function<Module*()> ModulesList::getFactoryMethod(const QString& name)
+std::function<std::shared_ptr<Module>()> ModulesList::getFactoryMethod(
+    const QString& name)
 {
     for (int i = 0; i < modulesInfo.count(); i++)
     {
@@ -256,12 +268,12 @@ QList<QString> ModulesList::getModulesNamesList()
 
 QList<ModuleInfo> ModulesList::getModulesInfo() { return modulesInfo; }
 
-Module* ModulesList::instantiateModule(ModuleId id)
+std::shared_ptr<Module> ModulesList::instantiateModule(ModuleId id)
 {
     return getFactoryMethod(id)();
 }
 
-Module* ModulesList::instantiateModule(const QString& name)
+std::shared_ptr<Module> ModulesList::instantiateModule(const QString& name)
 {
     return getFactoryMethod(name)();
 }
diff --git a/src/shared/Modules/ModulesList.h b/src/shared/Modules/ModulesList.h
index 65ef7099..0ae0cecf 100644
--- a/src/shared/Modules/ModulesList.h
+++ b/src/shared/Modules/ModulesList.h
@@ -32,15 +32,16 @@ public:
     QString getModuleName(ModuleId id);
     QString getXmlName(ModuleId id);
 
-    std::function<Module *()> getFactoryMethod(ModuleId id);
-    std::function<Module *()> getFactoryMethod(const QString &name);
+    std::function<std::shared_ptr<Module>()> getFactoryMethod(ModuleId id);
+    std::function<std::shared_ptr<Module>()> getFactoryMethod(
+        const QString &name);
 
     QList<QString> getModulesNamesList();
     bool getModuleId(QString name, ModuleId &id);
     QList<ModuleInfo> getModulesInfo();
 
-    Module *instantiateModule(ModuleId id);
-    Module *instantiateModule(const QString &name);
+    std::shared_ptr<Module> instantiateModule(ModuleId id);
+    std::shared_ptr<Module> instantiateModule(const QString &name);
 
 private:
     ModulesList();
-- 
GitLab