python-can icon indicating copy to clipboard operation
python-can copied to clipboard

Fix the sized rotating logger size bug for the blf writer

Open j-c-cook opened this issue 3 years ago • 3 comments

I see two options here:

  1. The -s option for the blf rolling logger will not define the file size to be written, but rather the buffer size prior to compression
  2. A mathematical equation is to be written that computes the expected file size of the buffer based on the compression integer passed to zlib.compress. (This would require either some knowledge of how the compression functions, or could be an equation fit made from a table of collected values based on a range of compressed buffer sizes vs. the input integer.)

I plan to move forward shortly with a commit that will implement the solution (1).

closes #1359

j-c-cook avatar Aug 04 '22 20:08 j-c-cook

Codecov Report

Merging #1360 (55401ea) into develop (b9d9d01) will increase coverage by 0.00%. The diff coverage is 85.71%.

@@           Coverage Diff            @@
##           develop    #1360   +/-   ##
========================================
  Coverage    66.06%   66.06%           
========================================
  Files           86       86           
  Lines         8973     8976    +3     
========================================
+ Hits          5928     5930    +2     
- Misses        3045     3046    +1     

codecov[bot] avatar Aug 04 '22 21:08 codecov[bot]

@zariiii9003 Ready for review.

j-c-cook avatar Aug 04 '22 21:08 j-c-cook

@hardbyte i see you approved this but personally i think bringing implementation details of the BlfWriter (options["max_container_size"] = results.file_size # bytes) into the logger script is not a pretty solution.

What do you think about adding a size property to all FileIOMessageWriter subclasses? That way we could delegate size estimation to the logger implementations.

zariiii9003 avatar Aug 07 '22 22:08 zariiii9003