Fix vedirect polling (#291)

* VE.Direct: remove polling interval the polling interval was meant to limit the amount of MQTT updates. however, that is already controlled by the global MQTT publish interval. the removed interval was instead used to limit polling of the VE.Direct UART for incoming data. the Victron device sends data unsolicited. the VeDirectFrameHandler does not implement any polling mechanism. no data is ever sent to the Victron device. what the removed polling interval did was cause a buffer overrun of the HardwareSerial class, since the incoming data was not processed in time. so every five seconds, we read a whole valid VE.Direct frame, plus some old data, which was not a whole frame, leading to VE.Direct error messages to pop up. with the polling interval removed, no framing errors are reported, and instead we gain new data from the charge controller approximately ever two seconds -- for free. * VE.Direct: change texts to correct VE.Direct capital letters * VE.Direct: improve "UpdatesOnly" switch labels especially since the publish interval setting is gone, the label makes it hard to comprehend what the switch does. update the texts to better explain what the switch is used for. use the same text on the VE.Direct info view. * VE.Direct: use StatusBadge on info view there were custom badges to indicate the VE.Direct settings. replace those by the common StatusBadge to make then look the same as the other badged on the info views.
2023-07-04 12:04:38 +02:00 · 2023-07-04 12:04:38 +02:00 · e457ab73f9
commit e457ab73f9
parent 006f63ed02
13 changed files with 48 additions and 104 deletions
--- a/include/Configuration.h
+++ b/include/Configuration.h
@ -122,7 +122,6 @@ struct CONFIG_T {

    bool Vedirect_Enabled;
    bool Vedirect_UpdatesOnly;
-    uint32_t Vedirect_PollInterval;

    char Mqtt_Hostname[MQTT_MAX_HOSTNAME_STRLEN + 1];

--- a/lib/VeDirectFrameHandler/VeDirectFrameHandler.cpp
+++ b/lib/VeDirectFrameHandler/VeDirectFrameHandler.cpp
@ -64,8 +64,7 @@ VeDirectFrameHandler::VeDirectFrameHandler() :
 	_name(""),
 	_value(""),
 	_tmpFrame(),
-	_pollInterval(5),
-	_lastPoll(0)
+	_lastUpdate(0)
 {
 }

@ -75,17 +74,8 @@ void VeDirectFrameHandler::init(int8_t rx, int8_t tx)
    VedirectSerial.flush();
 }

-void VeDirectFrameHandler::setPollInterval(unsigned long interval)
-{
-    _pollInterval = interval;
-}
-
 void VeDirectFrameHandler::loop()
 {
-	if ((millis() - getLastUpdate()) < _pollInterval * 1000) {
-		return;
-	}
-	
 	while ( VedirectSerial.available()) {
 		rxData(VedirectSerial.read());
 	}
@ -273,7 +263,7 @@ void VeDirectFrameHandler::frameEndEvent(bool valid) {
 		}

 		veFrame = _tmpFrame;
-		setLastUpdate();
+		_lastUpdate = millis();
 	}
 	_tmpFrame = {};
 }
