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

Support for multiple .dbc files #20

Closed
taylorjdlee opened this issue Jan 3, 2023 · 4 comments
Closed

Support for multiple .dbc files #20

taylorjdlee opened this issue Jan 3, 2023 · 4 comments
Labels
help wanted Extra attention is needed question Further information is requested wontfix This will not be worked on

Comments

@taylorjdlee
Copy link
Contributor

taylorjdlee commented Jan 3, 2023

Is there a chance we can look into being able to read multiple .dbc files and store them inside one Dbc object?
Currently Parser.cs doesn't have a function that achieves this https://github.com/EFeru/DbcParser/blob/main/DbcParserLib/Parser.cs

Below I've made a work around of sort but it's not optimal. If we could possibly look into this that would be great thanks!

string[] fileNames = Directory.GetFiles(DBC_FILES_PATH, "*.dbc");
File.WriteAllLines(DBC_PATH, fileNames.SelectMany(file => File.ReadLines(file)));
dbc = Parser.ParseFromPath(DBC_PATH);

@EFeru
Copy link
Owner

EFeru commented Jan 3, 2023

Hi,
Just wondering if it can be done at the user side using a List:

List<Dbc> DbcList = new List<Dbc>();
foreach (var f in files)
     DbcList.Add(Parser.ParseFromPath(f));

And then you can use the Dbc list for your purpose. What is your opinion?

@Adhara3
Copy link
Collaborator

Adhara3 commented Jan 14, 2023

Hi,

I think you have two ways to achieve your goal.

Empty multiple DBC into one

Easy, not optimal in performance, but can be achieved with existing objects

public Dbc CreateFromMultiplePaths(IEnemerable<string> fileNames )
{
  var nodes = new List<Node>();
  var messages = new List<Message>();

  foreach(var path in filenames)
  {
    var dbc = Parser.ParseFromPath(path);
    nodes.AddRange(dbc.Nodes);
    messages.AddRange(dbc.Messages);
  }

  return new Dbc(nodes, messages);
}

This could be made parallel very easily by using Parallel.ForEach instead and accordingly use concurrent containers instead of List for adding Node and Message.
Another advantage is that this thing is a native Dbc as such can be sent around with no issues hiding the fact that it was built from a single or a multi-file.

Composite object

This is similar to @EFeru solution, which works, you could create a new class called CompositeDbc.

public class CompositeDbc
{
  private readonly IEnumerable<Dbc> m_innerDbcs;

  public IEnumerable<Node> Nodes => m_innerDbcs.SelectMany(x => x.Nodes);
  public IEnumerable<Message> Messages => m_innerDbcs.SelectMany(x => x.Messages);

  public CompositeDbc(IEnumerable<Dbc> dbcs)
  {
    m_innerDbcs = dbcs;
  }
}
public CompositeDbc CreateFromMultiplePaths(IEnemerable<string> fileNames )
{
  // Remember to materialize to avoid re-parsing at each iteration...
  var dbcs = fileNames.Select(x => Parser.ParseFromPath(x)).ToArray();

  return new CompositeDbc(dbcs);
}

Again this could be built in parallel, but this solution has the advantage of not allocating anything more than what is required by single dbcs.
The cons here is that CompositeDbc is a different class. However, it shares Dbc interface.

I don't know if I like the idea of adding one of those into the code.

  • First we would need to do this for paths but also for the other overloads
  • As of today, Dbc just contains a couple of list so can be easily built from multiple. I am not sure this will last. For example you may need to keep track of which Dbc a node come from. So leaving to the user custom needs makes sense.

A compromise could be: we create an extension method to merge several Dbcs into one for now. This would leave the user the freedom to load them with the method they prefer but we take care of aggregating. Something like

public Dbc Merge(this IEnemerable<Dbc> dbcs )
{
  var nodes = new List<Node>();
  var messages = new List<Message>();

  foreach(var dbc in dbcs)
  {
    nodes.AddRange(dbc.Nodes);
    messages.AddRange(dbc.Messages);
  }

  return new Dbc(nodes, messages);
}

public void Main()
{
  var composite = Directory
                   .GetFiles(DBC_FILES_PATH, "*.dbc")
                   .Select(x => Parser.ParseFromPath(x))
                   .Merge();
}

Of course this extension method is for you a third solution now, if you like.
A

@taylorjdlee
Copy link
Contributor Author

taylorjdlee commented Jan 14, 2023

Thanks @Adhara3 this is a great write up and I'll add these changes to my code. I'll probably go for the first option as performance isn't our priority (we merge all the .dbc files at startup). This is being bookmarked for sure haha

@Adhara3
Copy link
Collaborator

Adhara3 commented Jan 14, 2023

The last is equivalent but a bit more elegant ;)

A couple of notes about the code you are using now

  • ReadAllLines is not efficient way to read a file and with multiple big files may allocate too much
  • You do not need to write the merged file, there is an overload to parse DBC from an in memory string
  • This method assumes that the parser is not strict with order. E.g usually a DBC starts with a version, in you case a version is found anywhere in the file. In the future the parser may be more strict on the order in which it expects tags to appear in which case physically merging DBC text may no longer work

Cheers
A

@Adhara3 Adhara3 added question Further information is requested wontfix This will not be worked on labels Jan 15, 2023
@Adhara3 Adhara3 closed this as not planned Won't fix, can't repro, duplicate, stale Jan 15, 2023
@Adhara3 Adhara3 added the help wanted Extra attention is needed label Jan 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants