From 07362df41f7d1efb1c6ff7bd1ae482d725a0a6a4 Mon Sep 17 00:00:00 2001
From: lukken <lukken@astron.nl>
Date: Wed, 7 Sep 2022 07:59:14 +0200
Subject: [PATCH] L2SS-876: Improve docstrings for antennafield

---
 .../devices/antennafield.py                   | 49 ++++++++++++-------
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/tangostationcontrol/tangostationcontrol/devices/antennafield.py b/tangostationcontrol/tangostationcontrol/devices/antennafield.py
index 86271d18c..2ac0f505b 100644
--- a/tangostationcontrol/tangostationcontrol/devices/antennafield.py
+++ b/tangostationcontrol/tangostationcontrol/devices/antennafield.py
@@ -397,12 +397,21 @@ class AntennaField(lofar_device):
         return mapped_values
     
     def set_mapped_attribute(self, mapped_point: str, value):
+        """Set the attribute to new value only for controlled points
+
+        :warning: This method is susceptible to a lost update race condition if the
+                  attribute on the RECV device is written to in between `read_attribute`
+                  and `write_attribute`!
+
+        """
         mapped_value = self.__mapper.map_write(mapped_point, value)
 
         for idx, recv_proxy in enumerate(self.recv_proxies):
             new_values = mapped_value[idx]
+
+            # TODO(Corne): Resolve potential lost update race condition
             current_values = recv_proxy.read_attribute(mapped_point)
-            new_values = self.__mapper.merge_write(new_values, current_values)
+            self.__mapper.merge_write(new_values, current_values)
             recv_proxy.write_attribute(mapped_point, new_values)
     
     # --------
@@ -465,7 +474,8 @@ class AntennaToRecvMapper(object):
     def __init__(self, control_to_recv_mapping, power_to_recv_mapping, number_of_receivers):
         number_of_antennas = len(control_to_recv_mapping)
 
-        # Reduce memory footprint of default values by creating single instance of common fields
+        # Reduce memory footprint of default values by creating single instance of
+        # common fields
         value_map_ant_32_int = numpy.zeros([number_of_antennas, 32], dtype=numpy.int64)
         value_map_ant_32_bool = numpy.full((number_of_antennas, 32), False)
         value_map_ant_bool = numpy.full(number_of_antennas, False)
@@ -503,14 +513,14 @@ class AntennaToRecvMapper(object):
             "HBAT_BF_delay_steps_RW": (96, 32),
         }
 
-    def map_read(self, mapped_attribute: str, recv_results: List[any]):
-        """
+    def map_read(self, mapped_attribute: str, recv_results: List[any]) -> List[any]:
+        """Perform a mapped read for the attribute using the recv_results
 
         :param mapped_attribute: attribute identifier as present in
                                  py:attribute:`~_default_value_mapping_read`
         :param recv_results: Results as gathered by appending attributes from RECV
                              devices
-        :return:
+        :return: recv_results as mapped given attribute dimensions and control mapping
         """
 
         default_values = self._default_value_mapping_read[mapped_attribute]
@@ -521,13 +531,13 @@ class AntennaToRecvMapper(object):
         
         return self._mapped_r_values(recv_results, default_values)
         
-    def map_write(self, mapped_attribute: str, set_values):
+    def map_write(self, mapped_attribute: str, set_values: List[any]) -> List[any]:
         """Perform a mapped write for the attribute using the set_values
 
         :param mapped_attribute: attribute identifier as present in
                                  py:attribute:`~_default_value_mapping_write`
-        :param set_values:
-        :return:
+        :param set_values: The values to be set for the specified attribute
+        :return: set_values as mapped given attribute dimensions and control mapping
         """
 
         default_values = self._masked_value_mapping_write[mapped_attribute]
@@ -540,22 +550,26 @@ class AntennaToRecvMapper(object):
 
         return mapped_values
 
-    def merge_write(self, mapped_values, current_values):
+    @staticmethod
+    def merge_write(merge_values: List[any], current_values: List[any]):
         """Merge values as retrieved from :py:func:`~map_write` with current_values
 
+        This method will modify the contents of merge_values.
+
         To be used by the :py:class:`~AntennaField` device to remove None fields
         from mapped_values with recently retrieved current_values from RECV device.
 
-        :param mapped_values:
-        :param current_values:
-        :return:
+        :param merge_values: values as retrieved from :py:func:`~map_write`
+        :param current_values: values retrieved from RECV device on specific attribute
         """
 
-        for idx, value in enumerate(mapped_values):
+        for idx, value in enumerate(merge_values):
             if value is None:
-                mapped_values[idx] = current_values[idx]
+                merge_values[idx] = current_values[idx]
+
+    def _mapped_r_values(self, recv_results: List[any], default_values: List[any]):
+        """Mapping for read using :py:attribute:`~_control_mapping` and shallow copy"""
 
-    def _mapped_r_values(self, recv_results, default_values):
         mapped_values = numpy.array(default_values)
 
         for idx, mapping in enumerate(self._control_mapping):
@@ -566,7 +580,9 @@ class AntennaToRecvMapper(object):
 
         return mapped_values
 
-    def _mapped_rw_values(self, set_values, default_values):
+    def _mapped_rw_values(self, set_values: List[any], default_values: List[any]):
+        """Mapping for write using :py:attribute:`~_control_mapping` and shallow copy"""
+
         mapped_values = []
 
         for _ in range(self._number_of_receivers):
@@ -579,7 +595,6 @@ class AntennaToRecvMapper(object):
             if recv > 0:
                 mapped_values[recv - 1, rcu] = set_values[idx]
 
-        # import pdb; pdb.set_trace()
         return mapped_values
 
 # ----------
-- 
GitLab