From 977e1e0c6dc85b709eb16b6fea9a719144440603 Mon Sep 17 00:00:00 2001 From: jack-cook_agco Date: Thu, 4 Aug 2022 15:29:05 -0500 Subject: [PATCH 1/9] Add print statements to highlight the bug --- can/io/blf.py | 1 + can/io/logger.py | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/can/io/blf.py b/can/io/blf.py index efeba9488..94cfd0183 100644 --- a/can/io/blf.py +++ b/can/io/blf.py @@ -531,6 +531,7 @@ def _add_object(self, obj_type, data, timestamp=None): self._buffer_size += obj_size + padding_size self.object_count += 1 if self._buffer_size >= self.max_container_size: + print("Flush the buffer to a file. Larger than max container!") self._flush() def _flush(self): diff --git a/can/io/logger.py b/can/io/logger.py index 37ba1e736..103896290 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -325,7 +325,14 @@ def should_rollover(self, msg: Message) -> bool: if self.max_bytes <= 0: return False + value = self.writer.file.tell() + + if value != 144: + print("NOT 144!") + if self.writer.file.tell() >= self.max_bytes: + print('Rollover: {} bytes'.format(value)) + print('Buffer size: {} bytes'.format(self._writer._buffer_size)) return True return False From 824893b5144290275502358f813dad76ae201115 Mon Sep 17 00:00:00 2001 From: jack-cook_agco Date: Thu, 4 Aug 2022 15:51:04 -0500 Subject: [PATCH 2/9] Fix the rollover for the blf writer --- can/io/blf.py | 1 - can/io/logger.py | 21 +++++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/can/io/blf.py b/can/io/blf.py index 94cfd0183..efeba9488 100644 --- a/can/io/blf.py +++ b/can/io/blf.py @@ -531,7 +531,6 @@ def _add_object(self, obj_type, data, timestamp=None): self._buffer_size += obj_size + padding_size self.object_count += 1 if self._buffer_size >= self.max_container_size: - print("Flush the buffer to a file. Larger than max container!") self._flush() def _flush(self): diff --git a/can/io/logger.py b/can/io/logger.py index 103896290..3123da7a0 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -325,14 +325,19 @@ def should_rollover(self, msg: Message) -> bool: if self.max_bytes <= 0: return False - value = self.writer.file.tell() - - if value != 144: - print("NOT 144!") - - if self.writer.file.tell() >= self.max_bytes: - print('Rollover: {} bytes'.format(value)) - print('Buffer size: {} bytes'.format(self._writer._buffer_size)) + # The blf writer initially writes the header (144 bytes) of data to the + # file, but then does not write again until the buffer size becomes + # larger than the max container size and is flushed to the file. This + # results in two cases: (1) the requested file size is less than 144 + # bytes and files are spam written, or (2) the size of the buffer that + # goes into the file is consistently the max container size. Therefore, + # the buffer size must be checked for the blf writer specifically. + if ( + self.writer.file.tell() >= self.max_bytes + or self._writer._buffer_size >= self.max_bytes + ): + print("file.tell(): {} bytes".format(self.writer.file.tell())) + print("Buffer size: {} bytes".format(self._writer._buffer_size)) return True return False From 6f0a298053b756b54cd8bfd7822db0d598d2bc3f Mon Sep 17 00:00:00 2001 From: jack-cook_agco Date: Thu, 4 Aug 2022 16:18:18 -0500 Subject: [PATCH 3/9] Fix change to run with all logger types --- can/io/blf.py | 12 ++++++++---- can/io/logger.py | 13 +------------ can/logger.py | 1 + 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/can/io/blf.py b/can/io/blf.py index efeba9488..71154903a 100644 --- a/can/io/blf.py +++ b/can/io/blf.py @@ -17,7 +17,7 @@ import datetime import time import logging -from typing import List, BinaryIO, Generator, Union, Tuple, Optional, cast +from typing import List, BinaryIO, Generator, Union, Tuple, Optional, cast, Any from ..message import Message from ..util import len2dlc, dlc2len, channel2int @@ -358,9 +358,6 @@ class BLFWriter(FileIOMessageWriter): file: BinaryIO - #: Max log container size of uncompressed data - max_container_size = 128 * 1024 - #: Application identifier for the log writer application_id = 5 @@ -370,6 +367,7 @@ def __init__( append: bool = False, channel: int = 1, compression_level: int = -1, + **options: Any, ) -> None: """ :param file: a path-like object or as file-like object to write to @@ -400,6 +398,11 @@ def __init__( self.compression_level = compression_level self._buffer: List[bytes] = [] self._buffer_size = 0 + #: Max log container size of uncompressed data + if "max_container_size" in options: + self.max_container_size = options["max_container_size"] + else: + self.max_container_size = 128 * 1024 # bytes if append: # Parse file header data = self.file.read(FILE_HEADER_STRUCT.size) @@ -530,6 +533,7 @@ def _add_object(self, obj_type, data, timestamp=None): self._buffer_size += obj_size + padding_size self.object_count += 1 + a = len(zlib.compress(memoryview(b"".join(self._buffer)), 1)) if self._buffer_size >= self.max_container_size: self._flush() diff --git a/can/io/logger.py b/can/io/logger.py index 3123da7a0..01a64c997 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -325,19 +325,8 @@ def should_rollover(self, msg: Message) -> bool: if self.max_bytes <= 0: return False - # The blf writer initially writes the header (144 bytes) of data to the - # file, but then does not write again until the buffer size becomes - # larger than the max container size and is flushed to the file. This - # results in two cases: (1) the requested file size is less than 144 - # bytes and files are spam written, or (2) the size of the buffer that - # goes into the file is consistently the max container size. Therefore, - # the buffer size must be checked for the blf writer specifically. - if ( - self.writer.file.tell() >= self.max_bytes - or self._writer._buffer_size >= self.max_bytes - ): + if self.writer.file.tell() >= self.max_bytes: print("file.tell(): {} bytes".format(self.writer.file.tell())) - print("Buffer size: {} bytes".format(self._writer._buffer_size)) return True return False diff --git a/can/logger.py b/can/logger.py index 5aff5e4e1..9419dd0e7 100644 --- a/can/logger.py +++ b/can/logger.py @@ -215,6 +215,7 @@ def main() -> None: options = {"append": results.append} if results.file_size: + options["max_container_size"] = results.file_size # bytes logger = SizedRotatingLogger( base_filename=results.log_file, max_bytes=results.file_size, **options ) From 276b253c6cb82fc79f07a924d31e050cf511f3a1 Mon Sep 17 00:00:00 2001 From: jack-cook_agco Date: Thu, 4 Aug 2022 16:23:16 -0500 Subject: [PATCH 4/9] Remove unnecessary statement --- can/io/blf.py | 1 - 1 file changed, 1 deletion(-) diff --git a/can/io/blf.py b/can/io/blf.py index 71154903a..0c8420ea6 100644 --- a/can/io/blf.py +++ b/can/io/blf.py @@ -533,7 +533,6 @@ def _add_object(self, obj_type, data, timestamp=None): self._buffer_size += obj_size + padding_size self.object_count += 1 - a = len(zlib.compress(memoryview(b"".join(self._buffer)), 1)) if self._buffer_size >= self.max_container_size: self._flush() From c1950015a82bf6d2f74617c697746f84f538584e Mon Sep 17 00:00:00 2001 From: jack-cook_agco Date: Thu, 4 Aug 2022 16:26:08 -0500 Subject: [PATCH 5/9] Remove unnecessary print statement for file.tell() --- can/io/logger.py | 1 - 1 file changed, 1 deletion(-) diff --git a/can/io/logger.py b/can/io/logger.py index 01a64c997..37ba1e736 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -326,7 +326,6 @@ def should_rollover(self, msg: Message) -> bool: return False if self.writer.file.tell() >= self.max_bytes: - print("file.tell(): {} bytes".format(self.writer.file.tell())) return True return False From c68f155a54bf533d0ba7ed52e14f937be476a4a2 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Fri, 5 Aug 2022 10:38:36 -0500 Subject: [PATCH 6/9] Add **options to API of all Writers --- can/io/asc.py | 1 + can/io/canutils.py | 1 + can/io/csv.py | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/can/io/asc.py b/can/io/asc.py index 3a320f007..a4c22fa0b 100644 --- a/can/io/asc.py +++ b/can/io/asc.py @@ -352,6 +352,7 @@ def __init__( self, file: Union[StringPathLike, TextIO], channel: int = 1, + **options ) -> None: """ :param file: a path-like object or as file-like object to write to diff --git a/can/io/canutils.py b/can/io/canutils.py index 0cca82eb8..3f3a6bbd4 100644 --- a/can/io/canutils.py +++ b/can/io/canutils.py @@ -132,6 +132,7 @@ def __init__( file: Union[StringPathLike, TextIO], channel: str = "vcan0", append: bool = False, + **options, ): """ :param file: a path-like object or as file-like object to write to diff --git a/can/io/csv.py b/can/io/csv.py index 0161b4f55..f49637c55 100644 --- a/can/io/csv.py +++ b/can/io/csv.py @@ -87,7 +87,7 @@ class CSVWriter(FileIOMessageWriter): file: TextIO def __init__( - self, file: Union[StringPathLike, TextIO], append: bool = False + self, file: Union[StringPathLike, TextIO], append: bool = False, **options ) -> None: """ :param file: a path-like object or a file-like object to write to. From 3c35108c680e8555aa1cef9d8fe0ba6afe708a6b Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Fri, 5 Aug 2022 19:48:13 -0500 Subject: [PATCH 7/9] Add type definition to **options --- can/io/asc.py | 5 +---- can/io/canutils.py | 4 ++-- can/io/csv.py | 4 ++-- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/can/io/asc.py b/can/io/asc.py index a4c22fa0b..c57866b02 100644 --- a/can/io/asc.py +++ b/can/io/asc.py @@ -349,10 +349,7 @@ class ASCWriter(FileIOMessageWriter): FORMAT_EVENT = "{timestamp: 9.6f} {message}\n" def __init__( - self, - file: Union[StringPathLike, TextIO], - channel: int = 1, - **options + self, file: Union[StringPathLike, TextIO], channel: int = 1, **options: Any ) -> None: """ :param file: a path-like object or as file-like object to write to diff --git a/can/io/canutils.py b/can/io/canutils.py index 3f3a6bbd4..5c11d4175 100644 --- a/can/io/canutils.py +++ b/can/io/canutils.py @@ -5,7 +5,7 @@ """ import logging -from typing import Generator, TextIO, Union +from typing import Generator, TextIO, Union, Any from can.message import Message from .generic import FileIOMessageWriter, MessageReader @@ -132,7 +132,7 @@ def __init__( file: Union[StringPathLike, TextIO], channel: str = "vcan0", append: bool = False, - **options, + **options: Any, ): """ :param file: a path-like object or as file-like object to write to diff --git a/can/io/csv.py b/can/io/csv.py index f49637c55..5c533939a 100644 --- a/can/io/csv.py +++ b/can/io/csv.py @@ -10,7 +10,7 @@ """ from base64 import b64encode, b64decode -from typing import TextIO, Generator, Union +from typing import TextIO, Generator, Union, Any from can.message import Message from .generic import FileIOMessageWriter, MessageReader @@ -87,7 +87,7 @@ class CSVWriter(FileIOMessageWriter): file: TextIO def __init__( - self, file: Union[StringPathLike, TextIO], append: bool = False, **options + self, file: Union[StringPathLike, TextIO], append: bool = False, **options: Any ) -> None: """ :param file: a path-like object or a file-like object to write to. From 611fc4f8329c42963e9a078a3a69ed185792bed9 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Fri, 5 Aug 2022 19:55:51 -0500 Subject: [PATCH 8/9] Update help documentation for -s logger option --- can/logger.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/can/logger.py b/can/logger.py index 9419dd0e7..2f5fe2e32 100644 --- a/can/logger.py +++ b/can/logger.py @@ -170,8 +170,8 @@ def main() -> None: "--file_size", dest="file_size", type=int, - help="Maximum file size in bytes. Rotate log file when size threshold " - "is reached.", + help="Maximum file size in bytes (or for the case of blf, maximum buffer size before compression and flush to " + "file). Rotate log file when size threshold is reached.", default=None, ) From 55401ea288653f17f1e6cf8644cb9f973db0caad Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Fri, 5 Aug 2022 20:01:58 -0500 Subject: [PATCH 9/9] Refactor line width --- can/logger.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/can/logger.py b/can/logger.py index 2f5fe2e32..1ec5afa48 100644 --- a/can/logger.py +++ b/can/logger.py @@ -170,8 +170,9 @@ def main() -> None: "--file_size", dest="file_size", type=int, - help="Maximum file size in bytes (or for the case of blf, maximum buffer size before compression and flush to " - "file). Rotate log file when size threshold is reached.", + help="Maximum file size in bytes (or for the case of blf, maximum " + "buffer size before compression and flush to file). Rotate log " + "file when size threshold is reached.", default=None, )