Merge pull request #1208 from OpenC3/ignore_offsets

IGNORE_OVERLAP in telemetry

openc3/data/config/telemetry_modifiers.yaml

@@ -170,4 +170,9 @@ ACCESSOR:
       required: true
       description: The name of the accessor class
       values: .+
-  since: 5.0.10
+  since: 5.0.10
+IGNORE_OVERLAP:
+  summary: Ignores any packet items which overlap
+  description: Packet items which overlap normally generate a warning unless each individual item has the OVERLAP keyword.
+    This ignores overlaps across the entire packet.
+  since: 5.15.3

openc3/lib/openc3/packets/packet.rb

@@ -101,6 +101,9 @@ class Packet < Structure
     # @return [Array<Array<Target Name, Packet Name, Item Name>>] Related items
     attr_accessor :related_items
 
+    # @return [Boolean] Whether to ignore overlapping items
+    attr_accessor :ignore_overlap
+
     # Valid format types
     VALUE_TYPES = [:RAW, :CONVERTED, :FORMATTED, :WITH_UNITS]
 
@@ -140,6 +143,7 @@ def initialize(target_name = nil, packet_name = nil, default_endianness = :BIG_E
         @cmd_or_tlm = nil
         @template = nil
         @packet_time = nil
+        @ignore_overlap = false
       end
 
       # Sets the target name this packet is associated with. Unidentified packets
@@ -151,12 +155,10 @@ def target_name=(target_name)
           if !(String === target_name)
             raise(ArgumentError, "target_name must be a String but is a #{target_name.class}")
           end
-
           @target_name = target_name.upcase.freeze
         else
           @target_name = nil
         end
-        @target_name
       end
 
       # Sets the packet name. Unidentified packets will have packet name set to
@@ -168,12 +170,10 @@ def packet_name=(packet_name)
           if !(String === packet_name)
             raise(ArgumentError, "packet_name must be a String but is a #{packet_name.class}")
           end
-
           @packet_name = packet_name.upcase.freeze
         else
           @packet_name = nil
         end
-        @packet_name
       end
 
       # Sets the description of the packet
@@ -184,12 +184,10 @@ def description=(description)
           if !(String === description)
             raise(ArgumentError, "description must be a String but is a #{description.class}")
           end
-
           @description = description.to_utf8.freeze
         else
           @description = nil
         end
-        @description
       end
 
       # Sets the received time of the packet
@@ -200,13 +198,11 @@ def received_time=(received_time)
           if !(Time === received_time)
             raise(ArgumentError, "received_time must be a Time but is a #{received_time.class}")
           end
-
           @received_time = received_time.clone.freeze
         else
           @received_time = nil
         end
         @read_conversion_cache.clear if @read_conversion_cache
-        @received_time
       end
 
       # Sets the received count of the packet
@@ -217,10 +213,8 @@ def received_count=(received_count)
         if !(Integer === received_count)
           raise(ArgumentError, "received_count must be an Integer but is a #{received_count.class}")
         end
-
         @received_count = received_count
         @read_conversion_cache.clear if @read_conversion_cache
-        @received_count
       end
 
     end # if RUBY_ENGINE != 'ruby' or ENV['OPENC3_NO_EXT']
@@ -401,6 +395,10 @@ def template=(template)
     #
     # @return [Array<String>] Warning messages for big definition overlaps
     def check_bit_offsets
+      if @ignore_overlap
+        Logger.instance.info("#{@target_name} #{@packet_name} has IGNORE_OVERLAP so bit overlaps ignored")
+        return []
+      end
       expected_next_offset = nil
       previous_item = nil
       warnings = []
@@ -617,7 +615,7 @@ def read_item(item, value_type = :CONVERTED, buffer = @buffer, given_raw = nil)
 
           unless using_cached_value
             if item.array_size
-              value.map! do |val, index|
+              value.map! do |val, _index|
                 item.read_conversion.call(val, self, buffer)
               end
             else
@@ -646,7 +644,7 @@ def read_item(item, value_type = :CONVERTED, buffer = @buffer, given_raw = nil)
         # Convert from value to state if possible
         if item.states
           if Array === value
-            value = value.map do |val, index|
+            value = value.map do |val, _index|
               if item.states.key(val)
                 item.states.key(val)
               elsif item.states.values.include?(ANY_STATE)
@@ -667,7 +665,7 @@ def read_item(item, value_type = :CONVERTED, buffer = @buffer, given_raw = nil)
           end
         else
           if Array === value
-            value = value.map do |val, index|
+            value = value.map do |val, _index|
               apply_format_string_and_units(item, val, value_type)
             end
           else
@@ -693,7 +691,7 @@ def read_item(item, value_type = :CONVERTED, buffer = @buffer, given_raw = nil)
     # @param value_type [Symbol] Value type to read for every item
     # @param buffer [String] The binary buffer to read the items from
     # @return Hash of read names and values
-    def read_items(items, value_type = :RAW, buffer = @buffer, raw_value = nil)
+    def read_items(items, value_type = :RAW, buffer = @buffer, _raw_value = nil)
       buffer = allocate_buffer_if_needed() unless buffer
       if value_type == :RAW
         result = super(items, value_type, buffer)
@@ -735,11 +733,11 @@ def write_item(item, value, value_type = :CONVERTED, buffer = @buffer)
         end
         begin
           super(item, value, :RAW, buffer)
-        rescue ArgumentError => err
-          if item.states and String === value and err.message =~ /invalid value for/
+        rescue ArgumentError => e
+          if item.states and String === value and e.message =~ /invalid value for/
             raise "Unknown state #{value} for #{item.name}"
           else
-            raise err
+            raise e
           end
         end
       when :FORMATTED, :WITH_UNITS
@@ -998,7 +996,7 @@ def reset
       end
       return unless @processors
 
-      @processors.each do |processor_name, processor|
+      @processors.each do |_processor_name, processor|
         processor.reset
       end
     end
@@ -1051,7 +1049,7 @@ def to_config(cmd_or_tlm)
         config << "COMMAND #{@target_name.to_s.quote_if_necessary} #{@packet_name.to_s.quote_if_necessary} #{@default_endianness} \"#{@description}\"\n"
       end
       if @accessor.class.to_s != 'OpenC3::BinaryAccessor'
-        config << "  ACCESSOR #{@accessor.class.to_s} #{@accessor.args.map { |a| a.to_s.quote_if_necessary }.join(" ")}\n"
+        config << "  ACCESSOR #{@accessor.class} #{@accessor.args.map { |a| a.to_s.quote_if_necessary }.join(" ")}\n"
       end
       # TODO: Add TEMPLATE_ENCODED so this can always be done inline regardless of content
       if @template
@@ -1067,7 +1065,7 @@ def to_config(cmd_or_tlm)
       end
 
       if @processors
-        @processors.each do |processor_name, processor|
+        @processors.each do |_processor_name, processor|
           config << processor.to_config
         end
       end
@@ -1106,6 +1104,9 @@ def to_config(cmd_or_tlm)
           config << "  RELATED_ITEM #{target_name.to_s.quote_if_necessary} #{packet_name.to_s.quote_if_necessary} #{item_name.to_s.quote_if_necessary}"
         end
       end
+      if @ignore_overlap
+        config << "  IGNORE_OVERLAP"
+      end
       config
     end
 
@@ -1128,7 +1129,7 @@ def as_json(*a)
 
       if @processors
         processors = []
-        @processors.each do |processor_name, processor|
+        @processors.each do |_processor_name, processor|
           processors << processor.as_json(*a)
         end
         config['processors'] = processors
@@ -1161,6 +1162,7 @@ def as_json(*a)
       if @related_items
         config['related_items'] = @related_items
       end
+      config['ignore_overlap'] = true if @ignore_overlap
 
       config
     end
@@ -1182,8 +1184,8 @@ def self.from_json(hash)
           else
             packet.accessor = accessor.new(packet)
           end
-        rescue => error
-          Logger.instance.error "#{packet.target_name} #{packet.packet_name} accessor of #{hash['accessor']} could not be found due to #{error}"
+        rescue => e
+          Logger.instance.error "#{packet.target_name} #{packet.packet_name} accessor of #{hash['accessor']} could not be found due to #{e}"
         end
       end
       packet.template = Base64.decode64(hash['template']) if hash['template']
@@ -1204,6 +1206,7 @@ def self.from_json(hash)
       if hash['related_items']
         packet.related_items = hash['related_items']
       end
+      packet.ignore_overlap = hash['ignore_overlap']
 
       packet
     end
@@ -1237,7 +1240,7 @@ def decom
     def process(buffer = @buffer)
       return unless @processors
 
-      @processors.each do |processor_name, processor|
+      @processors.each do |_processor_name, processor|
         processor.call(self, buffer)
       end
     end

openc3/lib/openc3/packets/packet_config.rb

@@ -220,7 +220,7 @@ def process_file(filename, process_target_name, language = 'ruby')
               'APPEND_PARAMETER', 'APPEND_ID_ITEM', 'APPEND_ID_PARAMETER', 'APPEND_ARRAY_ITEM',\
               'APPEND_ARRAY_PARAMETER', 'ALLOW_SHORT', 'HAZARDOUS', 'PROCESSOR', 'META',\
               'DISABLE_MESSAGES', 'HIDDEN', 'DISABLED', 'ACCESSOR', 'TEMPLATE', 'TEMPLATE_FILE',\
-              'RESPONSE', 'ERROR_RESPONSE', 'SCREEN', 'RELATED_ITEM'
+              'RESPONSE', 'ERROR_RESPONSE', 'SCREEN', 'RELATED_ITEM', 'IGNORE_OVERLAP'
             raise parser.error("No current packet for #{keyword}") unless @current_packet
 
             process_current_packet(parser, keyword, params)
@@ -539,6 +539,11 @@ def process_current_packet(parser, keyword, params)
         @current_packet.related_items ||= []
         @current_packet.related_items << [params[0].upcase, params[1].upcase, params[2].upcase]
 
+      when 'IGNORE_OVERLAP'
+        usage = "#{keyword}"
+        parser.verify_num_parameters(0, 0, usage)
+        @current_packet.ignore_overlap = true
+
       end
 
     end

openc3/python/openc3/packets/packet.py

@@ -95,6 +95,7 @@ def __init__(
         self.error_response = None
         self.screen = None
         self.related_items = None
+        self.ignore_overlap = False
 
     @property
     def target_name(self):
@@ -345,8 +346,14 @@ def template(self, template):
     # gaps in the packet, but not allow the same bits to be used for multiple
     # variables.
     #
-    # self.return [Array<String>] Warning messages for big definition overlaps
+    # self.return [Array<String>] Warning messages for bit definition overlaps
     def check_bit_offsets(self):
+        if self.ignore_overlap:
+            Logger.info(
+                f"{self.target_name} {self.packet_name} has IGNORE_OVERLAP so bit overlaps ignored"
+            )
+            return []
+
         expected_next_offset = None
         previous_item = None
         warnings = []
@@ -1042,6 +1049,9 @@ def to_config(self, cmd_or_tlm):
             for related_item in self.related_items:
                 config += f"  RELATED_ITEM {quote_if_necessary(related_item[0])} {quote_if_necessary(related_item[1])} {quote_if_necessary(related_item[2])}\n"
 
+        if self.ignore_overlap:
+            config += "  IGNORE_OVERLAP"
+
         return config
 
     def as_json(self):
@@ -1089,15 +1099,14 @@ def as_json(self):
 
         if self.response:
             config["response"] = self.response
-
         if self.error_response:
             config["error_response"] = self.error_response
-
         if self.screen:
             config["screen"] = self.screen
-
         if self.related_items:
             config["related_items"] = self.related_items
+        if self.ignore_overlap:
+            config["ignore_overlap"] = self.ignore_overlap
 
         return config
 
@@ -1136,15 +1145,14 @@ def from_json(cls, hash):
 
         if "response" in hash:
             packet.response = hash["response"]
-
         if "error_response" in hash:
             packet.error_response = hash["error_response"]
-
         if "screen" in hash:
             packet.screen = hash["screen"]
-
         if "related_items" in hash:
             packet.related_items = hash["related_items"]
+        if "ignore_overlap" in hash:
+            packet.ignore_overlap = hash["ignore_overlap"]
 
         return packet
 

openc3/python/openc3/packets/packet_config.py

@@ -222,6 +222,7 @@ def process_file(self, filename, process_target_name):
                         | "ERROR_RESPONSE"
                         | "SCREEN"
                         | "RELATED_ITEM"
+                        | "IGNORE_OVERLAP"
                     ):
                         if not self.current_packet:
                             raise parser.error(f"No current packet for {keyword}")
@@ -530,6 +531,11 @@ def process_current_packet(self, parser, keyword, params):
                     [params[0].upper(), params[1].upper(), params[2].upper()]
                 )
 
