huawei grid charger: avoid reboot on settings change

MCP215: make event-driven. in particular, this allows us to sleep for
some time in the loop task instead of only yield()ing, which effectively
keeps the core busy all the time because of this task. the newly added
ISR now wakes the loop task once the IRQ pin goes low.

use unique_ptr<> to manage the lifetime of the MCP2515 CAN lib object as
well as the respective SPI host object.

Author: schlimmchen

include/gridcharger/huawei/HardwareInterface.h

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 #pragma once
 
+#include <atomic>
 #include <array>
 #include <mutex>
 #include <map>
@@ -13,7 +14,9 @@ namespace GridCharger::Huawei {
 
 class HardwareInterface {
 public:
-    virtual ~HardwareInterface();
+    HardwareInterface() = default;
+
+    virtual ~HardwareInterface() = default;
 
     virtual bool init() = 0;
 
@@ -52,20 +55,22 @@ class HardwareInterface {
     using can_message_t = struct CAN_MESSAGE_T;
 
     bool startLoop();
+    void stopLoop();
+
+    TaskHandle_t getTaskHandle() const { return _taskHandle; }
 
 private:
     static void staticLoopHelper(void* context);
-    void loopHelper();
     void loop();
 
     virtual bool getMessage(can_message_t& msg) = 0;
 
-    virtual bool sendMessage(uint32_t valueId, std::array<uint8_t, 8> const& data) = 0;
+    virtual bool sendMessage(uint32_t canId, std::array<uint8_t, 8> const& data) = 0;
 
     mutable std::mutex _mutex;
 
     TaskHandle_t _taskHandle = nullptr;
-    bool _taskDone = false;
+    std::atomic<bool> _taskDone = false;
     bool _stopLoop = false;
 
     std::map<HardwareInterface::Property, HardwareInterface::property_t> _stats;

include/gridcharger/huawei/MCP2515.h

@@ -1,23 +1,32 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 #pragma once
 
+#include <memory>
 #include <SPI.h>
 #include <mcp_can.h>
+#include <SpiManager.h>
 #include <gridcharger/huawei/HardwareInterface.h>
 
 namespace GridCharger::Huawei {
 
 class MCP2515 : public HardwareInterface {
 public:
+    ~MCP2515();
+
     bool init() final;
 
     bool getMessage(HardwareInterface::can_message_t& msg) final;
 
-    bool sendMessage(uint32_t valueId, std::array<uint8_t, 8> const& data) final;
+    bool sendMessage(uint32_t canId, std::array<uint8_t, 8> const& data) final;
 
 private:
-    SPIClass *SPI;
-    MCP_CAN *_CAN;
+    // this is static because we cannot give back the bus once we claimed it.
+    // as we are going to use a shared host/bus in the future, we won't use a
+    // workaround for the limited time we use it like this.
+    static std::optional<uint8_t> _oSpiBus;
+
+    std::unique_ptr<SPIClass> _upSPI;
+    std::unique_ptr<MCP_CAN> _upCAN;
     uint8_t _huaweiIrq; // IRQ pin
 };
 

include/gridcharger/huawei/TWAI.h

@@ -7,11 +7,13 @@ namespace GridCharger::Huawei {
 
 class TWAI : public HardwareInterface {
 public:
+    ~TWAI();
+
     bool init() final;
 
     bool getMessage(HardwareInterface::can_message_t& msg) final;
 
-    bool sendMessage(uint32_t valueId, std::array<uint8_t, 8> const& data) final;
+    bool sendMessage(uint32_t canId, std::array<uint8_t, 8> const& data) final;
 };
 
 } // namespace GridCharger::Huawei

src/WebApi_Huawei.cpp

@@ -201,11 +201,5 @@ void WebApiHuaweiClass::onAdminPost(AsyncWebServerRequest* request)
 
     WebApi.sendJsonResponse(request, response, __FUNCTION__, __LINE__);
 
-    // TODO(schlimmchen): HuaweiCan has no real concept of the fact that the
-    // config might change. at least not regarding CAN parameters. until that
-    // changes, the ESP must restart for configuration changes to take effect.
-    yield();
-    delay(1000);
-    yield();
-    ESP.restart();
+    HuaweiCan.updateSettings();
 }

src/gridcharger/huawei/Controller.cpp

@@ -71,6 +71,7 @@ void Controller::updateSettings()
 
     if (!_upHardwareInterface->init()) {
         MessageOutput.println("[HuaweiCanClass::init] Error initializing hardware interface");
+        _upHardwareInterface.reset(nullptr);
         return;
     };
 

src/gridcharger/huawei/HardwareInterface.cpp

@@ -6,45 +6,48 @@
 
 namespace GridCharger::Huawei {
 
-HardwareInterface::~HardwareInterface()
+void HardwareInterface::staticLoopHelper(void* context)
 {
-    std::unique_lock<std::mutex> lock(_mutex);
-    _taskDone = false;
-    _stopLoop = true;
-    lock.unlock();
-
-    if (_taskHandle != nullptr) {
-        while (!_taskDone) { delay(10); }
-        _taskHandle = nullptr;
+    auto pInstance = static_cast<HardwareInterface*>(context);
+    static auto constexpr resetNotificationValue = pdTRUE;
+    static auto constexpr notificationTimeout = pdMS_TO_TICKS(500);
+
+    while (true) {
+        ulTaskNotifyTake(resetNotificationValue, notificationTimeout);
+        {
+            std::unique_lock<std::mutex> lock(pInstance->_mutex);
+            if (pInstance->_stopLoop) { break; }
+            pInstance->loop();
+        }
     }
+
+    pInstance->_taskDone = true;
+
+    vTaskDelete(nullptr);
 }
 
-void HardwareInterface::staticLoopHelper(void* context)
+bool HardwareInterface::startLoop()
 {
-    auto pInstance = static_cast<HardwareInterface*>(context);
-    pInstance->loopHelper();
-    vTaskDelete(nullptr);
+    uint32_t constexpr stackSize = 4096;
+    return pdPASS == xTaskCreate(HardwareInterface::staticLoopHelper,
+            "HuaweiHwIfc", stackSize, this, 1/*prio*/, &_taskHandle);
 }
 
-void HardwareInterface::loopHelper()
+void HardwareInterface::stopLoop()
 {
-    std::unique_lock<std::mutex> lock(_mutex);
+    if (_taskHandle == nullptr) { return; }
+
+    _taskDone = false;
 
-    while (!_stopLoop) {
-        loop();
-        lock.unlock();
-        yield();
-        lock.lock();
+    {
+        std::unique_lock<std::mutex> lock(_mutex);
+        _stopLoop = true;
     }
 
-    _taskDone = true;
-}
+    xTaskNotifyGive(_taskHandle);
 
-bool HardwareInterface::startLoop()
-{
-    uint32_t constexpr stackSize = 2048;
-    return pdPASS == xTaskCreate(HardwareInterface::staticLoopHelper,
-            "HuaweiHwIfc", stackSize, this, 1/*prio*/, &_taskHandle);
+    while (!_taskDone) { delay(10); }
+    _taskHandle = nullptr;
 }
 
 void HardwareInterface::loop()
@@ -80,7 +83,6 @@ void HardwareInterface::loop()
             static_cast<uint8_t>(val & 0xFF)
         };
 
-        // Send extended message
         if (!sendMessage(0x108180FE, data)) {
             MessageOutput.print("[Huawei::HwIfc] Failed to set parameter\r\n");
             _sendQueue.push({setting, val});
@@ -101,6 +103,8 @@ void HardwareInterface::setParameter(HardwareInterface::Setting setting, float v
 {
     std::lock_guard<std::mutex> lock(_mutex);
 
+    if (_taskHandle == nullptr) { return; }
+
     switch (setting) {
         case Setting::OfflineVoltage:
         case Setting::OnlineVoltage:
@@ -113,6 +117,8 @@ void HardwareInterface::setParameter(HardwareInterface::Setting setting, float v
     }
 
     _sendQueue.push({setting, static_cast<uint16_t>(val)});
+
+    xTaskNotifyGive(_taskHandle);
 }
 
 HardwareInterface::property_t HardwareInterface::getParameter(HardwareInterface::Property property) const

src/gridcharger/huawei/MCP2515.cpp

@@ -4,12 +4,34 @@
  */
 #include <gridcharger/huawei/MCP2515.h>
 #include "MessageOutput.h"
-#include "SpiManager.h"
 #include "PinMapping.h"
 #include "Configuration.h"
 
 namespace GridCharger::Huawei {
 
+TaskHandle_t sIsrTaskHandle = nullptr;
+
+void mcp2515Isr()
+{
+    if (sIsrTaskHandle == nullptr) { return; }
+    BaseType_t xHigherPriorityTaskWoken = pdFALSE;
+    vTaskNotifyGiveFromISR(sIsrTaskHandle, &xHigherPriorityTaskWoken);
+    // we assume we can wait until the lower-priority task is scheduled
+    // anyways, so we ignore xHigherPriorityTaskWoken == pdTRUE.
+}
+
+std::optional<uint8_t> MCP2515::_oSpiBus = std::nullopt;
+
+MCP2515::~MCP2515()
+{
+    detachInterrupt(digitalPinToInterrupt(_huaweiIrq));
+    sIsrTaskHandle = nullptr;
+    stopLoop();
+    _upCAN.reset(nullptr);
+    if (_upSPI) { _upSPI->end(); } // nullptr if init failed or never called
+    _upSPI.reset(nullptr);
+}
+
 bool MCP2515::init()
 {
     const PinMapping_t& pin = PinMapping.get();
@@ -22,51 +44,74 @@ bool MCP2515::init()
         return false;
     }
 
-    auto spi_bus = SpiManagerInst.claim_bus_arduino();
-    if (!spi_bus) { return false; }
+    if (!_oSpiBus) {
+        _oSpiBus = SpiManagerInst.claim_bus_arduino();
+    }
+
+    if (!_oSpiBus) {
+        MessageOutput.printf("[Huawei::MCP2515] no SPI host available\r\n");
+        return false;
+    }
 
-    SPI = new SPIClass(*spi_bus);
+    _upSPI = std::make_unique<SPIClass>(*_oSpiBus);
 
-    SPI->begin(pin.huawei_clk, pin.huawei_miso, pin.huawei_mosi, pin.huawei_cs);
+    _upSPI->begin(pin.huawei_clk, pin.huawei_miso, pin.huawei_mosi, pin.huawei_cs);
     pinMode(pin.huawei_cs, OUTPUT);
     digitalWrite(pin.huawei_cs, HIGH);
 
-    pinMode(pin.huawei_irq, INPUT_PULLUP);
-    _huaweiIrq = pin.huawei_irq;
-
     auto mcp_frequency = MCP_8MHZ;
     auto frequency = Configuration.get().Huawei.CAN_Controller_Frequency;
     if (16000000UL == frequency) { mcp_frequency = MCP_16MHZ; }
     else if (8000000UL != frequency) {
         MessageOutput.printf("[Huawei::MCP2515] unknown frequency %d Hz, using 8 MHz\r\n", mcp_frequency);
     }
 
-    _CAN = new MCP_CAN(SPI, pin.huawei_cs);
-    if (_CAN->begin(MCP_STDEXT, CAN_125KBPS, mcp_frequency) != CAN_OK) {
+    _upCAN = std::make_unique<MCP_CAN>(_upSPI.get(), pin.huawei_cs);
+    if (_upCAN->begin(MCP_STDEXT, CAN_125KBPS, mcp_frequency) != CAN_OK) {
         MessageOutput.printf("[Huawei::MCP2515] mcp_can begin() failed\r\n");
         return false;
     }
 
     const uint32_t myMask = 0xFFFFFFFF;         // Look at all incoming bits and...
     const uint32_t myFilter = 0x1081407F;       // filter for this message only
-    _CAN->init_Mask(0, 1, myMask);
-    _CAN->init_Filt(0, 1, myFilter);
-    _CAN->init_Mask(1, 1, myMask);
+    _upCAN->init_Mask(0, 1, myMask);
+    _upCAN->init_Filt(0, 1, myFilter);
+    _upCAN->init_Mask(1, 1, myMask);
 
     // Change to normal mode to allow messages to be transmitted
-    _CAN->setMode(MCP_NORMAL);
+    _upCAN->setMode(MCP_NORMAL);
+
+    if (!startLoop()) {
+        MessageOutput.printf("[Huawei::MCP2515] failed to start loop task\r\n");
+        return false;
+    }
+
+    if (sIsrTaskHandle != nullptr) {
+        // make the ISR aware of multiple instances if multiple instances of
+        // this driver should be able to co-exist. only one is supported now.
+        MessageOutput.printf("[Huawei::MCP2515] ISR task handle already in use\r\n");
+        stopLoop();
+        return false;
+    }
+
+    sIsrTaskHandle = getTaskHandle();
+    _huaweiIrq = pin.huawei_irq;
+    pinMode(_huaweiIrq, INPUT_PULLUP);
+    attachInterrupt(digitalPinToInterrupt(_huaweiIrq), mcp2515Isr, FALLING);
 
-    return startLoop();
+    return true;
 }
 
 bool MCP2515::getMessage(HardwareInterface::can_message_t& msg)
 {
+    if (!_upCAN) { return false; }
+
     INT32U rxId;
     unsigned char len = 0;
     unsigned char rxBuf[8];
 
     while (!digitalRead(_huaweiIrq)) {
-        _CAN->readMsgBuf(&rxId, &len, rxBuf); // Read data: len = data length, buf = data byte(s)
+        _upCAN->readMsgBuf(&rxId, &len, rxBuf); // Read data: len = data length, buf = data byte(s)
 
         // Determine if ID is standard (11 bits) or extended (29 bits)
         if ((rxId & 0x80000000) != 0x80000000) { continue; }
@@ -83,12 +128,14 @@ bool MCP2515::getMessage(HardwareInterface::can_message_t& msg)
     return false;
 }
 
-bool MCP2515::sendMessage(uint32_t valueId, std::array<uint8_t, 8> const& data)
+bool MCP2515::sendMessage(uint32_t canId, std::array<uint8_t, 8> const& data)
 {
+    if (!_upCAN) { return false; }
+
     // MCP2515 CAN library requires a non-const pointer to the data
     uint8_t rwData[8];
     memcpy(rwData, data.data(), data.size());
-    return _CAN->sendMsgBuf(valueId, 1, 8, rwData) == CAN_OK;
+    return _upCAN->sendMsgBuf(canId, 1, 8, rwData) == CAN_OK;
 }
 
 } // namespace GridCharger::Huawei