Extend ISignal::EMultiplexer, validation for advanced multiplexing, documentation

Author: Andreas Ringlstetter

include/dbcppp/Message.h

@@ -50,6 +50,9 @@ namespace dbcppp
         virtual const std::string& Comment() const = 0;
         virtual const ISignalGroup& SignalGroups_Get(std::size_t i) const = 0;
         virtual uint64_t SignalGroups_Size() const = 0;
+        /// \brief Optional multiplexor signal when this message is using simple multiplexing with a single switch.
+        ///
+        /// For advanced multiplexing support, ISignal::SignalMultiplexerValues needs to be followed instead.
         virtual const ISignal* MuxSignal() const = 0;
 
         DBCPPP_MAKE_ITERABLE(IMessage, MessageTransmitters, std::string);

include/dbcppp/Signal.h

@@ -25,11 +25,17 @@ namespace dbcppp
             MaschinesFloatEncodingNotSupported = 1,
             MaschinesDoubleEncodingNotSupported = 2,
             SignalExceedsMessageSize = 4,
-            WrongBitSizeForExtendedDataType = 8
+            WrongBitSizeForExtendedDataType = 8,
+            ConflictingMultiplexDefinition = 16,
         };
         enum class EMultiplexer
         {
-            NoMux, MuxSwitch, MuxValue
+            NoMux,
+            MuxSwitch = 1,
+            MuxValue = 2,
+            // For advanced multiplexing with ISignalMultiplexerValue, nesting is possible.
+            // This results in signals being both a multiplexor switch and a multiplexed value.
+            MuxSwitchAndValue = MuxSwitch | MuxValue
         };
         enum class EByteOrder
         {
@@ -44,7 +50,7 @@ namespace dbcppp
         {
             Integer, Float, Double
         };
-        
+
         static std::unique_ptr<ISignal> Create(
               uint64_t message_size
             , std::string&& name
@@ -65,13 +71,21 @@ namespace dbcppp
             , std::string&& comment
             , EExtendedValueType extended_value_type
             , std::vector<std::unique_ptr<ISignalMultiplexerValue>>&& signal_multiplexer_values);
-            
+
         virtual std::unique_ptr<ISignal> Clone() const = 0;
 
         virtual ~ISignal() = default;
         virtual const std::string& Name() const = 0;
         virtual EMultiplexer MultiplexerIndicator() const = 0;
+        /// \brief Single multiplexor value for simple multiplexing.
+        ///
+        /// Invalid if ISignal::SignalMultiplexerValues is not empty.
         virtual uint64_t MultiplexerSwitchValue() const = 0;
+        /// \brief Least significant bit of the signal.
+        ///
+        /// \note DBC uses the least signficant bit as start bit for both big and little endian.
+        ///       This means that for big endian, the StartBit points into the highest byte.
+        ///       This differs from FIBEX/Autosar where big endian signals are instead using the least significant bit in the lowest byte as offset.
         virtual uint64_t StartBit() const = 0;
         virtual uint64_t BitSize() const = 0;
         virtual EByteOrder ByteOrder() const = 0;
@@ -121,6 +135,12 @@ namespace dbcppp
         DBCPPP_MAKE_ITERABLE(ISignal, Receivers, std::string);
         DBCPPP_MAKE_ITERABLE(ISignal, ValueEncodingDescriptions, IValueEncodingDescription);
         DBCPPP_MAKE_ITERABLE(ISignal, AttributeValues, IAttribute);
+
+        /// \brief Mapping of this multiplexed signal to specific value ranges of a selected multiplexor switch signal.
+        /// 
+        /// In a valid DBC, this can only have 0 or 1 entries.
+        /// Requires EMultiplexer::MuxValue to be set in MultiplexerIndicator in order to be valid.
+        /// If empty, simple multiplexing rules by ISignal::MultiplexerSwitchValue and IMessage::MuxSignal apply instead.
         DBCPPP_MAKE_ITERABLE(ISignal, SignalMultiplexerValues, ISignalMultiplexerValue);
 
     protected:

include/dbcppp/SignalMultiplexerValue.h

@@ -6,12 +6,12 @@
 #include <memory>
 #include <vector>
 #include <functional>
-#include <cstdint>
 
 #include "Iterator.h"
 
 namespace dbcppp
 {
+    // Map from values of the multiplexor signal ISignalMultiplexerValue::SwitchName to the parent multiplexed ISignal.
     class ISignalMultiplexerValue
     {
     public:

src/DBCAST2Network.cpp

@@ -351,16 +351,20 @@ static auto getSignals(const G_Network& gnet, const G_Message& m, Cache const& c
         uint64_t multiplexer_switch_value = 0;
         if (s.multiplexer_indicator)
         {
-            auto m = *s.multiplexer_indicator;
-            if (m.substr(0, 1) == "M")
+            const auto& m = *s.multiplexer_indicator;
+            if (auto mux_value_pos = m.find('m', 0); mux_value_pos != std::string::npos)
             {
-                multiplexer_indicator = ISignal::EMultiplexer::MuxSwitch;
+                multiplexer_indicator = ISignal::EMultiplexer(
+                    std::underlying_type_t<ISignal::EMultiplexer>(multiplexer_indicator) |
+                    std::underlying_type_t<ISignal::EMultiplexer>(ISignal::EMultiplexer::MuxValue));
+                multiplexer_switch_value = std::atoi(m.c_str() + mux_value_pos + 1);
             }
-            else
+            if (m.find('M', 0) != std::string::npos)
             {
-                multiplexer_indicator = ISignal::EMultiplexer::MuxValue;
-                std::string value = m.substr(1, m.size());
-                multiplexer_switch_value = std::atoi(value.c_str());
+                multiplexer_indicator =
+                    ISignal::EMultiplexer(
+                        std::underlying_type_t<ISignal::EMultiplexer>(multiplexer_indicator) |
+                        std::underlying_type_t<ISignal::EMultiplexer>(ISignal::EMultiplexer::MuxSwitch));
             }
         }
 

src/MessageImpl.cpp

@@ -67,22 +67,68 @@ MessageImpl::MessageImpl(
     , _mux_signal(nullptr)
     , _error(EErrorCode::NoError)
 {
-    bool have_mux_value = false;
+    bool have_mux_values = false;
+    size_t num_mux_switches = 0;
+    bool have_mux_values_without_advanced_mapping = false;
     for (const auto& sig : _signals)
     {
-        switch (sig.MultiplexerIndicator())
+        if (std::underlying_type_t<ISignal::EMultiplexer>(sig.MultiplexerIndicator()) &
+            std::underlying_type_t<ISignal::EMultiplexer>(ISignal::EMultiplexer::MuxValue))
         {
-        case ISignal::EMultiplexer::MuxValue:
-            have_mux_value = true;
-            break;
-        case ISignal::EMultiplexer::MuxSwitch:
-            _mux_signal = &sig;
-            break;
+            have_mux_values = true;
+            if (sig.SignalMultiplexerValues_Size() == 0)
+            {
+                have_mux_values_without_advanced_mapping = true;
+            }
+            else
+            {
+                for (const auto& multiplexor : sig.SignalMultiplexerValues())
+                {
+                    if (!std::any_of(_signals.begin(), _signals.end(), [&multiplexor](const SignalImpl& sig)
+                        { return sig.Name() == multiplexor.SwitchName(); }))
+                    {
+                        _error = EErrorCode::MuxValeWithoutMuxSignal;
+                    }
+                }
+            }
+        }
+        if (std::underlying_type_t<ISignal::EMultiplexer>(sig.MultiplexerIndicator()) &
+            std::underlying_type_t<ISignal::EMultiplexer>(ISignal::EMultiplexer::MuxSwitch))
+        {
+            ++num_mux_switches;
         }
     }
-    if (have_mux_value && _mux_signal == nullptr)
+    if (num_mux_switches > 1)
     {
-        _error = EErrorCode::MuxValeWithoutMuxSignal;
+        if (have_mux_values_without_advanced_mapping)
+        {
+            _error = EErrorCode::MuxValeWithoutMuxSignal;
+        }
+    }
+    // MuxSignal property can be populated for simple multiplexing only.
+    else if (num_mux_switches == 1)
+    {
+        for (const auto& sig : _signals)
+        {
+            if (uint64_t(sig.MultiplexerIndicator()) & uint64_t(ISignal::EMultiplexer::MuxSwitch))
+            {
+                _mux_signal = &sig;
+            }
+        }
+        for (const auto& sig : _signals)
+        {
+            if (uint64_t(sig.MultiplexerIndicator()) & uint64_t(ISignal::EMultiplexer::MuxValue))
+            {
+                _mux_signal = &sig;
+            }
+        }
+    }
+    else if (num_mux_switches == 0)
+    {
+        if (have_mux_values)
+        {
+            _error = EErrorCode::MuxValeWithoutMuxSignal;
+        }
     }
 }
 MessageImpl::MessageImpl(const MessageImpl& other)

src/Network2DBC.cpp

@@ -499,10 +499,19 @@ DBCPPP_API std::ostream& dbcppp::Network2DBC::operator<<(std::ostream& os, const
 DBCPPP_API std::ostream& dbcppp::Network2DBC::operator<<(std::ostream& os, const ISignal& s)
 {
     os << "\tSG_ " << s.Name() << " ";
-    switch (s.MultiplexerIndicator())
+    if (s.MultiplexerIndicator() != ISignal::EMultiplexer::NoMux)
     {
-    case ISignal::EMultiplexer::MuxSwitch: os << "M "; break;
-    case ISignal::EMultiplexer::MuxValue: os << "m" << s.MultiplexerSwitchValue() << " "; break;
+        if (std::underlying_type_t<ISignal::EMultiplexer>(s.MultiplexerIndicator()) &
+            std::underlying_type_t<ISignal::EMultiplexer>(ISignal::EMultiplexer::MuxValue))
+        {
+            os << "m" << s.MultiplexerSwitchValue();
+        }
+        if (std::underlying_type_t<ISignal::EMultiplexer>(s.MultiplexerIndicator()) &
+            std::underlying_type_t<ISignal::EMultiplexer>(ISignal::EMultiplexer::MuxSwitch))
+        {
+            os << "M";
+        }
+        os << " ";
     }
     os << ": " << s.StartBit() << "|" << s.BitSize() << "@";
     switch (s.ByteOrder())

src/SignalImpl.cpp

@@ -517,6 +517,22 @@ SignalImpl::SignalImpl(
         _phys_to_raw = ::phys_to_raw<double>;
         break;
     }
+    // Permitted due to faulty interface design in ISignal only - doesn't happen with real DBC files.
+    if (_signal_multiplexer_values.size() > 1)
+    {
+        SetError(EErrorCode::ConflictingMultiplexDefinition);
+    }
+    else if (_signal_multiplexer_values.size() == 1)
+    {
+        const auto ranges = _signal_multiplexer_values[0].ValueRanges();
+        if (!std::any_of(std::begin(ranges), std::end(ranges), [&](const ISignalMultiplexerValue::Range& range) -> bool
+            {
+                return range.from <= _multiplexer_switch_value && range.to >= _multiplexer_switch_value;
+            }))
+        {
+            SetError(EErrorCode::ConflictingMultiplexDefinition);
+        }
+    }
 }
 std::unique_ptr<ISignal> SignalImpl::Clone() const
 {
@@ -612,11 +628,11 @@ uint64_t SignalImpl::SignalMultiplexerValues_Size() const
 }
 bool SignalImpl::Error(EErrorCode code) const
 {
-    return code == _error || (uint64_t(_error) & uint64_t(code));
+    return code == _error || (std::underlying_type_t<EErrorCode>(_error) & std::underlying_type_t<EErrorCode>(code));
 }
 void SignalImpl::SetError(EErrorCode code)
 {
-    _error = EErrorCode(uint64_t(_error) | uint64_t(code));
+    _error = EErrorCode(std::underlying_type_t<EErrorCode>(_error) | std::underlying_type_t<EErrorCode>(code));
 }
 bool SignalImpl::operator==(const ISignal& rhs) const
 {