+            case "IGNORE_OVERLAP":
+                usage = f"{keyword}"
+                parser.verify_num_parameters(0, 0, usage)
+                self.current_packet.ignore_overlap = True
+
     def process_current_item(self, parser, keyword, params):
         match keyword:
             # Add a state to the current telemety item

openc3/python/openc3/script/web_socket_api.py

@@ -156,13 +156,16 @@ def disconnect(self):
 
     # Generate the appropriate token for OpenC3
     def _generate_auth(self):
-        if os.environ.get("OPENC3_API_TOKEN") and os.environ.get("OPENC3_API_USER"):
-            return OpenC3KeycloakAuthentication(os.environ.get("OPENC3_KEYCLOAK_URL"))
-        else:
+        if (
+            os.environ.get("OPENC3_API_TOKEN") is None
+            and os.environ.get("OPENC3_API_USER") is None
+        ):
             if os.environ.get("OPENC3_API_PASSWORD"):
                 return OpenC3Authentication()
             else:
-                raise RuntimeError("Environment Variables Not Set for Authentication")
+                return None
+        else:
+            return OpenC3KeycloakAuthentication(os.environ.get("OPENC3_KEYCLOAK_URL"))
 
 
 # Base class for cmd-tlm-api websockets - Do not use directly

openc3/python/test/packets/test_packet_config.py

@@ -1229,3 +1229,27 @@ def test_allows_appending_derived_items(self):
             self.pc.telemetry["TGT1"]["PKT1"].items["ITEM1"].data_type, "DERIVED"
         )
         tf.close()
+
+    def test_detects_overlapping_items_without_IGNORE_OVERLAP(self):
+        tf = tempfile.NamedTemporaryFile(mode="w")
+        tf.write('TELEMETRY tgt1 pkt1 LITTLE_ENDIAN "Packet"\n')
+        tf.write("  ITEM item1 0 8 UINT\n")
+        tf.write("  ITEM item2 4 4 UINT\n")
+        tf.seek(0)
+        self.pc.process_file(tf.name, "TGT1")
+        self.assertIn(
+            "Bit definition overlap at bit offset 4 for packet TGT1 PKT1 items ITEM2 and ITEM1",
+            self.pc.warnings,
+        )
+        tf.close()
+
+    def test_ignores_overlapping_items_with_IGNORE_OVERLAP(self):
+        tf = tempfile.NamedTemporaryFile(mode="w")
+        tf.write('TELEMETRY tgt1 pkt1 LITTLE_ENDIAN "Packet"\n')
+        tf.write("  IGNORE_OVERLAP\n")
+        tf.write("  ITEM item1 0 8 UINT\n")
+        tf.write("  ITEM item2 4 4 UINT\n")
+        tf.seek(0)
+        self.pc.process_file(tf.name, "TGT1")
+        self.assertEqual(len(self.pc.warnings), 0)
+        tf.close()