check_all_frames working better for LittleEndian

Author: Avi SZYCHTER

src/analysis/CANFrameAnalysis.cpp

@@ -25,6 +25,9 @@ struct SignalLayoutEntry {
     }
 
     SignalLayoutEntry(const SignalLayoutEntry&) = default;
+    SignalLayoutEntry& operator=(const SignalLayoutEntry&) = default;
+    SignalLayoutEntry(SignalLayoutEntry&&) = default;
+    SignalLayoutEntry& operator=(SignalLayoutEntry&&) = default;
 
     const CANSignal* src_signal;
     SignalRanges ranges;
@@ -65,9 +68,21 @@ SignalRanges little_endian_ranges(const CANSignal& src) {
     // which does not really represent anything in terms of layout. So we need
     // to compute the ranges for each byte individually anyway.
 
-    unsigned byteAnalyzed = 0;
+    unsigned bitsLeft = src.length();
     bool is_start_byte = 0;
 
+    for(unsigned current_byte = src.start_bit() / 8; bitsLeft > 0; current_byte++, is_start_byte = false) {
+        unsigned lbit = 7 -( src.start_bit() + bitsLeft);
+        unsigned rbit = 7 - src.start_bit();
+
+        // The static_cast are not "necessary" but it removes some warnings
+        result.push_back({ static_cast<uint8_t>(current_byte), 
+                           static_cast<uint8_t>(lbit),
+                           static_cast<uint8_t>(rbit) });
+
+        bitsLeft -= (rbit - lbit);
+    }
+
     return result;
 }
 
@@ -91,32 +106,34 @@ std::vector<SignalLayoutEntry> compute_layout(const CANFrame& src) {
     return result;
 }
 
+bool overlap(const SignalLayoutEntry& e1, const SignalLayoutEntry& e2) {
+  for(const SignalRange& r1 : e1.ranges) {
+    for(const SignalRange& r2: e2.ranges) {
+      // Find if r2 shares a SignalRange with the same byte with r1 
+      if(r1.byte != r2.byte)
+          continue;
+        
+      // Now we know that the SignalRange(s) share a common byte
+        
+      // ordered.first is the leftmost SignalRange in the byte
+      // ordered.second is the rightmost SignalRange in the byte
+      auto ordered = std::minmax(r1, r2, [](const SignalRange& r, const SignalRange& rr) {
+          return r.lr_start_bit < rr.lr_start_bit;
+      });
+
+      // No overlapping if the last bit of the leftmost is before the first
+      // bit of the rightmost.
+      if(ordered.first.lr_end_bit < ordered.second.lr_start_bit)
+        return true;
+    }
+  }
+
+  return false;
+}
+
 bool CppCAN::analysis::is_frame_layout_ok(const CANFrame& src) {
     auto layout = compute_layout(src);
 
-    auto overlap = [](const SignalLayoutEntry& e1, const SignalLayoutEntry& e2) -> bool {
-        return std::any_of(e1.ranges.begin(), e1.ranges.end(), [&e2](const SignalRange& r1) {
-            // Find if r2 shares a SignalRange with the same byte with r1 
-            auto r2 = std::find_if(e2.ranges.begin(), e2.ranges.end(), [&r1](const SignalRange& e_range) {
-                return r1.byte == e_range.byte;
-            });
-
-            // The signals are on completely different bytes
-            if(r2 == e2.ranges.end())
-                return false;
-
-            // ordered.first is the leftmost SignalRange in the byte
-            // ordered.second is the rightmost SignalRange in the byte
-            auto ordered = std::minmax(r1, *r2, [](const SignalRange& r, const SignalRange& rr) {
-                return r.lr_start_bit < rr.lr_start_bit;
-            });
-
-            // No overlapping if the last bit of the leftmost is before the first
-            // bit of the rightmost.
-            return ordered.first.lr_end_bit < ordered.second.lr_start_bit;
-        });
-    };
-
     for(size_t i = 0; i < layout.size(); i++) {
         const SignalLayoutEntry& e = layout[i];
 

utils/can-parse/can-parse.cpp

@@ -7,8 +7,6 @@
 #include "operations.h"
 #include <iomanip>
 
-using namespace CppCAN::can_parse;
-
 enum CanParseAction {
   None,
   PrintAll,
@@ -102,6 +100,8 @@ std::tuple<CanParseAction, std::string, uint32_t> extractAction(int argc, char**
 }
 
 int main(int argc, char** argv) {
+  using namespace CppCAN::can_parse;
+
   std::string src_file;
   CanParseAction action;
   uint32_t detail_frame;
@@ -121,9 +121,10 @@ int main(int argc, char** argv) {
   }
 
   CANDatabase db;
+  std::vector<CANDatabase::parsing_warning> warnings;
 
   try {
-     db = std::move(CANDatabase::fromFile(src_file));
+     db = std::move(CANDatabase::fromFile(src_file, &warnings));
   }
   catch (const CANDatabaseException& e) {
     std::cerr << "An error happened while parsing the database: " 
@@ -149,7 +150,7 @@ int main(int argc, char** argv) {
     break;
 
     case CheckAll:
-      check_all_frames(db);
+      check_all_frames(db, warnings);
       break;
 
     case Help:

utils/can-parse/check-frame.cpp

@@ -3,18 +3,32 @@
 #include "CANDatabase.h"
 #include <algorithm>
 
-void CppCAN::can_parse::check_all_frames(CANDatabase& db) {
+void CppCAN::can_parse::check_all_frames(CANDatabase& db, const std::vector<CANDatabase::parsing_warning>& warnings) {
   std::vector<uint32_t> ids;
-    for(const auto& frame : db) {
-      if(!CppCAN::analysis::is_frame_layout_ok(frame.second))
-        ids.push_back(frame.second.can_id());
+
+  if(warnings.size() > 0) {
+    std::cout << "The following warnings have been detected during parsing: " << std::endl;
+    for(const auto& w : warnings) {
+      std::cout << "(Line " << w.line + 1 << ") " << w.description << std::endl;
     }
+    std::cout << std::endl;
+  }
+
+
+  for(const auto& frame : db) {
+    if(!CppCAN::analysis::is_frame_layout_ok(frame.second))
+      ids.push_back(frame.second.can_id());
+  }
+
+  if(ids.size() == 0)
+    std::cout << "No layout issue have been found in the CAN database." << std::endl;
+  else {
+    std::cout << "Some layout have been found in the database for the following frames: ";
+    std::cout << std::hex << std::showbase;
 
-    if(ids.size() == 0)
-      std::cout << "No layout issue have been found in the CAN database." << std::endl;
-    else {
-      std::cout << "Some layout have been found in the database for the following frames: ";
-      std::cout << std::hex << std::showbase;
-      std::for_each(ids.begin(), ids.end(), [](uint64_t id) { std::cout << id << ", "; });
+    for(size_t i = 0; i < ids.size() - 1; i++) {
+      std::cout << ids[i] << ", ";
     }
+    std::cout << ids.back() << std::endl;
+  }
 }

utils/can-parse/operations.h

@@ -4,8 +4,8 @@
 #include <cstdint>
 #include <string>
 #include <stdexcept>
-
-class CANDatabase;
+#include <vector>
+#include "CANDatabase.h"
 
 namespace CppCAN {
 namespace can_parse {
@@ -22,7 +22,7 @@ namespace can_parse {
     /**
      * @brief Checks the validity of all the frames of the database
      */
-    void check_all_frames(CANDatabase& db);
+    void check_all_frames(CANDatabase& db, const std::vector<CANDatabase::parsing_warning>& warnings);
 
     /**
      * Exception thrown by any operation if an error happens during their