Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/issue135 (CAN FD) #140

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
language: cpp
compiler: gcc
dist: trusty
sudo: true
env:
global:
# The next declaration is the encrypted COVERITY_SCAN_TOKEN, created
Expand All @@ -20,4 +22,4 @@ before_install:
- ./bootstrap.sh
before_script: "mkdir build && cd build && cmake .. -DCMAKE_BUILD_TYPE=Debug -DCONTINUOUS_INTEGRATION_BUILD=1"
script:
- if [ "${COVERITY_SCAN_BRANCH}" != 1 ] && [ "${TARGET}" == "native" ]; then make && make ARGS=-VV test; fi
- if [ "${COVERITY_SCAN_BRANCH}" != 1 ] && [ "${TARGET}" == "native" ]; then make -j4 && make ARGS=-VV test; fi
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,6 @@ Vagrant can be used to setup a compatible Ubuntu virtual image. Follow the instr
```bash
vagrant up
vagrant ssh
mkdir build
cd build
mkdir build && cd build && cmake .. -DCMAKE_BUILD_TYPE=Debug -DCONTINUOUS_INTEGRATION_BUILD=1
```

Expand Down
23 changes: 21 additions & 2 deletions libuavcan/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ include_directories(include)
#
# libuavcan
#
set(fd_flags "-DUAVCAN_USE_FD")

file(GLOB_RECURSE LIBUAVCAN_CXX_FILES RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} "src/*.cpp")
add_library(uavcan STATIC ${LIBUAVCAN_CXX_FILES})
add_dependencies(uavcan libuavcan_dsdlc)
Expand All @@ -79,6 +81,14 @@ install(DIRECTORY include/dsdlc_generated/uavcan DESTINATION include) # Genera
install(CODE "execute_process(COMMAND ${PYTHON} setup.py install --record installed_files.log
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/dsdl_compiler)")

#
# libuavcan_fd
#
add_library(uavcan_fd STATIC ${LIBUAVCAN_CXX_FILES})
add_dependencies(uavcan_fd libuavcan_dsdlc)
set_target_properties(uavcan_fd PROPERTIES COMPILE_FLAGS ${fd_flags})
add_dependencies(uavcan_fd uavcan)

#
# Tests and static analysis - only for debug builds
#
Expand Down Expand Up @@ -119,9 +129,12 @@ if (DEBUG_BUILD)
if (COMPILER_IS_GCC_COMPATIBLE)
# No such thing as too many warnings
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror -pedantic -Wfloat-equal -Wconversion")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wsign-conversion -Wcast-align -Wmissing-declarations -Wlogical-op")
# These flags are GCC-specific
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wlogical-op -Wzero-as-null-pointer-constant")

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wsign-conversion -Wcast-align -Wmissing-declarations")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wdouble-promotion -Wswitch-enum -Wtype-limits -Wno-error=array-bounds")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wzero-as-null-pointer-constant -Wnon-virtual-dtor")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wnon-virtual-dtor")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Woverloaded-virtual -Wsign-promo -Wold-style-cast")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error=deprecated-declarations")
set(optim_flags "-O3 -DNDEBUG -g0")
Expand All @@ -135,11 +148,17 @@ if (DEBUG_BUILD)
add_library(uavcan_optim STATIC ${LIBUAVCAN_CXX_FILES})
set_target_properties(uavcan_optim PROPERTIES COMPILE_FLAGS ${optim_flags})
add_dependencies(uavcan_optim libuavcan_dsdlc)

add_library(uavcan_fd_optim STATIC ${LIBUAVCAN_CXX_FILES})
set_target_properties(uavcan_fd_optim PROPERTIES COMPILE_FLAGS "${fd_flags} ${optim_flags}")
add_dependencies(uavcan_fd_optim libuavcan_dsdlc)

