Fix onError on write response

Author: bertmelis

platformio.ini

@@ -6,7 +6,6 @@ build_flags =
   -Wextra
   -Wpedantic
   -std=c++11
-  -pthread
 
 [env:native]
 platform = native
@@ -18,3 +17,10 @@ build_flags =
   -D VW_START_PAYLOAD_LENGTH=10
 extra_scripts = test_coverage.py
 build_type = debug
+test_testing_command =
+  valgrind
+  --leak-check=full
+  --show-leak-kinds=all
+  --track-origins=yes
+  --error-exitcode=1
+  ${platformio.build_dir}/${this.__env__}/program

src/VS2/PacketVS2.cpp

@@ -45,11 +45,11 @@ bool PacketVS2::createPacket(PacketType pt, FunctionCode fc, uint8_t id, uint16_
 
   // check arguments
   if (len == 0) {
-    vw_log_i("Length error: %u", len);
+    vw_log_w("Length error: %u", len);
     return false;
   }
   if (fc == FunctionCode::WRITE && !data) {
-    vw_log_i("Function code - data mismatch");
+    vw_log_w("Function code - data mismatch");
     return false;
   }
   if (id > 7) {
@@ -61,7 +61,7 @@ bool PacketVS2::createPacket(PacketType pt, FunctionCode fc, uint8_t id, uint16_
   if (toAllocate > _allocatedLength) {
     uint8_t* newBuffer = reinterpret_cast<uint8_t*>(realloc(_buffer, toAllocate));
     if (!newBuffer) {
-      vw_log_i("buffer not available");
+      vw_log_e("buffer not available");
       return false;
     }
     _buffer = newBuffer;
@@ -129,6 +129,7 @@ uint8_t PacketVS2::dataLength() const {
 }
 
 const uint8_t* PacketVS2::data() const {
+  if (functionCode() == FunctionCode::WRITE) return nullptr;
   return &_buffer[6];
 }
 

src/VS2/ParserVS2.cpp

@@ -77,17 +77,20 @@ ParserResult ParserVS2::parse(const uint8_t b) {
 
   case ParserStep::PAYLOADLENGTH:
     _packet[5] = b;
-    if (_packet.functionCode() == VitoWiFi::FunctionCode::WRITE ||
-        _packet.packetType() == VitoWiFi::PacketType::RESPONSE) {
+    if ((_packet.functionCode() == VitoWiFi::FunctionCode::READ &&
+        _packet.packetType() == VitoWiFi::PacketType::REQUEST) ||
+        (_packet.functionCode() == VitoWiFi::FunctionCode::WRITE &&
+        _packet.packetType() == VitoWiFi::PacketType::RESPONSE)) {
+      // read requests and write responses don't have a data payload
+      _step = ParserStep::CHECKSUM;
+    } else {
       if (b != _packet.length() - 6U) {
         vw_log_w("Invalid payload length: %u (expected %u)", b, _packet.length() - 6U);
         _step = ParserStep::STARTBYTE;
         return ParserResult::ERROR;
       }
       _payloadLength = b;
       _step = ParserStep::PAYLOAD;
-    } else {
-      _step = ParserStep::CHECKSUM;
     }
     break;
 

test/test_ParserVS2/test_ParserVS2.cpp

@@ -54,7 +54,7 @@ void test_ok_request() {
   TEST_ASSERT_EQUAL_UINT8(0x02, parser.packet().dataLength());
 }
 
-void test_ok_response() {
+void test_ok_readresponse() {
   const uint8_t stream[] = {
     0x41,  // start byte
     0x07,  // length
@@ -81,7 +81,7 @@ void test_ok_response() {
     }
   }
 
-  TEST_ASSERT_EQUAL(ParserResult::COMPLETE,  result);
+  TEST_ASSERT_EQUAL(ParserResult::COMPLETE, result);
   TEST_ASSERT_EQUAL_UINT(length, bytesRead);
   TEST_ASSERT_EQUAL_UINT8(packetLength, parser.packet().length());
   TEST_ASSERT_EQUAL_UINT8(PacketType::RESPONSE, parser.packet().packetType());
@@ -92,6 +92,41 @@ void test_ok_response() {
   TEST_ASSERT_EQUAL_UINT8_ARRAY(data, parser.packet().data(), 2);
 }
 
+void test_ok_writeresponse() {
+  const uint8_t stream[] = {
+    0x41,  // start byte
+    0x05,  // length
+    0x01,  // packet type (response)
+    0x02,  // flags: id + function code (0 + write)
+    0x23,  // address 1
+    0x23,  // address 2
+    0x01,  // payload length
+    0x4F   // cs
+  };
+  const std::size_t length = 8;
+  const std::size_t packetLength = 6;
+
+  std::size_t bytesRead = 0;
+  ParserResult result = ParserResult::ERROR;
+
+  while (bytesRead < length) {
+    result = parser.parse(stream[bytesRead++]);
+    if (result != ParserResult::CONTINUE) {
+      break;
+    }
+  }
+
+  TEST_ASSERT_EQUAL(ParserResult::COMPLETE, result);
+  TEST_ASSERT_EQUAL_UINT(length, bytesRead);
+  TEST_ASSERT_EQUAL_UINT8(packetLength, parser.packet().length());
+  TEST_ASSERT_EQUAL_UINT8(PacketType::RESPONSE, parser.packet().packetType());
+  TEST_ASSERT_EQUAL_UINT8(0x00, parser.packet().id());
+  TEST_ASSERT_EQUAL_UINT8(FunctionCode::WRITE, parser.packet().functionCode());
+  TEST_ASSERT_EQUAL_UINT16(0x2323, parser.packet().address());
+  TEST_ASSERT_EQUAL_UINT8(0x01, parser.packet().dataLength());
+  TEST_ASSERT_NULL(parser.packet().data());
+}
+
 void test_spuriousbytes() {
   const uint8_t stream[] = {
     0x05,
@@ -251,7 +286,8 @@ void test_invalidChecksum() {
 int main() {
   UNITY_BEGIN();
   RUN_TEST(test_ok_request);
-  RUN_TEST(test_ok_response);
+  RUN_TEST(test_ok_readresponse);
+  RUN_TEST(test_ok_writeresponse);
   RUN_TEST(test_spuriousbytes);
   RUN_TEST(test_invalidLength);
   RUN_TEST(test_invalidPacketType);