Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-34639: rename the <name>ID field to salIndex, implementing RFC-849 #240

Merged
merged 4 commits into from
May 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion doc/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ Version History
v7.1.0
------

* Update for ts_sal 6.2, which is required:

* Remove all references to the "priority" field (RFC-848).
* Rename "{component_name}ID" fields to "salIndex" (RFC-849).


* `BaseCsc`: make ``start`` easier to use by making the handling of the initial state occur after ``start`` is done (using the new ``start_phase2`` `Controller` method).
This allows CSCs to write SAL messages in ``start``, after calling ``await super().start()``, without worrying that transitioning to a non-default initial state writes contradictory information.
* `ConfigurableCsc`: always publish the configurationApplied event when transitioning from STANDBY to DISABLED state.
Expand Down Expand Up @@ -37,7 +43,7 @@ Requirements:
* ts_ddsconfig
* ts_idl 2
* ts_utils 1.1
* IDL files for Test and Script generated from ts_xml 11
* IDL files for Test and Script generated from ts_xml 11 using ts_sal 6.2

v7.0.1
------
Expand Down
6 changes: 2 additions & 4 deletions python/lsst/ts/salobj/csc_commander.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ def __init__(
enable: bool = False,
exclude: typing.Optional[typing.Sequence[str]] = None,
exclude_commands: typing.Sequence[str] = (),
fields_to_ignore: typing.Sequence[str] = ("ignored", "value", "priority"),
fields_to_ignore: typing.Sequence[str] = ("ignored", "value"),
telemetry_fields_to_not_compare: typing.Sequence[str] = ("timestamp",),
) -> None:
self.domain = domain.Domain()
Expand Down Expand Up @@ -340,9 +340,7 @@ def field_is_public(self, name: str) -> bool:
"""Return True if the specified field name is public,
False otherwise.
"""
if name.startswith("private_"):
return False
if name == f"{self.remote.salinfo.name}ID":
if name.startswith("private_") or name == "salIndex":
return False
if name in self.fields_to_ignore:
return False
Expand Down
6 changes: 2 additions & 4 deletions python/lsst/ts/salobj/sal_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,8 +527,7 @@ def parse_metadata(self) -> None:
] = f"{self.name}::{sal_topic_name}_{topic_metadata.version_hash}"

# Examine last topic (or any topic) to see if component is indexed.
indexed_field_name = f"{self.name}ID"
self.indexed = indexed_field_name in topic_metadata.field_info
self.indexed = "salIndex" in topic_metadata.field_info

self.command_names = tuple(command_names)
self.event_names = tuple(event_names)
Expand Down Expand Up @@ -709,9 +708,8 @@ async def start(self) -> None:
if reader.max_history > 0:
if self.index == 0 and self.indexed:
# Get the most recent sample for each index
index_field = f"{self.name}ID"
data_dict = {
getattr(data, index_field): data
getattr(data, "salIndex"): data
for data in full_data_list
}
data_list = data_dict.values()
Expand Down
11 changes: 6 additions & 5 deletions python/lsst/ts/salobj/testcsccommander.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,23 +45,24 @@ class TestCscCommander(csc_commander.CscCommander):
Names of topics (telemetry or events) to not support.
Topic names must not have a ``tel_`` or ``evt_`` prefix.
If `None` or empty then no topics are excluded.
fields_to_ignore : `List` [`str`], optional
SAL topic fields names to ignore when specifying command parameters,
and when printing events and telemetry.
"""

def __init__(
self,
index: typing.Optional[int],
enable: bool = False,
exclude: typing.Optional[typing.Sequence[str]] = None,
fields_to_ignore: typing.Sequence[str] = (
"ignored",
"value",
"priority",
),
fields_to_ignore: typing.Sequence[str] = ("ignored", "value"),
) -> None:
super().__init__(
name="Test",
index=index,
enable=enable,
exclude=exclude,
fields_to_ignore=fields_to_ignore,
)

def asbool(val: str) -> bool:
Expand Down
2 changes: 1 addition & 1 deletion python/lsst/ts/salobj/topics/read_topic.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ def __init__(
]
queries = []
if salinfo.index > 0:
queries.append(f"{salinfo.name}ID = {salinfo.index}")
queries.append(f"salIndex = {salinfo.index}")
if attr_name == "ack_ackcmd" and filter_ackcmd:
queries += [
f"origin = {salinfo.domain.origin}",
Expand Down
2 changes: 1 addition & 1 deletion python/lsst/ts/salobj/topics/write_topic.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ def _basic_write(self) -> None:
# when index is 0 use the default of 0 and give senders a chance
# to override it.
if self.salinfo.index != 0:
setattr(self.data, f"{self.salinfo.name}ID", self.salinfo.index)
self.data.salIndex = self.salinfo.index

try:
self._writer.write(self.data)
Expand Down
9 changes: 3 additions & 6 deletions python/lsst/ts/salobj/type_hints.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,16 @@
class BaseMsgType:
r"""Base DDS sample data type, for type annotations.