if (GTEST_FOUND)
message(STATUS "GTest found, tests will be built and run.")
add_libuavcan_test(libuavcan_test uavcan "") # Default
add_libuavcan_test(libuavcan_test_optim uavcan_optim "${optim_flags}") # Max optimization
add_libuavcan_test(libuavcan_fd_test uavcan_fd "${fd_flags}") # Default FD
add_libuavcan_test(libuavcan_fd_test_optim uavcan_fd_optim "${optim_flags} ${fd_flags}") # Max optimization FD
else (GTEST_FOUND)
message(STATUS "GTest was not found, tests will not be built")
endif (GTEST_FOUND)
Expand Down
20 changes: 17 additions & 3 deletions libuavcan/include/uavcan/build_config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ namespace uavcan
/**
* Memory pool block size.
*
* The default of 64 bytes should be OK for any target arch up to AMD64 and any compiler.
* The default of 64 bytes should be OK for any target arch up to AMD64 and any compiler. We add 32 bytes when
* compiling for CAN-FD to allow for the larger buffers needed for that type of CAN bus.
*
* The library leverages compile-time checks to ensure that all types that are subject to dynamic allocation
* fit this size, otherwise compilation fails.
Expand All @@ -218,13 +219,27 @@ namespace uavcan
#ifdef UAVCAN_MEM_POOL_BLOCK_SIZE
/// Explicitly specified by the user.
static const unsigned MemPoolBlockSize = UAVCAN_MEM_POOL_BLOCK_SIZE;

/// MemPoolBlockSize default value is based on uavcan::CanTxQueue::Entry which was the largest known
/// object allocated in a pool in the codebase at the time.
#elif defined(__BIGGEST_ALIGNMENT__) && (__BIGGEST_ALIGNMENT__ <= 8)
/// Convenient default for GCC-like compilers - if alignment allows, pool block size can be safely reduced.
#if defined(UAVCAN_USE_FD)
static const unsigned MemPoolBlockSize = 88;
#else
/// Convenient default for GCC-like compilers - if alignment allows, pool block size can be safely reduced.
static const unsigned MemPoolBlockSize = 56;
#endif
#else
#if defined(UAVCAN_USE_FD)
/// Safe default that should be OK for any platform.
static const unsigned MemPoolBlockSize = 96;
#else
/// Safe default that should be OK for any platform.
static const unsigned MemPoolBlockSize = 64;
#endif
#endif


#ifdef __BIGGEST_ALIGNMENT__
static const unsigned MemPoolAlignment = __BIGGEST_ALIGNMENT__;
Expand All @@ -243,8 +258,7 @@ struct UAVCAN_EXPORT IsDynamicallyAllocatable
{
static void check()
{
char dummy[(sizeof(T) <= MemPoolBlockSize) ? 1 : -1] = { '0' };
(void)dummy;
static_assert((sizeof(T) <= MemPoolBlockSize), "Type cannot be allocated dynamically.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we are officially leaving C++03 behind?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember you merged feature/issue100 already. So the switch has been thrown. It's just a matter of cleaning up now and fully embracing the new language. As I stated before: we should not add more c++11 into this PR and instead add it in one feature at a time when I have a nuttx verification environment setup. So far we are using, and should continue to use, static_assert and unique_ptr (i.e. do not use auto_ptr any more).

To that end: what should I do to be able to verify nuttx compatibility with these changes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what should I do to be able to verify nuttx compatibility with these changes?

Let me invoke @davids5. David, how difficult would it be to set up a testing environment with NuttX on some kind of commonly available STM32H7 demo board like NUCLEO-H743ZI? @LorenzMeier can we get some assistance with that, please?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plain NuttX or PX4/NuttX?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plain NuttX preferred, if possible.

}
};

Expand Down
104 changes: 96 additions & 8 deletions libuavcan/include/uavcan/driver/can.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
#define UAVCAN_DRIVER_CAN_HPP_INCLUDED

#include <cassert>
#include <type_traits>
#include <uavcan/std.hpp>
#include <uavcan/build_config.hpp>
#include <uavcan/driver/system_clock.hpp>
#include <uavcan/util/bus.hpp>

namespace uavcan
{
Expand All @@ -18,6 +20,32 @@ namespace uavcan
*/
enum { MaxCanIfaces = 3 };

/**
* Valid message data codes.
*
* CAN DLCs are only 4-bits long so FD had to use 9-15 to encode
* various lengths up to 64 bits.
*/
enum class CanFrameDLC : uint_fast8_t {
CodeForLength0 = 0, /**< Data Length Code: 0 bytes */
CodeForLength1 = 1, /**< Data Length Code: 1 bytes */
CodeForLength2 = 2, /**< Data Length Code: 2 bytes */
CodeForLength3 = 3, /**< Data Length Code: 3 bytes */
CodeForLength4 = 4, /**< Data Length Code: 4 bytes */
CodeForLength5 = 5, /**< Data Length Code: 5 bytes */
CodeForLength6 = 6, /**< Data Length Code: 6 bytes */
CodeForLength7 = 7, /**< Data Length Code: 7 bytes */
CodeForLength8 = 8, /**< Data Length Code: 8 bytes */
CodeForLength12 = 9, /**< Data Length Code: 12 bytes */
CodeForLength16 = 10, /**< Data Length Code: 16 bytes */
CodeForLength20 = 11, /**< Data Length Code: 20 bytes */
CodeForLength24 = 12, /**< Data Length Code: 24 bytes */
CodeForLength32 = 13, /**< Data Length Code: 32 bytes */
CodeForLength48 = 14, /**< Data Length Code: 48 bytes */
CodeForLength64 = 15, /**< Data Length Code: 64 bytes */
invalid_code = 16 /**< Not a valid DLC. */
};

