diff --git a/openc3/lib/openc3/api/tlm_api.rb b/openc3/lib/openc3/api/tlm_api.rb index 1b8e13827a..115f38a4a2 100644 --- a/openc3/lib/openc3/api/tlm_api.rb +++ b/openc3/lib/openc3/api/tlm_api.rb @@ -131,7 +131,16 @@ def inject_tlm(target_name, packet_name, item_hash = nil, type: :CONVERTED, manu if item_hash item_hash = item_hash.transform_keys(&:upcase) # Check that the items exist ... exceptions are raised if not - TargetModel.packet_items(target_name, packet_name, item_hash.keys, scope: scope) + items = TargetModel.packet_items(target_name, packet_name, item_hash.keys, scope: scope) + if type == :CONVERTED + # If the type is converted, check that the item states are valid + item_hash.each do |item_name, item_value| + item = items.find { |i| i['name'] == item_name.to_s.upcase } + if item['states'] && !item['states'][item_value] + raise "Unknown state '#{item_value}' for #{item['name']}, must be one of #{item['states'].keys.join(', ')}" + end + end + end else # Check that the packet exists ... exceptions are raised if not TargetModel.packet(target_name, packet_name, scope: scope) diff --git a/openc3/lib/openc3/microservices/interface_decom_common.rb b/openc3/lib/openc3/microservices/interface_decom_common.rb index 6011aad88e..c0563f141a 100644 --- a/openc3/lib/openc3/microservices/interface_decom_common.rb +++ b/openc3/lib/openc3/microservices/interface_decom_common.rb @@ -36,6 +36,10 @@ def handle_inject_tlm(inject_tlm_json) packet.received_count += 1 packet.received_time = Time.now.sys TelemetryTopic.write_packet(packet, scope: @scope) + # If the inject_tlm parameters are bad we rescue so + # interface_microservice and decom_microservice can continue + rescue => e + @logger.error "inject_tlm error due to #{e.message}" end def handle_build_cmd(build_cmd_json, msg_id) diff --git a/openc3/lib/openc3/packets/packet.rb b/openc3/lib/openc3/packets/packet.rb index fc353df3d0..136a9b9ae4 100644 --- a/openc3/lib/openc3/packets/packet.rb +++ b/openc3/lib/openc3/packets/packet.rb @@ -756,7 +756,7 @@ def write_item(item, value, value_type = :CONVERTED, buffer = @buffer) super(item, value, :RAW, buffer) rescue ArgumentError => e if item.states and String === value and e.message =~ /invalid value for/ - raise "Unknown state #{value} for #{item.name}, must be one of #{item.states.keys.join(', ')}" + raise "Unknown state '#{value}' for #{item.name}, must be one of #{item.states.keys.join(', ')}" else raise e end diff --git a/openc3/python/openc3/api/tlm_api.py b/openc3/python/openc3/api/tlm_api.py index 5603e831eb..912a2e35ea 100644 --- a/openc3/python/openc3/api/tlm_api.py +++ b/openc3/python/openc3/api/tlm_api.py @@ -132,7 +132,15 @@ def inject_tlm(target_name, packet_name, item_hash=None, type="CONVERTED", scope if item_hash: item_hash = {k.upper(): v for k, v in item_hash.items()} # Check that the items exist ... exceptions are raised if not - TargetModel.packet_items(target_name, packet_name, item_hash.keys(), scope=scope) + items = TargetModel.packet_items(target_name, packet_name, item_hash.keys(), scope=scope) + if type == 'CONVERTED': + # If the type is converted, check that the item states are valid + for item_name, item_value in item_hash.items(): + item = next((i for i in items if i['name'] == item_name.upper()), None) + if item and item.get('states') and item_value not in item['states']: + raise RuntimeError( + f"Unknown state '{item_value}' for {item['name']}, must be one of {', '.join(item['states'].keys())}" + ) else: # Check that the packet exists ... exceptions are raised if not TargetModel.packet(target_name, packet_name, scope=scope) diff --git a/openc3/python/openc3/packets/packet.py b/openc3/python/openc3/packets/packet.py index 94b1cd3d86..a82e2231b5 100644 --- a/openc3/python/openc3/packets/packet.py +++ b/openc3/python/openc3/packets/packet.py @@ -655,7 +655,7 @@ def write_item(self, item, value, value_type="CONVERTED", buffer=None): super().write_item(item, value, "RAW", buffer) except ValueError as error: if item.states and isinstance(value, str) and "invalid literal for" in repr(error): - raise ValueError(f"Unknown state {value} for {item.name}, must be one of f{', '.join(item.states.keys())}") from error + raise ValueError(f"Unknown state '{value}' for {item.name}, must be one of f{', '.join(item.states.keys())}") from error else: raise error case "FORMATTED" | "WITH_UNITS": diff --git a/openc3/python/test/api/test_limits_api.py b/openc3/python/test/api/test_limits_api.py index e3125fe091..38baf3ccca 100644 --- a/openc3/python/test/api/test_limits_api.py +++ b/openc3/python/test/api/test_limits_api.py @@ -424,8 +424,8 @@ def test_get_overall_limits_state_returns_the_overall_system_limits_state(self): "TEMP2": 0, "TEMP3": 0, "TEMP4": 0, - "GROUND1STATUS": 1, - "GROUND2STATUS": 1, + "GROUND1STATUS": 'CONNECTED', + "GROUND2STATUS": 'CONNECTED', }, ) time.sleep(0.1) diff --git a/openc3/python/test/packets/test_packet.py b/openc3/python/test/packets/test_packet.py index ab57e0b3c4..49b23d3c5d 100644 --- a/openc3/python/test/packets/test_packet.py +++ b/openc3/python/test/packets/test_packet.py @@ -684,7 +684,7 @@ def test_writes_the_converted_value_with_states(self): self.assertEqual(self.buffer, b"\x01\x00\x00\x00") self.p.write_item(i, "FALSE", "CONVERTED", self.buffer) self.assertEqual(self.buffer, b"\x02\x00\x00\x00") - with self.assertRaisesRegex(ValueError, "Unknown state BLAH for ITEM"): + with self.assertRaisesRegex(ValueError, "Unknown state 'BLAH' for ITEM"): self.p.write_item(i, "BLAH", "CONVERTED", self.buffer) i.write_conversion = GenericConversion("value / 2") self.p.write("ITEM", 4, "CONVERTED", self.buffer) diff --git a/openc3/spec/api/cmd_api_spec.rb b/openc3/spec/api/cmd_api_spec.rb index 56c1319e59..04030e60b1 100644 --- a/openc3/spec/api/cmd_api_spec.rb +++ b/openc3/spec/api/cmd_api_spec.rb @@ -168,7 +168,7 @@ def test_cmd_unknown(method) expect { @api.send(method, "INST COLLECT with TYPE #{type}, DURATION 10") }.not_to raise_error else # Non-raw commands still raise because the state parameter is checked during the write - expect { @api.send(method, "INST COLLECT with TYPE #{type}, DURATION 10") }.to raise_error("Unknown state OTHER for TYPE, must be one of NORMAL, SPECIAL") + expect { @api.send(method, "INST COLLECT with TYPE #{type}, DURATION 10") }.to raise_error("Unknown state 'OTHER' for TYPE, must be one of NORMAL, SPECIAL") end else # cmd(), cmd_raw() and (no_hazardous_check variants) check the state parameter and raise diff --git a/openc3/spec/api/limits_api_spec.rb b/openc3/spec/api/limits_api_spec.rb index 734c3f0388..889af2f97b 100644 --- a/openc3/spec/api/limits_api_spec.rb +++ b/openc3/spec/api/limits_api_spec.rb @@ -308,7 +308,7 @@ class ApiTest describe "get_overall_limits_state" do it "returns the overall system limits state" do @api.inject_tlm("INST", "HEALTH_STATUS", - { 'TEMP1' => 0, 'TEMP2' => 0, 'TEMP3' => 0, 'TEMP4' => 0, 'GROUND1STATUS' => 1, 'GROUND2STATUS' => 1 }) + { 'TEMP1' => 0, 'TEMP2' => 0, 'TEMP3' => 0, 'TEMP4' => 0, 'GROUND1STATUS' => 'CONNECTED', 'GROUND2STATUS' => 'CONNECTED' }) sleep 0.1 expect(@api.get_overall_limits_state).to eql "GREEN" # TEMP1 limits: -80.0 -70.0 60.0 80.0 -20.0 20.0 diff --git a/openc3/spec/api/tlm_api_spec.rb b/openc3/spec/api/tlm_api_spec.rb index 259b353c1e..5a93acb02f 100644 --- a/openc3/spec/api/tlm_api_spec.rb +++ b/openc3/spec/api/tlm_api_spec.rb @@ -40,6 +40,7 @@ class ApiTest before(:each) do mock_redis() setup_system() + local_s3() %w(INST SYSTEM).each do |target| model = TargetModel.new(folder_name: target, name: target, scope: "DEFAULT") @@ -57,6 +58,7 @@ class ApiTest end after(:each) do + local_s3_unset() Thread.list.each do |t| t.join if t != Thread.current end diff --git a/openc3/spec/packets/packet_spec.rb b/openc3/spec/packets/packet_spec.rb index 3f15f18a37..643cb966b9 100644 --- a/openc3/spec/packets/packet_spec.rb +++ b/openc3/spec/packets/packet_spec.rb @@ -729,7 +729,7 @@ module OpenC3 expect(@buffer).to eql "\x01\x00\x00\x00" @p.write_item(i, "FALSE", :CONVERTED, @buffer) expect(@buffer).to eql "\x02\x00\x00\x00" - expect { @p.write_item(i, "BLAH", :CONVERTED, @buffer) }.to raise_error(RuntimeError, "Unknown state BLAH for ITEM, must be one of TRUE, FALSE") + expect { @p.write_item(i, "BLAH", :CONVERTED, @buffer) }.to raise_error(RuntimeError, "Unknown state 'BLAH' for ITEM, must be one of TRUE, FALSE") i.write_conversion = GenericConversion.new("value / 2") @p.write("ITEM", 4, :CONVERTED, @buffer) expect(@buffer).to eql "\x02\x00\x00\x00"