@ -316,8 +306,8 @@ int VeDirectFrameHandler::hexRxEvent(uint8_t inbyte) {
 }

 bool VeDirectFrameHandler::isDataValid() {
- 	if ((millis() - getLastUpdate()) / 1000 > _pollInterval * 5) { 
-        return false;
+	if (_lastUpdate == 0) {
+		return false;
 	}
 	if (strlen(veFrame.SER) == 0) {
 		return false;
@ -327,16 +317,7 @@ bool VeDirectFrameHandler::isDataValid() {

 unsigned long VeDirectFrameHandler::getLastUpdate()
 {
-    return _lastPoll;
-}
-
-/*
- * setLastUpdate
- * This function is called every time a new ve.direct frame was read.
- */
-void VeDirectFrameHandler::setLastUpdate()
-{
-    _lastPoll = millis();
+    return _lastUpdate;
 }

 /*
--- a/lib/VeDirectFrameHandler/VeDirectFrameHandler.h
+++ b/lib/VeDirectFrameHandler/VeDirectFrameHandler.h
@ -88,7 +88,6 @@ public:

    VeDirectFrameHandler();
    void init(int8_t rx, int8_t tx);             // initialize HardewareSerial
-    void setPollInterval(unsigned long interval); // set poll intervall in seconds
    void loop();                                 // main loop to read ve.direct data
    unsigned long getLastUpdate();               // timestamp of last successful frame read
    bool isDataValid();                          // return true if data valid and not outdated
@ -118,8 +117,7 @@ private:
    char _value[VE_MAX_VALUE_LEN];             // buffer for the field value
    veStruct _tmpFrame{};                        // private struct for received name and value pairs
    MovingAverage<double, 5> _efficiency;
-    unsigned long _pollInterval;
-    unsigned long _lastPoll;
+    unsigned long _lastUpdate;
 };

 extern VeDirectFrameHandler VeDirect;
--- a/src/Configuration.cpp
+++ b/src/Configuration.cpp
@ -121,7 +121,6 @@ bool ConfigurationClass::write()
    JsonObject vedirect = doc.createNestedObject("vedirect");
    vedirect["enabled"] = config.Vedirect_Enabled;
    vedirect["updates_only"] = config.Vedirect_UpdatesOnly;
-    vedirect["poll_interval"] = config.Vedirect_PollInterval;

    JsonObject powermeter = doc.createNestedObject("powermeter");
    powermeter["enabled"] = config.PowerMeter_Enabled;
@ -330,7 +329,6 @@ bool ConfigurationClass::read()
    JsonObject vedirect = doc["vedirect"];
    config.Vedirect_Enabled = vedirect["enabled"] | VEDIRECT_ENABLED;
    config.Vedirect_UpdatesOnly = vedirect["updates_only"] | VEDIRECT_UPDATESONLY;
-    config.Vedirect_PollInterval = vedirect["poll_interval"] | VEDIRECT_POLL_INTERVAL;

    JsonObject powermeter = doc["powermeter"];
    config.PowerMeter_Enabled = powermeter["enabled"] | POWERMETER_ENABLED;
--- a/src/WebApi_vedirect.cpp
+++ b/src/WebApi_vedirect.cpp
@ -37,7 +37,6 @@ void WebApiVedirectClass::onVedirectStatus(AsyncWebServerRequest* request)
    const CONFIG_T& config = Configuration.get();

    root[F("vedirect_enabled")] = config.Vedirect_Enabled;
-    root[F("vedirect_pollinterval")] = config.Vedirect_PollInterval;
    root[F("vedirect_updatesonly")] = config.Vedirect_UpdatesOnly;

    response->setLength();
@ -55,7 +54,6 @@ void WebApiVedirectClass::onVedirectAdminGet(AsyncWebServerRequest* request)
    const CONFIG_T& config = Configuration.get();

    root[F("vedirect_enabled")] = config.Vedirect_Enabled;
-    root[F("vedirect_pollinterval")] = config.Vedirect_PollInterval;
    root[F("vedirect_updatesonly")] = config.Vedirect_UpdatesOnly;

    response->setLength();
@ -101,7 +99,7 @@ void WebApiVedirectClass::onVedirectAdminPost(AsyncWebServerRequest* request)
        return;
    }

-    if (!(root.containsKey("vedirect_enabled") && root.containsKey("vedirect_pollinterval") && root.containsKey("vedirect_updatesonly")) ) {
+    if (!(root.containsKey("vedirect_enabled") && root.containsKey("vedirect_updatesonly")) ) {
        retMsg[F("message")] = F("Values are missing!");
        retMsg[F("code")] = WebApiError::GenericValueMissing;
        response->setLength();
@ -109,21 +107,9 @@ void WebApiVedirectClass::onVedirectAdminPost(AsyncWebServerRequest* request)
        return;
    }

-     if (root[F("vedirect_pollinterval")].as<uint32_t>() == 0) {
-        retMsg[F("message")] = F("Poll interval must be a number between 5 and 65535!");
-        retMsg[F("code")] = WebApiError::MqttPublishInterval;
-        retMsg[F("param")][F("min")] = 5;
-        retMsg[F("param")][F("max")] = 65535;
-         
-        response->setLength();
-        request->send(response);
-        return;
-    }
-
    CONFIG_T& config = Configuration.get();
    config.Vedirect_Enabled = root[F("vedirect_enabled")].as<bool>();
    config.Vedirect_UpdatesOnly = root[F("vedirect_updatesonly")].as<bool>();
-    config.Vedirect_PollInterval = root[F("vedirect_pollinterval")].as<uint32_t>();
    Configuration.write();

    retMsg[F("type")] = F("success");
@ -132,6 +118,4 @@ void WebApiVedirectClass::onVedirectAdminPost(AsyncWebServerRequest* request)

    response->setLength();
    request->send(response);
-
-    VeDirect.setPollInterval(config.Vedirect_PollInterval);
-}
+}
--- a/src/main.cpp
+++ b/src/main.cpp
@ -165,7 +165,6 @@ void setup()
    if (PinMapping.isValidVictronConfig()) {
        MessageOutput.printf("ve.direct rx = %d, tx = %d\r\n", pin.victron_rx, pin.victron_tx);
        VeDirect.init(pin.victron_rx, pin.victron_tx);
-        VeDirect.setPollInterval(config.Vedirect_PollInterval);
        MessageOutput.println(F("done"));
    } else {
        MessageOutput.println(F("Invalid pin config"));
--- a/webapp/src/locales/de.json
+++ b/webapp/src/locales/de.json
@ -9,7 +9,7 @@
        "SecuritySettings": "Sicherheit",
        "DTUSettings": "DTU",
        "DeviceManager": "Hardware",
-        "VedirectSettings": "Ve.direct",
+        "VedirectSettings": "VE.Direct",
        "PowerMeterSettings": "Power Meter",
        "BatterySettings": "Batterie",
        "AcChargerSettings": "AC Ladegerät",
@ -22,7 +22,7 @@
        "NTP": "NTP",
        "MQTT": "MQTT",
        "Console": "Konsole",
-        "Vedirect": "Ve.direct",
+        "Vedirect": "VE.Direct",
        "About": "Über",
        "Logout": "Abmelden",
        "Login": "Anmelden"
@ -318,12 +318,12 @@
        "Disconnected": "getrennt"
    },
    "vedirectinfo": {
-        "VedirectInformation" : "Ve.direct Info",
+        "VedirectInformation" : "VE.Direct Info",
        "ConfigurationSummary": "@:ntpinfo.ConfigurationSummary",
        "Status": "@:ntpinfo.Status",
        "Enabled": "@:mqttinfo.Enabled",
        "Disabled": "@:mqttinfo.Disabled",
-        "UpdatesOnly": "Nur Änderungen senden",
+        "UpdatesOnly": "@:vedirectadmin.UpdatesOnly",
        "UpdatesEnabled": "@:mqttinfo.Enabled",
        "UpdatesDisabled": "@:mqttinfo.Disabled"
    },
@ -485,13 +485,12 @@
        "Save": "@:dtuadmin.Save"
    },
    "vedirectadmin": {
-        "VedirectSettings": "Ve.direct Einstellungen",
-        "VedirectConfiguration": "Ve.direct Konfiguration",
-        "EnableVedirect": "Aktiviere Ve.direct",
-        "VedirectParameter": "Ve.direct Parameter",
-        "PublishInterval": "Veröffentlichungsintervall:",
+        "VedirectSettings": "VE.Direct Einstellungen",
+        "VedirectConfiguration": "VE.Direct Konfiguration",
+        "EnableVedirect": "Aktiviere VE.Direct",
+        "VedirectParameter": "VE.Direct Parameter",
        "Seconds": "Sekunden",
-        "UpdatesOnly": "Nur Änderungen senden:",
+        "UpdatesOnly": "Werte nur bei Änderung an MQTT broker senden",
        "Save": "@:dtuadmin.Save"
    },
    "powermeteradmin":{
--- a/webapp/src/locales/en.json
+++ b/webapp/src/locales/en.json
@ -9,7 +9,7 @@
        "SecuritySettings": "Security",
        "DTUSettings": "DTU",
        "DeviceManager": "Device-Manager",
-        "VedirectSettings": "Ve.direct",
+        "VedirectSettings": "VE.Direct",
        "PowerMeterSettings": "Power Meter",
        "BatterySettings": "Battery",
        "AcChargerSettings": "AC Charger",
@ -22,7 +22,7 @@
        "NTP": "NTP",
        "MQTT": "MQTT",
        "Console": "Console",
-        "Vedirect": "Ve.direct",
+        "Vedirect": "VE.Direct",
        "About": "About",
        "Logout": "Logout",
        "Login": "Login"
@ -318,12 +318,12 @@
        "Disconnected": "disconnected"
    },
    "vedirectinfo": {
-        "VedirectInformation" : "Ve.direct Info",
+        "VedirectInformation" : "VE.Direct Info",
        "ConfigurationSummary": "@:ntpinfo.ConfigurationSummary",
        "Status": "@:ntpinfo.Status",
        "Enabled": "@:mqttinfo.Enabled",
        "Disabled": "@:mqttinfo.Disabled",
-        "UpdatesOnly": "Send updates only",
+        "UpdatesOnly": "@:vedirectadmin.UpdatesOnly",
        "UpdatesEnabled": "@:mqttinfo.Enabled",
        "UpdatesDisabled": "@:mqttinfo.Disabled"
    },
@ -485,13 +485,12 @@
        "Save": "@:dtuadmin.Save"
    },
    "vedirectadmin": {
-        "VedirectSettings": "Ve.direct Settings",
-        "VedirectConfiguration": "Ve.direct Configuration",
-        "EnableVedirect": "Enable Ve.direct",
-        "VedirectParameter": "Ve.direct Parameter",
-        "PublishInterval": "Publish Interval:",
+        "VedirectSettings": "VE.Direct Settings",
+        "VedirectConfiguration": "VE.Direct Configuration",
+        "EnableVedirect": "Enable VE.Direct",
+        "VedirectParameter": "VE.Direct Parameter",
        "Seconds": "seconds",
-        "UpdatesOnly": "Send only updates:",
+        "UpdatesOnly": "Publish values to MQTT only when they change",
        "Save": "@:dtuadmin.Save"
    },
    "powermeteradmin":{
--- a/webapp/src/locales/fr.json
+++ b/webapp/src/locales/fr.json
@ -9,7 +9,7 @@
        "SecuritySettings": "Sécurité",
        "DTUSettings": "DTU",
        "DeviceManager": "Périphériques",
-        "VedirectSettings": "Ve.direct",
+        "VedirectSettings": "VE.Direct",
        "PowerMeterSettings": "Power Meter",
        "BatterySettings": "Battery",
        "AcChargerSettings": "AC Charger",
@ -22,7 +22,7 @@
        "NTP": "NTP",
        "MQTT": "MQTT",
        "Console": "Console",
-        "Vedirect": "Ve.direct",
+        "Vedirect": "VE.Direct",
        "About": "A propos",
        "Logout": "Déconnexion",
        "Login": "Connexion"
@ -318,12 +318,12 @@
        "Disconnected": "déconnecté"
    },
    "vedirectinfo": {
-        "VedirectInformation" : "Ve.direct Info",
+        "VedirectInformation" : "VE.Direct Info",
        "ConfigurationSummary": "@:ntpinfo.ConfigurationSummary",
        "Status": "@:ntpinfo.Status",
        "Enabled": "@:mqttinfo.Enabled",
        "Disabled": "@:mqttinfo.Disabled",
-        "UpdatesOnly": "Send updates only",
+        "UpdatesOnly": "@:vedirectadmin.UpdatesOnly",
        "UpdatesEnabled": "@:mqttinfo.Enabled",
        "UpdatesDisabled": "@:mqttinfo.Disabled"
    },
@ -485,13 +485,12 @@
        "Save": "@:dtuadmin.Save"
    },
    "vedirectadmin": {
-        "VedirectSettings": "Ve.direct Settings",
-        "VedirectConfiguration": "Ve.direct Configuration",
-        "EnableVedirect": "Enable Ve.direct",
-        "VedirectParameter": "Ve.direct Parameter",
-        "PublishInterval": "Publish Interval:",
+        "VedirectSettings": "VE.Direct Settings",
+        "VedirectConfiguration": "VE.Direct Configuration",
+        "EnableVedirect": "Enable VE.Direct",
+        "VedirectParameter": "VE.Direct Parameter",
        "Seconds": "seconds",
-        "UpdatesOnly": "Send only updates:",
+        "UpdatesOnly": "Publish values to MQTT only when they change",
        "Save": "@:dtuadmin.Save"
    },
    "inverteradmin": {
--- a/webapp/src/router/index.ts
+++ b/webapp/src/router/index.ts
@ -70,7 +70,7 @@ const router = createRouter({
    },
    {
        path: '/info/vedirect',
-        name: 'Ve.direct',
+        name: 'VE.Direct',
        component: VedirectInfoView
    },
    {
@ -85,7 +85,7 @@ const router = createRouter({
    },
    {
        path: '/settings/vedirect',
-        name: 'Ve.direct Settings',
+        name: 'VE.Direct Settings',
        component: VedirectAdminView
    },
    {
--- a/webapp/src/types/VedirectConfig.ts
+++ b/webapp/src/types/VedirectConfig.ts
@ -1,5 +1,4 @@
 export interface VedirectConfig {
    vedirect_enabled: boolean;
-    vedirect_pollinterval: number;
    vedirect_updatesonly: boolean;
-}
+}
--- a/webapp/src/views/VedirectAdminView.vue
+++ b/webapp/src/views/VedirectAdminView.vue
@ -14,14 +14,9 @@
            <CardElement :text="$t('vedirectadmin.VedirectParameter')" textVariant="text-bg-primary" add-space
                         v-show="vedirectConfigList.vedirect_enabled"
            >
-                <InputElement :label="$t('vedirectadmin.PublishInterval')"
-                              v-model="vedirectConfigList.vedirect_pollinterval"
-                              type="number" min="5" max="86400"
-                              :postfix="$t('vedirectadmin.Seconds')"/>
-
                <InputElement :label="$t('vedirectadmin.UpdatesOnly')"
                              v-model="vedirectConfigList.vedirect_updatesonly"
-                              type="checkbox"/>
+                              type="checkbox" wide/>
            </CardElement>

            <button type="submit" class="btn btn-primary mb-3">{{ $t('vedirectadmin.Save') }}</button>
--- a/webapp/src/views/VedirectInfoView.vue
+++ b/webapp/src/views/VedirectInfoView.vue
@ -6,23 +6,15 @@
                    <tbody>
                        <tr>
                            <th>{{ $t('vedirectinfo.Status') }}</th>
-                            <td class="badge" :class="{
-                                'text-bg-danger': !vedirectDataList.vedirect_enabled,
-                                'text-bg-success': vedirectDataList.vedirect_enabled,
-                            }">
-                                <span v-if="vedirectDataList.vedirect_enabled">{{ $t('vedirectinfo.Enabled') }}</span>
-                                <span v-else>{{ $t('vedirectinfo.Disabled') }}</span>
+                            <td>
+                                <StatusBadge :status="vedirectDataList.vedirect_enabled" true_text="vedirectinfo.Enabled" false_text="vedirectinfo.Disabled" />
                            </td>
                        </tr>
-                        <tr v-show="vedirectDataList.vedirect_enabled">
-                                <th>{{ $t('vedirectinfo.UpdatesOnly') }}</th>
-                                <td class="badge" :class="{
-                                    'text-bg-danger': !vedirectDataList.vedirect_updatesonly,
-                                    'text-bg-success': vedirectDataList.vedirect_updatesonly,
-                                }">
-                                    <span v-if="vedirectDataList.vedirect_updatesonly">{{ $t('vedirectinfo.UpdatesEnabled') }}</span>
-                                    <span v-else>{{ $t('vedirectinfo.UpdatesDisabled') }}</span>
-                                </td>
+                        <tr>
+                            <th>{{ $t('vedirectinfo.UpdatesOnly') }}</th>
+                            <td>
+                                <StatusBadge :status="vedirectDataList.vedirect_updatesonly" true_text="vedirectinfo.UpdatesEnabled" false_text="vedirectinfo.UpdatesDisabled" />
+                            </td>
                        </tr>
                    </tbody>
                </table>
@ -34,6 +26,7 @@
 <script lang="ts">
 import BasePage from '@/components/BasePage.vue';
 import CardElement from '@/components/CardElement.vue';
+import StatusBadge from '@/components/StatusBadge.vue';
 import type { VedirectStatus } from "@/types/VedirectStatus";
 import { authHeader, handleResponse } from '@/utils/authentication';
 import { defineComponent } from 'vue';
@ -42,6 +35,7 @@ export default defineComponent({
    components: {
        BasePage,
        CardElement,
+        StatusBadge
    },
    data() {
        return {