Integrate CANTools Log File Readers

[Bokonon79]

On M3OC, @brian-man suggested using the log-readers in his CANTools project as a more complete solution for loading CAN logs in a variety of formats.

Based on what I currently see in that project, the only major addition would be the ability to stream a log file line-by-line (i.e., adding something like a LoadNext() method). With that capability, it should be straightforward to instantiate the appropriate log-reader at the beginning of the import and then read one message per iteration of the timerCallback() loop.

[brian-man]

Why is this needed? Why not just load all at once and then iterate through the Data enumerable (see DataWithColumns) at whatever rate you want?

I'm not saying "no" to the per-line capability (callback or whatever) but it doesn't seem to be required, right?

[brian-man]

@Bokonon79
Let me know if this (DataColumns.Read) covers what you need:
https://brianman.visualstudio.com/_git/CANTools/commit/2a1bba820dbf207b5594ea92d3e712fe96c120e7?refName=refs%2Fheads%2Fu%2Fbrianman%2FLoadIncremental

I'll push to master but I figured it would be good to have you test it first.

[Bokonon79]

Why is this needed? Why not just load all at once and then iterate through the Data enumerable (see DataWithColumns) at whatever rate you want?

I could be prematurely optimizing, but my reasoning was this: longer captures are going to be several hundred MBs or a few GBs in size. If we were to read the entire file into memory all at once, only to pass it to the timerCallback() loop one line at a time, we'd end up consuming ~twice the memory by the end of the load, which could become a problem on a multi-GB file. Since both DataWithColumns and CANBUS-Analyzer's timerCallback() loop are already built around line-by-line iteration, it seemed like a natural solution to connect the two with minimal effort.

Let me know if this (DataColumns.Read) covers what you need:

I could be missing something, but just from looking at the commit, I didn't see where the callback was actually invoked.... Does something like this need to be added to Read() at line 199?

if (callback != null) 
{
    callback(row);
}

I'll push to master but I figured it would be good to have you test it first.

Sounds good. I've added CANTools/u/brianman/LoadIncremental as a submodule on my development branch and will try it out later today (adding the code above if needed).

[brian-man]

Good catch on the missing callback(row). That pointed out a bug in my unit test. The unit test bug is now fixed in master and u/brianman/LoadIncremental has been rebased onto the new master and had the fix applied. Sorry for the inconvenience.

[Bokonon79]

No apologies necessary, thanks for making those changes! I've updated the submodule in my main dev branch and will try wiring it up this weekend.

[Bokonon79]

Sorry for the absence... I've got the CANTools log readers integrated in my CANToolsReaders branch, along with a bug fix for #26.

To get the log readers to work, I ended up making three small changes to DataWithColumns in a cloned copy of CANTools/LoadIncremental

1. Extract ReadHeaders() and ReadNext() methods from Read():
   Bokonon79/CANTools@ae7a4fd

2. Add support for multi-space delimited files:
   Bokonon79/CANTools@f32163b

@brian-man, I didn't see a way to submit a pull request to your CANTools repo on Azure DevOps, but if I could, it would contain the above two commits. Could you please have a look at them when you get a chance, and consider merging them into the LoadIncremental branch? Thanks.

[brian-man]

Initial feedback:

• ReadHeaders doesn't need to be public.
• What's the benefit of adding ReadNext, and why is it public?

Perhaps I'm missing something. Can you point me to your branch's commit in CANBUS-Analyzer the uses CANLib?

[brian-man]

Also, I don't like multi-space because it's lossy -- which means it breaks, for example, round-trip serialization. What benefit is it giving you?