Skip to content

Fix the compressed BLFWriter #1379

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

Closed
wants to merge 9 commits into from
Closed

Fix the compressed BLFWriter #1379

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9259ec0
Quick fix to make blf.gz writer functional
j-c-cook Aug 17, 2022
a911256
Merge branch 'develop' into Fixblfgz
j-c-cook Aug 17, 2022
5b9b49a
Fix rollover name creation
j-c-cook Aug 17, 2022
8129805
Fix formatting with black
j-c-cook Aug 22, 2022
4ed0a17
Clean up _default_name changes
j-c-cook Aug 22, 2022
28e5405
Compressed logger now acknowledges append kwarg
j-c-cook Aug 23, 2022
ac664ca
For gzip.GzipFile, don't call seek twice
j-c-cook Aug 23, 2022
876dacb
Format with black
j-c-cook Aug 23, 2022
f64b6e7
Hotfix blf.gz log by temporarily removing dflt header
j-c-cook Aug 24, 2022
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
15 changes: 8 additions & 7 deletions can/io/blf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,11 +583,12 @@ def file_size(self) -> int:
def stop(self):
"""Stops logging and closes the file."""
self._flush()
if self.file.seekable():
filesize = self.file.tell()
# Write header in the beginning of the file
self.file.seek(0)
self._write_header(filesize)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zariiii9003 The call self.file.seek(0) results in an OSError. It appears that the code block is not required if write mode wb is used (rather than ab). However, if the append option is requested, then some of this idea may be required.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The header contains things that are only known after the log has been written, so that's why we need to go back and write it later. Previously we closed and reopened the file to update it, but it seemed unnecessary if it was possible to seek instead.

else:
LOG.error("Could not write BLF header since file is not seekable")
# Note: This does not appear to be necessary if the append option is not being used.
# if self.file.seekable():
# filesize = self.file.tell()
# # Write header in the beginning of the file
# self.file.seek(0) # TODO: fix the OSError that occurs here
# self._write_header(filesize)
# else:
# LOG.error("Could not write BLF header since file is not seekable")
super().stop()
10 changes: 7 additions & 3 deletions can/io/logger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,9 @@ def compress(filename: StringPathLike) -> Tuple[str, FileLike]:
File will automatically recompress upon close.
"""
real_suffix = pathlib.Path(filename).suffixes[-2].lower()
mode = "ab" if real_suffix == ".blf" else "at"
# Why is this setup to append by default?
# mode = "ab" if real_suffix == ".blf" else "at"
mode = "wb" if real_suffix == ".blf" else "wt"

return real_suffix, gzip.open(filename, mode)

Expand Down Expand Up @@ -344,12 +346,14 @@ def do_rollover(self) -> None:
def _default_name(self) -> StringPathLike:
"""Generate the default rotation filename."""
path = pathlib.Path(self.base_filename)
stem = path.parts[-1].split(".")[0]
suffix = "".join(pathlib.Path(self.base_filename).suffixes[-2:])
new_name = (
path.stem
stem
+ "_"
+ datetime.now().strftime("%Y-%m-%dT%H%M%S")
+ "_"
+ f"#{self.rollover_count:03}"
+ path.suffix
+ suffix
)
return str(path.parent / new_name)