Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions openc3/lib/openc3/utilities/questdb_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -928,6 +928,10 @@ def self.decode_reduced_row(row)
# @return [Array, Hash] Array of [value, limits_state] pairs per row, or {} if no results.
# Single-row results return a flat array; multi-row results return array of arrays.
def self.tsdb_lookup(items, start_time:, end_time: nil, scope: "DEFAULT")
# Every item is a placeholder for an item which doesn't exist, so there's
# nothing to query. Return a single row of nil values, one per item.
return Array.new(items.length) { [nil, nil] } if items.all? { |item| item[2].nil? }

# Group items by db_shard number while preserving their original positions
db_shard_groups = {} # db_shard => { positions: [], items: [] }
items.each_with_index do |item, pos|
Expand Down Expand Up @@ -1060,6 +1064,11 @@ def self.tsdb_lookup_single_db_shard(items, start_time:, end_time: nil, scope: "
end
end

# Every item in this db_shard is a placeholder so there's no table to query.
# Return no results and let tsdb_lookup fill these positions with [nil, nil]
# when it merges the db_shards (an all placeholder lookup returns before this)
return {} if tables.empty?

# Add needed timestamp columns to the SELECT for calculated items
needed_timestamps.each do |table_index, ts_columns|
ts_columns.each do |ts_col|
Expand Down
23 changes: 17 additions & 6 deletions openc3/python/openc3/api/tlm_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,19 +393,30 @@ def get_tlm_values(
end_time=None,
scope=OPENC3_SCOPE,
):
if not isinstance(items, list) or len(items) == 0 or not isinstance(items[0], str):
if not isinstance(items, list) or len(items) == 0:
raise TypeError("items must be array of strings: ['TGT__PKT__ITEM__TYPE', ...]")
packets = []
cvt_items = []
for item in items:
try:
target_name, packet_name, item_name, value_type = item.upper().split("__")
except ValueError:
raise ValueError("items must be formatted as TGT__PKT__ITEM__TYPE") from None
# get_tlm_available returns None for items which don't exist and its result is
# passed directly here, so None is a placeholder which returns a None value
if item is None:
cvt_items.append([None, None, None, None, None])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't match ruby's tlm_api.rb at line 335. I think this is correct and Ruby should be fixed. Note that tlm_api.rb 335 is currently a noop because line 344 over writes it each time. Add tests for Ruby and make it match implementation.

continue
if not isinstance(item, str):
raise TypeError("items must be array of strings: ['TGT__PKT__ITEM__TYPE', ...]")
# get_tlm_available tacks on __LIMITS to indicate a limits value is available
# so accept both TGT__PKT__ITEM__TYPE and TGT__PKT__ITEM__TYPE__LIMITS
parts = item.upper().split("__")
if len(parts) < 4 or len(parts) > 5:
raise ValueError("items must be formatted as TGT__PKT__ITEM__TYPE")
target_name, packet_name, item_name, value_type = parts[0:4]
limits = parts[4] if len(parts) == 5 else None
if packet_name == "LATEST":
packet_name = CvtModel.determine_latest_packet_for_item(target_name, item_name, cache_timeout, scope)
# Change packet_name in case of LATEST and ensure upcase
cvt_items.append([target_name, packet_name, item_name, value_type])
# NOTE: limits is required by the historical (start_time) QuestDB lookup
cvt_items.append([target_name, packet_name, item_name, value_type, limits])
packets.append([target_name, packet_name])
# Make the array of arrays unique
packets = [list(x) for x in {tuple(x) for x in packets}]
Expand Down
15 changes: 13 additions & 2 deletions openc3/python/openc3/models/cvt_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,12 @@ def get_tlm_values(
for item in items:
cls._parse_item(now, lookups, overrides, item, cache_timeout=cache_timeout, scope=scope)

for target_packet_key, target_name, packet_name, value_keys in lookups:
for lookup in lookups:
# Set in _parse_item for an item which doesn't exist
if lookup is None:
results.append([None, None])
continue
target_packet_key, target_name, packet_name, value_keys = lookup
if target_packet_key not in packet_lookup:
packet_lookup[target_packet_key] = cls.get(
target_name,
Expand Down Expand Up @@ -438,7 +443,13 @@ def _get_overrides(cls, now, tgt_pkt_key, overrides, target_name, packet_name, c
# return an ordered array of dict with keys
@classmethod
def _parse_item(cls, now, lookups, overrides, item, cache_timeout, scope):
target_name, packet_name, item_name, value_type = item
# Items can also carry a trailing limits element (see get_tlm_values) which
# is only used by the historical QuestDB lookup
target_name, packet_name, item_name, value_type = item[0:4]
# They are all None when the item doesn't exist (see get_tlm_available)
if item_name is None:
lookups.append(None)
return

# We build lookup keys by including all the less formatted types to gracefully degrade lookups
# This allows the user to specify FORMATTED and if there is no conversions it will simply return the RAW value
Expand Down
5 changes: 5 additions & 0 deletions openc3/python/openc3/utilities/questdb_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,11 @@ def tsdb_lookup(cls, items, start_time, end_time=None, scope="DEFAULT"):
Array of [value, limits_state] pairs per row, or {} if no results.
Single-row results return a flat array; multi-row results return array of arrays.
"""
# Every item is a placeholder for an item which doesn't exist, so there's
# nothing to query. Return a single row of None values, one per item.
if all(item[2] is None for item in items):
return [[None, None] for _ in items]

tables = {}
names = []
nil_count = 0
Expand Down
58 changes: 58 additions & 0 deletions openc3/python/test/api/test_tlm_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,64 @@ def test_get_tlm_values_complains_about_bad_arguments(self):
get_tlm_values([["INST", "HEALTH_STATUS", "TEMP1"]])
with self.assertRaisesRegex(ValueError, "items must be formatted"):
get_tlm_values(["INST", "HEALTH_STATUS", "TEMP1"])
with self.assertRaisesRegex(ValueError, "items must be formatted"):
get_tlm_values(["INST__HEALTH_STATUS__TEMP1__CONVERTED__LIMITS__EXTRA"])

def test_get_tlm_values_historical_passes_the_limits_flag_to_the_tsdb_lookup(self):
# get_tlm_available tacks on __LIMITS which must be accepted and passed along
items = [
"INST__HEALTH_STATUS__TEMP1__CONVERTED",
"INST__HEALTH_STATUS__TEMP2__CONVERTED__LIMITS",
]
with patch("openc3.models.cvt_model.QuestDBClient.tsdb_lookup") as tsdb_lookup:
tsdb_lookup.return_value = [[[0.0, None], [(-100.0), "RED_LOW"]]]
vals = get_tlm_values(items, start_time="2026-09-13T00:00:00Z", end_time="2026-09-13T01:00:00Z")
self.assertEqual(vals, [[[0.0, None], [(-100.0), "RED_LOW"]]])
lookup_items = tsdb_lookup.call_args[0][0]
self.assertEqual(lookup_items[0], ["INST", "HEALTH_STATUS", "TEMP1", "CONVERTED", None])
self.assertEqual(lookup_items[1], ["INST", "HEALTH_STATUS", "TEMP2", "CONVERTED", "LIMITS"])

def test_get_tlm_values_accepts_the_limits_suffix_from_the_cvt(self):
vals = get_tlm_values(["INST__HEALTH_STATUS__TEMP1__CONVERTED__LIMITS"])
self.assertEqual(vals[0][0], (-100.0))
self.assertEqual(vals[0][1], "RED_LOW")

def test_get_tlm_values_returns_none_for_items_which_do_not_exist(self):
# get_tlm_available returns None for an item which doesn't exist
vals = get_tlm_values([None, "INST__HEALTH_STATUS__TEMP1__CONVERTED", None])
self.assertEqual(vals[0], [None, None])
self.assertEqual(vals[1][0], (-100.0))
self.assertEqual(vals[1][1], "RED_LOW")
self.assertEqual(vals[2], [None, None])

def test_get_tlm_values_takes_the_get_tlm_available_result_directly(self):
items = [
"INST__HEALTH_STATUS__TEMP1__CONVERTED",
"INST__HEALTH_STATUS__BLAH__CONVERTED",
]
available = get_tlm_available(items)
self.assertEqual(available[0], "INST__HEALTH_STATUS__TEMP1__CONVERTED__LIMITS")
self.assertIsNone(available[1])
vals = get_tlm_values(available)
self.assertEqual(vals[0][0], (-100.0))
self.assertEqual(vals[0][1], "RED_LOW")
self.assertEqual(vals[1], [None, None])

def test_get_tlm_values_historical_returns_nones_when_no_item_exists(self):
# Nothing to query, so this doesn't need (or touch) the time series database
vals = get_tlm_values([None, None], start_time="2026-09-13T00:00:00Z", end_time="2026-09-13T01:00:00Z")
self.assertEqual(vals, [[None, None], [None, None]])

def test_get_tlm_values_historical_passes_a_placeholder_for_items_which_do_not_exist(self):
with patch("openc3.models.cvt_model.QuestDBClient.tsdb_lookup") as tsdb_lookup:
tsdb_lookup.return_value = [[[0.0, None], [None, None]]]
get_tlm_values(
["INST__HEALTH_STATUS__TEMP1__CONVERTED", None],
start_time="2026-09-13T00:00:00Z",
end_time="2026-09-13T01:00:00Z",
)
lookup_items = tsdb_lookup.call_args[0][0]
self.assertEqual(lookup_items[1], [None, None, None, None, None])

def test_get_tlm_values_reads_all_the_specified_items(self):
items = []
Expand Down
18 changes: 18 additions & 0 deletions openc3/python/test/utilities/test_questdb_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,24 @@
from openc3.utilities.questdb_client import QuestDBClient


class TestTsdbLookup(unittest.TestCase):
def test_returns_a_row_of_nones_when_every_item_is_a_placeholder(self):
# get_tlm_available returns None for items which don't exist, which arrive
# here as [None, None, None, None, None]. There's no table to query so the
# values come back None rather than building a query with no FROM clause.
items = [[None] * 5, [None] * 5, [None] * 5]
self.assertEqual(
QuestDBClient.tsdb_lookup(items, start_time="2026-09-13T00:00:00Z", end_time="2026-09-13T01:00:00Z"),
[[None, None], [None, None], [None, None]],
)

def test_returns_a_row_of_nones_for_a_placeholder_without_an_end_time(self):
self.assertEqual(
QuestDBClient.tsdb_lookup([[None] * 5], start_time="2026-09-13T00:00:00Z"),
[[None, None]],
)


class TestBuildAggregationSelects(unittest.TestCase):
def test_aggregates_raw_column_for_raw_value_type(self):
selects, mapping = QuestDBClient.build_aggregation_selects("TEMP1", "RAW")
Expand Down
16 changes: 16 additions & 0 deletions openc3/spec/api/tlm_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,22 @@ def test_tlm_unknown(method)
expect { @api.get_tlm_values(["INST", "HEALTH_STATUS", "TEMP1"]) }.to raise_error(ArgumentError, /items must be formatted/)
end

it "returns nil values for items which do not exist" do
# get_tlm_available returns nil for items which don't exist and its result
# is passed directly to get_tlm_values
vals = @api.get_tlm_values([nil, "INST__HEALTH_STATUS__TEMP1__CONVERTED", nil])
expect(vals[0]).to eql([nil, nil])
expect(vals[1][0]).to eql(-100.0)
expect(vals[1][1]).to eql(:RED_LOW)
expect(vals[2]).to eql([nil, nil])
end

it "returns a row of nils when no item exists in a historical query" do
# Nothing to query, so this doesn't need (or touch) the time series database
vals = @api.get_tlm_values([nil, nil], start_time: "2026-09-13T00:00:00Z", end_time: "2026-09-13T01:00:00Z")
expect(vals).to eql([[nil, nil], [nil, nil]])
end

it "reads all the specified items" do
items = []
items << 'inst__Health_Status__Temp1__converted' # Case doesn't matter
Expand Down
14 changes: 14 additions & 0 deletions openc3/spec/utilities/questdb_client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,20 @@

module OpenC3
describe QuestDBClient, no_ext: true do
describe "tsdb_lookup" do
it "returns a row of nils when every item is a placeholder" do
# get_tlm_available returns nil for items which don't exist, which arrive
# here as [nil, nil, nil, nil, nil]. There's no table to query so the values
# come back nil rather than building a query with no FROM clause.
items = Array.new(3) { Array.new(5) }
expect(QuestDBClient.tsdb_lookup(items, start_time: "2026-09-13T00:00:00Z", end_time: "2026-09-13T01:00:00Z")).to eq([[nil, nil], [nil, nil], [nil, nil]])
end

it "returns a row of nils for a placeholder without an end_time" do
expect(QuestDBClient.tsdb_lookup([Array.new(5)], start_time: "2026-09-13T00:00:00Z")).to eq([[nil, nil]])
end
end

describe "numeric_column_type?" do
it "returns true for aggregatable numeric types (case-insensitive)" do
['BYTE', 'SHORT', 'INT', 'LONG', 'FLOAT', 'DOUBLE', 'double', 'float'].each do |type|
Expand Down
Loading