/**
* Raw CAN frame, as passed to/from the CAN driver.
*/
Expand All @@ -29,32 +57,92 @@ struct UAVCAN_EXPORT CanFrame
static const uint32_t FlagRTR = 1U << 30; ///< Remote transmission request
static const uint32_t FlagERR = 1U << 29; ///< Error frame

static const uint8_t MaxDataLen = 8;
static const uint8_t MaxDataLen = CanBusType::max_frame_size;

static CanFrameDLC length_to_dlc(uint_fast8_t length) {
/**
* Lookup table to map a CAN frame length to a DLC value
* that will accommodate the frame.
*/
constexpr uint8_t length_to_dlc_lookup[] = {
0,1,2,3,4,5,6,7,8,
9, 9, 9, 9,
10,10,10,10,
11,11,11,11,
12,12,12,12,
13,13,13,13,13,13,13,13,
14,14,14,14,14,14,14,14,14,14,14,14,14,14,14,14,
15,15,15,15,15,15,15,15,15,15,15,15,15,15,15,15
};

static_assert(std::extent<decltype(length_to_dlc_lookup)>::value == 65, "length_to_dlc_lookup table is malformed.");

if (length <= std::extent<decltype(length_to_dlc_lookup)>::value) {
return CanFrameDLC(length_to_dlc_lookup[length]);
} else {
return CanFrameDLC::invalid_code;
}
}

static uint_fast8_t dlc_to_length(CanFrameDLC dlc) {
/**
* Lookup table to map a DLC value to the maximum data
* payload length supported for the DLC.
*/
constexpr uint8_t dlc_to_length_lookup[] = {
0,1,2,3,4,5,6,7,8,12,16,20,24,32,48,64
};

static_assert(std::extent<decltype(dlc_to_length_lookup)>::value == 16, "dlc_to_length_lookup table is malformed.");

const auto dlc_value = static_cast<std::underlying_type<CanFrameDLC>::type>(dlc);
if (dlc_value <= std::extent<decltype(dlc_to_length_lookup)>::value) {
return dlc_to_length_lookup[dlc_value];
} else {
return 0;
}
}

uint32_t id; ///< CAN ID with flags (above)
uint8_t data[MaxDataLen];
uint8_t dlc; ///< Data Length Code

private:
CanFrameDLC dlc; ///< Data Length Code
public:

CanFrame() :
id(0),
dlc(0)
dlc(CanFrameDLC::CodeForLength0)
{
fill(data, data + MaxDataLen, uint8_t(0));
}

CanFrame(uint32_t can_id, const uint8_t* can_data, uint8_t data_len) :
CanFrame(uint32_t can_id, const uint8_t* can_data, CanFrameDLC in_dlc) :
id(can_id),
dlc((data_len > MaxDataLen) ? MaxDataLen : data_len)
dlc(in_dlc)
{
const uint_fast8_t data_len = dlc_to_length(dlc);
UAVCAN_ASSERT(can_data != UAVCAN_NULLPTR);
UAVCAN_ASSERT(data_len == dlc);
(void)copy(can_data, can_data + dlc, this->data);
UAVCAN_ASSERT(data_len <= MaxDataLen);
(void)copy(can_data, can_data + static_cast<uint8_t>(data_len), this->data);
}

CanFrameDLC getDLC() const {
return dlc;
}

void setDataLength(uint_fast8_t data_length) {
dlc = length_to_dlc(data_length);
}

uint_fast8_t getDataLength() const {
return dlc_to_length(dlc);
}

bool operator!=(const CanFrame& rhs) const { return !operator==(rhs); }
bool operator==(const CanFrame& rhs) const
{
return (id == rhs.id) && (dlc == rhs.dlc) && equal(data, data + dlc, rhs.data);
return (id == rhs.id) && (dlc == rhs.dlc) && equal(data, data + dlc_to_length(dlc), rhs.data);
}

bool isExtended() const { return id & FlagEFF; }
Expand Down
15 changes: 14 additions & 1 deletion libuavcan/include/uavcan/marshal/array.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <cmath>
#include <uavcan/error.hpp>
#include <uavcan/util/bitset.hpp>
#include <uavcan/util/bus.hpp>
#include <uavcan/util/templates.hpp>
#include <uavcan/build_config.hpp>
#include <uavcan/marshal/type_util.hpp>
Expand Down Expand Up @@ -426,11 +427,23 @@ class UAVCAN_EXPORT Array : public ArrayImpl<T, ArrayMode, MaxSize_>
typedef ArrayImpl<T, ArrayMode, MaxSize_> Base;
typedef Array<T, ArrayMode, MaxSize_> SelfType;

