refactor serial port manager: hand out UARTs FCFS

get rid of particular compile-time designations by UART index. just hand out the next free index of hardware UARTs, or indicate that none is available any more. use names as keys to register and free UARTs.
helgeerbe · Jun 2, 2024 · be41e6b · be41e6b
1 parent 5a1c3af
commit be41e6b
Show file tree

Hide file tree

Showing 10 changed files with 97 additions and 125 deletions.
diff --git a/include/Battery.h b/include/Battery.h
@@ -14,7 +14,6 @@ class BatteryProvider {
     virtual void deinit() = 0;
     virtual void loop() = 0;
     virtual std::shared_ptr<BatteryStats> getStats() const = 0;
-    virtual int usedHwUart() const { return -1; } // -1 => no HW UART used
 };
 
 class BatteryClass {

diff --git a/include/JkBmsController.h b/include/JkBmsController.h
@@ -7,12 +7,12 @@
 #include "Battery.h"
 #include "JkBmsSerialMessage.h"
 
+//#define JKBMS_DUMMY_SERIAL
+
 class DataPointContainer;
 
 namespace JkBms {
 
-uint8_t constexpr HwSerialPort = ((ARDUINO_USB_CDC_ON_BOOT != 1)?2:0);
-
 class Controller : public BatteryProvider {
     public:
         Controller() = default;
@@ -21,9 +21,16 @@ class Controller : public BatteryProvider {
         void deinit() final;
         void loop() final;
         std::shared_ptr<BatteryStats> getStats() const final { return _stats; }
-        int usedHwUart() const final { return HwSerialPort; }
 
     private:
+        static char constexpr _serialPortOwner[] = "JK BMS";
+
+#ifdef JKBMS_DUMMY_SERIAL
+        std::unique_ptr<DummySerial> _upSerial;
+#else
+        std::unique_ptr<HardwareSerial> _upSerial;
+#endif
+
         enum class Status : unsigned {
             Initializing,
             Timeout,

diff --git a/include/SerialPortManager.h b/include/SerialPortManager.h
@@ -1,29 +1,21 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 #pragma once
 
-#include <map>
+#include <array>
+#include <optional>
+#include <string>
 
 class SerialPortManagerClass {
 public:
     void init();
-    bool allocateMpptPort(uint8_t port);
-    bool allocateBatteryPort(uint8_t port);
-    void invalidateBatteryPort();
-    void invalidateMpptPorts();
 
-private:
-    enum class Owner {
-        Console,
-        Battery,
-        MPPT
-    };
-
-    std::map<uint8_t, Owner> allocatedPorts;
+    std::optional<uint8_t> allocatePort(std::string const& owner);
+    void freePort(std::string const& owner);
 
-    bool allocatePort(uint8_t port, Owner owner);
-    void invalidate(Owner owner);
-
-    static const char* print(Owner owner);
+private:
+    // the amount of hardare UARTs available on supported ESP32 chips
+    static size_t constexpr _num_controllers = 3;
+    std::array<std::string, _num_controllers> _ports = { "" };
 };
 
 extern SerialPortManagerClass SerialPortManager;
diff --git a/include/VictronMppt.h b/include/VictronMppt.h
@@ -55,8 +55,9 @@ class VictronMpptClass {
     using controller_t = std::unique_ptr<VeDirectMpptController>;
     std::vector<controller_t> _controllers;
 
+    std::vector<String> _serialPortOwners;
     bool initController(int8_t rx, int8_t tx, bool logging,
-        uint8_t instance, uint8_t hwSerialPort);
+        uint8_t instance);
 };
 
 extern VictronMpptClass VictronMppt;
diff --git a/include/VictronSmartShunt.h b/include/VictronSmartShunt.h
@@ -6,13 +6,13 @@
 class VictronSmartShunt : public BatteryProvider {
 public:
     bool init(bool verboseLogging) final;
-    void deinit() final { }
+    void deinit() final;
     void loop() final;
     std::shared_ptr<BatteryStats> getStats() const final { return _stats; }
-    int usedHwUart() const final { return _hwSerialPort; }
 
 private:
-    static uint8_t constexpr _hwSerialPort = ((ARDUINO_USB_CDC_ON_BOOT != 1)?2:0);
+    static char constexpr _serialPortOwner[] = "SmartShunt";
+
     uint32_t _lastUpdate = 0;
     std::shared_ptr<VictronSmartShuntStats> _stats =
         std::make_shared<VictronSmartShuntStats>();

diff --git a/src/Battery.cpp b/src/Battery.cpp
@@ -5,7 +5,6 @@
 #include "JkBmsController.h"
 #include "VictronSmartShunt.h"
 #include "MqttBattery.h"
-#include "SerialPortManager.h"
 
 BatteryClass Battery;
 
@@ -39,7 +38,6 @@ void BatteryClass::updateSettings()
         _upProvider->deinit();
         _upProvider = nullptr;
     }
-    SerialPortManager.invalidateBatteryPort();
 
     CONFIG_T& config = Configuration.get();
     if (!config.Battery.Enabled) { return; }
@@ -64,18 +62,7 @@ void BatteryClass::updateSettings()
             return;
     }
 
-    // port is -1 if provider is neither JK BMS nor SmartShunt. otherwise, port
-    // is 2, unless running on ESP32-S3 with USB CDC, then port is 0.
-    int port = _upProvider->usedHwUart();
-    if (port >= 0 && !SerialPortManager.allocateBatteryPort(port)) {
-        _upProvider = nullptr;
-        return;
-    }
-
-    if (!_upProvider->init(verboseLogging)) {
-        SerialPortManager.invalidateBatteryPort();
-        _upProvider = nullptr;
-    }
+    if (!_upProvider->init(verboseLogging)) { _upProvider = nullptr; }
 }
 
 void BatteryClass::loop()

diff --git a/src/JkBmsController.cpp b/src/JkBmsController.cpp
@@ -5,12 +5,11 @@
 #include "MessageOutput.h"
 #include "JkBmsDataPoints.h"
 #include "JkBmsController.h"
+#include "SerialPortManager.h"
 #include <frozen/map.h>
 
 namespace JkBms {
 
-//#define JKBMS_DUMMY_SERIAL
-
 #ifdef JKBMS_DUMMY_SERIAL
 class DummySerial {
     public:
@@ -198,9 +197,6 @@ class DummySerial {
         size_t _msg_idx = 0;
         size_t _byte_idx = 0;
 };
-DummySerial HwSerial;
-#else
-HardwareSerial HwSerial(HwSerialPort);
 #endif
 
 bool Controller::init(bool verboseLogging)
@@ -220,9 +216,18 @@ bool Controller::init(bool verboseLogging)
         return false;
     }
 
-    HwSerial.end(); // make sure the UART will be re-initialized
-    HwSerial.begin(115200, SERIAL_8N1, pin.battery_rx, pin.battery_tx);
-    HwSerial.flush();
+#ifdef JKBMS_DUMMY_SERIAL
+    _upSerial = std::make_unique<DummySerial>();
+#else
+    auto oHwSerialPort = SerialPortManager.allocatePort(_serialPortOwner);
+    if (!oHwSerialPort) { return false; }
+
+    _upSerial = std::make_unique<HardwareSerial>(*oHwSerialPort);
+#endif
+
+    _upSerial->end(); // make sure the UART will be re-initialized
+    _upSerial->begin(115200, SERIAL_8N1, pin.battery_rx, pin.battery_tx);
+    _upSerial->flush();
 
     if (Interface::Transceiver != getInterface()) { return true; }
 
@@ -242,10 +247,12 @@ bool Controller::init(bool verboseLogging)
 
 void Controller::deinit()
 {
-    HwSerial.end();
+    _upSerial->end();
 
     if (_rxEnablePin > 0) { pinMode(_rxEnablePin, INPUT); }
     if (_txEnablePin > 0) { pinMode(_txEnablePin, INPUT); }
+
+    SerialPortManager.freePort(_serialPortOwner);
 }
 
 Controller::Interface Controller::getInterface() const
@@ -296,7 +303,7 @@ void Controller::sendRequest(uint8_t pollInterval)
         return announceStatus(Status::WaitingForPollInterval);
     }
 
-    if (!HwSerial.availableForWrite()) {
+    if (!_upSerial->availableForWrite()) {
         return announceStatus(Status::HwSerialNotAvailableForWrite);
     }
 
@@ -307,10 +314,10 @@ void Controller::sendRequest(uint8_t pollInterval)
         digitalWrite(_txEnablePin, HIGH); // enable transmission
     }
 
-    HwSerial.write(readAll.data(), readAll.size());
+    _upSerial->write(readAll.data(), readAll.size());
 
     if (Interface::Transceiver == getInterface()) {
-        HwSerial.flush();
+        _upSerial->flush();
         digitalWrite(_rxEnablePin, LOW); // enable reception
         digitalWrite(_txEnablePin, LOW); // disable transmission (free the bus)
     }
@@ -326,8 +333,8 @@ void Controller::loop()
     CONFIG_T& config = Configuration.get();
     uint8_t pollInterval = config.Battery.JkBmsPollingInterval;
 
-    while (HwSerial.available()) {
-        rxData(HwSerial.read());
+    while (_upSerial->available()) {
+        rxData(_upSerial->read());
     }
 
     sendRequest(pollInterval);

diff --git a/src/SerialPortManager.cpp b/src/SerialPortManager.cpp
@@ -2,79 +2,46 @@
 #include "SerialPortManager.h"
 #include "MessageOutput.h"
 
-#define MAX_CONTROLLERS 3
-
 SerialPortManagerClass SerialPortManager;
 
 void SerialPortManagerClass::init()
 {
     if (ARDUINO_USB_CDC_ON_BOOT != 1) {
-        allocatePort(0, Owner::Console);
+        _ports[0] = "Serial Console";
+        MessageOutput.println("[SerialPortManager] HW UART port 0 now in use "
+                "by 'Serial Console'");
     }
 }
 
-bool SerialPortManagerClass::allocateBatteryPort(uint8_t port)
+std::optional<uint8_t> SerialPortManagerClass::allocatePort(std::string const& owner)
 {
-    return allocatePort(port, Owner::Battery);
-}
-
-bool SerialPortManagerClass::allocateMpptPort(uint8_t port)
-{
-    return allocatePort(port, Owner::MPPT);
-}
+    for (size_t i = 0; i < _ports.size(); ++i) {
+        if (_ports[i] != "") {
+            MessageOutput.printf("[SerialPortManager] HW UART %d already "
+                    "in use by '%s'\r\n", i, _ports[i].c_str());
+            continue;
+        }
 
-bool SerialPortManagerClass::allocatePort(uint8_t port, Owner owner)
-{
-    if (port >= MAX_CONTROLLERS) {
-        MessageOutput.printf("[SerialPortManager] Invalid serial port: %d\r\n", port);
-        return false;
-    }
+        _ports[i] = owner;
 
-    auto res = allocatedPorts.insert({port, owner});
+        MessageOutput.printf("[SerialPortManager] HW UART %d now in use "
+                "by '%s'\r\n", i, owner.c_str());
 
-    if (!res.second) {
-        MessageOutput.printf("[SerialPortManager] Cannot assign HW UART "
-                "port %d to %s: already in use by %s\r\n",
-                port, print(owner), print(res.first->second));
-        return false;
+        return i;
     }
 
-    MessageOutput.printf("[SerialPortManager] HW UART port %d now in use "
-            "by %s\r\n", port, print(owner));
-    return true;
-}
-
-void SerialPortManagerClass::invalidateBatteryPort()
-{
-    invalidate(Owner::Battery);
+    MessageOutput.printf("[SerialPortManager] Cannot assign another HW "
+            "UART port to '%s'\r\n", owner.c_str());
+    return std::nullopt;
 }
 
-void SerialPortManagerClass::invalidateMpptPorts()
+void SerialPortManagerClass::freePort(std::string const& owner)
 {
-    invalidate(Owner::MPPT);
-}
+    for (size_t i = 0; i < _ports.size(); ++i) {
+        if (_ports[i] != owner) { continue; }
 
-void SerialPortManagerClass::invalidate(Owner owner)
-{
-    for (auto it = allocatedPorts.begin(); it != allocatedPorts.end();) {
-        if (it->second == owner) {
-            MessageOutput.printf("[SerialPortManager] Removing port = %d, owner = %s \r\n", it->first, print(owner));
-            it = allocatedPorts.erase(it);
-        } else {
-            ++it;
-        }
-    }
-}
-
-const char* SerialPortManagerClass::print(Owner owner)
-{
-    switch (owner) {
-        case Owner::Console:
-            return "Serial Console";
-        case Owner::Battery:
-            return "Battery Interface";
-        case Owner::MPPT:
-            return "Victron MPPT";
+        MessageOutput.printf("[SerialPortManager] Freeing HW UART %d, owner "
+                "was '%s'\r\n", i, owner.c_str());
+        _ports[i] = "";
     }
-    return "unknown";
 }