Cleanup rpc interfaces

components/config/ConfigManager.cpp

@@ -52,6 +52,12 @@ std::string ConfigManager::get_wifi_password() const {
     return str;
 }
 
+CommunicationMethod ConfigManager::get_communication_method() const {
+    uint16_t type = DEFAULT_COMMUNICATION_METHOD;
+    get_uint16(COMMUNICATION_METHOD_KEY, &type);
+    return static_cast<CommunicationMethod>(type);
+}
+
 esp_err_t ConfigManager::get_string(const char *key, std::string& out) const {
     std::shared_lock lock(rw_lock);
     esp_err_t ret = ESP_OK;
@@ -133,6 +139,10 @@ void ConfigManager::set_wifi_password(const char* password) {
     set<nvs_set_cpp_str, std::string>(WIFI_PASSWORD_KEY, password);
 }
 
+void ConfigManager::set_communication_method(const CommunicationMethod method) {
+    set<nvs_set_u16, uint16_t>(COMMUNICATION_METHOD_KEY, method);
+}
+
 // Func - the esp write function to call (ie. nvs_set_u16)
 // T - the type of the value in the key value pair
 template<auto Func, typename T>

components/config/include/ConfigManager.h

@@ -9,6 +9,7 @@
 #include <unordered_map>
 #include <shared_mutex>
 
+#include "enums.h"
 #include "esp_check.h"
 #include "nvs.h"
 #include "flatbuffers_generated/RobotModule_generated.h"
@@ -30,11 +31,13 @@ public:
     ModuleType get_module_type() const;
     std::string get_wifi_ssid() const;
     std::string get_wifi_password() const;
+    CommunicationMethod get_communication_method() const;
 
     void set_module_id(uint16_t id);
     void set_module_type(ModuleType type);
     void set_wifi_ssid(const char* ssid);
     void set_wifi_password(const char* password);
+    void set_communication_method(CommunicationMethod method);
 
 private:
     ConfigManager() = default;

components/config/include/enums.h

@@ -0,0 +1,12 @@
+//
+// Created by Johnathon Slightham on 2025-10-22.
+//
+
+#ifndef ENUMS_H
+#define ENUMS_H
+
+enum CommunicationMethod {
+    Wireless,
+};
+
+#endif //ENUMS_H

components/constants/include/constants/config.h

@@ -5,13 +5,15 @@
 #ifndef CONFIG_H
 #define CONFIG_H
 
-#define NVS_FLASH_NAMESPACE     "app"
-#define DEFAULT_MODULE_ID       1
-#define DEFAULT_MODULE_TYPE     0
+#define NVS_FLASH_NAMESPACE             "app"
+#define DEFAULT_MODULE_ID               1
+#define DEFAULT_MODULE_TYPE             0
+#define DEFAULT_COMMUNICATION_METHOD    0
 
-#define MODULE_ID_KEY           "module_id"
-#define MODULE_TYPE_KEY         "module_type"
-#define WIFI_SSID_KEY           "wifi_ssid"
-#define WIFI_PASSWORD_KEY       "wifi_password"
+#define MODULE_ID_KEY               "module_id"
+#define MODULE_TYPE_KEY             "module_type"
+#define WIFI_SSID_KEY               "wifi_ssid"
+#define WIFI_PASSWORD_KEY           "wifi_password"
+#define COMMUNICATION_METHOD_KEY    "comm_method"
 
 #endif //CONFIG_H

components/ptrQueue/include/PtrQueue.h

@@ -6,7 +6,11 @@
 #define PTRQUEUE_H
 
 #include <memory>
-#include <iostream>
+
+#include "freertos/FreeRTOS.h"
+#include "portmacro.h"
+#include "freertos/projdefs.h"
+#include "freertos/queue.h"
 
 // Wrapped FreeRTOS queue to support unique_ptr
 

components/rpc/CMakeLists.txt

@@ -1,4 +1,10 @@
-idf_component_register(SRCS "WifiManager.cpp" "mDNSDiscoveryService.cpp" "TCPServer.cpp" "CommunicationRouter.cpp" "MessagingInterface.cpp" "OrientationDetection.cpp"
+file(GLOB_RECURSE ALL_SRCS
+        "${CMAKE_CURRENT_SOURCE_DIR}/*.c"
+        "${CMAKE_CURRENT_SOURCE_DIR}/*.cpp"
+        "${CMAKE_CURRENT_SOURCE_DIR}/*.S"
+)
+
+idf_component_register(SRCS ${ALL_SRCS}
                         PRIV_REQUIRES driver esp_event nvs_flash esp_netif espressif__mdns constants config flatbuffers dataLink rmt
                         REQUIRES ptrQueue esp_wifi
                         INCLUDE_DIRS "include")

components/rpc/CommunicationFactory.cpp

@@ -0,0 +1,47 @@
+//
+// Created by Johnathon Slightham on 2025-10-22.
+//
+
+#include <memory>
+
+#include "CommunicationFactory.h"
+#include "constants/tcp.h"
+#include "wireless/mDNSDiscoveryService.h"
+#include "wireless/TCPServer.h"
+#include "wireless/WifiManager.h"
+
+std::unique_ptr<IConnectionManager> CommunicationFactory::create_connection_manager(const CommunicationMethod type) {
+    switch (type) {
+        case Wireless:
+            return std::make_unique<WifiManager>();
+        default:
+            return nullptr;
+    }
+}
+
+std::unique_ptr<IDiscoveryService> CommunicationFactory::create_discovery_service(const CommunicationMethod type) {
+    switch (type) {
+        case Wireless:
+            return std::make_unique<mDNSDiscoveryService>();
+        default:
+            return nullptr;
+    }
+}
+
+std::unique_ptr<IRPCServer> CommunicationFactory::create_lossy_server(const CommunicationMethod type, const std::shared_ptr<PtrQueue<std::vector<uint8_t>>>& rx_queue) {
+    switch (type) {
+        case Wireless:
+            return std::make_unique<TCPServer>(TCP_PORT, rx_queue); // todo: replace with udp server
+        default:
+            return nullptr;
+    }
+}
+
+std::unique_ptr<IRPCServer> CommunicationFactory::create_lossless_server(const CommunicationMethod type, const std::shared_ptr<PtrQueue<std::vector<uint8_t>>>& rx_queue) {
+    switch (type) {
+        case Wireless:
+            return std::make_unique<TCPServer>(TCP_PORT, rx_queue);
+        default:
+            return nullptr;
+    }
+}

components/rpc/CommunicationRouter.cpp

@@ -2,9 +2,9 @@
 
 #include "CommunicationRouter.h"
 #include "AngleControlMessageBuilder.h"
-#include "mDNSDiscoveryService.h"
+#include "include/wireless/mDNSDiscoveryService.h"
 #include "MPIMessageBuilder.h"
-#include "WifiManager.h"
+#include "include/wireless/WifiManager.h"
 #include "freertos/FreeRTOS.h"
 #include "Tables.h"
 #include "PtrQueue.h"
@@ -19,7 +19,6 @@ CommunicationRouter::~CommunicationRouter() {
 // todo: we really need to change all char to uint8_t everywhere
 // todo: get rid of copying going on, need to pass around sharedptrs/uniqueptrs
 
-// todo: this needs to be combined with the 4 rmt threads
 [[noreturn]] void CommunicationRouter::router_thread(void *args) {
     const auto that = static_cast<CommunicationRouter *>(args);
 
@@ -40,7 +39,6 @@ CommunicationRouter::~CommunicationRouter() {
     that->m_data_link_manager->start_receive_frames(channel);
     while (true) {
         char buffer[512];
-        // todo: very c style function calls
         const auto err = that->m_data_link_manager->receive(reinterpret_cast<uint8_t *>(buffer), 512, &bytes_received, channel);
         that->m_data_link_manager->start_receive_frames(channel);
 
@@ -66,7 +64,6 @@ int CommunicationRouter::send_msg(char* buffer, const size_t length) const {
     return 0;
 }
 
-// todo: the number of things this is doing in so many different places is crazy...
 void CommunicationRouter::update_leader() {
     RIPRow_public table[RIP_MAX_ROUTES];
     size_t table_size = RIP_MAX_ROUTES;
@@ -95,26 +92,24 @@ void CommunicationRouter::update_leader() {
     this->m_leader = max;
 
     if (this->m_leader == m_module_id) {
-        mDNSDiscoveryService::set_connected_boards(connected_module_ids);
+        this->m_discovery_service->set_connected_boards(connected_module_ids);
     }
 }
 
 void CommunicationRouter::route(uint8_t* buffer, const size_t length) const {
 	flatbuffers::Verifier verifier(buffer, length);
-	bool ok = Messaging::VerifyMPIMessageBuffer(verifier);
-	if (!ok) { // This could be moved to just be called on wireline data to save cpu cycles.
+    // This could be moved to just be called on wireline data to save cpu cycles.
+    if (bool ok = Messaging::VerifyMPIMessageBuffer(verifier); !ok) {
 		ESP_LOGW(TAG, "route: got an invalid MPI message, disregarding");
 		return;
 	}
 
-    const auto& mpi_message = Flatbuffers::MPIMessageBuilder::parse_mpi_message(buffer);
-
-    if (mpi_message->destination() == m_module_id) {
+    if (const auto& mpi_message = Flatbuffers::MPIMessageBuilder::parse_mpi_message(buffer); mpi_message->destination() == m_module_id) {
         ESP_LOGD(TAG, "Routing to this module [dest: %d, length: %d]", static_cast<int>(mpi_message->destination()), length);
         this->m_rx_callback(reinterpret_cast<char *>(buffer), 512);
     } else if (mpi_message->destination() == PC_ADDR && this->m_leader == m_module_id) {
         ESP_LOGD(TAG, "Routing to wifi [dest: %d, length: %d]", static_cast<int>(mpi_message->destination()), length);
-        this->m_tcp_server->send_msg(reinterpret_cast<char *>(buffer), 512);
+        this->m_lossless_server->send_msg(reinterpret_cast<char *>(buffer), 512);
     } else if (mpi_message->destination() == PC_ADDR) {
         ESP_LOGD(TAG, "Routing to wireline for wifi [dest: %d, length: %d]", static_cast<int>(mpi_message->destination()), length);
         this->m_data_link_manager->send(this->m_leader, buffer, length, FrameType::MOTOR_TYPE, 0);

components/rpc/MessagingInterface.cpp

@@ -5,6 +5,7 @@
 #include "MessagingInterface.h"
 #include "AngleControlMessageBuilder.h"
 #include "ConfigManager.h"
+#include "freertos/FreeRTOS.h"
 #include "freertos/queue.h"
 #include "freertos/semphr.h"
 #include "MPIMessageBuilder.h"

components/rpc/include/CommunicationFactory.h

@@ -0,0 +1,27 @@
+//
+// Created by Johnathon Slightham on 2025-10-22.
+//
+
+// Although we currently only support wireless, this is architected to be easy to add other connection methods
+
+#ifndef COMMUNICATIONFACTORY_H
+#define COMMUNICATIONFACTORY_H
+
+#include <memory>
+#include <vector>
+
+#include "IConnectionManager.h"
+#include "IDiscoveryService.h"
+#include "IRPCServer.h"
+#include "PtrQueue.h"
+#include "enums.h"
+
+class CommunicationFactory {
+public:
+    static std::unique_ptr<IConnectionManager> create_connection_manager(CommunicationMethod type);
+    static std::unique_ptr<IDiscoveryService> create_discovery_service(CommunicationMethod type);
+    static std::unique_ptr<IRPCServer> create_lossy_server(CommunicationMethod type, const std::shared_ptr<PtrQueue<std::vector<uint8_t>>> &rx_queue);
+    static std::unique_ptr<IRPCServer> create_lossless_server(CommunicationMethod type, const std::shared_ptr<PtrQueue<std::vector<uint8_t>>>& rx_queue);
+};
+
+#endif //COMMUNICATIONFACTORY_H

components/rpc/include/CommunicationRouter.h

@@ -2,6 +2,8 @@
 // Created by Johnathon Slightham on 2025-05-25.
 //
 
+// todo: this class is getting a bit large
+
 #ifndef COMMUNICATIONROUTER_H
 #define COMMUNICATIONROUTER_H
 
@@ -9,13 +11,14 @@
 #include <memory>
 #include <chrono>
 
+#include "CommunicationFactory.h"
+#include "freertos/FreeRTOS.h"
+#include "IDiscoveryService.h"
 #include "ConfigManager.h"
 #include "OrientationDetection.h"
-#include "WifiManager.h"
-#include "freertos/FreeRTOS.h"
-#include "TCPServer.h"
+#include "wireless/WifiManager.h"
+#include "wireless/TCPServer.h"
 #include "DataLinkManager.h"
-#include "constants/tcp.h"
 #include "constants/module.h"
 #include "PtrQueue.h"
 
@@ -29,15 +32,16 @@ class CommunicationRouter {
     };
 
 public:
-    CommunicationRouter(const std::function<void(char*, int)> &rx_callback, std::unique_ptr<WifiManager>&& pc_connection)
+    explicit CommunicationRouter(const std::function<void(char*, int)> &rx_callback)
         : m_tcp_rx_queue(std::make_shared<PtrQueue<std::vector<uint8_t>>>(10)),
             m_rx_callback(rx_callback),
             m_config_manager(ConfigManager::get_instance()),
-            m_tcp_server(std::make_unique<TCPServer>(TCP_PORT, m_tcp_rx_queue)),
+            m_pc_connection(CommunicationFactory::create_connection_manager(m_config_manager.get_communication_method())),
+            m_lossless_server(CommunicationFactory::create_lossless_server(m_config_manager.get_communication_method(), m_tcp_rx_queue)),
             m_data_link_manager(std::make_unique<DataLinkManager>(m_config_manager.get_module_id(), MODULE_TO_NUM_CHANNELS_MAP[m_config_manager.get_module_type()])),
-            m_pc_connection(std::move(pc_connection)),
             m_module_id(m_config_manager.get_module_id()),
-            m_last_leader_updated(std::chrono::system_clock::now()){
+            m_last_leader_updated(std::chrono::system_clock::now()),
+            m_discovery_service(CommunicationFactory::create_discovery_service(m_config_manager.get_communication_method())){
         OrientationDetection::init();
         update_leader();
 
@@ -67,13 +71,14 @@ public:
 private:
     TaskHandle_t m_router_thread = nullptr;
     ConfigManager &m_config_manager;
+    std::unique_ptr<IConnectionManager> m_pc_connection;
     std::vector<TaskHandle_t> m_link_layer_threads;
-    std::unique_ptr<TCPServer> m_tcp_server; // todo: dependency injection
+    std::unique_ptr<IRPCServer> m_lossless_server;
     std::unique_ptr<DataLinkManager> m_data_link_manager;
-    std::unique_ptr<WifiManager> m_pc_connection; // todo: change to dependency inject
     uint8_t m_leader = 0;
     uint8_t m_module_id;
     std::chrono::time_point<std::chrono::system_clock> m_last_leader_updated;
+    std::unique_ptr<IDiscoveryService> m_discovery_service;
 };
 
 #endif //COMMUNICATIONROUTER_H

components/rpc/include/IConnectionManager.h

@@ -2,12 +2,12 @@
 // Created by Johnathon Slightham on 2025-05-26.
 //
 
-#ifndef IWIFIMANAGER_H
-#define IWIFIMANAGER_H
+#ifndef ICONNECTIONMANAGER_H
+#define ICONNECTIONMANAGER_H
 
-class IWifiManager {
+class IConnectionManager{
 public:
-    virtual ~IWifiManager() = default;
+    virtual ~IConnectionManager() = default;
     virtual int connect() = 0;
     virtual int disconnect() = 0;
 };

components/rpc/include/IDiscoveryService.h

@@ -8,8 +8,8 @@
 class IDiscoveryService {
 public:
     virtual ~IDiscoveryService() = default;
-    virtual static void setup() = 0;
-    virtual static void set_connected_boards(const std::vector<int>& boards) = 0;
-}
+
+    virtual void set_connected_boards(const std::vector<int>& boards) = 0;
+};
 
 #endif //IDISCOVERYSERVICE_H

components/rpc/include/MessagingInterface.h

@@ -14,10 +14,10 @@
 
 class MessagingInterface {
 public:
-    explicit MessagingInterface(std::unique_ptr<WifiManager>&& pc_connection)
+    explicit MessagingInterface()
         : m_config_manager(ConfigManager::get_instance()),
             m_mpi_rx_queue(xQueueCreate(MAX_RX_BUFFER_SIZE, RX_QUEUE_SIZE)),
-            m_router(std::make_unique<CommunicationRouter>([this](const char* buffer, const int size) { handleRecv(buffer, size); }, std::move(pc_connection))),
+            m_router(std::make_unique<CommunicationRouter>([this](const char* buffer, const int size) { handleRecv(buffer, size); })),
             m_map_semaphore(xSemaphoreCreateMutex()) {};
 
     ~MessagingInterface();

components/rpc/include/RPCFactory.h

@@ -1,14 +0,0 @@
-//
-// Created by Johnathon Slightham on 2025-05-25.
-//
-
-#ifndef RPCFACTORY_H
-#define RPCFACTORY_H
-
-class RPCFactory {
-public:
-static std::shared_ptr<IRPCServer> createRPCServer() {
-    return std::make_shared<TCPServer>();
-}
-
-#endif //RPCFACTORY_H

components/rpc/include/mDNSDiscoveryService.h

@@ -1,19 +0,0 @@
-//
-// Created by Johnathon Slightham on 2025-05-25.
-//
-
-#ifndef DISCOVERYSERVICE_H
-#define DISCOVERYSERVICE_H
-
-#include <vector>
-
-class mDNSDiscoveryService final {
-public:
-    mDNSDiscoveryService() = delete;
-    ~mDNSDiscoveryService() = delete;
-
-    static void setup();
-    static void set_connected_boards(const std::vector<int>& boards);
-};
-
-#endif //DISCOVERYSERVICE_H

components/rpc/include/wireless/TCPServer.h

@@ -13,7 +13,7 @@
 #include "IRPCServer.h"
 #include "PtrQueue.h"
 
-class TCPServer final : IRPCServer {
+class TCPServer final : public IRPCServer {
 public:
     TCPServer(int port, const std::shared_ptr<PtrQueue<std::vector<uint8_t>>>& rx_queue);
     ~TCPServer() override;

components/rpc/include/wireless/WifiManager.h

@@ -8,9 +8,9 @@
 #include "freertos/FreeRTOS.h"
 #include "freertos/semphr.h"
 #include "ConfigManager.h"
-#include "IWifiManager.h"
+#include "IConnectionManager.h"
 
-class WifiManager final : IWifiManager {
+class WifiManager final : public IConnectionManager {
 public:
   WifiManager() : m_config_manager(ConfigManager::get_instance()),
                   m_mutex(xSemaphoreCreateMutex()),

components/rpc/include/wireless/mDNSDiscoveryService.h

@@ -0,0 +1,20 @@
+//
+// Created by Johnathon Slightham on 2025-05-25.
+//
+
+#ifndef DISCOVERYSERVICE_H
+#define DISCOVERYSERVICE_H
+
+#include <vector>
+
+#include "IDiscoveryService.h"
+
+class mDNSDiscoveryService final : public IDiscoveryService {
+public:
+    mDNSDiscoveryService();
+    ~mDNSDiscoveryService() override;
+
+    void set_connected_boards(const std::vector<int>& boards) override;
+};
+
+#endif //DISCOVERYSERVICE_H

components/rpc/wireless/TCPServer.cpp

@@ -11,7 +11,7 @@
 #include "lwip/sockets.h"
 #include "lwip/sys.h"
 #include "lwip/netdb.h"
-#include "TCPServer.h"
+#include "wireless/TCPServer.h"
 #include "bits/shared_ptr_base.h"
 #include "constants/app_comms.h"
 #include "constants/tcp.h"
@@ -212,11 +212,11 @@ bool TCPServer::is_network_connected() {
 }
 
 bool TCPServer::authenticate_client(int sock) {
-    // todo: authentication (wait for a passphrase from the client)
+    // todo: authentication (key?)
     return 0;
 }
 
-int TCPServer::send_msg(char *buffer, uint32_t length) const {
+int TCPServer::send_msg(char *buffer, const uint32_t length) const {
     if (!is_network_connected()) {
         return -1;
     }

components/rpc/wireless/WifiManager.cpp

@@ -1,9 +1,8 @@
 #include <cstring>
 
-#include "WifiManager.h"
+#include "wireless/WifiManager.h"
 #include "ConfigManager.h"
 #include "constants/wifi.h"
-#include "mDNSDiscoveryService.h"
 
 #define TAG "WifiManager"
 #define SOFTAP_SCAN_FREQUENCY_MS 30000
@@ -219,7 +218,6 @@ void WifiManager::wifi_event_handler(void *event_handler_arg, esp_event_base_t e
     } else if (WIFI_EVENT_STA_CONNECTED == event_id) {
         ESP_LOGI(TAG, "Connected to wifi in station mode\n");
         that->update_state(wifi_state::connected);
-        mDNSDiscoveryService::setup();
     } else if (WIFI_EVENT_STA_DISCONNECTED == event_id) {
         ESP_LOGI(TAG, "Station mode shutdown\n");
         xSemaphoreTake(that->m_mutex, portMAX_DELAY);
@@ -236,7 +234,6 @@ void WifiManager::wifi_event_handler(void *event_handler_arg, esp_event_base_t e
     } else if (WIFI_EVENT_AP_STADISCONNECTED == event_id) {
         ESP_LOGI(TAG, "User disconnected from AP\n");
     } else if (WIFI_EVENT_AP_START == event_id) {
-        mDNSDiscoveryService::setup();
         ESP_LOGI(TAG, "AP started\n");
     }
 }

components/rpc/wireless/mDNSDiscoveryService.cpp

@@ -9,10 +9,10 @@
 
 #include "mdns.h"
 #include "ConfigManager.h"
-#include "mDNSDiscoveryService.h"
+#include "wireless/mDNSDiscoveryService.h"
 #include "constants/tcp.h"
 
-void mDNSDiscoveryService::setup() {
+mDNSDiscoveryService::mDNSDiscoveryService() {
     mdns_init();
 
     const auto& config_manager = ConfigManager::get_instance();
@@ -32,6 +32,10 @@ void mDNSDiscoveryService::setup() {
     mdns_service_txt_set("_robotcontrol", "_tcp", service_txt_data, 3);
 }
 
+mDNSDiscoveryService::~mDNSDiscoveryService() {
+    mdns_free();
+}
+
 void mDNSDiscoveryService::set_connected_boards(const std::vector<int>& boards) {
     std::stringstream ss;