LOG_STREAM instead of LOG_RAW #540
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #540 +/- ##
==========================================
+ Coverage 74.02% 74.09% +0.07%
==========================================
Files 457 457
Lines 28303 28277 -26
Branches 585 585
==========================================
+ Hits 20950 20953 +3
+ Misses 7259 7232 -27
+ Partials 94 92 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 8 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
openc3/lib/openc3/io/raw_logger.rb
Outdated
| logging_enabled = false, | ||
| cycle_size = 2000000000 | ||
| cycle_size = 2000000000 # TODO: Need to be able to change this |
There was a problem hiding this comment.
We need to implement cycle_time
| @@ -183,7 +184,7 @@ def as_json(*a) | |||
| 'secret_options' => @secret_options, | |||
| 'protocols' => @protocols, | |||
| 'log' => @log, | |||
| 'log_raw' => @log_raw, | |||
| 'log_stream' => @log_stream, | |||
There was a problem hiding this comment.
It should be ok to change the json output of this right?
openc3/lib/openc3/io/raw_logger.rb
Outdated
| @@ -155,12 +153,14 @@ def close_file | |||
| if @file | |||
| begin | |||
| @file.close unless @file.closed? | |||
| File.chmod(0444, @file.path) # Make file read only | |||
| Logger.instance.info "Raw Log File Closed : #{@filename}" | |||
| remote_log_directory = "#{ENV['OPENC3_SCOPE']}/stream_logs/" | |||
There was a problem hiding this comment.
New top level stream_logs directory
| @@ -252,9 +253,10 @@ def handle_config(parser, keyword, parameters) | |||
| parser.verify_num_parameters(0, 0, "#{keyword}") | |||
| @log = false | |||
|
|
|||
| when 'LOG_RAW' | |||
| when 'LOG_STREAM', 'LOG_RAW' | |||
| # TODO: Take parameters for the raw log class? See bridge_config.rb | |||
There was a problem hiding this comment.
This would allow us to change the size but requires specific knowledge of the class initialize parameters. I was thinking something more like the existing TLM_LOG_CYCLE_TIME and TLM_LOG_CYCLE_SIZE with TLM as STREAM.
Do we instantiate the RawLoggerPair right here like in bridge_config? There's no consistency between the two.
| @@ -165,6 +165,7 @@ def build | |||
| klass = OpenC3.require_class(protocol[1]) | |||
| interface_or_router.add_protocol(klass, protocol[2..-1], protocol[0].upcase.intern) | |||
| end | |||
| interface_or_router.start_raw_logging if @log_stream | |||
There was a problem hiding this comment.
Raw logging is currently broken except for the APIs manually starting it. This allows it to start with the LOG_RAW or LOG_STREAM flag. But maybe this should move to the keyword parsing like in bridge_config?
openc3/lib/openc3/logs/log_writer.rb
Outdated
| @@ -40,6 +40,11 @@ class LogWriter | |||
| # independently. | |||
| attr_reader :cycle_time | |||
|
|
|||
| # @return cycle_size [Integer] The amount of data in bytes before creating | |||
| # a new log file. This can be combined with cycle_time but is better used | |||
| # independently. | |||
There was a problem hiding this comment.
I don't think it is better used independenly. The ideal is both, time and size, whichever comes first.
There was a problem hiding this comment.
Copy / paste. I'll change the other comments.
openc3/lib/openc3/logs/stream_log.rb
Outdated
| def initialize( | ||
| log_name, | ||
| log_type, | ||
| cycle_time = 600, # 5 minutes, matches time in target_model |
| required: true | ||
| description: Ruby filename or class name which implements the stream log. Default is StreamLog. | ||
| values: .* | ||
| - name: Log specific parameters |
There was a problem hiding this comment.
You should list these out specifically, because the class isn't configurable
| @@ -82,7 +82,8 @@ VARIABLE reduced_log_retain_time 2592000 | |||
| PROTOCOL WRITE MyRejectProtocol | |||
| MAP_TARGET <%= example_target_name %> | |||
| DONT_CONNECT | |||
| LOG_STREAM | |||
| # Override the default log time of 600 | |||
| LOG_STREAM StreamLog 60 | |||
There was a problem hiding this comment.
Hardcode StreamLog and remove from given parameters
closes #519