From 076d86fc58dc01f8b90d100a870df1d913e239ac Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Wed, 2 Aug 2023 07:40:35 +0200 Subject: [PATCH 01/21] Extend CanMsg to allow to distinguish between standard and extended IDs following a inspired approach. --- api/CanMsg.h | 46 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/api/CanMsg.h b/api/CanMsg.h index 5b120344..a95c1b08 100644 --- a/api/CanMsg.h +++ b/api/CanMsg.h @@ -31,7 +31,12 @@ namespace arduino class CanMsg : public Printable { public: - static size_t constexpr MAX_DATA_LENGTH = 8; + static size_t constexpr MAX_DATA_LENGTH = 8; + + static uint32_t constexpr CAN_EFF_FLAG = 0x80000000U; + static uint32_t constexpr CAN_SFF_MASK = 0x000007FFU; /* standard frame format (SFF) */ + static uint32_t constexpr CAN_EFF_MASK = 0x1FFFFFFFU; /* extended frame format (EFF) */ + CanMsg(uint32_t const can_id, uint8_t const can_data_len, uint8_t const * can_data_ptr) : id{can_id} @@ -68,7 +73,10 @@ class CanMsg : public Printable size_t len = 0; /* Print the header. */ - len = snprintf(buf, sizeof(buf), "[%08X] (%d) : ", id, data_length); + if (isStandardId()) + len = snprintf(buf, sizeof(buf), "[%03X] (%d) : ", id, data_length); + else + len = snprintf(buf, sizeof(buf), "[%08X] (%d) : ", id, data_length); size_t n = p.write(buf, len); /* Print the data. */ @@ -82,11 +90,45 @@ class CanMsg : public Printable return n; } + + uint32_t getStandardId() const { + return (id & CAN_SFF_MASK); + } + uint32_t getExtendedId() const { + return (id & CAN_EFF_MASK); + } + bool isStandardId() const { + return ((id & CAN_EFF_FLAG) == 0); + } + bool isExtendedId() const { + return ((id & CAN_EFF_FLAG) == CAN_EFF_FLAG); + } + + + /* + * CAN ID semantics (mirroring definition by linux/can.h): + * |- Bit 31 : frame format flag (0 = standard 11 bit, 1 = extended 29 bit) + * |- Bit 30 : reserved (future remote transmission request flag) + * |- Bit 29 : reserved (future error frame flag) + * |- Bit 0-28 : CAN identifier (11/29 bit) + */ uint32_t id; uint8_t data_length; uint8_t data[MAX_DATA_LENGTH]; }; +/************************************************************************************** + * FREE FUNCTIONS + **************************************************************************************/ + +static uint32_t CanStandardId(uint32_t const id) { + return (id & CanMsg::CAN_SFF_MASK); +} + +static uint32_t CanExtendedId(uint32_t const id) { + return (CanMsg::CAN_EFF_FLAG | (id & CanMsg::CAN_EFF_MASK)); +} + /************************************************************************************** * NAMESPACE **************************************************************************************/ From 36cc4b16cd02d70a464980265b6a9afb4c11ed98 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Wed, 2 Aug 2023 08:35:15 +0200 Subject: [PATCH 02/21] Obtain the actual printable ID via getStandardId(), getExtendedId() in order to eliminate the 3 MSBits encoded status flags. --- api/CanMsg.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/CanMsg.h b/api/CanMsg.h index a95c1b08..653b3c9f 100644 --- a/api/CanMsg.h +++ b/api/CanMsg.h @@ -74,9 +74,9 @@ class CanMsg : public Printable /* Print the header. */ if (isStandardId()) - len = snprintf(buf, sizeof(buf), "[%03X] (%d) : ", id, data_length); + len = snprintf(buf, sizeof(buf), "[%03X] (%d) : ", getStandardId(), data_length); else - len = snprintf(buf, sizeof(buf), "[%08X] (%d) : ", id, data_length); + len = snprintf(buf, sizeof(buf), "[%08X] (%d) : ", getExtendedId(), data_length); size_t n = p.write(buf, len); /* Print the data. */ From 27a7b079f6a1615e66b8d579a961927e8a773edc Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Wed, 2 Aug 2023 13:37:02 +0200 Subject: [PATCH 03/21] Implementing test code for "CanStandardId", "CanExtendedId", "isStandardId", "isExtendedId". --- test/CMakeLists.txt | 5 ++++ test/src/CanMsg/test_CanExtendedId.cpp | 41 ++++++++++++++++++++++++++ test/src/CanMsg/test_CanStandardId.cpp | 40 +++++++++++++++++++++++++ test/src/CanMsg/test_isExtendedId.cpp | 33 +++++++++++++++++++++ test/src/CanMsg/test_isStandardId.cpp | 33 +++++++++++++++++++++ 5 files changed, 152 insertions(+) create mode 100644 test/src/CanMsg/test_CanExtendedId.cpp create mode 100644 test/src/CanMsg/test_CanStandardId.cpp create mode 100644 test/src/CanMsg/test_isExtendedId.cpp create mode 100644 test/src/CanMsg/test_isStandardId.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 2df2eacf..c834fd14 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -25,6 +25,10 @@ set(TEST_TARGET ${CMAKE_PROJECT_NAME}) ########################################################################## set(TEST_SRCS + src/CanMsg/test_CanExtendedId.cpp + src/CanMsg/test_CanStandardId.cpp + src/CanMsg/test_isExtendedId.cpp + src/CanMsg/test_isStandardId.cpp src/Common/test_makeWord.cpp src/Common/test_map.cpp src/Common/test_max.cpp @@ -95,6 +99,7 @@ set(TEST_SRCS ) set(TEST_DUT_SRCS + ../api/CanMsg.cpp ../api/Common.cpp ../api/IPAddress.cpp ../api/String.cpp diff --git a/test/src/CanMsg/test_CanExtendedId.cpp b/test/src/CanMsg/test_CanExtendedId.cpp new file mode 100644 index 00000000..72f8ece5 --- /dev/null +++ b/test/src/CanMsg/test_CanExtendedId.cpp @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2020 Arduino. All rights reserved. + */ + +/************************************************************************************** + * INCLUDE + **************************************************************************************/ + +#include + +#include + +/************************************************************************************** + * NAMESPACE + **************************************************************************************/ + +using namespace arduino; + +/************************************************************************************** + * TEST CODE + **************************************************************************************/ + +TEST_CASE ("Verify correct conversion to 29-Bit CAN ID for lowest valid 29-Bit CAN ID", "[CanMsg-CanExtendedId-01]") +{ + REQUIRE(CanExtendedId(0) == (CanMsg::CAN_EFF_FLAG | 0U)); +} + +TEST_CASE ("Verify correct conversion to 29-Bit CAN ID for highest valid 29-Bit CAN ID", "[CanMsg-CanExtendedId-02]") +{ + REQUIRE(CanExtendedId(0x1FFFFFFFU) == (CanMsg::CAN_EFF_FLAG | 0x1FFFFFFFU)); +} + +TEST_CASE ("Verify capping of CAN IDs exceeding 29-Bit CAN ID range", "[CanMsg-CanExtendedId-03]") +{ + REQUIRE(CanExtendedId(0x2FFFFFFFU) == (CanMsg::CAN_EFF_FLAG | 0x0FFFFFFFU)); + REQUIRE(CanExtendedId(0x3FFFFFFFU) == (CanMsg::CAN_EFF_FLAG | 0x1FFFFFFFU)); + REQUIRE(CanExtendedId(0x4FFFFFFFU) == (CanMsg::CAN_EFF_FLAG | 0x0FFFFFFFU)); + REQUIRE(CanExtendedId(0x5FFFFFFFU) == (CanMsg::CAN_EFF_FLAG | 0x1FFFFFFFU)); + /* ... */ + REQUIRE(CanExtendedId(0xFFFFFFFFU) == (CanMsg::CAN_EFF_FLAG | 0x1FFFFFFFU)); +} diff --git a/test/src/CanMsg/test_CanStandardId.cpp b/test/src/CanMsg/test_CanStandardId.cpp new file mode 100644 index 00000000..02496241 --- /dev/null +++ b/test/src/CanMsg/test_CanStandardId.cpp @@ -0,0 +1,40 @@ +/* + * Copyright (c) 2020 Arduino. All rights reserved. + */ + +/************************************************************************************** + * INCLUDE + **************************************************************************************/ + +#include + +#include + +/************************************************************************************** + * NAMESPACE + **************************************************************************************/ + +using namespace arduino; + +/************************************************************************************** + * TEST CODE + **************************************************************************************/ + +TEST_CASE ("Verify correct conversion to 11-Bit CAN ID for lowest valid 11-Bit CAN ID", "[CanMsg-CanStandardId-01]") +{ + REQUIRE(CanStandardId(0) == 0U); +} + +TEST_CASE ("Verify correct conversion to 11-Bit CAN ID for highest valid 11-Bit CAN ID", "[CanMsg-CanStandardId-02]") +{ + REQUIRE(CanStandardId(0x7FF) == 0x7FF); +} + +TEST_CASE ("Verify capping of CAN IDs exceeding 11-Bit CAN ID range", "[CanMsg-CanStandardId-03]") +{ + REQUIRE(CanStandardId(0x800U) == 0x000U); + REQUIRE(CanStandardId(0x801U) == 0x001U); + REQUIRE(CanStandardId(0x8FFU) == 0x0FFU); + REQUIRE(CanStandardId(0x1FFFFFFFU) == 0x7FFU); + REQUIRE(CanStandardId(0xFFFFFFFFU) == 0x7FFU); +} diff --git a/test/src/CanMsg/test_isExtendedId.cpp b/test/src/CanMsg/test_isExtendedId.cpp new file mode 100644 index 00000000..3a12ab8f --- /dev/null +++ b/test/src/CanMsg/test_isExtendedId.cpp @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2020 Arduino. All rights reserved. + */ + +/************************************************************************************** + * INCLUDE + **************************************************************************************/ + +#include + +#include + +/************************************************************************************** + * NAMESPACE + **************************************************************************************/ + +using namespace arduino; + +/************************************************************************************** + * TEST CODE + **************************************************************************************/ + +TEST_CASE ("\"isExtendedId\" should return true for extended CAN ID", "[CanMsg-isExtendedId-01]") +{ + CanMsg const msg_ext_id(CanExtendedId(0x020), 0, nullptr); + REQUIRE(msg_ext_id.isExtendedId() == true); +} + +TEST_CASE ("\"isExtendedId\" should return false for standard CAN ID", "[CanMsg-isExtendedId-02]") +{ + CanMsg const msg_std_id(CanStandardId(0x020), 0, nullptr); + REQUIRE(msg_std_id.isExtendedId() == false); +} diff --git a/test/src/CanMsg/test_isStandardId.cpp b/test/src/CanMsg/test_isStandardId.cpp new file mode 100644 index 00000000..718d2535 --- /dev/null +++ b/test/src/CanMsg/test_isStandardId.cpp @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2020 Arduino. All rights reserved. + */ + +/************************************************************************************** + * INCLUDE + **************************************************************************************/ + +#include + +#include + +/************************************************************************************** + * NAMESPACE + **************************************************************************************/ + +using namespace arduino; + +/************************************************************************************** + * TEST CODE + **************************************************************************************/ + +TEST_CASE ("\"isStandardId\" should return true for standard CAN ID", "[CanMsg-isStandardId-01]") +{ + CanMsg const msg_std_id(CanStandardId(0x020), 0, nullptr); + REQUIRE(msg_std_id.isStandardId() == true); +} + +TEST_CASE ("\"isStandardId\" should return false for extended CAN ID", "[CanMsg-isStandardId-02]") +{ + CanMsg const msg_ext_id(CanExtendedId(0x020), 0, nullptr); + REQUIRE(msg_ext_id.isStandardId() == false); +} From 70d393933c3dc3ceab10deea7759d2ef06d4a07f Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Wed, 2 Aug 2023 13:37:57 +0200 Subject: [PATCH 04/21] Replace broad include file with only the required include files, inline functions to prevent linker errors, and define static constants so that the compiler can place them in memory. All of those changes broaden adoption. --- api/CanMsg.cpp | 34 ++++++++++++++++++++++++++++++++++ api/CanMsg.h | 14 ++++++++------ 2 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 api/CanMsg.cpp diff --git a/api/CanMsg.cpp b/api/CanMsg.cpp new file mode 100644 index 00000000..a87900ca --- /dev/null +++ b/api/CanMsg.cpp @@ -0,0 +1,34 @@ +/* + * This file is free software; you can redistribute it and/or modify + * it under the terms of either the GNU General Public License version 2 + * or the GNU Lesser General Public License version 2.1, both as + * published by the Free Software Foundation. + */ + +/************************************************************************************** + * INCLUDE + **************************************************************************************/ + +#include "CanMsg.h" + +/************************************************************************************** + * NAMESPACE + **************************************************************************************/ + +namespace arduino +{ + +/************************************************************************************** + * STATIC CONST DEFINITION + **************************************************************************************/ + +size_t const CanMsg::MAX_DATA_LENGTH; +uint32_t const CanMsg::CAN_EFF_FLAG; +uint32_t const CanMsg::CAN_SFF_MASK; +uint32_t const CanMsg::CAN_EFF_MASK; + +/************************************************************************************** + * NAMESPACE + **************************************************************************************/ + +} /* arduino */ diff --git a/api/CanMsg.h b/api/CanMsg.h index 653b3c9f..4025c177 100644 --- a/api/CanMsg.h +++ b/api/CanMsg.h @@ -15,7 +15,9 @@ #include #include -#include +#include "Print.h" +#include "Printable.h" +#include "Common.h" /************************************************************************************** * NAMESPACE @@ -112,20 +114,20 @@ class CanMsg : public Printable * |- Bit 29 : reserved (future error frame flag) * |- Bit 0-28 : CAN identifier (11/29 bit) */ - uint32_t id; - uint8_t data_length; - uint8_t data[MAX_DATA_LENGTH]; + private: uint32_t id; + public: uint8_t data_length; + public: uint8_t data[MAX_DATA_LENGTH]; }; /************************************************************************************** * FREE FUNCTIONS **************************************************************************************/ -static uint32_t CanStandardId(uint32_t const id) { +inline uint32_t CanStandardId(uint32_t const id) { return (id & CanMsg::CAN_SFF_MASK); } -static uint32_t CanExtendedId(uint32_t const id) { +inline uint32_t CanExtendedId(uint32_t const id) { return (CanMsg::CAN_EFF_FLAG | (id & CanMsg::CAN_EFF_MASK)); } From bba303d2e6016e68d214696b50fed951e563dd35 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 3 Aug 2023 08:18:46 +0200 Subject: [PATCH 05/21] Adding test code for CanMsg::CanMsg. --- test/CMakeLists.txt | 1 + test/src/CanMsg/test_CanMsg.cpp | 52 +++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 test/src/CanMsg/test_CanMsg.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index c834fd14..6f1819cb 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -25,6 +25,7 @@ set(TEST_TARGET ${CMAKE_PROJECT_NAME}) ########################################################################## set(TEST_SRCS + src/CanMsg/test_CanMsg.cpp src/CanMsg/test_CanExtendedId.cpp src/CanMsg/test_CanStandardId.cpp src/CanMsg/test_isExtendedId.cpp diff --git a/test/src/CanMsg/test_CanMsg.cpp b/test/src/CanMsg/test_CanMsg.cpp new file mode 100644 index 00000000..09502df8 --- /dev/null +++ b/test/src/CanMsg/test_CanMsg.cpp @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2020 Arduino. All rights reserved. + */ + +/************************************************************************************** + * INCLUDE + **************************************************************************************/ + +#include + +#include + +/************************************************************************************** + * NAMESPACE + **************************************************************************************/ + +using namespace arduino; + +/************************************************************************************** + * TEST CODE + **************************************************************************************/ + +TEST_CASE ("Test constructor with no data (data length = 0)", "[CanMsg-CanMsg-01]") +{ + CanMsg const msg(CanStandardId(0x20), 0, nullptr); + + REQUIRE(msg.data_length == 0); + for (size_t i = 0; i < CanMsg::MAX_DATA_LENGTH; i++) + REQUIRE(msg.data[i] == 0); +} + +TEST_CASE ("Test constructor with data (data length < CanMsg::MAX_DATA_LENGTH)", "[CanMsg-CanMsg-02]") +{ + uint8_t const msg_data[4] = {0xDE, 0xAD, 0xC0, 0xDE}; + + CanMsg const msg(CanStandardId(0x20), sizeof(msg_data), msg_data); + + REQUIRE(msg.data_length == 4); + for (size_t i = 0; i < msg.data_length; i++) + REQUIRE(msg.data[i] == msg_data[i]); +} + +TEST_CASE ("Test constructor with data (data length > CanMsg::MAX_DATA_LENGTH)", "[CanMsg-CanMsg-03]") +{ + uint8_t const msg_data[12] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}; + + CanMsg const msg(CanStandardId(0x20), sizeof(msg_data), msg_data); + + REQUIRE(msg.data_length == 8); + for (size_t i = 0; i < msg.data_length; i++) + REQUIRE(msg.data[i] == msg_data[i]); +} From 39aa6841f5bb5a8e7de4220e1c8fbb7a6c551974 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 3 Aug 2023 08:20:45 +0200 Subject: [PATCH 06/21] Fixing bug leading to incorrect length stored when frame lenght exceeds max data lenght. --- api/CanMsg.cpp | 2 +- api/CanMsg.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/CanMsg.cpp b/api/CanMsg.cpp index a87900ca..1137f066 100644 --- a/api/CanMsg.cpp +++ b/api/CanMsg.cpp @@ -22,7 +22,7 @@ namespace arduino * STATIC CONST DEFINITION **************************************************************************************/ -size_t const CanMsg::MAX_DATA_LENGTH; +uint8_t const CanMsg::MAX_DATA_LENGTH; uint32_t const CanMsg::CAN_EFF_FLAG; uint32_t const CanMsg::CAN_SFF_MASK; uint32_t const CanMsg::CAN_EFF_MASK; diff --git a/api/CanMsg.h b/api/CanMsg.h index 4025c177..2b00871d 100644 --- a/api/CanMsg.h +++ b/api/CanMsg.h @@ -33,7 +33,7 @@ namespace arduino class CanMsg : public Printable { public: - static size_t constexpr MAX_DATA_LENGTH = 8; + static uint8_t constexpr MAX_DATA_LENGTH = 8; static uint32_t constexpr CAN_EFF_FLAG = 0x80000000U; static uint32_t constexpr CAN_SFF_MASK = 0x000007FFU; /* standard frame format (SFF) */ @@ -42,10 +42,10 @@ class CanMsg : public Printable CanMsg(uint32_t const can_id, uint8_t const can_data_len, uint8_t const * can_data_ptr) : id{can_id} - , data_length{can_data_len} + , data_length{min(can_data_len, MAX_DATA_LENGTH)} , data{0} { - memcpy(data, can_data_ptr, min(can_data_len, MAX_DATA_LENGTH)); + memcpy(data, can_data_ptr, data_length); } CanMsg() : CanMsg(0, 0, nullptr) { } From 16c74c18645be24f01f81227fd228d6f4fed4c5f Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 3 Aug 2023 13:19:17 +0200 Subject: [PATCH 07/21] Test CanMsg copy constructor. --- test/CMakeLists.txt | 1 + test/src/CanMsg/test_CanMsg_CopyCtor.cpp | 38 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 test/src/CanMsg/test_CanMsg_CopyCtor.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 6f1819cb..d701e588 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -26,6 +26,7 @@ set(TEST_TARGET ${CMAKE_PROJECT_NAME}) set(TEST_SRCS src/CanMsg/test_CanMsg.cpp + src/CanMsg/test_CanMsg_CopyCtor.cpp src/CanMsg/test_CanExtendedId.cpp src/CanMsg/test_CanStandardId.cpp src/CanMsg/test_isExtendedId.cpp diff --git a/test/src/CanMsg/test_CanMsg_CopyCtor.cpp b/test/src/CanMsg/test_CanMsg_CopyCtor.cpp new file mode 100644 index 00000000..a1442c09 --- /dev/null +++ b/test/src/CanMsg/test_CanMsg_CopyCtor.cpp @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2020 Arduino. All rights reserved. + */ + +/************************************************************************************** + * INCLUDE + **************************************************************************************/ + +#include + +#include + +/************************************************************************************** + * NAMESPACE + **************************************************************************************/ + +using namespace arduino; + +/************************************************************************************** + * TEST CODE + **************************************************************************************/ + +TEST_CASE ("Test copy constructor", "[CanMsg-CopyCtor-01]") +{ + uint8_t const msg_data[4] = {0xDE, 0xAD, 0xC0, 0xDE}; + + CanMsg const msg_1(CanStandardId(0x20), sizeof(msg_data), msg_data); + CanMsg const msg_2(msg_1); + + REQUIRE(msg_1.data_length == msg_2.data_length); + + for (size_t i = 0; i < msg_1.data_length; i++) + { + REQUIRE(msg_1.data[i] == msg_data[i]); + REQUIRE(msg_2.data[i] == msg_data[i]); + } + +} From ed1c507270270ce3cf2c0d726c1b8dd8367cdac1 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 3 Aug 2023 13:29:18 +0200 Subject: [PATCH 08/21] Completing test code for all APIs of CanMsg. --- test/CMakeLists.txt | 2 + test/src/CanMsg/test_CanMsg_CopyCtor.cpp | 1 - test/src/CanMsg/test_operator_assignment.cpp | 39 +++++++++++++++++ test/src/CanMsg/test_printTo.cpp | 44 ++++++++++++++++++++ 4 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 test/src/CanMsg/test_operator_assignment.cpp create mode 100644 test/src/CanMsg/test_printTo.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index d701e588..e1ca18d2 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -31,6 +31,8 @@ set(TEST_SRCS src/CanMsg/test_CanStandardId.cpp src/CanMsg/test_isExtendedId.cpp src/CanMsg/test_isStandardId.cpp + src/CanMsg/test_operator_assignment.cpp + src/CanMsg/test_printTo.cpp src/Common/test_makeWord.cpp src/Common/test_map.cpp src/Common/test_max.cpp diff --git a/test/src/CanMsg/test_CanMsg_CopyCtor.cpp b/test/src/CanMsg/test_CanMsg_CopyCtor.cpp index a1442c09..b303369a 100644 --- a/test/src/CanMsg/test_CanMsg_CopyCtor.cpp +++ b/test/src/CanMsg/test_CanMsg_CopyCtor.cpp @@ -34,5 +34,4 @@ TEST_CASE ("Test copy constructor", "[CanMsg-CopyCtor-01]") REQUIRE(msg_1.data[i] == msg_data[i]); REQUIRE(msg_2.data[i] == msg_data[i]); } - } diff --git a/test/src/CanMsg/test_operator_assignment.cpp b/test/src/CanMsg/test_operator_assignment.cpp new file mode 100644 index 00000000..ae2d2ecb --- /dev/null +++ b/test/src/CanMsg/test_operator_assignment.cpp @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2020 Arduino. All rights reserved. + */ + +/************************************************************************************** + * INCLUDE + **************************************************************************************/ + +#include + +#include + +/************************************************************************************** + * NAMESPACE + **************************************************************************************/ + +using namespace arduino; + +/************************************************************************************** + * TEST CODE + **************************************************************************************/ + +TEST_CASE ("Testing CanMsg::operator = (CanMsg const &)", "[CanMsg-Operator-=-1]") +{ + uint8_t const msg_data[4] = {0xDE, 0xAD, 0xC0, 0xDE}; + + CanMsg const msg_1(CanStandardId(0x20), sizeof(msg_data), msg_data); + CanMsg msg_2(CanStandardId(0x21), 0, nullptr); + + msg_2 = msg_1; + + REQUIRE(msg_1.data_length == msg_2.data_length); + + for (size_t i = 0; i < msg_1.data_length; i++) + { + REQUIRE(msg_1.data[i] == msg_data[i]); + REQUIRE(msg_2.data[i] == msg_data[i]); + } +} diff --git a/test/src/CanMsg/test_printTo.cpp b/test/src/CanMsg/test_printTo.cpp new file mode 100644 index 00000000..d55973ac --- /dev/null +++ b/test/src/CanMsg/test_printTo.cpp @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2020 Arduino. All rights reserved. + */ + +/************************************************************************************** + * INCLUDE + **************************************************************************************/ + +#include + +#include +#include + +/************************************************************************************** + * NAMESPACE + **************************************************************************************/ + +using namespace arduino; + +/************************************************************************************** + * TEST CODE + **************************************************************************************/ + +TEST_CASE ("Print CAN frame with standard ID", "[CanMsg-printTo-1]") +{ + uint8_t const std_msg_data[] = {0xBE, 0xEF}; + CanMsg const std_msg(CanStandardId(0x20), sizeof(std_msg_data), std_msg_data); + + PrintMock mock; + mock.print(std_msg); + + REQUIRE(mock._str == "[020] (2) : BEEF"); +} + +TEST_CASE ("Print CAN frame with extended ID", "[CanMsg-printTo-1]") +{ + uint8_t const ext_msg_data[] = {0xDE, 0xAD, 0xC0, 0xDE}; + CanMsg const ext_msg(CanExtendedId(0x20), sizeof(ext_msg_data), ext_msg_data); + + PrintMock mock; + mock.print(ext_msg); + + REQUIRE(mock._str == "[00000020] (4) : DEADC0DE"); +} From 437020eb4bf517413afdd89192758b4a86be8742 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 3 Aug 2023 13:29:55 +0200 Subject: [PATCH 09/21] Fix wrong test case number. --- test/src/CanMsg/test_printTo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/CanMsg/test_printTo.cpp b/test/src/CanMsg/test_printTo.cpp index d55973ac..dd1fb4b8 100644 --- a/test/src/CanMsg/test_printTo.cpp +++ b/test/src/CanMsg/test_printTo.cpp @@ -32,7 +32,7 @@ TEST_CASE ("Print CAN frame with standard ID", "[CanMsg-printTo-1]") REQUIRE(mock._str == "[020] (2) : BEEF"); } -TEST_CASE ("Print CAN frame with extended ID", "[CanMsg-printTo-1]") +TEST_CASE ("Print CAN frame with extended ID", "[CanMsg-printTo-2]") { uint8_t const ext_msg_data[] = {0xDE, 0xAD, 0xC0, 0xDE}; CanMsg const ext_msg(CanExtendedId(0x20), sizeof(ext_msg_data), ext_msg_data); From ba86aefb521d104cbf69cb1a21373e6821e46631 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 3 Aug 2023 13:35:32 +0200 Subject: [PATCH 10/21] Restore full public access to "id", even if that enables some foot-shooting. --- api/CanMsg.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/CanMsg.h b/api/CanMsg.h index 2b00871d..b561bf97 100644 --- a/api/CanMsg.h +++ b/api/CanMsg.h @@ -114,9 +114,9 @@ class CanMsg : public Printable * |- Bit 29 : reserved (future error frame flag) * |- Bit 0-28 : CAN identifier (11/29 bit) */ - private: uint32_t id; - public: uint8_t data_length; - public: uint8_t data[MAX_DATA_LENGTH]; + uint32_t id; + uint8_t data_length; + uint8_t data[MAX_DATA_LENGTH]; }; /************************************************************************************** From ab4a66c65310525fe2eab699cce460ba31fb4bf4 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 3 Aug 2023 13:38:31 +0200 Subject: [PATCH 11/21] Encode expectations for value of "id" in unit test. --- test/src/CanMsg/test_CanMsg.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/src/CanMsg/test_CanMsg.cpp b/test/src/CanMsg/test_CanMsg.cpp index 09502df8..14b8a76a 100644 --- a/test/src/CanMsg/test_CanMsg.cpp +++ b/test/src/CanMsg/test_CanMsg.cpp @@ -50,3 +50,17 @@ TEST_CASE ("Test constructor with data (data length > CanMsg::MAX_DATA_LENGTH)", for (size_t i = 0; i < msg.data_length; i++) REQUIRE(msg.data[i] == msg_data[i]); } + +TEST_CASE ("Test constructor constructing a CAN frame with standard ID", "[CanMsg-CanMsg-04]") +{ + CanMsg const msg(CanStandardId(0x20), 0, nullptr); + + REQUIRE(msg.id == 0x20); +} + +TEST_CASE ("Test constructor constructing a CAN frame with extended ID", "[CanMsg-CanMsg-05]") +{ + CanMsg const msg(CanExtendedId(0x20), 0, nullptr); + + REQUIRE(msg.id == (CanMsg::CAN_EFF_FLAG | 0x20)); +} From b997cbfd07c99dd66f3f9094cb2c97dc7b8ff8ea Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 3 Aug 2023 13:39:46 +0200 Subject: [PATCH 12/21] Ignore visual studio code and clion artifacts. --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 00000000..93c700e8 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +.vscode/ +.idea/ From e09108034ffb002f366d79646a67607e66fadf45 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 10 Aug 2023 07:36:00 +0200 Subject: [PATCH 13/21] Fix copy-assignment operator. --- api/CanMsg.h | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/api/CanMsg.h b/api/CanMsg.h index b561bf97..fe0b920c 100644 --- a/api/CanMsg.h +++ b/api/CanMsg.h @@ -59,14 +59,15 @@ class CanMsg : public Printable virtual ~CanMsg() { } - void operator = (CanMsg const & other) + CanMsg & operator = (CanMsg const & other) { - if (this == &other) - return; - - this->id = other.id; - this->data_length = other.data_length; - memcpy(this->data, other.data, this->data_length); + if (this != &other) + { + this->id = other.id; + this->data_length = other.data_length; + memcpy(this->data, other.data, this->data_length); + } + return (*this); } virtual size_t printTo(Print & p) const override From a5b8e43c9cefd0f3f1d0485ee4c5a8119460770d Mon Sep 17 00:00:00 2001 From: Hayden Roche Date: Fri, 25 Aug 2023 10:47:33 -0700 Subject: [PATCH 14/21] Fix format specifier for printing CAN message ID in CanMsg.h. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regarding the X format specifier: C standard §7.21.6.1: "The unsigned int argument is converted to ..." On some platforms (e.g. mbed_nano), uint32_t, which is what's being printed here, is an unsigned long int, so this code causes a compiler warning. The fix is to use the format specifiers from §7.8.1, specifically PRIX32. --- api/CanMsg.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/CanMsg.h b/api/CanMsg.h index 5b120344..522e6541 100644 --- a/api/CanMsg.h +++ b/api/CanMsg.h @@ -12,7 +12,7 @@ * INCLUDE **************************************************************************************/ -#include +#include #include #include @@ -68,7 +68,7 @@ class CanMsg : public Printable size_t len = 0; /* Print the header. */ - len = snprintf(buf, sizeof(buf), "[%08X] (%d) : ", id, data_length); + len = snprintf(buf, sizeof(buf), "[%08" PRIX32 "] (%d) : ", id, data_length); size_t n = p.write(buf, len); /* Print the data. */ From f6764f41fe3b9cbcb94b042944adefa17af0de2f Mon Sep 17 00:00:00 2001 From: PiotrekB416 Date: Wed, 30 Aug 2023 16:39:11 +0200 Subject: [PATCH 15/21] Fix String::replace replace multiple occurrences of longer String by shorter one --- api/String.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/api/String.cpp b/api/String.cpp index 0a5c11fe..7e76a510 100644 --- a/api/String.cpp +++ b/api/String.cpp @@ -234,10 +234,10 @@ void String::move(String &rhs) String & String::operator = (const String &rhs) { if (this == &rhs) return *this; - + if (rhs.buffer) copy(rhs.buffer, rhs.len); else invalidate(); - + return *this; } @@ -253,7 +253,7 @@ String & String::operator = (const char *cstr) { if (cstr) copy(cstr, strlen(cstr)); else invalidate(); - + return *this; } @@ -484,7 +484,7 @@ bool String::equalsIgnoreCase( const String &s2 ) const const char *p2 = s2.buffer; while (*p1) { if (tolower(*p1++) != tolower(*p2++)) return false; - } + } return true; } @@ -515,7 +515,7 @@ char String::charAt(unsigned int loc) const return operator[](loc); } -void String::setCharAt(unsigned int loc, char c) +void String::setCharAt(unsigned int loc, char c) { if (loc < len) buffer[loc] = c; } @@ -652,9 +652,9 @@ void String::replace(const String& find, const String& replace) } } else if (diff < 0) { unsigned int size = len; // compute size needed for result + diff = 0 - diff; while ((foundAt = strstr(readFrom, find.buffer)) != NULL) { readFrom = foundAt + find.len; - diff = 0 - diff; size -= diff; } if (size == len) return; From f782ba4bd546659f9c60ee448ea643e180535910 Mon Sep 17 00:00:00 2001 From: PiotrekB416 Date: Wed, 30 Aug 2023 16:42:00 +0200 Subject: [PATCH 16/21] Add tests --- test/src/String/test_replace.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/src/String/test_replace.cpp b/test/src/String/test_replace.cpp index 5d46fafb..d4f28bf5 100644 --- a/test/src/String/test_replace.cpp +++ b/test/src/String/test_replace.cpp @@ -66,3 +66,24 @@ TEST_CASE ("Testing String::replace(String, String) substr 'find' smaller than ' str.replace(arduino::String("ll"), arduino::String("111")); REQUIRE(str == "He111o Arduino!"); } + +TEST_CASE ("Testing String::replace(String, String) substr 'find' smaller than 'replace' multiple occurencies", "[String-replace-08]") +{ + arduino::String str("Hello Arduino! Hello, Hello, Hello"); + str.replace(arduino::String("ll"), arduino::String("lll")); + REQUIRE(str == "Helllo Arduino! Helllo, Helllo, Helllo"); +} + +TEST_CASE ("Testing String::replace(String, String) substr 'find' same length as 'replace' multiple occurencies", "[String-replace-09]") +{ + arduino::String str("Hello Arduino! Hello, Hello, Hello"); + str.replace(arduino::String("ll"), arduino::String("11")); + REQUIRE(str == "He11o Arduino! He11o, He11o, He11o"); +} + +TEST_CASE ("Testing String::replace(String, String) substr 'find' larger than 'replace' multiple occurencies", "[String-replace-10]") +{ + arduino::String str("Helllo Arduino! Helllo, Helllo, Helllo"); + str.replace(arduino::String("lll"), arduino::String("ll")); + REQUIRE(str == "Hello Arduino! Hello, Hello, Hello"); +} From 12fb6ba05e44385052a65de88f79003467bc8ce5 Mon Sep 17 00:00:00 2001 From: Piotr Bartoszewicz <89194651+PiotrekB416@users.noreply.github.com> Date: Wed, 30 Aug 2023 23:15:05 +0200 Subject: [PATCH 17/21] Update api/String.cpp Co-authored-by: per1234 --- api/String.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/String.cpp b/api/String.cpp index 7e76a510..69125aa2 100644 --- a/api/String.cpp +++ b/api/String.cpp @@ -652,7 +652,7 @@ void String::replace(const String& find, const String& replace) } } else if (diff < 0) { unsigned int size = len; // compute size needed for result - diff = 0 - diff; + diff = 0 - diff; while ((foundAt = strstr(readFrom, find.buffer)) != NULL) { readFrom = foundAt + find.len; size -= diff; From b052976fafa1bf4e4840a23ca5670b2608c02eb7 Mon Sep 17 00:00:00 2001 From: Martino Facchin Date: Mon, 4 Sep 2023 15:09:25 +0200 Subject: [PATCH 18/21] Release version 1.4.2 --- api/ArduinoAPI.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/ArduinoAPI.h b/api/ArduinoAPI.h index 8464396c..c1ce5d2d 100644 --- a/api/ArduinoAPI.h +++ b/api/ArduinoAPI.h @@ -20,8 +20,8 @@ #ifndef ARDUINO_API_H #define ARDUINO_API_H -// version 1.4.1 -#define ARDUINO_API_VERSION 10401 +// version 1.4.2 +#define ARDUINO_API_VERSION 10402 #include "Binary.h" From b2e375aad4a1ed5aed05bf11cdb7ff7d975f11d0 Mon Sep 17 00:00:00 2001 From: Umberto Baldi <34278123+umbynos@users.noreply.github.com> Date: Mon, 4 Sep 2023 17:16:25 +0200 Subject: [PATCH 19/21] Bump codecov-action to v3, and use token --- .github/workflows/unit-tests.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index f72d58b1..55d5b650 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -38,8 +38,9 @@ jobs: coverage-data-path: ${{ env.COVERAGE_DATA_PATH }} # See: https://github.com/codecov/codecov-action/blob/master/README.md - - name: Upload coverage report to Codecov - uses: codecov/codecov-action@v1 + - name: Code coverage + uses: codecov/codecov-action@v3 with: - file: ${{ env.COVERAGE_DATA_PATH }} + token: ${{ secrets.CODECOV_TOKEN }} + files: ${{ env.COVERAGE_DATA_PATH }} fail_ci_if_error: true From 8ce59d204de0644e5804547dce83fab9e95ec247 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Mon, 4 Sep 2023 17:17:39 +0200 Subject: [PATCH 20/21] Add a test case demonstrating the issue when trying to print a zero value for uint64_t. --- test/src/Print/test_print.cpp | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/test/src/Print/test_print.cpp b/test/src/Print/test_print.cpp index 92e9c083..ac731a89 100644 --- a/test/src/Print/test_print.cpp +++ b/test/src/Print/test_print.cpp @@ -110,12 +110,24 @@ TEST_CASE ("Print::print(unsigned long long, int = DEC|HEX|OCT|BIN)", "[Print-pr { PrintMock mock; - unsigned long long const val = 17; + GIVEN("a value of zero ...") + { + unsigned long long const val = 0; - WHEN("DEC") { mock.print(val, DEC); REQUIRE(mock._str == "17"); } - WHEN("HEX") { mock.print(val, HEX); REQUIRE(mock._str == "11"); } - WHEN("OCT") { mock.print(val, OCT); REQUIRE(mock._str == "21"); } - WHEN("BIN") { mock.print(val, BIN); REQUIRE(mock._str == "10001"); } + WHEN("DEC") { mock.print(val, DEC); REQUIRE(mock._str == "0"); } + WHEN("HEX") { mock.print(val, HEX); REQUIRE(mock._str == "0"); } + WHEN("OCT") { mock.print(val, OCT); REQUIRE(mock._str == "0"); } + WHEN("BIN") { mock.print(val, BIN); REQUIRE(mock._str == "0"); } + } + GIVEN("a non-zero value ...") + { + unsigned long long const val = 17; + + WHEN("DEC") { mock.print(val, DEC); REQUIRE(mock._str == "17"); } + WHEN("HEX") { mock.print(val, HEX); REQUIRE(mock._str == "11"); } + WHEN("OCT") { mock.print(val, OCT); REQUIRE(mock._str == "21"); } + WHEN("BIN") { mock.print(val, BIN); REQUIRE(mock._str == "10001"); } + } } TEST_CASE ("Print::print(double, int = 2)", "[Print-print-10]") From 754aa9b1a65f15c190d49b2478b95ccbe55b7f7a Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Sun, 3 Sep 2023 13:18:47 -0700 Subject: [PATCH 21/21] Fix Print::print(0ULL) Fixes #1691 To be removed once upstream bug is fixed and core-api imported: https://github.com/arduino/ArduinoCore-API/issues/178 --- api/Print.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/api/Print.cpp b/api/Print.cpp index 8c3e1930..f1e82469 100644 --- a/api/Print.cpp +++ b/api/Print.cpp @@ -287,6 +287,12 @@ size_t Print::printULLNumber(unsigned long long n64, uint8_t base) uint8_t i = 0; uint8_t innerLoops = 0; + // Special case workaround https://github.com/arduino/ArduinoCore-API/issues/178 + if (n64 == 0) { + write('0'); + return 1; + } + // prevent crash if called with base == 1 if (base < 2) base = 10;