Merge pull request #194 from arduino/fix-11-bit-can-id

Extend CanMsg to allow to distinguish between standard and extended ids …

Author: aentinger

Diff for: .gitignore

@@ -0,0 +1,2 @@
+.vscode/
+.idea/

Diff for: 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
+ **************************************************************************************/
+
+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;
+
+/**************************************************************************************
+ * NAMESPACE
+ **************************************************************************************/
+
+} /* arduino */

Diff for: api/CanMsg.h

@@ -15,7 +15,9 @@
 #include <inttypes.h>
 #include <string.h>
 
-#include <Arduino.h>
+#include "Print.h"
+#include "Printable.h"
+#include "Common.h"
 
 /**************************************************************************************
  * NAMESPACE
@@ -31,14 +33,19 @@ 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) */
+  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}
-  , 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) { }
@@ -52,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
@@ -68,7 +76,10 @@ class CanMsg : public Printable
     size_t len = 0;
 
     /* Print the header. */
-    len = snprintf(buf, sizeof(buf), "[%08" PRIX32 "] (%d) : ", id, data_length);
+    if (isStandardId())
+      len = snprintf(buf, sizeof(buf), "[%03" PRIX32 "] (%d) : ", getStandardId(), data_length);
+    else
+      len = snprintf(buf, sizeof(buf), "[%08" PRIX32 "] (%d) : ", getExtendedId(), data_length);
     size_t n = p.write(buf, len);
 
     /* Print the data. */
@@ -82,11 +93,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
+ **************************************************************************************/
+
+inline uint32_t CanStandardId(uint32_t const id) {
+  return (id & CanMsg::CAN_SFF_MASK);
+}
+
+inline uint32_t CanExtendedId(uint32_t const id) {
+  return (CanMsg::CAN_EFF_FLAG | (id & CanMsg::CAN_EFF_MASK));
+}
+
 /**************************************************************************************
  * NAMESPACE
  **************************************************************************************/

Diff for: test/CMakeLists.txt

@@ -25,6 +25,14 @@ 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
+  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
@@ -95,6 +103,7 @@ set(TEST_SRCS
 )
 
 set(TEST_DUT_SRCS
+  ../api/CanMsg.cpp
   ../api/Common.cpp
   ../api/IPAddress.cpp
   ../api/String.cpp

Diff for: test/src/CanMsg/test_CanExtendedId.cpp

@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2020 Arduino.  All rights reserved.
+ */
+
+/**************************************************************************************
+ * INCLUDE
+ **************************************************************************************/
+
+#include <catch.hpp>
+
+#include <CanMsg.h>
+
+/**************************************************************************************
+ * 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 for: test/src/CanMsg/test_CanMsg.cpp

@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2020 Arduino.  All rights reserved.
+ */
+
+/**************************************************************************************
+ * INCLUDE
+ **************************************************************************************/
+
+#include <catch.hpp>
+
+#include <CanMsg.h>
+
+/**************************************************************************************
+ * 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]);
+}
+
+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));
+}

Diff for: test/src/CanMsg/test_CanMsg_CopyCtor.cpp

@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2020 Arduino.  All rights reserved.
+ */
+
+/**************************************************************************************
+ * INCLUDE
+ **************************************************************************************/
+
+#include <catch.hpp>
+
+#include <CanMsg.h>
+
+/**************************************************************************************
+ * 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]);
+  }
+}

Diff for: test/src/CanMsg/test_CanStandardId.cpp

@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2020 Arduino.  All rights reserved.
+ */
+
+/**************************************************************************************
+ * INCLUDE
+ **************************************************************************************/
+
+#include <catch.hpp>
+
+#include <CanMsg.h>
+
+/**************************************************************************************
+ * 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 for: test/src/CanMsg/test_isExtendedId.cpp

@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2020 Arduino.  All rights reserved.
+ */
+
+/**************************************************************************************
+ * INCLUDE
+ **************************************************************************************/
+
+#include <catch.hpp>
+
+#include <CanMsg.h>
+
+/**************************************************************************************
+ * 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);
+}