Merge pull request microsoft#2008 from jenshnielsen/rewrite_cache_get

Refactor parameter cache.get

qcodes/instrument/parameter.py

@@ -1861,6 +1861,23 @@ def _update_with(self, *,
         else:
             self._timestamp = timestamp
 
+    def _timestamp_expired(self) -> bool:
+        if self._timestamp is None:
+            # parameter has never been captured
+            return True
+        if self._max_val_age is None:
+            # parameter cannot expire
+            return False
+        oldest_accepted_timestamp = (
+                datetime.now() - timedelta(seconds=self._max_val_age))
+        if self._timestamp < oldest_accepted_timestamp:
+            # Time of last get exceeds max_val_age seconds, need to
+            # perform new .get()
+            return True
+        else:
+            # parameter is still valid
+            return False
+
     def get(self, get_if_invalid: bool = True) -> ParamDataType:
         """
         Return cached value if time since get was less than ``max_val_age``,
@@ -1874,43 +1891,46 @@ def get(self, get_if_invalid: bool = True) -> ParamDataType:
                 example, due to ``max_val_age``, or because the parameter has
                 never been captured)
         """
-        no_get = not self._parameter.gettable
 
-        # the parameter has never been captured so `get` it but only
-        # if `get_if_invalid` is True
-        if self._timestamp is None:
-            if get_if_invalid:
-                if no_get:
-                    raise RuntimeError(f"Value of parameter "
-                                       f"{(self._parameter.full_name)} "
-                                       f"is unknown and the Parameter does "
-                                       f"not have a get command. "
-                                       f"Please set the value before "
-                                       f"attempting to get it.")
-                return self._parameter.get()
-            else:
-                return self._value
+        gettable = self._parameter.gettable
+        cache_valid = not self._timestamp_expired()
 
-        if self._max_val_age is None:
-            # Return last value since max_val_age is not specified
+        if cache_valid:
             return self._value
         else:
-            if no_get:
-                # TODO: this check should really be at the time of setting
-                #  max_val_age unfortunately this happens in init before
-                #  get wrapping is performed.
-                raise RuntimeError("`max_val_age` is not supported for a "
-                                   "parameter without get command.")
-
-            oldest_accepted_timestamp = (
-                    datetime.now() - timedelta(seconds=self._max_val_age))
-            if self._timestamp < oldest_accepted_timestamp:
-                # Time of last get exceeds max_val_age seconds, need to
-                # perform new .get()
-                return self._parameter.get()
+            if get_if_invalid:
+                if gettable:
+                    return self._parameter.get()
+                else:
+                    error_msg = self._construct_error_msg()
+                    raise RuntimeError(error_msg)
             else:
                 return self._value
 
+    def _construct_error_msg(self) -> str:
+        if self._timestamp is None:
+            error_msg = (f"Value of parameter "
+                         f"{self._parameter.full_name} "
+                         f"is unknown and the Parameter "
+                         f"does not have a get command. "
+                         f"Please set the value before "
+                         f"attempting to get it.")
+        elif self._max_val_age is not None:
+            # TODO: this check should really be at the time
+            #  of setting max_val_age unfortunately this
+            #  happens in init before get wrapping is performed.
+            error_msg = ("`max_val_age` is not supported "
+                         "for a parameter without get "
+                         "command.")
+        else:
+            # max_val_age is None and TS is not None but cache is
+            # invalid with the current logic that should never
+            # happen
+            error_msg = ("Cannot return cache of a parameter "
+                         "that does not have a get command "
+                         "and has an invalid cache")
+        return error_msg
+
     def __call__(self) -> ParamDataType:
         """
         Same as :meth:`get` but always call ``get`` on parameter if the

qcodes/tests/test_parameter.py

@@ -1842,3 +1842,17 @@ def test_get_raw_and_get_cmd_raises(working_get_cmd, working_set_cmd):
     with pytest.raises(TypeError, match="set_raw"):
         GetSetRawParameter(name="param2", set_cmd="HereIsTheValue {}", get_cmd=working_get_cmd)
     GetSetRawParameter("param3", get_cmd=working_get_cmd, set_cmd=working_set_cmd)
+
+
+def test_get_from_cache_does_not_trigger_real_get_if_get_if_invalid_false():
+    """
+    assert that calling get on the cache with get_if_invalid=False does
+    not trigger a get of the parameter when parameter has expired due to max_val_age
+    """
+    param = BetterGettableParam(name="param", max_val_age=1)
+    param.get()
+    assert param._get_count == 1
+    # let the cache expire
+    time.sleep(2)
+    param.cache.get(get_if_invalid=False)
+    assert param._get_count == 1