From 7de795e2e8a72e31629aeb6dd42ea9bb0b080f70 Mon Sep 17 00:00:00 2001 From: Ton Huisman Date: Sat, 2 Nov 2024 15:09:51 +0100 Subject: [PATCH] [P176] Fix checksum validation, was ignoring the first line after receiving a valid checksum --- docs/source/Plugin/P176.rst | 2 +- src/src/PluginStructs/P176_data_struct.cpp | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/docs/source/Plugin/P176.rst b/docs/source/Plugin/P176.rst index d1a753559..95ddfd6de 100644 --- a/docs/source/Plugin/P176.rst +++ b/docs/source/Plugin/P176.rst @@ -91,7 +91,7 @@ This example shows the data as can be received from a VE.Direct device. The *Name* column is what should be used in the Values fields, or when as a ```` from rules or in a display configuration. The exact meaning and unit of each field can be found in the VE.Direct protocol documentation, available from Victron Energy. -The *Data* column shows the actual data as received. +The *Data* column shows the actual data as received. If the checksum validation is enabled, this column may be empty if the checksum could not be verified, like the first (possibly incomplete) packet, but as the frequency of packets is rather high, this column should not often (or long) be empty. The *Value* column shows a factored result based on the value, as mV is not always very useful, so that's converted to V, mA to A, Wh to kWh, etc. diff --git a/src/src/PluginStructs/P176_data_struct.cpp b/src/src/PluginStructs/P176_data_struct.cpp index d68851b1b..3adeea0cf 100644 --- a/src/src/PluginStructs/P176_data_struct.cpp +++ b/src/src/PluginStructs/P176_data_struct.cpp @@ -272,7 +272,7 @@ bool P176_data_struct::handleSerial() { if (Checksum_state_e::ValidateNext == _checksumState) { _checksumState = Checksum_state_e::Validating; } else - if (Checksum_state_e::Starting == _checksumState) { // Start counting after a Checksum was received + if (Checksum_state_e::Starting == _checksumState) { // Start counting after a Checksum (aka 'end of packet') was received _checksumState = Checksum_state_e::Counting; _checksum = 0; # if P176_DEBUG @@ -298,6 +298,10 @@ bool P176_data_struct::handleSerial() { # endif // if P176_DEBUG } else { _checksumState = Checksum_state_e::Starting; + # if P176_FAIL_CHECKSUM + + commitTempData(!_failChecksum); // Discard any data received so far, as we don't know their checksum status + # endif // if P176_FAIL_CHECKSUM } } # endif // if P176_HANDLE_CHECKSUM @@ -317,10 +321,11 @@ bool P176_data_struct::handleSerial() { _checksumDelta = 0; if (loglevelActiveFor(LOG_LEVEL_ERROR)) { - addLog(LOG_LEVEL_ERROR, strformat(F("Victron: Checksum error, expected 0 but got %d"), _checksum)); + addLog(LOG_LEVEL_ERROR, strformat(F("Victron: Checksum error, expected 0 but got %d (success: %d errors: %d)"), + _checksum, _successCounter, _checksumErrors)); } } else { - _checksumState = Checksum_state_e::Starting; + _checksumState = Checksum_state_e::Counting; // New packet is expected, start counting immediately _successCounter++; _checksumDelta++; result = true; @@ -334,6 +339,7 @@ bool P176_data_struct::handleSerial() { addLog(LOG_LEVEL_INFO, F("Victron: Checksum validated Ok")); } } + _checksum = 0; // Clean start # if P176_FAIL_CHECKSUM