static bool isOptimizedTailArray(TailArrayOptimizationMode tao_mode)
public:
template<typename C = CanBusType>
static typename EnableIf<IsSameType<C, CanBusType2_0>::Result, bool>::Type
isOptimizedTailArray(TailArrayOptimizationMode tao_mode)
{
return (T::MinBitLen >= 8) && (tao_mode == TailArrayOptEnabled);
}

template<typename C = CanBusType>
static typename EnableIf<IsSameType<C, CanBusTypeFd>::Result, bool>::Type
isOptimizedTailArray(TailArrayOptimizationMode tao_mode)
{
(void)tao_mode;
return false;
}
private:

int encodeImpl(ScalarCodec& codec, const TailArrayOptimizationMode tao_mode, FalseType) const /// Static
{
UAVCAN_ASSERT(size() > 0);
Expand Down
67 changes: 66 additions & 1 deletion libuavcan/include/uavcan/marshal/type_util.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
#define UAVCAN_MARSHAL_TYPE_UTIL_HPP_INCLUDED

#include <uavcan/build_config.hpp>
#include <uavcan/util/bus.hpp>
#include <uavcan/util/templates.hpp>
#include <uavcan/util/comparison.hpp>
#include <array>

namespace uavcan
{
Expand Down Expand Up @@ -35,15 +37,78 @@ struct IntegerBitLen<0>
enum { Result = 0 };
};

// +--------------------------------------------------------------------------+
// | ByteLenToByteLenWithPadding
// +--------------------------------------------------------------------------+
/*
* DO NOT USE. Use ByteLenToByteLenWithPadding instead.
*/
template <unsigned ByteLen, bool isMultiFrame>
struct ByteLenToByteLenWithPaddingImpl;

/*
* DO NOT USE. Use ByteLenToByteLenWithPadding instead.
*/
template <unsigned ByteLen>
struct ByteLenToByteLenWithPaddingImpl<ByteLen, false>
{
enum { Result = ByteLen };
};

/*
* DO NOT USE. Use ByteLenToByteLenWithPadding instead.
*/
template <unsigned ByteLen>
struct ByteLenToByteLenWithPaddingImpl<ByteLen, true>
{
constexpr static unsigned amount_of_data_in_first_frame = uavcan::CanBusType::max_frame_size - 3;
constexpr static unsigned max_data_in_frames = (uavcan::CanBusType::max_frame_size - 1);
constexpr static unsigned amount_of_data_in_last_frame = (ByteLen - amount_of_data_in_first_frame) % max_data_in_frames;

static_assert(amount_of_data_in_last_frame >= 0, "SFINAE should have prevented us from getting here?");
static_assert(amount_of_data_in_last_frame <= 0xFF, "Tried to calculate padding for a frame that does not fit in a uint8_t");
static_assert(uavcan::CanBusType::max_frame_size <= CanBusType::max_frame_size, "Lookup table would overflow.");
static_assert(uavcan::CanBusType::max_frame_size >= 3, "Expected a canbus that has at least 3 bytes in a frame.");

enum {
Result = ByteLen + static_cast<unsigned>((CanBusType::payload_length_to_frame_length[amount_of_data_in_last_frame] - static_cast<uint8_t>(amount_of_data_in_last_frame)))
};
};

/**
* Compile-time: Returns the number of bytes needed to contain the given number of bytes
* where transfer padding must be accounted for.
*/
template <unsigned ByteLen>
struct ByteLenToByteLenWithPadding
{
enum {
Result = uavcan::ByteLenToByteLenWithPaddingImpl<ByteLen, (ByteLen > (uavcan::CanBusType::max_frame_size - 1))>::Result
};
};

/**
* Compile-time: Returns the number of bytes needed to contain the given number of bits. Assumes 1 byte == 8 bit.
* Run-time: Returns the number of padding bytes that would be added to a buffer
* for a given data payload length.
*/
unsigned calculatePaddingBytes(size_t payload_length);

// +--------------------------------------------------------------------------+

template <unsigned long BitLen>
struct BitLenToByteLen
{
enum { Result = (BitLen + 7) / 8 };
};

template <unsigned long BitLen>
struct BitLenToByteLenWithPadding
{
enum {
Result = uavcan::ByteLenToByteLenWithPadding<uavcan::BitLenToByteLen<BitLen>::Result>::Result
};
};

/**
* Compile-time: Helper for integer and float specialization classes. Returns the platform-specific storage type.
*/
Expand Down
Loading