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.

include/Battery.h

@@ -14,7 +14,6 @@ public:
     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 {

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,

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);
+    std::optional<uint8_t> allocatePort(std::string const& owner);
-    void invalidateBatteryPort();
+    void freePort(std::string const& owner);
-    void invalidateMpptPorts();
 
 private:
-    enum class Owner {
+    // the amount of hardare UARTs available on supported ESP32 chips
-        Console,
+    static size_t constexpr _num_controllers = 3;
-        Battery,
+    std::array<std::string, _num_controllers> _ports = { "" };
-        MPPT
-    };
-
-    std::map<uint8_t, Owner> allocatedPorts;
-
-    bool allocatePort(uint8_t port, Owner owner);
-    void invalidate(Owner owner);
-
-    static const char* print(Owner owner);
 };
 
 extern SerialPortManagerClass SerialPortManager;

include/VictronMppt.h

@@ -55,8 +55,9 @@ private:
     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;

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>();

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
+    if (!_upProvider->init(verboseLogging)) { _upProvider = nullptr; }
-    // 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;
-    }
 }
 
 void BatteryClass::loop()

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
+#ifdef JKBMS_DUMMY_SERIAL
-    HwSerial.begin(115200, SERIAL_8N1, pin.battery_rx, pin.battery_tx);
+    _upSerial = std::make_unique<DummySerial>();
-    HwSerial.flush();
+#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()) {
+    while (_upSerial->available()) {
-        rxData(HwSerial.read());
+        rxData(_upSerial->read());
     }
 
     sendRequest(pollInterval);

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);
+    for (size_t i = 0; i < _ports.size(); ++i) {
-}
+        if (_ports[i] != "") {
-
+            MessageOutput.printf("[SerialPortManager] HW UART %d already "
-bool SerialPortManagerClass::allocateMpptPort(uint8_t port)
+                    "in use by '%s'\r\n", i, _ports[i].c_str());
-{
+            continue;
-    return allocatePort(port, Owner::MPPT);
-}
-
-bool SerialPortManagerClass::allocatePort(uint8_t port, Owner owner)
-{
-    if (port >= MAX_CONTROLLERS) {
-        MessageOutput.printf("[SerialPortManager] Invalid serial port: %d\r\n", port);
-        return false;
-    }
-
-    auto res = allocatedPorts.insert({port, owner});
-
-    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;
-    }
-
-    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);
-}
-
-void SerialPortManagerClass::invalidateMpptPorts()
-{
-    invalidate(Owner::MPPT);
-}
-
-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;
         }
+
+        _ports[i] = owner;
+
+        MessageOutput.printf("[SerialPortManager] HW UART %d now in use "
+                "by '%s'\r\n", i, owner.c_str());
+
+        return i;
     }
+
+    MessageOutput.printf("[SerialPortManager] Cannot assign another HW "
+            "UART port to '%s'\r\n", owner.c_str());
+    return std::nullopt;
 }
 
-const char* SerialPortManagerClass::print(Owner owner)
+void SerialPortManagerClass::freePort(std::string const& owner)
 {
-    switch (owner) {
+    for (size_t i = 0; i < _ports.size(); ++i) {
-        case Owner::Console:
+        if (_ports[i] != owner) { continue; }
-            return "Serial Console";
+
-        case Owner::Battery:
+        MessageOutput.printf("[SerialPortManager] Freeing HW UART %d, owner "
-            return "Battery Interface";
+                "was '%s'\r\n", i, owner.c_str());
-        case Owner::MPPT:
+        _ports[i] = "";
-            return "Victron MPPT";
     }
-    return "unknown";
 }

src/VictronMppt.cpp

@@ -22,42 +22,46 @@ void VictronMpptClass::updateSettings()
     std::lock_guard<std::mutex> lock(_mutex);
 
     _controllers.clear();
-    SerialPortManager.invalidateMpptPorts();
+    for (auto const& o: _serialPortOwners) {
+        SerialPortManager.freePort(o.c_str());
+    }
+    _serialPortOwners.clear();
 
     CONFIG_T& config = Configuration.get();
     if (!config.Vedirect.Enabled) { return; }
 
     const PinMapping_t& pin = PinMapping.get();
 
-    // HW UART 1 has always been the designated UART to connect a Victron MPPT
+    initController(pin.victron_rx, pin.victron_tx,
-    if (!initController(pin.victron_rx, pin.victron_tx,
+            config.Vedirect.VerboseLogging, 1);
-            config.Vedirect.VerboseLogging, 1, 1)) { return; }
 
-    // HW UART 2 conflicts with the SDM power meter and the battery interface
+    initController(pin.victron_rx2, pin.victron_tx2,
-    if (!initController(pin.victron_rx2, pin.victron_tx2,
+            config.Vedirect.VerboseLogging, 2);
-            config.Vedirect.VerboseLogging, 2, 2)) { return; }
 
-    // HW UART 0 is only available on ESP32-S3 with logging over USB CDC, and
-    // furthermore still conflicts with the battery interface in that case
     initController(pin.victron_rx3, pin.victron_tx3,
-            config.Vedirect.VerboseLogging, 3, 0);
+            config.Vedirect.VerboseLogging, 3);
 }
 
 bool VictronMpptClass::initController(int8_t rx, int8_t tx, bool logging,
-        uint8_t instance, uint8_t hwSerialPort)
+        uint8_t instance)
 {
-    MessageOutput.printf("[VictronMppt Instance %d] rx = %d, tx = %d, "
+    MessageOutput.printf("[VictronMppt Instance %d] rx = %d, tx = %d\r\n",
-            "hwSerialPort = %d\r\n", instance, rx, tx, hwSerialPort);
+            instance, rx, tx);
 
     if (rx < 0) {
         MessageOutput.printf("[VictronMppt Instance %d] invalid pin config\r\n", instance);
         return false;
     }
 
-    if (!SerialPortManager.allocateMpptPort(hwSerialPort)) { return false; }
+    String owner("Victron MPPT ");
+    owner += String(instance);
+    auto oHwSerialPort = SerialPortManager.allocatePort(owner.c_str());
+    if (!oHwSerialPort) { return false; }
+
+    _serialPortOwners.push_back(owner);
 
     auto upController = std::make_unique<VeDirectMpptController>();
-    upController->init(rx, tx, &MessageOutput, logging, hwSerialPort);
+    upController->init(rx, tx, &MessageOutput, logging, *oHwSerialPort);
     _controllers.push_back(std::move(upController));
     return true;
 }

src/VictronSmartShunt.cpp

@@ -3,7 +3,12 @@
 #include "Configuration.h"
 #include "PinMapping.h"
 #include "MessageOutput.h"
+#include "SerialPortManager.h"
 
+void VictronSmartShunt::deinit()
+{
+    SerialPortManager.freePort(_serialPortOwner);
+}
 
 bool VictronSmartShunt::init(bool verboseLogging)
 {
@@ -21,7 +26,10 @@ bool VictronSmartShunt::init(bool verboseLogging)
     auto tx = static_cast<gpio_num_t>(pin.battery_tx);
     auto rx = static_cast<gpio_num_t>(pin.battery_rx);
 
-    VeDirectShunt.init(rx, tx, &MessageOutput, verboseLogging, _hwSerialPort);
+    auto oHwSerialPort = SerialPortManager.allocatePort(_serialPortOwner);
+    if (!oHwSerialPort) { return false; }
+
+    VeDirectShunt.init(rx, tx, &MessageOutput, verboseLogging, *oHwSerialPort);
     return true;
 }