From 4113bf0c4c14d6d95e87a6968aafed0d3941727d Mon Sep 17 00:00:00 2001
From: David Hammer <dhammer@mailbox.org>
Date: Mon, 7 Feb 2022 19:37:27 +0100
Subject: [PATCH] Validate frame filter in preReconfigure

---
 src/calng/base_correction.py | 74 +++++++++++++++++++-----------------
 src/calng/utils.py           | 13 +++++++
 2 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/src/calng/base_correction.py b/src/calng/base_correction.py
index 358ac605..48769362 100644
--- a/src/calng/base_correction.py
+++ b/src/calng/base_correction.py
@@ -1,3 +1,4 @@
+import collections
 import enum
 import pathlib
 import threading
@@ -642,6 +643,11 @@ class BaseCorrection(PythonDevice):
         self.calcat_friend = self._calcat_friend_class(
             self, pathlib.Path.cwd() / "calibration-client-secrets.json"
         )
+        try:
+            self._frame_filter = _parse_frame_filter(self._parameters)
+        except (ValueError, TypeError):
+            self.log_status_warn("Failed to parse initial frame filter, will not use")
+            self._frame_filter = None
         self._update_frame_filter()
 
         self._buffered_status_update = Hash(
@@ -674,6 +680,7 @@ class BaseCorrection(PythonDevice):
         super().__del__()
 
     def preReconfigure(self, config):
+        # validation
         for ts_path in (
             "constantParameters.deviceMappingSnapshotAt",
             "constantParameters.constantVersionEventAt",
@@ -683,15 +690,19 @@ class BaseCorrection(PythonDevice):
                 if ts_string.strip() == "":
                     config.set(ts_path, "")
                 else:
-                    try:
-                        timestamp = dateutil.parser.isoparse(ts_string)
-                    except ValueError as error:
-                        self.log_status_warn(f"Failed to parse {ts_path}; {error}")
-                        config.erase(ts_path)
-                    else:
-                        config.set(ts_path, timestamp.isoformat())
+                    # allow exception to reach operator
+                    timestamp = dateutil.parser.isoparse(ts_string)
+                    config.set(ts_path, timestamp.isoformat())
+
         if config.has("constantParameters.deviceMappingSnapshotAt"):
             self.calcat_friend.flush_pdu_mapping()
+
+        # update device based on changes
+        if config.has("frameFilter"):
+            self._frame_filter = _parse_frame_filter(
+                utils.ChainHash(config, self._parameters)
+            )
+
         self._prereconfigure_update_hash = config
 
     def postReconfigure(self):
@@ -704,7 +715,7 @@ class BaseCorrection(PythonDevice):
         if update.has("frameFilter"):
             with self._buffer_lock:
                 self._update_frame_filter()
-        if any(
+        elif any(
             update.has(shape_param)
             for shape_param in (
                 "dataFormat.pixelsX",
@@ -827,32 +838,6 @@ class BaseCorrection(PythonDevice):
         afterwards."""
         # TODO: add some validation to preReconfigure
         self.log.DEBUG("Updating frame filter")
-        filter_type = FramefilterSpecType(self.get("frameFilter.type"))
-        filter_string = self.get("frameFilter.spec")
-
-        if filter_type is FramefilterSpecType.NONE or filter_string.strip() == "":
-            self._frame_filter = None
-        elif filter_type is FramefilterSpecType.RANGE:
-            try:
-                numbers = tuple(int(part) for part in filter_string.split(","))
-            except (ValueError, TypeError):
-                self.log_status_warn(
-                    f"Invalid frame filter specification: {filter_string}"
-                )
-            else:
-                self._frame_filter = np.arange(*numbers, dtype=np.uint16)
-        elif filter_type is FramefilterSpecType.COMMASEPARATED:
-            try:
-                self._frame_filter = np.fromstring(
-                    filter_string, sep=",", dtype=np.uint16
-                )
-            except ValueError:
-                # note: only in the future will numpy actually give ValueError
-                self.log_status_warn(
-                    f"Invalid frame filter specification: {filter_string}"
-                )
-        else:
-            self.log_status_warn(f"Invalid frame filter type '{filter_type}'")
 
         if self._frame_filter is None:
             self.set("dataFormat.filteredFrames", self.get("dataFormat.memoryCells"))
@@ -1104,3 +1089,24 @@ def add_correction_step_schema(schema, managed_keys, field_flag_mapping):
         )
         managed_keys.add(f"{node_name}.enable")
         managed_keys.add(f"{node_name}.preview")
+
+
+def _parse_frame_filter(config):
+    print("Get type")
+    _t = config["frameFilter.type"]
+    print(_t)
+    filter_type = FramefilterSpecType(_t)
+    print("Get spec")
+    filter_string = config["frameFilter.spec"]
+
+    if filter_type is FramefilterSpecType.NONE or filter_string.strip() == "":
+        return None
+    elif filter_type is FramefilterSpecType.RANGE:
+        # allow exceptions
+        numbers = tuple(int(part) for part in filter_string.split(","))
+        return np.arange(*numbers, dtype=np.uint16)
+    elif filter_type is FramefilterSpecType.COMMASEPARATED:
+        # np.fromstring is too lenient I think
+        return np.array([int(s) for s in filter_string.split(",")], dtype=np.uint16)
+    else:
+        raise TypeError(f"Unknown frame filter type {filter_type}")
diff --git a/src/calng/utils.py b/src/calng/utils.py
index 940e0aa3..a623731d 100644
--- a/src/calng/utils.py
+++ b/src/calng/utils.py
@@ -342,3 +342,16 @@ class TrainRatioTracker:
                 f"{self._train_id_queue[-1]}, just thought you should know..."
             )
         self._train_id_queue.append(train_id)
+
+
+class ChainHash:
+    """Like read-only ChainMap, but for karabo.bound.Hash(es) instead!"""
+
+    def __init__(self, *hashes):
+        self._hashes = hashes
+
+    def __getitem__(self, key):
+        for h in self._hashes:
+            if h.has(key):
+                return h[key]
+        raise KeyError()
-- 
GitLab