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.

Sign up for GitHub

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

Jump to bottom

Basic parsing for extended multiplexing #76

Closed
wants to merge 9 commits into from
Closed

Basic parsing for extended multiplexing #76

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d1d3cf9
Basic parsing for extended multiplexing
Uight Aug 27, 2024
4f7969d
Merge remote-tracking branch 'upstream/main' into ExtendedMultiplexin…
Uight Aug 28, 2024
565466b
Solve merge conflict
Uight Aug 28, 2024
42b056c
Merge remote-tracking branch 'upstream/main' into ExtendedMultiplexin…
Uight Aug 29, 2024
15fe8db
Fix merge conflict in unittest
Uight Aug 29, 2024
def6bc3
cleanup tests
Uight Aug 29, 2024
37036f0
(not final) add dedicated objects for parsing.
Uight Aug 29, 2024
02fdd1e
Small change
Uight Aug 29, 2024
3a665dc
probably a better way to parse multiplexing (Test not running but idc…
Uight Aug 30, 2024
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
124 changes: 124 additions & 0 deletions DbcParserLib.Tests/ExtendedMultiplexingParserTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
using System.Linq;
using NUnit.Framework;


namespace DbcParserLib.Tests
{
internal class ExtendedMultiplexingParserTests
{
[Test]
public void ParseSignalExtendedMultiplexingCase1()
{
var dbcString = @"
BO_ 2024 OBD2: 8 Vector__XXX
SG_ S1_PID_0D_VehicleSpeed m13 : 31|8@0+ (1,0) [0|255] ""km/h"" Vector__XXX
SG_ S1_PID_11_ThrottlePosition m17 : 31|8@0+ (0.39216,0) [0|100] ""%"" Vector__XXX
SG_ S1 m1M : 23|8@0+ (1,0) [0|255] """" Vector__XXX
SG_ Service M : 11|4@0+ (1,0) [0|15] """" Vector__XXX

SG_MUL_VAL_ 2024 S1_PID_0D_VehicleSpeed S1 13-13;
SG_MUL_VAL_ 2024 S1_PID_11_ThrottlePosition S1 17-17;
SG_MUL_VAL_ 2024 S1 Service 1-1;";


var dbc = Parser.Parse(dbcString);

Assert.That(dbc.Messages.Count(), Is.EqualTo(1));

var message = dbc.Messages.First();

Assert.That(message.Signals, Has.Count.EqualTo(4));

var signal1 = message.Signals.FirstOrDefault(x => x.Name.Equals("S1_PID_0D_VehicleSpeed"));
var signal2 = message.Signals.FirstOrDefault(x => x.Name.Equals("S1_PID_11_ThrottlePosition"));
var signal3 = message.Signals.FirstOrDefault(x => x.Name.Equals("S1"));
var signal4 = message.Signals.FirstOrDefault(x => x.Name.Equals("Service"));

Assert.Multiple(() =>
{
Assert.That(signal1?.ExtendedMultiplexing, Is.Not.Null);
Assert.That(signal1?.ExtendedMultiplexing.MultiplexorSignal, Is.EqualTo("S1"));
Assert.That(signal1?.ExtendedMultiplexing.MultiplexingRanges, Has.Count.EqualTo(1));
Assert.That(signal1?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.IsRange, Is.EqualTo(false));
Assert.That(signal1?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.Lower, Is.EqualTo(13));
Assert.That(signal1?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.Upper, Is.EqualTo(13));

Assert.That(signal2?.ExtendedMultiplexing, Is.Not.Null);
Assert.That(signal2?.ExtendedMultiplexing.MultiplexorSignal, Is.EqualTo("S1"));
Assert.That(signal2?.ExtendedMultiplexing.MultiplexingRanges, Has.Count.EqualTo(1));
Assert.That(signal2?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.IsRange, Is.EqualTo(false));
Assert.That(signal2?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.Lower, Is.EqualTo(17));
Assert.That(signal2?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.Upper, Is.EqualTo(17));

Assert.That(signal3?.ExtendedMultiplexing, Is.Not.Null);
Assert.That(signal3?.ExtendedMultiplexing.MultiplexorSignal, Is.EqualTo("Service"));
Assert.That(signal3?.ExtendedMultiplexing.MultiplexingRanges, Has.Count.EqualTo(1));
Assert.That(signal3?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.IsRange, Is.EqualTo(false));
Assert.That(signal3?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.Lower, Is.EqualTo(1));
Assert.That(signal3?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.Upper, Is.EqualTo(1));

Assert.That(signal4, Is.Not.Null);
Assert.That(signal4?.ExtendedMultiplexing, Is.Null);
});
}

[Test]
public void ParseSignalExtendedMultiplexingCase2()
{
var dbcString = @"
BO_ 100 MuxMsg: 1 Vector__XXX
SG_ Mux_4 m2 : 6|2@1+ (1,0) [0|0] """" Vector__XXX
SG_ Mux_3 m3M : 4|2@1+ (1,0) [0|0] """" Vector__XXX
SG_ Mux_2 m3M : 2|2@1+ (1,0) [0|0] """" Vector__XXX
SG_ Mux_1 M : 0|2@1+ (1,0) [0|0] """" Vector__XXX

SG_MUL_VAL_ 100 Mux_2 Mux_1 3-3, 5-10;
SG_MUL_VAL_ 100 Mux_3 Mux_2 3-3;
SG_MUL_VAL_ 100 Mux_4 Mux_3 2-2;";


var dbc = Parser.Parse(dbcString);

Assert.That(dbc.Messages.Count(), Is.EqualTo(1));

var message = dbc.Messages.First();

Assert.That(message.Signals, Has.Count.EqualTo(4));

var signal1 = message.Signals.FirstOrDefault(x => x.Name.Equals("Mux_1"));
var signal2 = message.Signals.FirstOrDefault(x => x.Name.Equals("Mux_2"));
var signal3 = message.Signals.FirstOrDefault(x => x.Name.Equals("Mux_3"));
var signal4 = message.Signals.FirstOrDefault(x => x.Name.Equals("Mux_4"));

Assert.Multiple(() =>
{
Assert.That(signal1, Is.Not.Null);
Assert.That(signal1?.ExtendedMultiplexing, Is.Null);

Assert.That(signal2?.ExtendedMultiplexing, Is.Not.Null);
Assert.That(signal2?.ExtendedMultiplexing.MultiplexorSignal, Is.EqualTo("Mux_1"));
Assert.That(signal2?.ExtendedMultiplexing.MultiplexingRanges, Has.Count.EqualTo(2));
Assert.That(signal2?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.IsRange, Is.EqualTo(false));
Assert.That(signal2?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.Lower, Is.EqualTo(3));
Assert.That(signal2?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.Upper, Is.EqualTo(3));
Assert.That(signal2?.ExtendedMultiplexing.MultiplexingRanges.LastOrDefault()?.IsRange, Is.EqualTo(true));
Assert.That(signal2?.ExtendedMultiplexing.MultiplexingRanges.LastOrDefault()?.Lower, Is.EqualTo(5));
Assert.That(signal2?.ExtendedMultiplexing.MultiplexingRanges.LastOrDefault()?.Upper, Is.EqualTo(10));

Assert.That(signal3?.ExtendedMultiplexing, Is.Not.Null);
Assert.That(signal3?.ExtendedMultiplexing.MultiplexorSignal, Is.EqualTo("Mux_2"));
Assert.That(signal3?.ExtendedMultiplexing.MultiplexingRanges, Has.Count.EqualTo(1));
Assert.That(signal3?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.IsRange, Is.EqualTo(false));
Assert.That(signal3?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.Lower, Is.EqualTo(3));
Assert.That(signal3?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.Upper, Is.EqualTo(3));

Assert.That(signal4?.ExtendedMultiplexing, Is.Not.Null);
Assert.That(signal4?.ExtendedMultiplexing.MultiplexorSignal, Is.EqualTo("Mux_3"));
Assert.That(signal4?.ExtendedMultiplexing.MultiplexingRanges, Has.Count.EqualTo(1));
Assert.That(signal4?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.IsRange, Is.EqualTo(false));
Assert.That(signal4?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.Lower, Is.EqualTo(2));
Assert.That(signal4?.ExtendedMultiplexing.MultiplexingRanges.FirstOrDefault()?.Upper, Is.EqualTo(2));
});
}
}
}
12 changes: 12 additions & 0 deletions DbcParserLib/DbcBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,18 @@ public void AddSignalValueType(uint messageId, string signalName, DbcValueType v
m_observer.SignalNameNotFound(messageId, signalName);
}

public void AddSignalExtendedMultiplexingInfo(uint messageId, string signalName, ParsingExtendedMultiplexing extendedMultiplexing)
{
if (TryGetValueMessageSignal(messageId, signalName, out var signal))
{
signal.ExtendedMultiplexing = extendedMultiplexing;
}
else
{
m_observer.SignalNameNotFound(messageId, signalName);
}
}

public void AddNodeComment(string nodeName, string comment)
{
var node = m_nodes.FirstOrDefault(n => n.Name.Equals(nodeName));
Expand Down
1 change: 1 addition & 0 deletions DbcParserLib/IDbcBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ internal interface IDbcBuilder
void AddSignal(Signal signal);
void AddSignalComment(uint messageId, string signalName, string comment);
void AddSignalValueType(uint messageId, string signalName, DbcValueType valueType);
void AddSignalExtendedMultiplexingInfo(uint messageId, string signalName, ParsingExtendedMultiplexing extendedMultiplexing);
void LinkNamedTableToSignal(uint messageId, string signalName, string tableName);
void LinkTableValuesToSignal(uint messageId, string signalName, IReadOnlyDictionary<int, string> dictValues);
void LinkTableValuesToEnvironmentVariable(string variableName, IReadOnlyDictionary<int, string> dictValues);
Expand Down
16 changes: 16 additions & 0 deletions DbcParserLib/Model/MultiplexingRange.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
namespace DbcParserLib.Model
{
public class MultiplexingRange
{
public bool IsRange { get; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about expanding the valid values?
So that 1-1, 5-7 is exploded into [1, 5, 6, 7].
I was wondering if that would simplify, after all range or not, from a decoding point of view, we only need to read a Muxer value and pick the right layout based on that value.

Just reasoning

public uint Lower { get; }
public uint Upper { get; }

public MultiplexingRange(uint lower, uint upper)
{
Lower = lower;
Upper = upper;
IsRange = lower != upper;
}
}
}
16 changes: 16 additions & 0 deletions DbcParserLib/Model/ParsingExtendedMultiplexing.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using System.Collections.Generic;

namespace DbcParserLib.Model
{
internal class ParsingExtendedMultiplexing
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's say we have instead:

internal class ParsingGeneralMultiplexing
{
  public MultiplexingRole Role{ get; set; }

  /* 
       For muxers the below is always null
       For standard mux info, the following is null at the end of the parsing but can be filled in the finish up
       phase by picking the only muxer expected in the message (if more than one, parsing error). Moreover for standard 
       the multiplexer value is filled with a single element, what is called group in the multiplexing info
       For advanced mux multiplexor signal is filled and multiplexer values are replaced (SG_ parsed values should be 
       overwritten)
  */
  public string MultiplexorSignal { get; set;}
  public ISet<uint> MultiplexerValue { get; } 

  // May be handy for checks in the wrap up phase. Should not be reported in the immutable public object
  public bool Extended {get; set;}
}

So, simple one

 BO_ 2348875519 SimpleMX_Message: 5 Vector__XXX
   SG_ S7 : 36|4@1+ (1,0) [0|0] "" Vector__XXX
   SG_ S5 m4: 4|24@1- (1,0) [0|0] "" Vector__XXX
   SG_ S4 m3: 4|24@1- (1,0) [0|0] "" Vector__XXX
   SG_ S3 m2: 20|16@1- (1,0) [0|0] "" Vector__XXX
   SG_ S2 m2: 4|16@1- (1,0) [0|0] "" Vector__XXX
   SG_ S6 m1: 2|32@1- (1,0) [0|0] "" Vector__XXX
   SG_ S0_m M : 0|2@1+ (1,0) [0|0] "" Vector__XXX

Phase 1: SG parsing

S7 will have role None, S0_m role Multiplexer, the other role Multiplexed and multiplexer value 1 | 2 | 3 | 4.

Wrap up

In the FinishUp or WrapUp phase, S0_m is forced into MultiplexerSignal for all Multiplexed for that message since only one M is present

The extended

BO_ 2348875518 ExtMX_Message: 5 Vector__XXX
  SG_ S7 : 36|4@1+ (1,0) [0|0] "" Vector__XXX
  SG_ S5 m6 : 4|24@1- (1,0) [0|0] "" Vector__XXX
  SG_ S4 m5 : 4|24@1- (1,0) [0|0] "" Vector__XXX
  SG_ S3 m4 : 20|16@1- (1,0) [0|0] "" Vector__XXX
  SG_ S2 m4 : 4|16@1- (1,0) [0|0] "" Vector__XXX
  SG_ S6 m1 : 2|32@1- (1,0) [0|0] "" Vector__XXX
  SG_ S1_m m0M : 2|2@1+ (1,0) [0|0] "" Vector__XXX
  SG_ S0_m M : 0|2@1+ (1,0) [0|0] "" Vector__XXX

SG_MUL_VAL_ 2348875518 S5 S1_m 6-6;
SG_MUL_VAL_ 2348875518 S4 S1_m 5-5;
SG_MUL_VAL_ 2348875518 S3 S1_m 4-4;
SG_MUL_VAL_ 2348875518 S2 S1_m 4-4;
SG_MUL_VAL_ 2348875518 S6 S0_m 1-1;
SG_MUL_VAL_ 2348875518 S1_m S0_m 0-0, 2-2;

This becomes a 3-phase stuff.

Phase 1: SG parsing

As the standard one, values will contain numbers extracted from standard muxing (i.e. m5, m0M) since at this stage we do not know if SG_MUL_VAL_ will be present.

Phase 2: SG_MUL_VAL_

We set the muxor name and we clear and fill values with ranges exploded.
If needed we can set Extended to true

Wrap up

Names are filled, so no action needed.

Layouts

For unpacking this whole information should be converted into something more handy that I would call layouts and look like what CANdb++ is showing. The idea is that not the user, but the message itself can unpack a byte[] of data autodetencting the layout by reading muxer values and using the right layout to read.
So, using the above example:

Read S0_m value. 
  if 1, then the layout will include S0_m, S7 and S6
  If 0 or 2, the read S1_m
    If 4 layout is S0_m, S7, S1_m, S2 and S3
    If 5 layout is S0_m, S7, S1_m, S4
    If 6 layout is S0_m, S7, S1_m, S5

I like this abstraction because the procedure above can be set up by the lib itself in the wrap up phase so having even more info that what we want to expose in the public API.
Moreover a layout il nothing but a virtual message definition. A multiplexed message can be exploded into several message definitions with no muxed signals in it, the logic part is how to retrieve the correct one given a byte[] but that should not be rocket science, it's just a matter of ordering the muxers and reding values.

These are more or less my ideas, I hope that they are clearer with this example.

Cheers
A

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Best way forward for me would be if i wait for the proof of concept for immutability go thorugh as i would have to do pretty much the same stuff as in there to achieve that and rebase this after that.

main resons being:
i would like to also have the extension method for Multiplexing Info internal;
i need the message when building signal multiplexing;
The current code would be easy to merge as its mainly new files anyway

Some remarks on your comment:
"by picking the only muxer expected in the message (if more than one, parsing error)" => not sure how you would do that. You want to have a real exception or expose the parsing observer to the builder functions or create a builder observer? My idea would have been to set everything to unknown if that happens witch unknow marking an error but in addition you could still create an entry in the observer.
"public ISet MultiplexerValue { get; } " you expect this to contain one element in case of normal multiplexing and 2*n elements for n ranges in extended multiplexing?
=> signal with multiplexing "m2M" is parsed then it would conatin "2" and if then a extended mutiplexing is found specififying ranges 2-10, 15-15 you would replace it with "2","10","15","15"?

To assign the single multiplexor to any signal (in simple multiplexing mode) i still need some strange logic i dont like. the message parses signals one by one and each signals builds it multiplexing info. Either pre parse some info that can then be using in calculating the multiplexing info or add the value later

for the packer stuff i agree i would probably reverse the multiplexing tree and always start with the multiplexors and see which signals they apply to.

Copy link
Collaborator

@Adhara3 Adhara3 Aug 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Best way forward for me would be if i wait for the proof of concept for immutability go thorugh as i would have to do pretty much the same stuff as in there to achieve that and rebase this after that.

Yep. If we are fine with the immutability poc I can merge that. That will have some API changes, but if we think it's the first step....

not sure how you would do that. You want to have a real exception or expose the parsing observer to the builder functions or create a builder

Generally speaking, if the dbc is flawed, we should fail parsing. I prefer that that say all good and then decode the wrong stuff. I would not throw, but use the parsing observer

=> signal with multiplexing "m2M" is parsed then it would conatin "2" and if then a extended mutiplexing is found specififying ranges 2-10, 15-15 you would replace it with "2","10","15","15"?

no, would replace with {2, 3, 4, 5, 6, 7, 8, 9, 10, 15}

To assign the single multiplexor to any signal (in simple multiplexing mode) i still need some strange logic i dont like. the message parses signals one by one and each signals builds it multiplexing info. Either pre parse some info that can then be using in calculating the multiplexing info or add the value later

I do not like that logic either but that's a dbc format limitation, it's not indexed. Moreover, in parsing reading the file is most expensive thing, so pre parsing is weird the same way. The disconfort here is minimal and if we keep the pain in the parsing phase then fixing in the wrap up phase we have to safe places for the checks and the public item is fine, which is the most important thing. With the SG_ parsing it's intrinsic that the multiplexer is unknown until the message parsing is over (for simple mux) and until the file is over (for advanced).

Anyway, the way we handle parsing and wrap up can be changed anytime without affecting the user, so it doesn't need to be perfect out of the box. As long as we provide a consistent user API, how we manage our internals can be changed any time so to me it is a bit lower priority to agree on a solution, especially for details.

A

{
public string MultiplexorSignal { get; }
public List<MultiplexingRange> MultiplexingRanges { get; }

public ParsingExtendedMultiplexing(string multiplexorSignal, List<MultiplexingRange> multiplexingRanges)
{
MultiplexorSignal = multiplexorSignal;
MultiplexingRanges = multiplexingRanges;
}
}
}
5 changes: 3 additions & 2 deletions DbcParserLib/Model/Signal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ internal class ImmutableSignal
public string[] Receiver { get; }
public IReadOnlyDictionary<int, string> ValueTableMap { get; }
public string Comment { get; }
public string Multiplexing { get; }
public MultiplexingInfo Multiplexing { get; }
public IReadOnlyDictionary<string, CustomProperty> CustomProperties { get; }

internal ImmutableSignal(Signal signal)
Expand All @@ -42,7 +42,7 @@ internal ImmutableSignal(Signal signal)
Receiver = signal.Receiver;
ValueTableMap = signal.ValueTableMap;
Comment = signal.Comment;
Multiplexing = signal.Multiplexing;
Multiplexing = signal.MultiplexingInfo();
CustomProperties = signal.CustomProperties;
}
}
Expand All @@ -66,6 +66,7 @@ public class Signal
public IReadOnlyDictionary<int, string> ValueTableMap = new Dictionary<int, string>();
public string Comment;
public string Multiplexing;
internal ParsingExtendedMultiplexing ExtendedMultiplexing;
public Message Parent;
public readonly Dictionary<string, CustomProperty> CustomProperties = new Dictionary<string, CustomProperty>();
public double InitialValue
Expand Down
1 change: 1 addition & 0 deletions DbcParserLib/Observers/IParseFailureObserver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public interface IParseFailureObserver
void PropertyDefaultSyntaxError();
void SignalSyntaxError();
void SignalValueTypeSyntaxError();
void SignalExtendedMultiplexingError();
void ValueTableDefinitionSyntaxError();
void ValueTableSyntaxError();
void SignalNameNotFound(uint messageId, string signalName);
Expand Down
4 changes: 4 additions & 0 deletions DbcParserLib/Observers/SilentFailureObserver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ public void SignalValueTypeSyntaxError()
{
}

public void SignalExtendedMultiplexingError()
{
}

public void ValueTableDefinitionSyntaxError()
{
}
Expand Down
5 changes: 5 additions & 0 deletions DbcParserLib/Observers/SimpleFailureObserver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ public void SignalValueTypeSyntaxError()
AddError("[SIG_VALTYPE_] Signal value type syntax error");
}

