From be41e6b906cf02ab743d5f89626bcbf011f921a8 Mon Sep 17 00:00:00 2001 From: Bernhard Kirchen Date: Sat, 1 Jun 2024 15:45:36 +0200 Subject: [PATCH] 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 | 1 - include/JkBmsController.h | 13 ++++-- include/SerialPortManager.h | 26 ++++------- include/VictronMppt.h | 3 +- include/VictronSmartShunt.h | 6 +-- src/Battery.cpp | 15 +------ src/JkBmsController.cpp | 35 +++++++++------ src/SerialPortManager.cpp | 87 ++++++++++++------------------------- src/VictronMppt.cpp | 34 ++++++++------- src/VictronSmartShunt.cpp | 10 ++++- 10 files changed, 101 insertions(+), 129 deletions(-) diff --git a/include/Battery.h b/include/Battery.h index c8cf42f1..5130381c 100644 --- a/include/Battery.h +++ b/include/Battery.h @@ -14,7 +14,6 @@ public: virtual void deinit() = 0; virtual void loop() = 0; virtual std::shared_ptr 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 index 60e31652..7156d760 100644 --- 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 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 _upSerial; +#else + std::unique_ptr _upSerial; +#endif + enum class Status : unsigned { Initializing, Timeout, diff --git a/include/SerialPortManager.h b/include/SerialPortManager.h index b1a58447..372b9c50 100644 --- a/include/SerialPortManager.h +++ b/include/SerialPortManager.h @@ -1,29 +1,21 @@ // SPDX-License-Identifier: GPL-2.0-or-later #pragma once -#include +#include +#include +#include class SerialPortManagerClass { public: void init(); - bool allocateMpptPort(uint8_t port); - bool allocateBatteryPort(uint8_t port); - void invalidateBatteryPort(); - void invalidateMpptPorts(); + + std::optional allocatePort(std::string const& owner); + void freePort(std::string const& owner); private: - enum class Owner { - Console, - Battery, - MPPT - }; - - std::map allocatedPorts; - - bool allocatePort(uint8_t port, Owner owner); - void invalidate(Owner owner); - - static const char* print(Owner owner); + // the amount of hardare UARTs available on supported ESP32 chips + static size_t constexpr _num_controllers = 3; + std::array _ports = { "" }; }; extern SerialPortManagerClass SerialPortManager; diff --git a/include/VictronMppt.h b/include/VictronMppt.h index 11254ed6..963a46cd 100644 --- a/include/VictronMppt.h +++ b/include/VictronMppt.h @@ -55,8 +55,9 @@ private: using controller_t = std::unique_ptr; std::vector _controllers; + std::vector _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 index cc123e9f..10bf1c52 100644 --- 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 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 _stats = std::make_shared(); diff --git a/src/Battery.cpp b/src/Battery.cpp index 8d24c466..794afa51 100644 --- 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 index 9fdb7922..c39664e0 100644 --- 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 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(); +#else + auto oHwSerialPort = SerialPortManager.allocatePort(_serialPortOwner); + if (!oHwSerialPort) { return false; } + + _upSerial = std::make_unique(*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 index d489cbd5..fbcc7f03 100644 --- 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 SerialPortManagerClass::allocatePort(std::string const& owner) { - return allocatePort(port, Owner::Battery); -} - -bool SerialPortManagerClass::allocateMpptPort(uint8_t port) -{ - 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; + 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; } + + _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) { - case Owner::Console: - return "Serial Console"; - case Owner::Battery: - return "Battery Interface"; - case Owner::MPPT: - return "Victron MPPT"; + for (size_t i = 0; i < _ports.size(); ++i) { + if (_ports[i] != owner) { continue; } + + MessageOutput.printf("[SerialPortManager] Freeing HW UART %d, owner " + "was '%s'\r\n", i, owner.c_str()); + _ports[i] = ""; } - return "unknown"; } diff --git a/src/VictronMppt.cpp b/src/VictronMppt.cpp index d1366b86..bc0122d4 100644 --- a/src/VictronMppt.cpp +++ b/src/VictronMppt.cpp @@ -22,42 +22,46 @@ void VictronMpptClass::updateSettings() std::lock_guard 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 - if (!initController(pin.victron_rx, pin.victron_tx, - config.Vedirect.VerboseLogging, 1, 1)) { return; } + initController(pin.victron_rx, pin.victron_tx, + config.Vedirect.VerboseLogging, 1); - // HW UART 2 conflicts with the SDM power meter and the battery interface - if (!initController(pin.victron_rx2, pin.victron_tx2, - config.Vedirect.VerboseLogging, 2, 2)) { return; } + initController(pin.victron_rx2, pin.victron_tx2, + config.Vedirect.VerboseLogging, 2); - // 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, " - "hwSerialPort = %d\r\n", instance, rx, tx, hwSerialPort); + MessageOutput.printf("[VictronMppt Instance %d] rx = %d, tx = %d\r\n", + 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(); - upController->init(rx, tx, &MessageOutput, logging, hwSerialPort); + upController->init(rx, tx, &MessageOutput, logging, *oHwSerialPort); _controllers.push_back(std::move(upController)); return true; } diff --git a/src/VictronSmartShunt.cpp b/src/VictronSmartShunt.cpp index 067c7fd4..c64f9a83 100644 --- a/src/VictronSmartShunt.cpp +++ b/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(pin.battery_tx); auto rx = static_cast(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; }