Trc file support

[pkess]

trafficstars

With this i added basic support for the TRC file format as logging for can messages. There is only a very basic support at the moment but comments are welcome on what file versions might be relevant and if the basic code looks good

[codecov[bot]]

Codecov Report

Merging #1217 (c51ba62) into develop (4b1acde) will increase coverage by 0.79%. The diff coverage is 94.80%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1217      +/-   ##
===========================================
+ Coverage    65.16%   65.96%   +0.79%     
===========================================
  Files           81       82       +1     
  Lines         8853     9084     +231     
===========================================
+ Hits          5769     5992     +223     
- Misses        3084     3092       +8     

[zariiii9003]

@Mergifyio rebase

[mergify[bot]]

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (1/109)
Auto-merging .travis.yml
CONFLICT (content): Merge conflict in .travis.yml
error: could not apply 2a30f25... update wheel package before deploy
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 2a30f25... update wheel package before deploy

err-code: A920D

[zariiii9003]

@pkess can you rebase or start from the latest develop branch?

[pkess]

@pkess can you rebase or start from the latest develop branch?

Hello @zariiii9003, sure i can rebase all those commits. But can you give me some reasons for the rebase process? IMO the original history of the commits is relevant and not any rebased history which may lead to commits that are not working as other files changed.

I will push a merge commit with the current develop branch.

[pkess]

I fixed some mypy stuff now, but it looks like i have to add some work here. Can you tell me why mypy is showing errors her for the asc parser as well? I did not intentionally add changes here so they may exist in develop branch?

[zariiii9003]

I fixed some mypy stuff now, but it looks like i have to add some work here. Can you tell me why mypy is showing errors her for the asc parser as well? I did not intentionally add changes here so they may exist in develop branch?

Ignore the asc errors. They will be fixed after #1221 is merged.

[zariiii9003]

@pkess May i resolve the conflicts?

[pkess]

@pkess May i resolve the conflicts?

Would be great. I will try to resolve the mypy errors in the evening.

[zariiii9003]

Are trc-files with version 1.* still widely used? I think it makes sense to drop the old ones and instead focus on properly supporting versions 2.*. Or at least we should split 1.* and 2.* into subclasses.

The current implementation of 2.* ignores most message types, does not consider optional columns and is generally far from ready to merge.

[pkess]

Well, i think the most widely used file version is 1.1. As it was used by default when using the pcan tools installed until 2020 or something i think (not sure).

For any reason the version 1.3 is very similar to version 2 format. I would like to implement this all in one. At least the reader, as this should handle every file ending with trc

[pkess]

And yes the support for version 2 is very poor at the moment and i had to implement some more on this but it is not ready for publish at the moment. Any suggestion on how to roll out the features?

[pkess]

Hello, i updated the writer now to support text io and enabled the unittests for this again. Now i would really like to implement some class and subclass procedure for the support of multiple versions, but i am not really sure how to do this. The class type has to be defined before opening the file. The actual file version can only be determined by the content of the file. So maybe i have to add an extra "worker" class where the exact subclass will be used. Or do you have any other idea on this?

[felixdivo]

Version 4.0.0 of python-can is out and since no major problems were brought up, I think new features like this can be merged.

[hardbyte]

@zariiii9003 up to you if you'd like to merge this now to include in 4.1 release.

[felixdivo]

I think we are a bit slow with merging new large features. There are many open PRs for new interfaces that we did not react to. Maybe we should actively find someone who is willing to review these larger PRs in particular.

[hardbyte]

Oh totally agree that the volunteers including myself are not keeping up. Any ideas how to recruit people to carry out code reviews though? I have thought about approaching some of the very large companies using python-can to contribute time or money. I wouldn't feel right taking donated money myself for this project (I'm barely active enough to call myself a maintainer!), but I also don't have the time to interview (and likely supervise) someone.

[felixdivo]

Yeah, It's the same with me. Some occasional comments here and there are currently fine, but not proper maintenance.

Maybe we can do the low-effort think and pin an issue stating that we are looking for people to help? Maybe ask for people to do something like peer-review?

Orthogonal to that, it might be a good idea to reconsider which interfaces (+ file types) we want to support? Do we want to add any interface even if only 5 people will ever use it? Or do we want some relevance of the interface? While it is appealing to "catch them all 🎶", it does add a lot of

1. review and development burden on all, and
2. some significant maintenance effort, especially when changing base implementations, helper classes, adding typing, changes to the documentation, ... and makes the code base more heterogeneous each time.

[felixdivo]

@pkess This is not to say that your file type is irrelevant! This discussion just happens to take place here. ;)

[zariiii9003]

I'm not opposed to merging this if @pkess is available to fix bug reports and complaints. However, this should be mentioned in the docs.

[zariiii9003]

Orthogonal to that, it might be a good idea to reconsider which interfaces (+ file types) we want to support? Do we want to add any interface even if only 5 people will ever use it? Or do we want some relevance of the interface?

We should start telling people to use the plugin interface/entry point. If we merge all the interfaces, then this lib will become an unmaintainable mess. We'll never be able to make API changes if we have to update 30 interfaces without access to the hardware.

[felixdivo]

Let's continue the discussion in #1431

[pkess]

Hello all, Thank you for this reminder. I was not able to work on this for a longer time now. From my personal point of view the trc file format is the most relevant in connection with agricultural machines (this is my normal job). So I would really like to get this merged. Otherwise it might also be good to add this as plugin. But until now I did not know that there is some plug in architecture with this package. Can you point some information for this?

[felixdivo]

Well, the discussion above (about plugins) is generally less about the file I/O like this one but more about what we call "interfaces" (connections to actual buses like socketcan, vector, pcan, ...). This certainly was the wrong place to kick it off. 😁 It is still good to motivate this PR tough!

We could also extend the discussion to file I/O, but there I see less of a problem. These code parts are most often simpler (less concurrency, no linking to binary code) and most of the time easy to test. It is therefore easier to maintain.

I'll comment on that in the linked issue.

[zariiii9003]

Just for completeness, we do have entry points for io, but the documentation is lacking there: https://github.com/hardbyte/python-can/pull/783

[pkess]

So if you aggree i will continue to work on this. Maybe for the very first support of trc files you can leave comment to this code so i can fix them to all relevant requirements?

[j-c-cook]

From my personal point of view the trc file format is the most relevant in connection with agricultural machines (this is my normal job).

Interesting. Vector is the dominant application used for opening CAN data logs by my colleagues (agriculture as well). Therefore, I typically utilize the blf format. This is not to discount your comment, I just think your perspective is interesting.

[pkess]

From my personal point of view the trc file format is the most relevant in connection with agricultural machines (this is my normal job).

Interesting. Vector is the dominant application used for opening CAN data logs by my colleagues (agriculture as well). Therefore, I typically utilize the blf format. This is not to discount your comment, I just think your perspective is interesting.

Yes, vector is the other big player in this environment. Peak with trc files is some kind of low cost solution and vector is the premium solution. I don't know exactly all companies but I know companies using only peak I know some only vector and I know some using both

[hardbyte]

I'm going to merge this and cut a pre-release of 4.1.0

[hardbyte]

Eek actually I might rebase and check the tests pass first.