Skip to content

can.LogReader error on opening "filename.with.too.many.dots.blf" #1210

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

Closed
ghost opened this issue Jan 13, 2022 · 9 comments · Fixed by #1221
Closed

can.LogReader error on opening "filename.with.too.many.dots.blf" #1210

ghost opened this issue Jan 13, 2022 · 9 comments · Fixed by #1221
Labels
Milestone

Comments

@ghost
Copy link

ghost commented Jan 13, 2022

Describe the bug

in python3 repl

>> can.LogReader("./2021.11.23.test.blf") 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/mnt/c/Users/username/Documents/various/python-can/can/io/player.py", line 87, in __new__
    raise ValueError(
ValueError: No read support for this unknown log format ".11.23.test.blf"

To Reproduce

Open a log file with more then one period in it.

Expected behavior

The file should be opened as a blf file
If the file is renamed to "test.blf" the conversion works properly

Additional context

OS and version: Ubuntu20.04 in WSL Linux TL0213507 4.4.0-19041-Microsoft #1237-Microsoft Sat Sep 11 14:32:00 PST 2021 x86_64 x86_64 x86_64 GNU/Linux
Python version: python 3.8.10
python-can version: python develop commit 2e24af0

@Tbruno25
Copy link
Contributor

I can take this issue.

@Tbruno25
Copy link
Contributor

Tbruno25 commented Jan 16, 2022

I'm unable to reproduce on Ubuntu 20.04.3 LTS.

Maybe this only occurs when using WSL? Unfortunately I don't have a Windows system available to test.

edit: my mistake, was on main not develop

@felixdivo
Copy link
Collaborator

felixdivo commented Jan 16, 2022

I can reproduce it on Python 3.10 @ Win 10 (not WSL, but classic installation) on develop. I think the problem is here:

suffix = "".join(s.lower() for s in pathlib.PurePath(filename).suffixes)

We should probably just replace it with suffix = pathlib.PurePath(filename).suffix.

@felixdivo
Copy link
Collaborator

In any case, we should add a regression test for this. Strange that this never occurred ...

@felixdivo felixdivo added the file-io about reading & writing to files label Jan 16, 2022
@felixdivo
Copy link
Collaborator

Incidently, that was the case some while ago, and apparently it was changed ... (I found this PR (#895) while compiling the new changelog.)

Git blame revealed that #1138 eventually changed it in order to detect files like *.asc.gz. Seems like that change was not tested with this (not so corner) case.

So maybe the fix is not as simple, but it should still be fairly straightforward to fix, I guess.

@felixdivo
Copy link
Collaborator

We could replace GzipASCReader by a GzipReader that just consumes the last suffix and then uses LogReader internally again to determine the reader once compression was removed (from the file and file name). This would actually be a nice feature since other readers would be supported too.

@Tbruno25
Copy link
Contributor

@felixdivo took a first pass at this in #1221 -- let's continue the discussion there.

@felixdivo
Copy link
Collaborator

Thanks @Tbruno25 for tackling this!

@pierreluctg
Copy link
Collaborator

This bug this I have introduced 👎 is also affecting the LogWritter.

See https://github.com/hardbyte/python-can/blob/develop/can/io/logger.py#L88

@felixdivo felixdivo added this to the 4.0.0 Release milestone Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants