From 166b2fff27c40e4a78793c881f330ff2ea1052aa Mon Sep 17 00:00:00 2001 From: Ryan Melton Date: Sun, 25 Oct 2026 18:50:57 -0600 Subject: [PATCH 1/3] fix(python): repair packet to_config, reset and as_json processor handling Packet looped over the processors and meta dicts as if they were lists of pairs, so to_config, reset and as_json raised for any packet with processors. PacketItem.to_config wrote a literal "{self.endianness}", raised KeyError when only some states had colors, hazardous or DISABLE_MESSAGES set, and dropped hazardous states with no description. Co-Authored-By: Claude Opus 5.5 (1M context) --- openc3/python/openc3/packets/packet.py | 10 +++---- openc3/python/openc3/packets/packet_item.py | 8 +++--- openc3/python/test/packets/test_packet.py | 26 +++++++++++++++++++ .../python/test/packets/test_packet_item.py | 17 ++++++++++++ 4 files changed, 52 insertions(+), 9 deletions(-) diff --git a/openc3/python/openc3/packets/packet.py b/openc3/python/openc3/packets/packet.py index 1e176dae9f..188c4fc2f6 100644 --- a/openc3/python/openc3/packets/packet.py +++ b/openc3/python/openc3/packets/packet.py @@ -1008,7 +1008,7 @@ def reset(self): if not self.processors: return - for _, processor in self.processors: + for processor in self.processors.values(): processor.reset() # Make a light weight clone of this packet. This only creates a new buffer @@ -1080,11 +1080,11 @@ def to_config(self, cmd_or_tlm): config += " RESTRICTED\n" if self.processors: - for _, processor in self.processors: - config += processor.to_config + for processor in self.processors.values(): + config += processor.to_config() if self.meta: - for key, values in self.meta: + for key, values in self.meta.items(): vals = " ".join([quote_if_necessary(a) for a in values]) config += f" META {quote_if_necessary(key)} {vals}\n" @@ -1151,7 +1151,7 @@ def as_json(self): if self.processors: processors = [] - for _, processor in self.processors: + for processor in self.processors.values(): processors.append(processor.as_json()) config["processors"] = processors diff --git a/openc3/python/openc3/packets/packet_item.py b/openc3/python/openc3/packets/packet_item.py index 1f65c34bad..9afee65ce7 100644 --- a/openc3/python/openc3/packets/packet_item.py +++ b/openc3/python/openc3/packets/packet_item.py @@ -398,7 +398,7 @@ def to_config(self, cmd_or_tlm, default_endianness): else: config += self.parameter_config() if self.endianness != default_endianness and self.data_type != "STRING" and self.data_type != "BLOCK": - config += " {self.endianness}" + config += f" {self.endianness}" config += "\n" if self.required: @@ -415,11 +415,11 @@ def to_config(self, cmd_or_tlm, default_endianness): if self.states: for state_name, state_value in self.states.items(): config += f" STATE {quote_if_necessary(state_name)} {quote_if_necessary(str(state_value))}" - if self.hazardous and self.hazardous[state_name]: + if self.hazardous and self.hazardous.get(state_name) is not None: config += f" HAZARDOUS {quote_if_necessary(self.hazardous[state_name])}" - if self.messages_disabled and self.messages_disabled[state_name]: + if self.messages_disabled and self.messages_disabled.get(state_name): config += " DISABLE_MESSAGES" - if self.state_colors and self.state_colors[state_name]: + if self.state_colors and self.state_colors.get(state_name): config += f" {self.state_colors[state_name]}" config += "\n" diff --git a/openc3/python/test/packets/test_packet.py b/openc3/python/test/packets/test_packet.py index 2b70cae579..de6df04419 100644 --- a/openc3/python/test/packets/test_packet.py +++ b/openc3/python/test/packets/test_packet.py @@ -2340,3 +2340,29 @@ def test_does_not_identify_a_packet_whose_id_item_is_outside_the_buffer(self): self.assertFalse(packet.identify(b"\x00\x01")) self.assertTrue(packet.identify(b"\x00\x01\x00\x05")) self.assertFalse(packet.identify(b"\x00\x01\x00\x06")) + + +class TestPacketProcessors(unittest.TestCase): + def test_reset_resets_each_processor(self): + p = Packet("TGT", "PKT") + processor = Processor() + processor.reset = Mock() + p.processors["PROC"] = processor + p.reset() + processor.reset.assert_called_once() + + def test_as_json_includes_each_processor(self): + p = Packet("TGT", "PKT") + p.processors["PROC"] = Processor() + self.assertEqual(p.as_json()["processors"], [Processor().as_json()]) + + +class TestPacketToConfig(unittest.TestCase): + def test_writes_processors_and_meta(self): + p = Packet("TGT", "PKT") + p.append_item("ITEM", 8, "UINT") + p.processors["PROC"] = Processor() + p.meta["TYPE"] = ["SENSOR", "LIGHT"] + config = p.to_config("TELEMETRY") + self.assertIn(" PROCESSOR PROCESSOR Processor CONVERTED\n", config) + self.assertIn(" META TYPE SENSOR LIGHT\n", config) diff --git a/openc3/python/test/packets/test_packet_item.py b/openc3/python/test/packets/test_packet_item.py index c6d3c98b59..aa7d98f689 100644 --- a/openc3/python/test/packets/test_packet_item.py +++ b/openc3/python/test/packets/test_packet_item.py @@ -440,6 +440,23 @@ def test_accepts_state_colors_as_a_hash(self): self.assertIn("STATE TRUE 1 GREEN", config) self.assertIn("STATE FALSE 0 RED", config) + def test_to_config_with_attributes_on_only_some_states(self): + self.pi.minimum = 0 + self.pi.maximum = 2 + self.pi.states = {"OFF": 0, "ON": 1, "FAULT": 2} + self.pi.state_colors = {"ON": "GREEN"} + self.pi.hazardous = {"FAULT": ""} + self.pi.messages_disabled = {"ON": True} + config = self.pi.to_config("TELEMETRY", "BIG_ENDIAN") + self.assertIn(" STATE OFF 0\n", config) + self.assertIn(" STATE ON 1 DISABLE_MESSAGES GREEN\n", config) + self.assertIn(" STATE FAULT 2 HAZARDOUS", config) + + def test_to_config_writes_non_default_endianness(self): + pi = PacketItem("test", 0, 32, "UINT", "LITTLE_ENDIAN", None) + self.assertIn('ITEM TEST 0 32 UINT "" LITTLE_ENDIAN\n', pi.to_config("TELEMETRY", "BIG_ENDIAN")) + self.assertNotIn("ENDIAN", pi.to_config("TELEMETRY", "LITTLE_ENDIAN")) + def test_sets_the_state_colors_to_None(self): self.pi.state_colors = None self.assertIsNone(self.pi.state_colors) From 2119cd2888dc1ecc5c47611be42a6b63f1462b11 Mon Sep 17 00:00:00 2001 From: Ryan Melton Date: Sun, 25 Oct 2026 20:20:57 -0600 Subject: [PATCH 2/3] fix(packets): write only STATE attributes valid for the packet type to_config wrote HAZARDOUS and DISABLE_MESSAGES on telemetry states and colors on command states, which the state parser rejects, so the generated config could not be loaded back. An empty hazardous description no longer leaves a trailing space. Applied to Ruby and Python to keep their output identical. Co-Authored-By: Claude Opus 5.5 (1M context) --- openc3/lib/openc3/packets/packet_item.rb | 21 ++++++++++++------- openc3/python/openc3/packets/packet_item.py | 17 +++++++++------ .../python/test/packets/test_packet_item.py | 18 +++++++++++++--- openc3/spec/packets/packet_item_spec.rb | 16 ++++++++++++++ 4 files changed, 55 insertions(+), 17 deletions(-) diff --git a/openc3/lib/openc3/packets/packet_item.rb b/openc3/lib/openc3/packets/packet_item.rb index 64b638f435..477f2375a7 100644 --- a/openc3/lib/openc3/packets/packet_item.rb +++ b/openc3/lib/openc3/packets/packet_item.rb @@ -398,14 +398,19 @@ def to_config(cmd_or_tlm, default_endianness) if @states @states.each do |state_name, state_value| config << " STATE #{state_name.to_s.quote_if_necessary} #{state_value.to_s.quote_if_necessary}" - if @hazardous and @hazardous[state_name] - config << " HAZARDOUS #{@hazardous[state_name].to_s.quote_if_necessary}" - end - if @messages_disabled and @messages_disabled[state_name] - config << " DISABLE_MESSAGES" - end - if @state_colors and @state_colors[state_name] - config << " #{@state_colors[state_name]}" + # Only write the attributes STATE accepts for this packet type so the config parses back + if cmd_or_tlm == :TELEMETRY + if @state_colors and @state_colors[state_name] + config << " #{@state_colors[state_name]}" + end + else + if @hazardous and @hazardous[state_name] + config << " HAZARDOUS" + config << " #{@hazardous[state_name].to_s.quote_if_necessary}" unless @hazardous[state_name].to_s.empty? + end + if @messages_disabled and @messages_disabled[state_name] + config << " DISABLE_MESSAGES" + end end config << "\n" end diff --git a/openc3/python/openc3/packets/packet_item.py b/openc3/python/openc3/packets/packet_item.py index 9afee65ce7..3ff7ea9f1f 100644 --- a/openc3/python/openc3/packets/packet_item.py +++ b/openc3/python/openc3/packets/packet_item.py @@ -415,12 +415,17 @@ def to_config(self, cmd_or_tlm, default_endianness): if self.states: for state_name, state_value in self.states.items(): config += f" STATE {quote_if_necessary(state_name)} {quote_if_necessary(str(state_value))}" - if self.hazardous and self.hazardous.get(state_name) is not None: - config += f" HAZARDOUS {quote_if_necessary(self.hazardous[state_name])}" - if self.messages_disabled and self.messages_disabled.get(state_name): - config += " DISABLE_MESSAGES" - if self.state_colors and self.state_colors.get(state_name): - config += f" {self.state_colors[state_name]}" + # Only write the attributes STATE accepts for this packet type so the config parses back + if cmd_or_tlm == "TELEMETRY": + if self.state_colors and self.state_colors.get(state_name): + config += f" {self.state_colors[state_name]}" + else: + if self.hazardous and self.hazardous.get(state_name) is not None: + config += " HAZARDOUS" + if self.hazardous[state_name]: + config += f" {quote_if_necessary(self.hazardous[state_name])}" + if self.messages_disabled and self.messages_disabled.get(state_name): + config += " DISABLE_MESSAGES" config += "\n" if self.read_conversion: diff --git a/openc3/python/test/packets/test_packet_item.py b/openc3/python/test/packets/test_packet_item.py index aa7d98f689..1923441116 100644 --- a/openc3/python/test/packets/test_packet_item.py +++ b/openc3/python/test/packets/test_packet_item.py @@ -440,7 +440,19 @@ def test_accepts_state_colors_as_a_hash(self): self.assertIn("STATE TRUE 1 GREEN", config) self.assertIn("STATE FALSE 0 RED", config) - def test_to_config_with_attributes_on_only_some_states(self): + def test_to_config_with_command_attributes_on_only_some_states(self): + self.pi.minimum = 0 + self.pi.maximum = 2 + self.pi.states = {"OFF": 0, "ON": 1, "FAULT": 2} + self.pi.hazardous = {"FAULT": ""} + self.pi.messages_disabled = {"ON": True} + self.pi.state_colors = {"ON": "GREEN"} + config = self.pi.to_config("COMMAND", "BIG_ENDIAN") + self.assertIn(" STATE OFF 0\n", config) + self.assertIn(" STATE ON 1 DISABLE_MESSAGES\n", config) + self.assertIn(" STATE FAULT 2 HAZARDOUS\n", config) + + def test_to_config_with_telemetry_colors_on_only_some_states(self): self.pi.minimum = 0 self.pi.maximum = 2 self.pi.states = {"OFF": 0, "ON": 1, "FAULT": 2} @@ -449,8 +461,8 @@ def test_to_config_with_attributes_on_only_some_states(self): self.pi.messages_disabled = {"ON": True} config = self.pi.to_config("TELEMETRY", "BIG_ENDIAN") self.assertIn(" STATE OFF 0\n", config) - self.assertIn(" STATE ON 1 DISABLE_MESSAGES GREEN\n", config) - self.assertIn(" STATE FAULT 2 HAZARDOUS", config) + self.assertIn(" STATE ON 1 GREEN\n", config) + self.assertIn(" STATE FAULT 2\n", config) def test_to_config_writes_non_default_endianness(self): pi = PacketItem("test", 0, 32, "UINT", "LITTLE_ENDIAN", None) diff --git a/openc3/spec/packets/packet_item_spec.rb b/openc3/spec/packets/packet_item_spec.rb index 9310d960e5..ba2c97cbf6 100644 --- a/openc3/spec/packets/packet_item_spec.rb +++ b/openc3/spec/packets/packet_item_spec.rb @@ -417,6 +417,22 @@ module OpenC3 expect(config).to match(/STATE FALSE 0 RED/) end + it "only writes the state attributes valid for the packet type" do + @pi.range = (0..2) + @pi.states = { "OFF" => 0, "ON" => 1, "FAULT" => 2 } + @pi.hazardous = { "FAULT" => "" } + @pi.messages_disabled = { "ON" => true } + @pi.state_colors = { "ON" => :GREEN } + config = @pi.to_config(:COMMAND, :BIG_ENDIAN) + expect(config).to include(" STATE OFF 0\n") + expect(config).to include(" STATE ON 1 DISABLE_MESSAGES\n") + expect(config).to include(" STATE FAULT 2 HAZARDOUS\n") + config = @pi.to_config(:TELEMETRY, :BIG_ENDIAN) + expect(config).to include(" STATE OFF 0\n") + expect(config).to include(" STATE ON 1 GREEN\n") + expect(config).to include(" STATE FAULT 2\n") + end + it "sets the state_colors to nil" do @pi.state_colors = nil expect(@pi.state_colors).to be_nil From ad554de3a54679fe44099aa3752e25ab82f0b249 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Mon, 26 Oct 2026 21:59:59 -0600 Subject: [PATCH 3/3] fix(python): write loadable class filenames in to_config - Write the module path, not the class name, for PROCESSOR, READ/WRITE_CONVERSION, LIMITS_RESPONSE, ACCESSOR, VALIDATOR and SUBPACKETIZER via a new class_to_filename helper, so output loads back - Fix WatermarkProcessor.as_json, which raised AttributeError - Fix IpWriteConversion.to_config missing its read_or_write argument and PacketItem.to_config not calling limits.response.to_config() - Omit an unset microseconds item in UnixTimeConversion.to_config instead of writing a literal None Co-Authored-By: Claude Opus 5.5 --- .../python/openc3/conversions/conversion.py | 7 ++- .../openc3/conversions/ip_write_conversion.py | 4 +- .../conversions/processor_conversion.py | 5 +- .../conversions/unix_time_conversion.py | 6 ++- .../python/openc3/packets/limits_response.py | 5 +- openc3/python/openc3/packets/packet.py | 7 +-- openc3/python/openc3/packets/packet_item.py | 2 +- openc3/python/openc3/processors/processor.py | 3 +- .../openc3/processors/statistics_processor.py | 3 +- .../openc3/processors/watermark_processor.py | 5 +- openc3/python/openc3/utilities/string.py | 9 ++++ openc3/python/test/packets/test_packet.py | 2 +- .../python/test/packets/test_packet_config.py | 49 +++++++++++++++++++ .../python/test/packets/test_packet_item.py | 4 +- .../processors/test_watermark_processor.py | 6 +++ 15 files changed, 98 insertions(+), 19 deletions(-) diff --git a/openc3/python/openc3/conversions/conversion.py b/openc3/python/openc3/conversions/conversion.py index 85bfb89c1f..ae677a4c84 100644 --- a/openc3/python/openc3/conversions/conversion.py +++ b/openc3/python/openc3/conversions/conversion.py @@ -1,4 +1,4 @@ -# Copyright 2024 OpenC3, Inc. +# Copyright 2026 OpenC3, Inc. # All Rights Reserved. # # This program is distributed in the hope that it will be useful, @@ -10,6 +10,9 @@ # if purchased from OpenC3, Inc. +from openc3.utilities.string import class_to_filename + + class Conversion: """Performs a general conversion via the implementation of the call method""" @@ -47,7 +50,7 @@ def __str__(self): # self.param read_or_write [String] Either 'READ' or 'WRITE' # self.return [String] Config fragment for this conversion def to_config(self, read_or_write): - return f" {read_or_write}_CONVERSION {self.__class__.__name__}\n" + return f" {read_or_write}_CONVERSION {class_to_filename(self.__class__)}\n" def as_json(self): result = {} diff --git a/openc3/python/openc3/conversions/ip_write_conversion.py b/openc3/python/openc3/conversions/ip_write_conversion.py index 260b272603..f84b9b0cc3 100644 --- a/openc3/python/openc3/conversions/ip_write_conversion.py +++ b/openc3/python/openc3/conversions/ip_write_conversion.py @@ -36,5 +36,5 @@ def __str__(self): # @param read_or_write [String] Either 'READ' or 'WRITE' # @return [String] Config fragment for this conversion - def to_config(self): - return f"{self}_CONVERSION openc3/conversions/ip_write_conversion.py\n" + def to_config(self, read_or_write): + return f"{read_or_write}_CONVERSION openc3/conversions/ip_write_conversion.py\n" diff --git a/openc3/python/openc3/conversions/processor_conversion.py b/openc3/python/openc3/conversions/processor_conversion.py index bdf134527c..00002cd9ce 100644 --- a/openc3/python/openc3/conversions/processor_conversion.py +++ b/openc3/python/openc3/conversions/processor_conversion.py @@ -1,4 +1,4 @@ -# Copyright 2024 OpenC3, Inc. +# Copyright 2026 OpenC3, Inc. # All Rights Reserved. # # This program is distributed in the hope that it will be useful, @@ -13,6 +13,7 @@ from openc3.accessors.binary_accessor import BinaryAccessor from openc3.config.config_parser import ConfigParser from openc3.conversions.conversion import Conversion +from openc3.utilities.string import class_to_filename class ProcessorConversion(Conversion): @@ -61,7 +62,7 @@ def __str__(self): # @param (see Conversion#to_config) # @return [String] Config fragment for this conversion def to_config(self, read_or_write): - config = f" {read_or_write}_CONVERSION {self.__class__.__name__} {self.processor_name} {self.result_name}" + config = f" {read_or_write}_CONVERSION {class_to_filename(self.__class__)} {self.processor_name} {self.result_name}" if self.converted_type is not None: config += f" {self.converted_type}" if self.converted_bit_size is not None: diff --git a/openc3/python/openc3/conversions/unix_time_conversion.py b/openc3/python/openc3/conversions/unix_time_conversion.py index 0d86467a9b..928a4f369d 100644 --- a/openc3/python/openc3/conversions/unix_time_conversion.py +++ b/openc3/python/openc3/conversions/unix_time_conversion.py @@ -13,6 +13,7 @@ from datetime import datetime, timezone from openc3.conversions.conversion import Conversion +from openc3.utilities.string import class_to_filename # Converts a unix format time: Epoch Jan 1 1970, seconds and microseconds @@ -54,4 +55,7 @@ def __str__(self): # @param (see Conversion#to_config) # @return [String] Config fragment for this conversion def to_config(self, read_or_write): - return f" {read_or_write}_CONVERSION {self.__class__.__name__} {self.seconds_item_name} {self.microseconds_item_name}\n" + config = f" {read_or_write}_CONVERSION {class_to_filename(self.__class__)} {self.seconds_item_name}" + if self.microseconds_item_name: + config += f" {self.microseconds_item_name}" + return config + "\n" diff --git a/openc3/python/openc3/packets/limits_response.py b/openc3/python/openc3/packets/limits_response.py index 0e116d127f..a3afbdc028 100644 --- a/openc3/python/openc3/packets/limits_response.py +++ b/openc3/python/openc3/packets/limits_response.py @@ -10,6 +10,9 @@ # if purchased from OpenC3, Inc. +from openc3.utilities.string import class_to_filename + + class LimitsResponse: """This class defines a #call method which is called case a PacketItem goes out of limits. This class must be subclassed and the call method @@ -27,7 +30,7 @@ def call(self, packet, item, old_limits_state): raise RuntimeError("call method must be defined by subclass") def to_config(self): - return f" LIMITS_RESPONSE {self.__class__.__name__}\n" + return f" LIMITS_RESPONSE {class_to_filename(self.__class__)}\n" def as_json(self): return {"class": self.__class__.__name__} diff --git a/openc3/python/openc3/packets/packet.py b/openc3/python/openc3/packets/packet.py index 188c4fc2f6..1e21d81e9a 100644 --- a/openc3/python/openc3/packets/packet.py +++ b/openc3/python/openc3/packets/packet.py @@ -33,6 +33,7 @@ ) from openc3.utilities.logger import Logger from openc3.utilities.string import ( + class_to_filename, quote_if_necessary, simple_formatted, ) @@ -1051,13 +1052,13 @@ def to_config(self, cmd_or_tlm): config += f'COMMAND {quote_if_necessary(self.target_name)} {quote_if_necessary(self.packet_name)} {self.default_endianness} "{self.description}"\n' if self.subpacketizer: args_str = " ".join([quote_if_necessary(str(a)) for a in self.subpacketizer.args]) - config += f" SUBPACKETIZER {self.subpacketizer.__class__.__name__} {args_str}\n" + config += f" SUBPACKETIZER {class_to_filename(self.subpacketizer.__class__)} {args_str}\n" if self.accessor.__class__.__name__ != "BinaryAccessor": args_str = " ".join([quote_if_necessary(str(a)) for a in self.accessor.args]) - config += f" ACCESSOR {self.accessor.__class__.__name__} {args_str}\n" + config += f" ACCESSOR {class_to_filename(self.accessor.__class__)} {args_str}\n" if self.validator: args_str = " ".join([quote_if_necessary(str(a)) for a in self.validator.args]) - config += f" VALIDATOR {self.validator.__class__.__name__} {args_str}\n" + config += f" VALIDATOR {class_to_filename(self.validator.__class__)} {args_str}\n" if self.template: config += f" TEMPLATE_BASE64 {base64.b64encode(self.template).decode('ascii')}\n" if self.short_buffer_allowed: diff --git a/openc3/python/openc3/packets/packet_item.py b/openc3/python/openc3/packets/packet_item.py index 3ff7ea9f1f..6863ea0545 100644 --- a/openc3/python/openc3/packets/packet_item.py +++ b/openc3/python/openc3/packets/packet_item.py @@ -443,7 +443,7 @@ def to_config(self, cmd_or_tlm, default_endianness): else: config += "\n" if limits.response: - config += limits.response.to_config + config += limits.response.to_config() if self.__meta: for key, values in self.__meta.items(): diff --git a/openc3/python/openc3/processors/processor.py b/openc3/python/openc3/processors/processor.py index 935b1962e3..7f6fb9b036 100644 --- a/openc3/python/openc3/processors/processor.py +++ b/openc3/python/openc3/processors/processor.py @@ -12,6 +12,7 @@ import copy from openc3.packets.packet import Packet +from openc3.utilities.string import class_to_filename class Processor: @@ -60,7 +61,7 @@ def clone(self): # Convert to configuration file string def to_config(self): - return f" PROCESSOR {self.name} {self.__class__.__name__} {self.value_type}\n" + return f" PROCESSOR {self.name} {class_to_filename(self.__class__)} {self.value_type}\n" def as_json(self): return { diff --git a/openc3/python/openc3/processors/statistics_processor.py b/openc3/python/openc3/processors/statistics_processor.py index daece825e3..228ddf743f 100644 --- a/openc3/python/openc3/processors/statistics_processor.py +++ b/openc3/python/openc3/processors/statistics_processor.py @@ -13,6 +13,7 @@ import statistics from openc3.processors.processor import Processor +from openc3.utilities.string import class_to_filename class StatisticsProcessor(Processor): @@ -63,7 +64,7 @@ def clone(self): # Convert to configuration file string def to_config(self): - return f" PROCESSOR {self.name} {self.__class__.__name__} {self.item_name} {self.samples_to_average} {self.value_type}\n" + return f" PROCESSOR {self.name} {class_to_filename(self.__class__)} {self.item_name} {self.samples_to_average} {self.value_type}\n" def as_json(self): return { diff --git a/openc3/python/openc3/processors/watermark_processor.py b/openc3/python/openc3/processors/watermark_processor.py index 75354d2673..7b29ab2ac0 100644 --- a/openc3/python/openc3/processors/watermark_processor.py +++ b/openc3/python/openc3/processors/watermark_processor.py @@ -10,6 +10,7 @@ # if purchased from OpenC3, Inc. from openc3.processors.processor import Processor +from openc3.utilities.string import class_to_filename class WatermarkProcessor(Processor): @@ -39,11 +40,11 @@ def reset(self): # Convert to configuration file string def to_config(self): - return f" PROCESSOR {self.name} {self.__class__.__name__} {self.item_name} {self.value_type}\n" + return f" PROCESSOR {self.name} {class_to_filename(self.__class__)} {self.item_name} {self.value_type}\n" def as_json(self): return { "name": self.name, - "class": self.__class__.__name__.name, + "class": self.__class__.__name__, "params": [self.item_name, self.value_type], } diff --git a/openc3/python/openc3/utilities/string.py b/openc3/python/openc3/utilities/string.py index 4dc5341a79..d799a5f2bd 100644 --- a/openc3/python/openc3/utilities/string.py +++ b/openc3/python/openc3/utilities/string.py @@ -142,6 +142,15 @@ def class_name_to_filename(string, include_extension=False): return filename +# Returns the filename config keywords load a class from, i.e. the class +# MyGreatClass in module path.to.my_great_class returns "path/to/my_great_class.py". +# This is the inverse of filename_to_module and filename_to_class_name. +# +# self.return [String] Filename which implements the class +def class_to_filename(klass): + return klass.__module__.replace(".", "/") + ".py" + + # Converts a String representing a filename (i.e. "path/to/my_great_class.py") # to a Python class name (i.e. "MyGreatClass"). # diff --git a/openc3/python/test/packets/test_packet.py b/openc3/python/test/packets/test_packet.py index de6df04419..8a9aef99a3 100644 --- a/openc3/python/test/packets/test_packet.py +++ b/openc3/python/test/packets/test_packet.py @@ -2364,5 +2364,5 @@ def test_writes_processors_and_meta(self): p.processors["PROC"] = Processor() p.meta["TYPE"] = ["SENSOR", "LIGHT"] config = p.to_config("TELEMETRY") - self.assertIn(" PROCESSOR PROCESSOR Processor CONVERTED\n", config) + self.assertIn(" PROCESSOR PROCESSOR openc3/processors/processor.py CONVERTED\n", config) self.assertIn(" META TYPE SENSOR LIGHT\n", config) diff --git a/openc3/python/test/packets/test_packet_config.py b/openc3/python/test/packets/test_packet_config.py index 4b27db68d5..aa3b231840 100644 --- a/openc3/python/test/packets/test_packet_config.py +++ b/openc3/python/test/packets/test_packet_config.py @@ -1776,6 +1776,55 @@ def test_to_config_exports_limits_groups(self): os.unlink(tf.name) + def test_to_config_output_parses_back_to_the_same_classes(self): + """Test that keywords naming a class are written as a filename the parser can load""" + with tempfile.NamedTemporaryFile(mode="w", suffix=".txt", delete=False) as tf: + tf.write('TELEMETRY TGT1 PKT1 BIG_ENDIAN "Packet"\n') + tf.write(" SUBPACKETIZER openc3/subpacketizers/subpacketizer.py\n") + tf.write(" PROCESSOR WATER openc3/processors/watermark_processor.py ITEM1\n") + tf.write(" PROCESSOR STATS openc3/processors/statistics_processor.py ITEM1 10\n") + tf.write(" ITEM ITEM1 0 16 UINT\n") + tf.write(" READ_CONVERSION openc3/conversions/received_count_conversion.py\n") + tf.write(" ITEM ITEM2 16 16 UINT\n") + tf.write(" READ_CONVERSION openc3/conversions/unix_time_conversion.py ITEM1\n") + tf.write(" ITEM ITEM3 32 16 UINT\n") + tf.write(" READ_CONVERSION openc3/conversions/processor_conversion.py WATER HIGH_WATER UINT 16\n") + tf.write(" LIMITS DEFAULT 1 ENABLED 1 2 6 7\n") + tf.write(" LIMITS_RESPONSE openc3/packets/limits_response.py\n") + tf.write('TELEMETRY TGT1 PKT2 BIG_ENDIAN "Packet"\n') + tf.write(" ACCESSOR openc3/accessors/json_accessor.py\n") + tf.write(" ITEM ITEM1 0 0 UINT\n") + tf.write(" KEY $.item1\n") + tf.write('COMMAND TGT1 CMD1 BIG_ENDIAN "Command"\n') + tf.write(" VALIDATOR openc3/packets/command_validator.py\n") + tf.write(' PARAMETER PARAM1 0 32 STRING "1.2.3.4"\n') + tf.write(" WRITE_CONVERSION openc3/conversions/ip_write_conversion.py\n") + tf.seek(0) + self.pc.process_file(tf.name, "TGT1") + + with tempfile.TemporaryDirectory() as output_dir: + self.pc.to_config(output_dir) + pc = PacketConfig() + pc.process_file(os.path.join(output_dir, "TGT1", "cmd_tlm", "tgt1_tlm.txt"), "TGT1") + pc.process_file(os.path.join(output_dir, "TGT1", "cmd_tlm", "tgt1_cmd.txt"), "TGT1") + + def classes(packets): + pkt1 = packets.telemetry["TGT1"]["PKT1"] + cmd1 = packets.commands["TGT1"]["CMD1"] + return { + "subpacketizer": pkt1.subpacketizer.as_json(), + "processors": [processor.as_json() for processor in pkt1.processors.values()], + "conversions": [pkt1.get_item(name).read_conversion.as_json() for name in ["ITEM1", "ITEM2", "ITEM3"]], + "limits_response": pkt1.get_item("ITEM3").limits.response.as_json(), + "accessor": packets.telemetry["TGT1"]["PKT2"].accessor.__class__.__name__, + "validator": cmd1.validator.__class__.__name__, + "write_conversion": cmd1.get_item("PARAM1").write_conversion.as_json(), + } + + self.assertEqual(classes(pc), classes(self.pc)) + + os.unlink(tf.name) + def test_to_config_skips_unknown_target(self): """Test that to_config skips UNKNOWN target""" # PacketConfig always creates UNKNOWN target/packets by default diff --git a/openc3/python/test/packets/test_packet_item.py b/openc3/python/test/packets/test_packet_item.py index 1923441116..fabb902875 100644 --- a/openc3/python/test/packets/test_packet_item.py +++ b/openc3/python/test/packets/test_packet_item.py @@ -51,7 +51,7 @@ def test_accepts_read_conversion_instances(self): c = Conversion() self.pi.read_conversion = c config = self.pi.to_config("TELEMETRY", "BIG_ENDIAN") - self.assertIn("READ_CONVERSION Conversion", config) + self.assertIn("READ_CONVERSION openc3/conversions/conversion.py\n", config) def test_sets_the_read_conversion_to_None(self): self.pi.read_conversion = None @@ -68,7 +68,7 @@ def test_accepts_write_conversion_instances(self): c = Conversion() self.pi.write_conversion = c config = self.pi.to_config("TELEMETRY", "BIG_ENDIAN") - self.assertIn("WRITE_CONVERSION Conversion", config) + self.assertIn("WRITE_CONVERSION openc3/conversions/conversion.py\n", config) def test_sets_the_write_conversion_to_None(self): self.pi.write_conversion = None diff --git a/openc3/python/test/processors/test_watermark_processor.py b/openc3/python/test/processors/test_watermark_processor.py index 1fd3f208f5..56c62585d1 100644 --- a/openc3/python/test/processors/test_watermark_processor.py +++ b/openc3/python/test/processors/test_watermark_processor.py @@ -42,3 +42,9 @@ def test_generates_a_high_and_low_water_mark(self): p.reset() self.assertEqual(p.results["HIGH_WATER"], None) self.assertEqual(p.results["LOW_WATER"], None) + + def test_as_json(self): + wp = WatermarkProcessor("TEST", "RAW") + self.assertEqual( + wp.as_json(), {"name": "WATERMARKPROCESSOR", "class": "WatermarkProcessor", "params": ["TEST", "RAW"]} + )