Dependency updates / trivial fixes

.gitmodules

@@ -7,3 +7,6 @@
 [submodule "deps/fmt"]
 	path = deps/fmt
 	url = git@github.com:fmtlib/fmt.git
+[submodule "deps/Catch2"]
+	path = deps/Catch2
+	url = https://github.com/catchorg/Catch2.git

CMakeLists.txt

@@ -1,4 +1,4 @@
-CMAKE_MINIMUM_REQUIRED (VERSION 3.0 FATAL_ERROR)
+CMAKE_MINIMUM_REQUIRED (VERSION 3.5 FATAL_ERROR)
 
 if(COMMAND cmake_policy)
     cmake_policy(SET CMP0003 NEW)
@@ -10,7 +10,7 @@ endif(COMMAND cmake_policy)
 
 PROJECT (cpp-can-isotp)
 SET (CMAKE_CXX_FLAGS "-std=c++17 -Wall -fsanitize=address -fno-omit-frame-pointer" CACHE INTERNAL "cxx compiler flags")
-INCLUDE_DIRECTORIES("src" "deps/GSL/include" "deps/etl/include" "deps/fmt/include")
+INCLUDE_DIRECTORIES("src" "deps/GSL/include" "deps/etl/include" "deps/fmt/include" "deps/Catch2/single_include")
 add_subdirectory (test/example)
 add_subdirectory (test/unit-test)
 add_subdirectory (test/socket-test)

README.md

