Merge pull request #240 from lsst-ts/tickets/DM-34639

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

doc/version_history.rst

@@ -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.
@@ -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
 ------

python/lsst/ts/salobj/csc_commander.py

@@ -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()
@@ -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

python/lsst/ts/salobj/sal_info.py

@@ -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)
@@ -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()

python/lsst/ts/salobj/testcsccommander.py

@@ -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:

python/lsst/ts/salobj/topics/read_topic.py

@@ -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}",

python/lsst/ts/salobj/topics/write_topic.py

@@ -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)

python/lsst/ts/salobj/type_hints.py

@@ -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()

tests/data/sal_revCoded_SimpleWithMetadata.idl

@@ -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")
@@ -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")
@@ -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
 };

tests/data/sal_revCoded_SimpleWithoutMetadata.idl

@@ -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;
@@ -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;
@@ -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
 };

tests/test_csc_commander.py

@@ -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)

tests/test_idl_parser.py

@@ -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",
@@ -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():
@@ -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
@@ -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"
@@ -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",
@@ -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"):
@@ -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":
@@ -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

tests/test_topics.py

@@ -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):