This is missing:

* _SAL_component_name_\ ID (e.g. ScriptID): only present
for indexed SAL components
* priority: present for events, but not used
* all topic-specific public fields
This has the ``salIndex`` field, which is only present for indexed
SAL components, and is missing all topic-specific fields.
"""
private_revCode: str = ""
private_sndStamp: float = 0
private_rcvStamp: float = 0
private_seqNum: int = 0
private_identity: str = ""
private_origin: int = 0
salIndex: int = 0

def get_vars(self) -> typing.Dict[str, typing.Any]:
raise NotImplementedError()
Expand Down
9 changes: 4 additions & 5 deletions tests/data/sal_revCoded_SimpleWithMetadata.idl
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SAL_VERSION=5.1.1 XML_VERSION=9.2.0
module Simple {
struct command_setArrays_0dd79125 { // @Metadata=(Description="Description of command_setArrays")
long TestID; //private // @Metadata=(Description="Description of TestID")
long salIndex; //private // @Metadata=(Description="Description of salIndex")
string<8> private_revCode; //private // @Metadata=(Units="unitless",Description="Description of private_revCode")
double private_sndStamp; //private // @Metadata=(Units="second",Description="Description of private_sndStamp")
double private_rcvStamp; //private // @Metadata=(Units="second",Description="Description of private_rcvStamp")
Expand All @@ -20,13 +20,13 @@ struct command_setArrays_0dd79125 { // @Metadata=(Description="Description of c
float float0[5]; // @Metadata=(Units="unitless",Description="Description of float0")
double double0[5]; // @Metadata=(Units="unitless",Description="Description of double0")
};
#pragma keylist command_setArrays_0dd79125 SimpleID
#pragma keylist command_setArrays_0dd79125 salIndex
const long arrays_Int0ValueEnum_Zero=0;
const long arrays_Int0ValueEnum_Two=2;
const long arrays_Int0ValueEnum_Four=04;
const long arrays_Int0ValueEnum_Five=0x05;
struct logevent_scalars_0ad55b18 { // @Metadata=(Description="Description of logevent_scalars")
long TestID; //private // @Metadata=(Description="Description of TestID")
long salIndex; //private // @Metadata=(Description="Description of salIndex")
string<8> private_revCode; //private // @Metadata=(Units="unitless",Description="Description of private_revCode")
double private_sndStamp; //private // @Metadata=(Units="second",Description="Description of private_sndStamp")
double private_rcvStamp; //private // @Metadata=(Units="second",Description="Description of private_rcvStamp")
Expand All @@ -45,7 +45,6 @@ struct logevent_scalars_0ad55b18 { // @Metadata=(Description="Description of lo
float float0; // @Metadata=(Units="unitless",Description="Description of float0")
double double0; // @Metadata=(Units="unitless",Description="Description of double0")
string<20> string0; // @Metadata=(Units="unitless",Description="Description of string0")
long priority; // @Metadata=(Units="unitless",Description="Description of priority")
};
#pragma keylist logevent_scalars_0ad55b18 SimpleID
#pragma keylist logevent_scalars_0ad55b18 salIndex
};
11 changes: 5 additions & 6 deletions tests/data/sal_revCoded_SimpleWithoutMetadata.idl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module Simple {
struct command_setArrays_0dd79125 {
long TestID; //private
long salIndex; //private
string<8> private_revCode;
double private_sndStamp;
double private_rcvStamp;
Expand All @@ -19,14 +19,14 @@ struct command_setArrays_0dd79125 {
float float0[5];
double double0[5];
};
#pragma keylist command_setArrays_0dd79125 TestID
#pragma keylist command_setArrays_1234abcd TestID
#pragma keylist command_setArrays_0dd79125 salIndex
#pragma keylist command_setArrays_1234abcd salIndex
const long arrays_Int0ValueEnum_Zero=0;
const long arrays_Int0ValueEnum_Two=2;
const long arrays_Int0ValueEnum_Four=04;
const long arrays_Int0ValueEnum_Five=0x05;
struct logevent_scalars_0ad55b18 {
long TestID; //private
long salIndex; //private
string<8> private_revCode;
double private_sndStamp;
double private_rcvStamp;
Expand All @@ -45,7 +45,6 @@ struct logevent_scalars_0ad55b18 {
float float0;
double double0;
string<20> string0;
long priority;
};
#pragma keylist logevent_scalars_0ad55b18 TestID
#pragma keylist logevent_scalars_0ad55b18 salIndex
};
2 changes: 1 addition & 1 deletion tests/test_csc_commander.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ async def test_basics(self) -> None:
scalar_fields = [
field
for field in self.csc.cmd_setScalars.DataType().get_vars().keys()
if not field.startswith("private_") and field != "TestID"
if not field.startswith("private_") and field != "salIndex"
]
bool_index = scalar_fields.index("boolean0")
n_scalar_fields = len(scalar_fields)
Expand Down
21 changes: 4 additions & 17 deletions tests/test_idl_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def check_parse_simple(self, has_metadata: bool) -> None:

# Dict of field name: expected type name
field_types = dict(
TestID="long",
salIndex="long",
private_revCode="string",
private_sndStamp="double",
private_rcvStamp="double",
Expand All @@ -83,7 +83,6 @@ def check_parse_simple(self, has_metadata: bool) -> None:
unsignedLong0="unsigned long",
float0="float",
double0="double",
priority="long",
)

for sal_topic_name, topic_metadata in metadata.topic_info.items():
Expand All @@ -103,9 +102,7 @@ def check_parse_simple(self, has_metadata: bool) -> None:
expected_field_names = set(field_types.keys())
if sal_topic_name == "command_setArrays":
expected_field_names = set(
name
for name in expected_field_names
if name not in ("string0", "priority")
name for name in expected_field_names if name != "string0"
)

assert set(topic_metadata.field_info.keys()) == expected_field_names
Expand All @@ -114,7 +111,7 @@ def check_parse_simple(self, has_metadata: bool) -> None:
if has_metadata:
if field_metadata.name.endswith("Stamp"):
expected_units: typing.Optional[str] = "second"
elif field_metadata.name == "TestID":
elif field_metadata.name == "salIndex":
expected_units = None
else:
expected_units = "unitless"
Expand Down Expand Up @@ -166,7 +163,7 @@ def test_test_idl(self) -> None:

# Dict of field name: expected type name
field_types = dict(
TestID="long",
salIndex="long",
private_revCode="string",
private_sndStamp="double",
private_rcvStamp="double",
Expand All @@ -185,14 +182,8 @@ def test_test_idl(self) -> None:
unsignedLong0="unsigned long",
float0="float",
double0="double",
priority="long",
)

# The private_host field is deprecated but may still exist
topic_metadata = metadata.topic_info["scalars"]
if "private_host" in topic_metadata.field_info:
field_types["private_host"] = "long"

# Check some details of arrays topics, including data type,
# array length and string length.
for topic_name in ("arrays", "logevent_arrays", "command_setArrays"):
Expand All @@ -203,8 +194,6 @@ def test_test_idl(self) -> None:

expected_field_names = set(field_types.keys())
expected_field_names.remove("string0") # only in scalars
if not topic_name.startswith("logevent_"):
expected_field_names.remove("priority")
assert set(topic_metadata.field_info.keys()) == expected_field_names
for field_metadata in topic_metadata.field_info.values():
if field_metadata.name[-1] != "0":
Expand All @@ -222,8 +211,6 @@ def test_test_idl(self) -> None:
for deprecated_field in ("char0", "octet0"):
topic_metadata.field_info.pop(deprecated_field, None)
expected_field_names = set(field_types.keys())
if not topic_name.startswith("logevent_"):
expected_field_names.remove("priority")
assert set(topic_metadata.field_info.keys()) == expected_field_names
for field_metadata in topic_metadata.field_info.values():
assert field_metadata.array_length is None
Expand Down
14 changes: 7 additions & 7 deletions tests/test_topics.py
Original file line number Diff line number Diff line change
Expand Up @@ -670,20 +670,20 @@ async def test_bad_put(self) -> None:
await self.remote.cmd_wait.start(self.csc.cmd_setScalars.DataType())

async def test_put_id(self) -> None:
"""Test that one can set the TestID field of a write topic
"""Test that one can set the salIndex field of a write topic
if index=0 and not otherwise.
"""
async with salobj.Controller(name="Test", index=0) as controller0:
async with salobj.Controller(name="Test", index=1) as controller1:
for ind in (0, 1, 2, 3):
# for a controller with zero index
# TestID will be whatever you set it to
await controller0.evt_scalars.set_write(TestID=ind)
assert controller0.evt_scalars.data.TestID == ind
# salIndex will be whatever you set it to
await controller0.evt_scalars.set_write(salIndex=ind)
assert controller0.evt_scalars.data.salIndex == ind
# for a controller with non-zero index
# TestID always matches that index
await controller1.evt_scalars.set_write(TestID=ind)
assert controller1.evt_scalars.data.TestID == 1
# salIndex always matches that index
await controller1.evt_scalars.set_write(salIndex=ind)
assert controller1.evt_scalars.data.salIndex == 1

async def test_command_timeout(self) -> None:
async with self.make_csc(initial_state=salobj.State.ENABLED):
Expand Down