@@ -2,13 +2,13 @@
 This library implements [ISO 15765-2](https://en.wikipedia.org/wiki/ISO_15765-2) transport layer known also as CAN bus ISO TP or simply *Transport Protocol*. It was developed with microcontrollers in mind, but was also tested on a Linux box (although on a Linux system there is a [better option](https://github.com/hartkopp/can-isotp)). 
 
 ## Notes
-Whenever I refer to some cryptic paragraph numer in this document or in the source code I have **ISO 15765-2 First edition (2004-10-15)** on mind. This PDF id widely available in the Net though I'm not sure about lagality of this stuff, so I'm not including it in the repo. Normally an ISO document like that costs around $150.
+Whenever I refer to some cryptic paragraph numer in this document or in the source code I have **ISO 15765-2 First edition (2004-10-15)** on mind. This PDF id widely available in the Net though I'm not sure about legality of this stuff, so I'm not including it in the repo. Normally an ISO document like that costs around $150.
 
 This library is influenced by and was tested against [python-can-isotp](https://github.com/pylessard/python-can-isotp).
 
 # Using the library
 ## Building
-This library is header only, but you can build unit-tests and other simple examples, which can help you understand how to use the library in case of some trouble. This is a possible scenario of acheiving this:
+This library is header only, but you can build unit-tests and other simple examples, which can help you understand how to use the library in case of some trouble. This is a possible scenario of achieving this:
 
 ```sh
 git clone --recurse-submodules git@github.com:iwasz/cpp-can-isotp.git
@@ -31,6 +31,7 @@ Run-time dependencies (libraries bundled with the code in deps directory as git
 * C++17 (```if constexpr```).
 
 Unit tests
+* [Catch2](https://github.com/catchorg/Catch2)
 * [fmt](https://github.com/fmtlib/fmt)
 
 ## Using in your project
@@ -41,7 +42,7 @@ Just
 ```
 
 ## Instantiating
-First you have to instantiate the ```TransportProtocol``` class which encapsulates the protocol state. TransportProtocol is a class template and can be customized depending on underlying CAN-bus implementation, memory constraints, error (exception) handling sheme and time related stuff. This makes the API of TransportProtocol a little bit verbose, but also enables one to use it on a *normal* computer (see ```socket-test``` for Linux example) as well as on microcontrollers which this library was meant for at the first place. 
+First you have to instantiate the ```TransportProtocol``` class which encapsulates the protocol state. TransportProtocol is a class template and can be customized depending on underlying CAN-bus implementation, memory constraints, error (exception) handling sheme and time related stuff. This makes the API of TransportProtocol a little bit verbose, but also enables one to use it on a *normal* computer (see ```socket-test``` for Linux example) as well as on microcontrollers which this library was meant for at the first place.
 
 ```cpp
 #include "LinuxCanFrame.h"
@@ -69,7 +70,7 @@ listenSocket (socketFd, [&tp] (auto const &frame) { tp.onCanNewFrame (frame); })
 The code you see above is more or less all that's needed for **receiving** ISO-TP messages. In (1) we somehow connect to the underlying CAN-bus subsystem and then, using ```socketFd``` we are able to send and receive raw CAN-frames (see examples). 
 
 ## Callbacks
-Callback is the second parameter to ```create``` function, and it can have 3 different forms. The simplest (called *simple* througout this document and the source code) is :
+Callback is the second parameter to ```create``` function, and it can have 3 different forms. The simplest (called *simple* througout this document and the source code) is:
 
 ```cpp
 void indication (tp::IsoMessage const &msg) { /* ... */ }
@@ -78,7 +79,7 @@ void indication (tp::IsoMessage const &msg) { /* ... */ }
 auto tp = tp::create<can_frame> (tp::Address{0x789ABC, 0x123456}, indication, socketSend);
 ```
 
-This one implements ```N_USData.indication``` (see par. 5.2.4)  and gets called whan new ISO message was successfulty assembled, or in case of an error to inform the user, that current ISO message could not be assembled fully. In the latter case, the ```msg``` argument will be empty, but no other further information on the error will be available. Of course name ```indication``` is used here as an example, and even a lambda can be used (in fact in unit tests lambdas are used almost exclusively).
+This one implements ```N_USData.indication``` (see par. 5.2.4)  and gets called whan new ISO message was successfully assembled, or in case of an error to inform the user, that current ISO message could not be assembled fully. In the latter case, the ```msg``` argument will be empty, but no other further information on the error will be available. Of course name ```indication``` is used here as an example, and even a lambda can be used (in fact in unit tests lambdas are used almost exclusively).
 
 The next one is called an *advanced* callback, because it has more parameters:
 
@@ -89,8 +90,8 @@ void indication (tp::Address const &a, tp::IsoMessage const &msg, tp::Result res
 Meaning of the parameters is :
 
 1. Address ```a``` is the address of a peer who sent the message. Depending on the addressing scheme ```a.getTxId ()``` or ```a.getTargetAddress ()``` will be of interest to the user. See paragraph on addresses.
-1. Message ```msg``` is the ISO message (in case of a success) or empty if there was an error.
-1. Result ```res``` will have value ```Result::N_OK``` in case of a success or something other if case of a failed transmission.
+2. Message ```msg``` is the ISO message (in case of a success) or empty if there was an error.
+3. Result ```res``` will have value ```Result::N_OK``` in case of a success or something other if case of a failed transmission.
 
 Lastly there is *advancedMethod* or *full* callback (again, those are the terms used in the unit tests and througout the comments in the source) which not only implements ```N_USData.indication``` but also ```N_USData.confirm``` and N_USData_FF.indication:
 
@@ -145,9 +146,9 @@ ISO messages can be moved, copied or passed by reference_wrapper (std::ref) if m
 - [x] It is possible to define a TP object without a default address and then use its send method also without an address. This way you are sending a message into oblivion. Have it sorted out.
 - [ ] Get rid of all warinigs and c-tidy issues.
 - [x] Check if separationTime and blockSize received from a peer is taken into account during sending (check both sides of communication BS is faulty for sure, no flow frame is sent other than first one).
-- [x] blockSize is hardcoede to 8 for testiung purposes. Revert to 0.
+- [x] blockSize is hardcoded to 8 for testing purposes. Revert to 0.
 - [ ] If errors occur during multi frame message receiving, the isoMessage should be removed (eventually. Probably some timeouts are mentioned in the ISO). Now it is not possible to receive second message if first has failed to be received entirely.
-- [x] Check if retyurn value from sendFrame is taken into account .
+- [x] Check if return value from sendFrame is taken into account.
 - [x] Implement all types of addressing.
 - [ ] Use some better means of unit testing. Test time dependent calls, maybe use some clever unit testing library like trompeleoleil for mocking.
 - [x] Test crosswise connected objects. They should be able to speak to each other.

src/Address.h

@@ -16,10 +16,10 @@ static constexpr uint32_t MAX_29_ID = 0x1FFFFFFF;
 
 /**
  * This is the address class which represents ISO TP addresses. It lives in
- * slihtly higher level of abstraction than addresses describved in the ISO
+ * slightly higher level of abstraction than addresses described in the ISO
  * document (N_AI) as it use the same variables for all 7 address types (localAddress
  * and remoteAddress). This are "address encoders" which further convert this
- * "Address" to apropriate data inside Can Frames.
+ * "Address" to appropriate data inside Can Frames.
  */
 struct Address {
 
@@ -30,7 +30,7 @@ struct Address {
 
         /// N_TAtype Network Target Address Type
         enum class TargetAddressType : uint8_t {
-                PHYSICAL,  /// 1 to 1 communiaction supported for multiple and single frame communications
+                PHYSICAL,  /// 1 to 1 communication supported for multiple and single frame communications
                 FUNCTIONAL /// 1 to n communication (like broadcast?) is allowed only for single frame comms.
         };
 
@@ -234,10 +234,6 @@ struct Extended11AddressEncoder {
         {
                 auto fId = f.getId ();
 
-                if (fId > MAX_11_ID) {
-                        return {};
-                }
-
                 if (f.isExtended () || fId > MAX_11_ID || f.getDlc () < 1) {
                         return {};
                 }
@@ -279,10 +275,6 @@ struct Extended29AddressEncoder {
         {
                 auto fId = f.getId ();
 
-                if (fId > MAX_29_ID) {
-                        return {};
-                }
-
                 if (!f.isExtended () || fId > MAX_29_ID || f.getDlc () < 1) {
                         return {};
                 }

src/CanFrame.h

@@ -66,9 +66,9 @@ template <> class CanFrameWrapper<CanFrame> {
         void set (size_t i, uint8_t b) { gsl::at (frame.data, i) = b; }
 
 private:
-        // TODO this should be some kind of handler if we wantto call this class a "wrapper".
+        // TODO this should be some kind of handler if we want to call this class a "wrapper".
         // This way we would get rid of one copy during reception of a CAN frame.
-        // But at the other hand there must be a possibility to create new "wrappers" withgout
+        // But at the other hand there must be a possibility to create new "wrappers" without
         // providing the internal object when sending frames.
         CanFrame frame{};
 };

src/CppCompat.h

@@ -8,6 +8,10 @@
 
 #pragma once
 
+#include <etl/array.h>
+#include <etl/map.h>
+#include <etl/optional.h>
+
 /**
  * This file is for maintaining compatibility with systems where C++ standard library is
  * not available i.e. for Arduino. I was totally unaware, that there is no C++ standard
@@ -19,10 +23,6 @@
 #include <stddef.h>
 #include <stdint.h>
 
-#include <etl/array.h>
-#include <etl/map.h>
-#include <etl/optional.h>
-
 #if !defined Expects
 #define Expects(x) assert (x)
 #endif
@@ -64,6 +64,3 @@ template <typename _Tp> auto declval () noexcept -> decltype (__declval<_Tp> (0)
 #include <cstdint>
 #include <gsl/gsl>
 #endif
-
-#include <etl/map.h>
-#include <etl/optional.h>

src/LinuxCanFrame.h

@@ -13,7 +13,7 @@
 namespace tp {
 
 /**
- * Wrapper for particular can frame type we are using ini this program (namely
+ * Wrapper for particular can frame type we are using in this program (namely
  * can_frame in this case).
  */
 template <> class CanFrameWrapper<can_frame> {
@@ -68,11 +68,11 @@ template <> class CanFrameWrapper<can_frame> {
 
 /**
  * This interface assumes that sending single CAN frame is instant and we
- * now the status (whether it failed or succeeded) instantly (thus boolean)
+ * know the status (whether it failed or succeeded) instantly (thus boolean)
  * return type.
  *
- * Important! If this interface fails, it shoud return false. Also, ISO requires,
- * that sending a CAN message shoud take not more as 1000ms + 50% i.e. 1500ms. If
+ * Important! If this interface fails, it should return false. Also, ISO requires,
+ * that sending a CAN message should take not more as 1000ms + 50% i.e. 1500ms. If
  * this interface detects (internally) that, this time constraint wasn't met, it
  * also should return false. Those are N_As and N_Ar timeouts.
  */

src/MiscTypes.h

@@ -15,19 +15,19 @@ namespace tp {
  * Codes signaled to the user via indication and confirmation callbacks.
  */
 enum class Result {
-        // standared result codes
+        // standard result codes
         N_OK,           /// Service execution performed successfully.
         N_TIMEOUT_A,    /// Timer N_Ar / N_As exceeded N_Asmax / N_Armax
         N_TIMEOUT_BS,   /// N_Bs time exceeded N_Bsmax
         N_TIMEOUT_CR,   /// N_Cr time exceeded N_Crmax
         N_WRONG_SN,     /// Unexpected sequence number in incoming consecutive frame
-        N_INVALID_FS,   /// Invalid FlowStatus value received in flow controll frame.
+        N_INVALID_FS,   /// Invalid FlowStatus value received in flow control frame.
         N_UNEXP_PDU,    /// Unexpected protocol data unit received.
-        N_WFT_OVRN,     /// This signals thge user that
-        N_BUFFER_OVFLW, /// When receiving flow controll with FlowStatus = OVFLW. Transmission is aborted.
+        N_WFT_OVRN,     /// This signals the user that we have received too many wait frames while awaiting a flow control frame.
+        N_BUFFER_OVFLW, /// When receiving flow control with FlowStatus = OVFLW. Transmission is aborted.
         N_ERROR,        /// General error.
         // implementation defined result codes
-        N_MESSAGE_NUM_MAX /// Not a standard error. This one means that there is too many different isoMessages beeing assembled from multiple
+        N_MESSAGE_NUM_MAX /// Not a standard error. This one means that there is too many different isoMessages being assembled from multiple
                           /// chunks of CAN frames now.
 
 };

src/StlTypes.h

@@ -18,7 +18,7 @@ class CanFrame;
 
 /**
  * @brief The TimeProvider struct
- * TODO if I'm not mistaken, those classes ought to have 100µs resolutiuon instead of 1ms (1000µs).
+ * TODO if I'm not mistaken, those classes ought to have 100µs resolution instead of 1ms (1000µs).
  */
 struct ChronoTimeProvider {
         long operator() () const
@@ -35,7 +35,7 @@ struct CoutPrinter {
 };
 
 /**
- * Buffer for all input and ouptut operations
+ * Buffer for all input and output operations
  */
 using IsoMessage = std::vector<uint8_t>;