public void SignalExtendedMultiplexingError()
{
AddError("[SG_MUL_VAL_] Signal extended multiplexing syntax error");
}

public void ValueTableDefinitionSyntaxError()
{
AddError("[VAL_TABLE_] Value table definition syntax error");
Expand Down
1 change: 1 addition & 0 deletions DbcParserLib/Parser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ private static void CreateLineParsers()
new PropertiesLineParser(m_parseObserver),
new EnvironmentVariableLineParser(m_parseObserver),
new EnvironmentDataVariableLineParser(m_parseObserver),
new ExtendedMultiplexingLineParser(m_parseObserver),
new UnknownLineParser(m_parseObserver) // Used as a catch all
};
}
Expand Down
62 changes: 62 additions & 0 deletions DbcParserLib/Parsers/ExtendedMultiplexingLineParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
using DbcParserLib.Model;
using DbcParserLib.Observers;
using System;
using System.Collections.Generic;
using System.Text.RegularExpressions;

namespace DbcParserLib.Parsers
{
internal class ExtendedMultiplexingLineParser : ILineParser
{
private const string SignalLineStarter = "SG_MUL_VAL_ ";
private const string SignalRegex = @"SG_MUL_VAL_\s+(\d+)\s+(\S+)\s+(\S+)\s+((?:\d+-\d+,?\s*)+);?";

private readonly IParseFailureObserver m_observer;

public ExtendedMultiplexingLineParser(IParseFailureObserver observer)
{
m_observer = observer;
}

public bool TryParse(string line, IDbcBuilder builder, INextLineProvider nextLineProvider)
{
if (!line.TrimStart().StartsWith(SignalLineStarter))
{
return false;
}

var match = Regex.Match(line, SignalRegex);

if (match.Success)
{
var multiplexorSignal = match.Groups[3].Value;
var multiplexorRanges = match.Groups[4].Value;

var rangesArray = multiplexorRanges.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);

var rangesParsed = new List<MultiplexingRange>();

foreach (var range in rangesArray)
{
var rangeClean = range.Trim();
var numbers = rangeClean.Split('-');

var lower = uint.Parse(numbers[0]);
var upper = uint.Parse(numbers[1]);

rangesParsed.Add(new MultiplexingRange(lower, upper));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, just wondering

var triggerValues = new List<uint>();

foreach (var range in rangesArray)
{
    var rangeClean = range.Trim();
    var numbers = rangeClean.Split('-');

    var lower = uint.Parse(numbers[0]);
    var upper = uint.Parse(numbers[1]);
    
    // Maybe check if upper >= lower?
    rangesParsed.AddRange(Enumerable.Range(lower, upper - lower + 1));
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

theres so much that could be checked here:

  1. Upper is higher or than lower
  2. the mux value in the signal definition (m2M) is in the ranges?
  3. Is the every mux value possible from the muxor?
  4. is the muxor a integer? (or bool which is also an integer in dbc scope)
  5. duplicates?

I would go with this:

  1. check if upper is higher then lower or else parsing error?
  2. use something like a try add to ignore duplicates (not sure atm but ISet is a hashset ? should have a try add just like dictionary)
  3. add the value from the signal (m15M) <= 15 in this case too.

for the final message parsing having ranges sucks anyway it way slower than checking i hashset on contains.


var extendendMultiplexing = new ParsingExtendedMultiplexing(multiplexorSignal, rangesParsed);

builder.AddSignalExtendedMultiplexingInfo(uint.Parse(match.Groups[1].Value), match.Groups[2].Value, extendendMultiplexing);
}
else
{
m_observer.SignalExtendedMultiplexingError();
}

return true;
}
}
}
Loading