From b22d2f15b3b0b3c14c50b0276d2cd313e843a67c Mon Sep 17 00:00:00 2001
From: woutdenolf <woutdenolf@users.sf.net>
Date: Fri, 28 Aug 2020 23:51:19 +0200
Subject: [PATCH 01/19] trapped CTRL-C when a key from a motor's setting or
 config

---
 bliss/controllers/motor.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bliss/controllers/motor.py b/bliss/controllers/motor.py
index 26573e4935..1b6889f319 100644
--- a/bliss/controllers/motor.py
+++ b/bliss/controllers/motor.py
@@ -33,7 +33,7 @@ def get_setting_or_config_value(axis, name):
     if value is None:
         try:
             value = axis.config.get(name, converter)
-        except BaseException:
+        except Exception:
             return None
     return value
 
-- 
GitLab


From 5ef0a5dccbd16ee5a5a3934d48e8ef324826d2c5 Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Fri, 28 Aug 2020 16:38:39 +0200
Subject: [PATCH 02/19] axis: do not need to call controller to clear an axis
 setting

---
 bliss/common/axis.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/bliss/common/axis.py b/bliss/common/axis.py
index d3cc2be02c..ab02851815 100644
--- a/bliss/common/axis.py
+++ b/bliss/common/axis.py
@@ -2065,16 +2065,16 @@ class Axis:
         )
 
         if velocity:
-            self.controller.axis_settings._clear(self, "velocity")
+            self.settings.clear("velocity")
         if acceleration:
-            self.controller.axis_settings._clear(self, "acceleration")
+            self.settings.clear("acceleration")
         if limits:
-            self.controller.axis_settings._clear(self, "low_limit")
-            self.controller.axis_settings._clear(self, "high_limit")
+            self.settings.clear("low_limit")
+            self.settings.clear("high_limit")
         if sign:
-            self.controller.axis_settings._clear(self, "sign")
+            self.settings.clear("sign")
         if backlash:
-            self.controller.axis_settings._clear(self, "backlash")
+            self.settings.clear("backlash")
 
         self.controller._init_settings(self)
 
-- 
GitLab


From 0800893cd42ee7f994bfc3c42235c541904d69d1 Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Fri, 28 Aug 2020 16:39:34 +0200
Subject: [PATCH 03/19] forgotten breakpoint

---
 tests/motors/test_axis.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/motors/test_axis.py b/tests/motors/test_axis.py
index bb0773339b..dd98d3c200 100644
--- a/tests/motors/test_axis.py
+++ b/tests/motors/test_axis.py
@@ -725,7 +725,6 @@ def test_no_offset(roby):
 
 @pytest.mark.parametrize("motor_name", ["roby", "nsa"])
 def test_offset_property(beacon, motor_name):
-    breakpoint()
     mot = beacon.get(motor_name)
 
     mot.move(1)
-- 
GitLab


From 876f0f1ad6f3c4d09eb8d585a0025b332f2a6ddf Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Fri, 28 Aug 2020 16:40:35 +0200
Subject: [PATCH 04/19] mockup motor controller: do not use axis settings for
 storing controller-side axis data

---
 bliss/controllers/motors/mockup.py | 47 +++++++++++++++++++-----------
 tests/motors/test_axis.py          |  2 +-
 2 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/bliss/controllers/motors/mockup.py b/bliss/controllers/motors/mockup.py
index da63e8226e..a4ff07906a 100644
--- a/bliss/controllers/motors/mockup.py
+++ b/bliss/controllers/motors/mockup.py
@@ -8,6 +8,7 @@ import math
 import time
 import random
 import gevent
+import collections
 
 from bliss.physics.trajectory import LinearTrajectory
 from bliss.controllers.motor import Controller, CalcController
@@ -15,7 +16,7 @@ from bliss.common.axis import Axis, AxisState
 from bliss.common.switch import Switch as BaseSwitch
 from bliss.common import event
 from bliss.config.static import get_config
-
+from bliss.config.settings import SimpleSetting
 from bliss.common.hook import MotionHook
 from bliss.common.utils import object_method
 from bliss.common.utils import object_attribute_get, object_attribute_set
@@ -58,6 +59,7 @@ class Mockup(Controller):
         self._axis_moves = {}
         self.__encoders = {}
         self.__switches = {}
+        self._axes_data = collections.defaultdict(dict)
 
         # Custom attributes.
         self.__voltages = {}
@@ -68,30 +70,36 @@ class Mockup(Controller):
 
         self._hw_state.create_state("PARKED", "mot au parking")
 
-        # Adds Mockup-specific settings.
-        self.axis_settings.add("init_count", int)
-        self.axis_settings.add("hw_position", float)
-        # those 2 are to simulate a real controller (one with internal settings, that
-        # keep those for multiple clients)
-        self.axis_settings.add("curr_acc", float)
-        self.axis_settings.add("curr_velocity", float)
-
     def steps_position_precision(self, axis):
         """Mockup is really a stepper motor controller"""
         return 1
 
     def read_hw_position(self, axis):
-        return axis.settings.get("hw_position")
+        return self._axes_data[axis]["hw_position"].get()
 
     def set_hw_position(self, axis, position):
-        axis.settings.set("hw_position", position)
+        self._axes_data[axis]["hw_position"].set(position)
 
     """
     Axes initialization actions.
     """
 
     def _add_axis(self, axis):
-        axis.settings.set("init_count", 0)
+        # this is a counter to check if an axis is added multiple times,
+        # it is incremented in `initalize_axis()`
+        self._axes_data[axis]["init_count"] = 0
+        # those 3 are to simulate a real controller (one with internal settings, that
+        # keep those for multiple clients)
+        self._axes_data[axis]["hw_position"] = SimpleSetting(
+            f"motor_mockup:{axis.name}:hw_position"
+        )
+        self._axes_data[axis]["curr_acc"] = SimpleSetting(
+            f"motor_mockup:{axis.name}:curr_acc"
+        )
+        self._axes_data[axis]["curr_velocity"] = SimpleSetting(
+            f"motor_mockup:{axis.name}:curr_velocity"
+        )
+
         encoder = axis.config.get("encoder", converter=None, default=None)
         if encoder:
             self.initialize_encoder(encoder)
@@ -100,6 +108,11 @@ class Mockup(Controller):
         if self.read_hw_position(axis) is None:
             self.set_hw_position(axis, 0)
 
+    def initialize_hardware_axis(self, axis):
+        self._axes_data[axis]["hw_position"].set(0)
+        self._axes_data[axis]["curr_acc"].set(0)
+        self._axes_data[axis]["curr_velocity"].set(0)
+
     def initialize_axis(self, axis):
         log_debug(self, "initializing axis %s", axis.name)
 
@@ -109,7 +122,7 @@ class Mockup(Controller):
         )
 
         # this is to test axis are initialized only once
-        axis.settings.set("init_count", axis.settings.get("init_count") + 1)
+        self._axes_data[axis]["init_count"] += 1
         axis.stop_jog_called = False
 
         # the next lines are there to test issue #1601
@@ -250,7 +263,7 @@ class Mockup(Controller):
         Return the current velocity taken from controller
         in motor units.
         """
-        return axis.settings.get("curr_velocity") * abs(axis.steps_per_unit)
+        return self._axes_data[axis]["curr_velocity"].get() * abs(axis.steps_per_unit)
 
     def set_velocity(self, axis, new_velocity):
         """
@@ -259,7 +272,7 @@ class Mockup(Controller):
         vel = new_velocity / abs(axis.steps_per_unit)
         if vel >= 1e9:
             raise RuntimeError("Invalid velocity")
-        axis.settings.set("curr_velocity", vel)
+        self._axes_data[axis]["curr_velocity"].set(vel)
         return vel
 
     """
@@ -270,7 +283,7 @@ class Mockup(Controller):
         """
         must return acceleration in controller units / s2
         """
-        return axis.settings.get("curr_acc") * abs(axis.steps_per_unit)
+        return self._axes_data[axis]["curr_acc"].get() * abs(axis.steps_per_unit)
 
     def set_acceleration(self, axis, new_acceleration):
         """
@@ -279,7 +292,7 @@ class Mockup(Controller):
         acc = new_acceleration / abs(axis.steps_per_unit)
         if acc >= 1e9:
             raise RuntimeError("Invalid acceleration")
-        axis.settings.set("curr_acc", acc)
+        self._axes_data[axis]["curr_acc"].set(acc)
         return acc
 
     """
diff --git a/tests/motors/test_axis.py b/tests/motors/test_axis.py
index dd98d3c200..6a8b3ffe05 100644
--- a/tests/motors/test_axis.py
+++ b/tests/motors/test_axis.py
@@ -248,7 +248,7 @@ def test_axis_multiple_move(robz):
 
 def test_axis_init(robz):
     assert robz.state.READY
-    assert robz.settings.get("init_count") == 1
+    assert robz.controller._axes_data[robz]["init_count"] == 1
 
 
 def test_stop(robz):
-- 
GitLab


From 4dc2a65abd52f40a834f2248ccdf2933b3b8bb98 Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Fri, 28 Aug 2020 16:41:07 +0200
Subject: [PATCH 05/19] new soft axis class, with default steps_per_unit == 1

---
 bliss/controllers/motors/soft.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/bliss/controllers/motors/soft.py b/bliss/controllers/motors/soft.py
index c658c6cbb7..4f8f3da45c 100644
--- a/bliss/controllers/motors/soft.py
+++ b/bliss/controllers/motors/soft.py
@@ -75,12 +75,18 @@ def get_state_func(obj, state):
     return state_func
 
 
+class SoftAxis(NoSettingsAxis):
+    pass
+
+
 class SoftController(Controller):
     def __init__(
         self, axis_name, obj, axis_config, position, move, stop=None, state=None
     ):
 
-        axes = {axis_name: (NoSettingsAxis, axis_config)}
+        axis_config.setdefault("steps_per_unit", 1)
+
+        axes = {axis_name: (SoftAxis, axis_config)}
 
         super(SoftController, self).__init__(
             "__soft_controller__", {}, axes, {}, {}, {}
-- 
GitLab


From f49d198bf88e815fd51ccee69985e455f993e043 Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Sun, 30 Aug 2020 00:56:00 +0200
Subject: [PATCH 06/19] common/axis.py: added missing lazy_init decorator to
 ensure proper initialization

This was missing for 'limits' setter, for example ; better to have them
all, than counting on init. when accessing a property in another prop.
---
 bliss/common/axis.py      | 23 +++++++++++++++++++++--
 tests/motors/test_axis.py | 22 +++++++++++++++++-----
 2 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/bliss/common/axis.py b/bliss/common/axis.py
index ab02851815..85d4949f16 100644
--- a/bliss/common/axis.py
+++ b/bliss/common/axis.py
@@ -784,11 +784,13 @@ class Axis:
             self.__config_backlash = self.config.get("backlash", float, 0)
 
     @property
+    @lazy_init
     def steps_per_unit(self):
         """Current steps per unit (:obj:`float`)"""
         return self.__steps_per_unit
 
     @property
+    @lazy_init
     def config_backlash(self):
         """Current backlash in user units (:obj:`float`)"""
         return self.__config_backlash
@@ -807,6 +809,7 @@ class Axis:
         self.settings.set("backlash", backlash)
 
     @property
+    @lazy_init
     def tolerance(self):
         """Current Axis tolerance in dial units (:obj:`float`)"""
         return self.__tolerance
@@ -851,6 +854,7 @@ class Axis:
         return sign
 
     @sign.setter
+    @lazy_init
     def sign(self, new_sign):
         new_sign = float(
             new_sign
@@ -922,6 +926,7 @@ class Axis:
         return position
 
     @_set_position.setter
+    @lazy_init
     def _set_position(self, new_set_pos):
         new_set_pos = float(
             new_set_pos
@@ -992,6 +997,7 @@ class Axis:
         return dial_pos
 
     @dial.setter
+    @lazy_init
     def dial(self, new_dial):
         if self.is_moving:
             raise RuntimeError(
@@ -1041,6 +1047,7 @@ class Axis:
         return pos
 
     @position.setter
+    @lazy_init
     def position(self, new_pos):
         log_debug(self, "axis.py : position(new_pos=%r)" % new_pos)
         if self.is_moving:
@@ -1222,6 +1229,7 @@ class Axis:
         return _user_vel
 
     @velocity.setter
+    @lazy_init
     def velocity(self, new_velocity):
         # Write -> Converts into motor units to change velocity of axis."
         new_velocity = float(
@@ -1233,6 +1241,7 @@ class Axis:
         return _user_vel
 
     @property
+    @lazy_init
     def config_velocity(self):
         """
         Return the config velocity.
@@ -1327,6 +1336,7 @@ class Axis:
         return _user_jog_vel
 
     @jog_velocity.setter
+    @lazy_init
     def jog_velocity(self, new_velocity):
         new_velocity = float(
             new_velocity
@@ -1336,6 +1346,7 @@ class Axis:
             self._jog_velocity_channel.value = new_velocity
 
     @property
+    @lazy_init
     def config_jog_velocity(self):
         """
         Return the config jog velocity.
@@ -1359,6 +1370,7 @@ class Axis:
         return _acceleration
 
     @acceleration.setter
+    @lazy_init
     def acceleration(self, new_acc):
         if self.is_moving:
             raise RuntimeError(
@@ -1373,6 +1385,7 @@ class Axis:
         return _acceleration
 
     @property
+    @lazy_init
     def config_acceleration(self):
         return self.__config_acceleration
 
@@ -1388,6 +1401,7 @@ class Axis:
         return abs(self.velocity / self.acceleration)
 
     @acctime.setter
+    @lazy_init
     def acctime(self, new_acctime):
         # Converts acctime into acceleration.
         new_acctime = float(
@@ -1432,6 +1446,7 @@ class Axis:
         return ll, hl
 
     @dial_limits.setter
+    @lazy_init
     def dial_limits(self, limits):
         """
         Set low, high limits in dial units
@@ -1458,13 +1473,14 @@ class Axis:
         return tuple(map(self.dial2user, self.dial_limits))
 
     @limits.setter
+    @lazy_init
     def limits(self, limits):
         # Set limits (low, high) in user units.
         try:
             if len(limits) != 2:
                 raise TypeError
         except TypeError:
-            raise ValueError("Usage: .limits(low, high)")
+            raise ValueError("Usage: .limits = low, high")
 
         # accepts iterable (incl. numpy array)
         self.low_limit, self.high_limit = (
@@ -1479,6 +1495,7 @@ class Axis:
         return self.dial2user(ll)
 
     @low_limit.setter
+    @lazy_init
     def low_limit(self, limit):
         # Sets Low Limit
         # <limit> must be given in USER units
@@ -1496,6 +1513,7 @@ class Axis:
         return self.dial2user(hl)
 
     @high_limit.setter
+    @lazy_init
     def high_limit(self, limit):
         # Sets High Limit (given in USER units)
         # Saved in settings in DIAL units.
@@ -1505,6 +1523,7 @@ class Axis:
         self.settings.set("high_limit", limit)
 
     @property
+    @lazy_init
     def config_limits(self):
         """
         Return a tuple (low_limit, high_limit) from IN-MEMORY config in
@@ -2434,6 +2453,6 @@ class ModuloAxis(Axis):
 
 class NoSettingsAxis(Axis):
     def __init__(self, *args, **kwags):
-        Axis.__init__(self, *args, **kwags)
+        super().__init__(*args, **kwags)
         for setting_name in self.settings:
             self.settings.disable_cache(setting_name)
diff --git a/tests/motors/test_axis.py b/tests/motors/test_axis.py
index 6a8b3ffe05..6ec539d69b 100644
--- a/tests/motors/test_axis.py
+++ b/tests/motors/test_axis.py
@@ -756,18 +756,30 @@ def test_settings_to_config(beacon, motor_name):
     cfg = beacon.get_config(motor_name)
     cfg_acc = cfg.get("acceleration")
     cfg_vel = cfg.get("velocity")
+    cfg_ll = cfg.get("low_limit")
+    cfg_hl = cfg.get("high_limit")
 
     mot.velocity = 3
     mot.acceleration = 10
     mot.limits = None, None
     assert mot.config_velocity == cfg_vel
     assert mot.config_acceleration == cfg_acc
+    assert mot.config_limits == (cfg_ll, cfg_hl)
     mot.settings_to_config()
-    assert mot.config_velocity == 3
-    assert mot.config_acceleration == 10
-    mot.velocity = cfg_vel
-    mot.acceleration = cfg_acc
-    mot.settings_to_config()
+    try:
+        assert mot.config_velocity == 3
+        assert mot.config_acceleration == 10
+        assert mot.config_limits == (float("-inf"), float("inf"))
+    finally:
+        # put back config files as they were,
+        # since the fixture that starts
+        # all servers (and that copy config. to a temp folder
+        # for the test) has a session scope
+        mot.velocity = cfg_vel
+        mot.acceleration = cfg_acc
+        mot.low_limit = cfg_ll
+        mot.high_limit = cfg_hl
+        mot.settings_to_config()
 
 
 @pytest.mark.parametrize("motor_name", ["roby", "nsa"])
-- 
GitLab


From 259e7c5876543e3d5e2b5cfd5f28514b73282215 Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Sun, 30 Aug 2020 20:21:06 +0200
Subject: [PATCH 07/19] motor settings: properly implements "no cache" for
 settings

Preserve the same behaviour as normal settings, regarding config and events
Methods which were defined in ControllerAxisSettings but which belong more to AxisSettings class were moved
---
 bliss/common/axis.py           |   2 +
 bliss/common/motor_settings.py | 167 +++++++++++++++++++--------------
 tests/motors/test_axis.py      |  10 +-
 3 files changed, 105 insertions(+), 74 deletions(-)

diff --git a/bliss/common/axis.py b/bliss/common/axis.py
index 85d4949f16..d74b2bfbf0 100644
--- a/bliss/common/axis.py
+++ b/bliss/common/axis.py
@@ -1044,6 +1044,7 @@ class Axis:
         pos = self.settings.get("position")
         if pos is None:
             pos = self.dial2user(self.dial)
+            self.settings.set("position", pos)
         return pos
 
     @position.setter
@@ -1104,6 +1105,7 @@ class Axis:
         if state is None:
             # really read from hw
             state = self.hw_state
+            self.settings.set("state", state)
         return state
 
     @property
diff --git a/bliss/common/motor_settings.py b/bliss/common/motor_settings.py
index 6333e53424..b49f61dfc5 100644
--- a/bliss/common/motor_settings.py
+++ b/bliss/common/motor_settings.py
@@ -8,6 +8,7 @@
 from bliss.common import event
 from bliss.common.greenlet_utils import KillMask
 from bliss.config import settings, settings_cache
+import collections
 import sys
 
 
@@ -41,7 +42,6 @@ def stateSetting(state):
 class ControllerAxisSettings:
     def __init__(self):
         self.setting_names = []
-        self.disabled_settings = {}
         self.convert_func = {}
         self.persistent_setting = {}
         self.config_setting = {}
@@ -73,56 +73,71 @@ class ControllerAxisSettings:
             self.persistent_setting[setting_name] = persistent
             self.config_setting[setting_name] = config
 
-    def get(self, axis, setting_name):
-        if setting_name not in self.setting_names:
-            raise ValueError(
-                "No setting '%s` for axis '%s`" % (setting_name, axis.name)
-            )
 
-        disabled_settings = self.disabled_settings.get(axis, set())
+disabled_settings_namedtuple = collections.namedtuple(
+    "disabled_settings", "names config_dict"
+)
 
-        if (
-            self.persistent_setting[setting_name]
-            and not setting_name in disabled_settings
-        ):
-            with KillMask():
-                value = axis.settings._hash.get(setting_name)
-        else:
-            chan = axis._beacon_channels.get(setting_name)
-            if chan:
-                value = chan.value
-            else:
-                value = None
-        if value is not None:
-            convert_func = self.convert_func.get(setting_name)
-            if convert_func is not None:
-                value = convert_func(value)
-        return value
-
-    def _clear(self, axis, setting_name):
-        if not setting_name in self.disabled_settings:
-            axis.settings._hash[setting_name] = None
 
-    def set(self, axis, setting_name, value):
+class AxisSettings:
+    def __init__(self, axis):
+        self.__axis = axis
+        self.__state = None
+        self._disabled_settings = disabled_settings_namedtuple(
+            set(), dict(axis.config.config_dict)
+        )
+        cnx_cache = settings_cache.get_redis_client_cache()
+        self._hash = settings.HashSetting(
+            "axis.%s" % axis.name,
+            default_values=axis.config.config_dict,
+            connection=cnx_cache,
+        )
+        # Activate prefetch
+        cnx_cache.add_prefetch(self._hash)
+
+    def __iter__(self):
+        for name in self.__axis.controller.axis_settings.setting_names:
+            yield name
+
+    def convert_func(self, setting_name):
+        return self.__axis.controller.axis_settings.convert_func.get(setting_name)
+
+    def config_settings(self):
+        return self.__axis.controller.axis_settings.config_settings()
+
+    def set(self, setting_name, value):
+        if setting_name == "state":
+            if self.__state == value:
+                return
+            self.__state = value
         """
         * set setting
         * send event
         * write
         """
-        if setting_name not in self.setting_names:
+        axis = self.__axis
+        axis_settings = axis.controller.axis_settings
+        if setting_name not in axis_settings.setting_names:
             raise ValueError(
                 "No setting '%s` for axis '%s`" % (setting_name, axis.name)
             )
-        convert_func = self.convert_func.get(setting_name)
+        convert_func = self.convert_func(setting_name)
         if convert_func is not None:
             value = convert_func(value)
 
-        if not setting_name in self.disabled_settings:
-            if self.persistent_setting[setting_name]:
+        disabled_settings = self._disabled_settings
+        if setting_name in disabled_settings.names:
+            if (
+                setting_name not in ("position", "dial_position")
+                and axis_settings.persistent_setting[setting_name]
+            ):
+                disabled_settings.config_dict[setting_name] = value
+        else:
+            if axis_settings.persistent_setting[setting_name]:
                 with KillMask():
                     axis.settings._hash[setting_name] = value
 
-        axis._beacon_channels[setting_name].value = value
+            axis._beacon_channels[setting_name].value = value
 
         event.send(axis, "internal_" + setting_name, value)
         try:
@@ -130,40 +145,51 @@ class ControllerAxisSettings:
         except Exception:
             sys.excepthook(*sys.exc_info())
 
+    def get(self, setting_name):
+        axis = self.__axis
+        axis_settings = axis.controller.axis_settings
+        disabled_settings = self._disabled_settings
 
-class AxisSettings:
-    def __init__(self, axis):
-        self.__axis = axis
-        self.__state = None
-        cnx_cache = settings_cache.get_redis_client_cache()
-        self._hash = settings.HashSetting(
-            "axis.%s" % axis.name,
-            default_values=axis.config.config_dict,
-            connection=cnx_cache,
-        )
-        # Activate prefetch
-        cnx_cache.add_prefetch(self._hash)
-
-    def set(self, setting_name, value):
-        if setting_name == "state":
-            if self.__state == value:
-                return
-            self.__state = value
-        return self.__axis.controller.axis_settings.set(
-            self.__axis, setting_name, value
-        )
-
-    def convert_func(self, setting_name):
-        return self.__axis.controller.axis_settings.convert_func[setting_name]
-
-    def config_settings(self):
-        return self.__axis.controller.axis_settings.config_settings()
+        if setting_name not in axis_settings.setting_names:
+            raise ValueError(
+                "No setting '%s` for axis '%s`" % (setting_name, axis.name)
+            )
 
-    def get(self, setting_name):
-        return self.__axis.controller.axis_settings.get(self.__axis, setting_name)
+        if setting_name in disabled_settings.names:
+            return disabled_settings.config_dict.get(setting_name)
+        else:
+            if axis_settings.persistent_setting[setting_name]:
+                with KillMask():
+                    value = axis.settings._hash.get(setting_name)
+            else:
+                chan = axis._beacon_channels.get(setting_name)
+                if chan:
+                    value = chan.value
+                else:
+                    value = None
+
+            if value is not None:
+                convert_func = self.convert_func(setting_name)
+                if convert_func is not None:
+                    value = convert_func(value)
+            return value
 
     def clear(self, setting_name):
-        self.__axis.controller.axis_settings._clear(self.__axis, setting_name)
+        axis = self.__axis
+        axis_settings = axis.controller.axis_settings
+        disabled_settings = self._disabled_settings
+
+        if setting_name in disabled_settings.names:
+            disabled_settings.config_dict[setting_name] = None
+        else:
+            axis.settings._hash[setting_name] = None
+            # reset beacon channel, if it is there
+            try:
+                channel = axis._beacon_channels[setting_name]
+            except KeyError:
+                pass
+            else:
+                channel.value = channel.default_value
 
     def disable_cache(self, setting_name, flag=True):
         """
@@ -172,17 +198,12 @@ class AxisSettings:
         if setting_name == "position":
             self.disable_cache("dial_position", flag)
 
-        disabled_settings = self.__axis.controller.axis_settings.disabled_settings.setdefault(
-            self.__axis, set()
-        )
+        self.clear(setting_name)
+
         if flag:
-            disabled_settings.add(setting_name)
+            self._disabled_settings.names.add(setting_name)
         else:
             try:
-                disabled_settings.remove(setting_name)
+                self._disabled_settings.names.remove(setting_name)
             except KeyError:
                 pass
-
-    def __iter__(self):
-        for name in self.__axis.controller.axis_settings.setting_names:
-            yield name
diff --git a/tests/motors/test_axis.py b/tests/motors/test_axis.py
index 6ec539d69b..b550a6d310 100644
--- a/tests/motors/test_axis.py
+++ b/tests/motors/test_axis.py
@@ -902,10 +902,18 @@ def test_axis_no_state_setting(m1):
     with mock.patch.object(m1.controller, "state") as new_state:
         new_state.return_value = AxisState("FAULT")
         assert m1.state == state
+        assert m1.hw_state == AxisState("FAULT")
         m1.settings.disable_cache("state")
         assert m1.state == AxisState("FAULT")
+        # re-enable cache
+        new_state.return_value = AxisState("READY")
         m1.settings.disable_cache("state", False)
-        assert m1.state == state
+        assert m1.hw_state == AxisState("READY")
+        # the beacon channel has been removed, there is no source of data
+        # so this will read the hw_state
+        assert m1.state == m1.hw_state
+        new_state.return_value = AxisState("FAULT")
+        assert m1.state == AxisState("READY")
 
 
 def test_axis_disable_cache_settings_from_config(beacon):
-- 
GitLab


From 73f3d9622af6e9168f3867509154007702afb2de Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Sun, 30 Aug 2020 02:16:55 +0200
Subject: [PATCH 08/19] common/axis.py: fixed typo

---
 bliss/common/axis.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bliss/common/axis.py b/bliss/common/axis.py
index d74b2bfbf0..bbf44517bb 100644
--- a/bliss/common/axis.py
+++ b/bliss/common/axis.py
@@ -703,8 +703,8 @@ class Axis:
                 "disabled_cache", []
             )  # get it from controller (parent)
         disabled_cache.extend(config.get("disabled_cache", []))  # get it for this axis
-        for settings_name in disabled_cache:
-            self.settings.disable_cache(settings_name)
+        for setting_name in disabled_cache:
+            self.settings.disable_cache(setting_name)
         self._unit = self.config.get("unit", str, None)
         self._polling_time = config.get("polling_time", DEFAULT_POLLING_TIME)
         global_map.register(self, parents_list=["axes", controller])
-- 
GitLab


From b88879eda15c512f8ed33ba41b176c4e8597fa8f Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Sun, 30 Aug 2020 19:52:08 +0200
Subject: [PATCH 09/19] calc motor controller: avoid recursion in
 _real_position_update

---
 bliss/controllers/motor.py | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/bliss/controllers/motor.py b/bliss/controllers/motor.py
index 1b6889f319..43d4e655bb 100644
--- a/bliss/controllers/motor.py
+++ b/bliss/controllers/motor.py
@@ -583,11 +583,17 @@ class CalcController(Controller):
             setpos_dict[self._axis_tag(axis)] = axis.user2dial(axis._set_position)
         return setpos_dict
 
-    def _real_position_update(self, *args):
+    def _real_position_update(self, pos, sender=None):
         for axis in self.pseudos:
             self._initialize_axis(axis)
 
-        return self._calc_from_real(*args)
+        try:
+            # avoid recursion by disconnecting the signal
+            event.disconnect(sender, "internal_position", self._real_position_update)
+            return self._calc_from_real()
+        finally:
+            # reconnect
+            event.connect(sender, "internal_position", self._real_position_update)
 
     def _real_setpos_update(self, _):
         real_setpos = dict()
@@ -674,7 +680,7 @@ class CalcController(Controller):
         )
         return self.calc_from_real(real_positions)
 
-    def _calc_from_real(self, *args, **kwargs):
+    def _calc_from_real(self, *args):
         new_positions = self._do_calc_from_real()
 
         for tagged_axis_name, dial_pos in new_positions.items():
-- 
GitLab


From 6ebfd78dc251d867d041aba1acb1f5b779cacf03 Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Tue, 1 Sep 2020 11:32:09 +0200
Subject: [PATCH 10/19] conductor/connection.py: remove custom finalization for
 connection objects

---
 bliss/config/conductor/connection.py | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/bliss/config/conductor/connection.py b/bliss/config/conductor/connection.py
index 96c76cff12..373d9c6ddf 100644
--- a/bliss/config/conductor/connection.py
+++ b/bliss/config/conductor/connection.py
@@ -32,23 +32,8 @@ class GreenletSafeConnectionPool(redis.ConnectionPool):
         # we do not care of being "fork-safe"
         return
 
-    def get_connection(self, command_name, *keys, **options):
-        connection = super().get_connection(command_name, *keys, **options)
-
-        if command_name == "pubsub":
-            finalize = weakref.finalize(
-                gevent.getcurrent(), self.clean_pubsub, connection
-            )
-        else:
-            finalize = weakref.finalize(gevent.getcurrent(), self.release, connection)
-        connection.__finalize__ = finalize
-
-        return connection
-
     def release(self, connection):
         with self._lock:
-            if hasattr(connection, "__finalize__"):
-                connection.__finalize__.detach()
             # As we register callback when greenlet disappear,
             # Connection might been removed before the greenlet
             try:
-- 
GitLab


From a3124790078ba55bc4642121e73f6843dab54786 Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Tue, 1 Sep 2020 13:05:35 +0200
Subject: [PATCH 11/19] tests/conftest.py: close cache clients after config is
 closed

Previously, cache was closed before config.close() => some channels
could still receive data and may want to read/write settings whereas
the connection is closed. Now config is closed and channels are
closed, then settings are closed.
---
 tests/conftest.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/conftest.py b/tests/conftest.py
index df592d09e4..cf473dceb4 100755
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -239,11 +239,11 @@ def beacon(ports):
     client._default_connection = connection.Connection("localhost", ports.beacon_port)
     config = static.get_config()
     yield config
-    settings_cache.close_all_client_cache()
     config.close()
     client._default_connection.close()
     # Ensure no connections are created due to garbage collection:
     client._default_connection = None
+    settings_cache.close_all_client_cache()
 
 
 @pytest.fixture
-- 
GitLab


From 7c89003d17d8c6d10cb797fbeec261387088e0f3 Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Tue, 1 Sep 2020 21:33:17 +0200
Subject: [PATCH 12/19] channels: remove unused code

---
 bliss/config/channels.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/bliss/config/channels.py b/bliss/config/channels.py
index f4f61b7a0a..2e22aa4721 100644
--- a/bliss/config/channels.py
+++ b/bliss/config/channels.py
@@ -497,9 +497,6 @@ class Channel(AdvancedInstantiationInterface):
             finally:
                 self._firing_callbacks = False
 
-        # Clean up
-        self._callbacks = {ref for ref in self._callback_refs if ref() is not None}
-
     # Representation
 
     def __repr__(self):
-- 
GitLab


From f13ce3dd896713c7ee5a32b6b9b7c3f6ad5f8a26 Mon Sep 17 00:00:00 2001
From: Matias Guijarro <matias.guijarro@esrf.fr>
Date: Mon, 31 Aug 2020 02:33:16 +0200
Subject: [PATCH 13/19] motor config: make .get() behaving like a Python
 dictionary

---
 bliss/common/motor_config.py               | 18 ++++--------
 bliss/controllers/motors/esrf_undulator.py | 34 ++++++++--------------
 2 files changed, 17 insertions(+), 35 deletions(-)

diff --git a/bliss/common/motor_config.py b/bliss/common/motor_config.py
index ffc13e1745..cd7acf3a38 100644
--- a/bliss/common/motor_config.py
+++ b/bliss/common/motor_config.py
@@ -11,9 +11,6 @@ import time
 
 
 class StaticConfig:
-
-    NO_VALUE = Null()
-
     def __init__(self, config_node):
         self.__config_dict = config_node
         self.__config_has_changed = False
@@ -32,14 +29,13 @@ class StaticConfig:
     def config_dict(self):
         return self.__config_dict
 
-    def get(self, property_name, converter=str, default=NO_VALUE):
+    def get(self, property_name, converter=str, default=None):
         """Get static property
 
         Args:
             property_name (str): Property name
             converter (function): Default :func:`str`, Conversion function from configuration format to Python
-            default: Default: NO_VALUE, default value for property
-            inherited (bool): Default: False, Property can be inherited
+            default: default value for property if key does not exist (defaults to None)
 
         Returns:
             Property value
@@ -51,18 +47,14 @@ class StaticConfig:
         if self.__config_has_changed:
             self.reload()
             self.__config_has_changed = False
-
         property_value = self.config_dict.get(property_name)
-        if property_value is not None:
+        if property_value is None:
+            return default
+        else:
             if callable(converter):
                 return converter(property_value)
             else:
                 return property_value
-        else:
-            if default != self.NO_VALUE:
-                return default
-
-            raise KeyError("no property '%s` in config" % property_name)
 
     def set(self, property_name, value):
         if self.__config_has_changed:
diff --git a/bliss/controllers/motors/esrf_undulator.py b/bliss/controllers/motors/esrf_undulator.py
index 8096cbc6e8..e9211f8cb3 100644
--- a/bliss/controllers/motors/esrf_undulator.py
+++ b/bliss/controllers/motors/esrf_undulator.py
@@ -87,24 +87,15 @@ class ESRF_Undulator(Controller):
 
         log_debug(self, f"attr_pos_name={attr_pos_name}")
 
-        try:
-            attr_vel_name = axis.config.get("attribute_velocity", str)
-        except KeyError:
-            attr_vel_name = "Velocity"
+        attr_vel_name = axis.config.get("attribute_velocity", str, "Velocity")
         log_debug(self, f"attr_vel_name={attr_vel_name}")
 
-        try:
-            attr_fvel_name = axis.config.get("attribute_first_velocity", str)
-        except KeyError:
-            attr_fvel_name = "FirstVelocity"
-
+        attr_fvel_name = axis.config.get(
+            "attribute_first_velocity", str, "FirstVelocity"
+        )
         log_debug(self, f"attr_fvel_name={attr_fvel_name}")
 
-        try:
-            attr_acc_name = axis.config.get("attribute_acceleration", str)
-        except KeyError:
-            attr_acc_name = "Acceleration"
-
+        attr_acc_name = axis.config.get("attribute_acceleration", str, "Acceleration")
         log_debug(self, f"attr_acc_name={attr_acc_name}")
 
         alpha = axis.config.get("alpha", float, 0.0)
@@ -112,19 +103,18 @@ class ESRF_Undulator(Controller):
 
         log_debug(self, f"alpha={alpha}  period={period}")
 
-        try:
-            undu_prefix = axis.config.get("undu_prefix", str)
-            attr_pos_name = undu_prefix + attr_pos_name
-            attr_vel_name = undu_prefix + attr_vel_name
-            attr_fvel_name = undu_prefix + attr_fvel_name
-            attr_acc_name = undu_prefix + attr_acc_name
-
-        except KeyError:
+        undu_prefix = axis.config.get("undu_prefix", str)
+        if undu_prefix is None:
             log_debug(self, "'undu_prefix' not specified in config")
             if attr_pos_name == "Position":
                 raise RuntimeError("'undu_prefix' must be specified in config")
             else:
                 undu_prefix = ""
+        else:
+            attr_pos_name = undu_prefix + attr_pos_name
+            attr_vel_name = undu_prefix + attr_vel_name
+            attr_fvel_name = undu_prefix + attr_fvel_name
+            attr_acc_name = undu_prefix + attr_acc_name
 
         # check for revolver undulator
         is_revolver = False
-- 
GitLab


From 593a3bdaf6d188fef636f76cd0f76e8c920dc8ca Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Thu, 3 Sep 2020 15:40:18 +0200
Subject: [PATCH 14/19] linting

---
 bliss/common/axis.py     | 28 +++++++++++++++-------------
 bliss/config/channels.py | 10 +++++-----
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/bliss/common/axis.py b/bliss/common/axis.py
index bbf44517bb..f3b9cc0611 100644
--- a/bliss/common/axis.py
+++ b/bliss/common/axis.py
@@ -489,7 +489,7 @@ class Motion:
                 return f"Moving {self.axis.name} from {start_} to {end_}"
 
 
-class Trajectory(object):
+class Trajectory:
     """ Trajectory information
 
     Represents a specific trajectory motion.
@@ -677,20 +677,9 @@ class Axis:
         self.__config = StaticConfig(config)
         self.__settings = AxisSettings(self)
         self.__init_config_properties()
+        self.__no_offset = False
         self._group_move = GroupMove()
-        self._beacon_channels = dict()
-        self._move_stop_channel = Channel(
-            f"axis.{self.name}.move_stop",
-            default_value=False,
-            callback=self._external_stop,
-        )
-        self._jog_velocity_channel = Channel(
-            f"axis.{self.name}.change_jog_velocity",
-            default_value=None,
-            callback=self._set_jog_velocity,
-        )
         self._lock = gevent.lock.Semaphore()
-        self.__no_offset = False
 
         try:
             config.parent
@@ -709,6 +698,19 @@ class Axis:
         self._polling_time = config.get("polling_time", DEFAULT_POLLING_TIME)
         global_map.register(self, parents_list=["axes", controller])
 
+        # create Beacon channels
+        self.settings.init_channels()
+        self._move_stop_channel = Channel(
+            f"axis.{self.name}.move_stop",
+            default_value=False,
+            callback=self._external_stop,
+        )
+        self._jog_velocity_channel = Channel(
+            f"axis.{self.name}.change_jog_velocity",
+            default_value=None,
+            callback=self._set_jog_velocity,
+        )
+
     def __close__(self):
         try:
             controller_close = self.__controller.close
diff --git a/bliss/config/channels.py b/bliss/config/channels.py
index 2e22aa4721..949941e9ed 100644
--- a/bliss/config/channels.py
+++ b/bliss/config/channels.py
@@ -270,7 +270,7 @@ class Bus(AdvancedInstantiationInterface):
             raise ConnectionError(
                 "Connection to Beacon server lost. "
                 + "This is a serious problem! "
-                + "Please quite the bliss session and try to restart it. ("
+                + "Quit the bliss session and try to restart it. ("
                 + str(e)
                 + ")"
             )
@@ -482,7 +482,7 @@ class Channel(AdvancedInstantiationInterface):
 
     def _fire_callbacks(self):
         value = self._raw_value.value
-        callbacks = [_f for _f in [ref() for ref in self._callback_refs] if _f]
+        callbacks = filter(None, [ref() for ref in self._callback_refs])
 
         # Run callbacks
         for cb in callbacks:
@@ -491,7 +491,7 @@ class Channel(AdvancedInstantiationInterface):
                 self._firing_callbacks = True
                 cb(value)
             # Catch and display exception
-            except:
+            except Exception:
                 sys.excepthook(*sys.exc_info())
             # Clean up the flag
             finally:
@@ -603,9 +603,9 @@ class EventChannel(AdvancedInstantiationInterface):
 
     def _set_raw_value(self, raw_value):
         value = raw_value.value
-        callbacks = [_f for _f in [ref() for ref in self._callback_refs] if _f]
+        callbacks = filter(None, [ref() for ref in self._callback_refs])
         for cb in callbacks:
             try:
                 cb(value)
-            except:
+            except Exception:
                 sys.excepthook(*sys.exc_info())
-- 
GitLab


From da64659fc38e2042228fafbefbea4cc8f0282fa4 Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Thu, 3 Sep 2020 15:45:24 +0200
Subject: [PATCH 15/19] motor settings: move code from controller and axis to
 motor_settings.py

---
 bliss/common/axis.py           |  29 ++---
 bliss/common/motor_settings.py | 201 ++++++++++++++++++++++++++-------
 bliss/controllers/motor.py     | 119 ++-----------------
 bliss/tango/servers/axis_ds.py |   4 +-
 4 files changed, 184 insertions(+), 169 deletions(-)

diff --git a/bliss/common/axis.py b/bliss/common/axis.py
index f3b9cc0611..1684a8eaf6 100644
--- a/bliss/common/axis.py
+++ b/bliss/common/axis.py
@@ -341,8 +341,7 @@ class GroupMove:
             for motion in motions:
                 motion.axis._set_moving_state()
 
-                for _, chan in motion.axis._beacon_channels.items():
-                    chan.unregister_callback(chan._setting_update_cb)
+                motion.axis.settings.unregister_channels_callbacks()
 
         with capture_exceptions(raise_index=0) as capture:
             with capture():
@@ -402,10 +401,9 @@ class GroupMove:
                     for hook in axis.motion_hooks:
                         hooks[hook].append(motion)
 
-                    # set move done
-                    for _, chan in axis._beacon_channels.items():
-                        chan.register_callback(chan._setting_update_cb)
+                    axis.settings.register_channels_callbacks()
 
+                    # set move done
                     motion.axis._set_move_done()
 
             if self._interrupted_move:
@@ -676,7 +674,7 @@ class Axis:
             self.__encoder.axis = self
         self.__config = StaticConfig(config)
         self.__settings = AxisSettings(self)
-        self.__init_config_properties()
+        self._init_config_properties()
         self.__no_offset = False
         self._group_move = GroupMove()
         self._lock = gevent.lock.Semaphore()
@@ -762,20 +760,20 @@ class Axis:
         """
         return not self.__move_done.is_set()
 
-    def __init_config_properties(
+    def _init_config_properties(
         self, velocity=True, acceleration=True, limits=True, sign=True, backlash=True
     ):
         self.__steps_per_unit = self.config.get("steps_per_unit", float, 1)
         self.__tolerance = self.config.get("tolerance", float, 1e-4)
         if velocity:
-            if self.controller.axis_settings.config_setting["velocity"]:
+            if "velocity" in self.settings.config_settings:
                 self.__config_velocity = self.config.get("velocity", float)
-            if self.controller.axis_settings.config_setting["jog_velocity"]:
+            if "jog_velocity" in self.settings.config_settings:
                 self.__config_jog_velocity = self.config.get(
                     "jog_velocity", float, self.__config_velocity
                 )
         if acceleration:
-            if self.controller.axis_settings.config_setting["acceleration"]:
+            if "acceleration" in self.settings.config_settings:
                 self.__config_acceleration = self.config.get("acceleration", float)
         if limits:
             self.__config_low_limit = self.config.get("low_limit", float, float("-inf"))
@@ -786,13 +784,11 @@ class Axis:
             self.__config_backlash = self.config.get("backlash", float, 0)
 
     @property
-    @lazy_init
     def steps_per_unit(self):
         """Current steps per unit (:obj:`float`)"""
         return self.__steps_per_unit
 
     @property
-    @lazy_init
     def config_backlash(self):
         """Current backlash in user units (:obj:`float`)"""
         return self.__config_backlash
@@ -811,7 +807,6 @@ class Axis:
         self.settings.set("backlash", backlash)
 
     @property
-    @lazy_init
     def tolerance(self):
         """Current Axis tolerance in dial units (:obj:`float`)"""
         return self.__tolerance
@@ -2056,7 +2051,7 @@ class Axis:
 
         if any((velocity, acceleration, limits, sign, backlash)):
             self.__config.save()
-            self.__init_config_properties(
+            self._init_config_properties(
                 velocity=velocity,
                 acceleration=acceleration,
                 limits=limits,
@@ -2079,7 +2074,7 @@ class Axis:
         if reload:
             self.config.reload()
 
-        self.__init_config_properties(
+        self._init_config_properties(
             velocity=velocity,
             acceleration=acceleration,
             limits=limits,
@@ -2099,7 +2094,7 @@ class Axis:
         if backlash:
             self.settings.clear("backlash")
 
-        self.controller._init_settings(self)
+        self.settings.init()
 
         # update position (needed for sign change)
         pos = self.dial2user(self.dial)
@@ -2458,5 +2453,5 @@ class ModuloAxis(Axis):
 class NoSettingsAxis(Axis):
     def __init__(self, *args, **kwags):
         super().__init__(*args, **kwags)
-        for setting_name in self.settings:
+        for setting_name in self.settings.setting_names:
             self.settings.disable_cache(setting_name)
diff --git a/bliss/common/motor_settings.py b/bliss/common/motor_settings.py
index b49f61dfc5..a6277ffb70 100644
--- a/bliss/common/motor_settings.py
+++ b/bliss/common/motor_settings.py
@@ -5,11 +5,16 @@
 # Copyright (c) 2015-2020 Beamline Control Unit, ESRF
 # Distributed under the GNU LGPLv3. See LICENSE for more info.
 
+import collections
+import functools
+import inspect
+import math
+import sys
+
 from bliss.common import event
 from bliss.common.greenlet_utils import KillMask
+from bliss.config.channels import Channel
 from bliss.config import settings, settings_cache
-import collections
-import sys
 
 
 def setting_update_from_channel(value, setting_name=None, axis=None):
@@ -47,23 +52,24 @@ class ControllerAxisSettings:
         self.config_setting = {}
 
         # 'offset' must be set BEFORE limits to ensure good dial/user conversion.
-        self.add("offset", float)
         self.add("sign", int)
+        self.add("offset", float)
         self.add("backlash", float)
+        self.add("low_limit", floatOrNone)
+        self.add("high_limit", floatOrNone)
         self.add("velocity", float, config=True)
         self.add("jog_velocity", float)
         self.add("acceleration", float, config=True)
-        self.add("low_limit", floatOrNone)
-        self.add("high_limit", floatOrNone)
         self.add("dial_position", float)
         self.add("_set_position", float)
         self.add("position", float)
         self.add("state", stateSetting, persistent=False)
         self.add("steps_per_unit", float, config=True)
 
+    @property
     def config_settings(self):
         return tuple(
-            [setting for setting, config in self.config_setting.items() if config]
+            setting for setting, config in self.config_setting.items() if config
         )
 
     def add(self, setting_name, convert_func=str, persistent=True, config=False):
@@ -83,6 +89,7 @@ class AxisSettings:
     def __init__(self, axis):
         self.__axis = axis
         self.__state = None
+        self._beacon_channels = {}
         self._disabled_settings = disabled_settings_namedtuple(
             set(), dict(axis.config.config_dict)
         )
@@ -95,26 +102,145 @@ class AxisSettings:
         # Activate prefetch
         cnx_cache.add_prefetch(self._hash)
 
-    def __iter__(self):
-        for name in self.__axis.controller.axis_settings.setting_names:
-            yield name
+    @property
+    def setting_names(self):
+        yield from self.__axis.controller.axis_settings.setting_names
 
     def convert_func(self, setting_name):
         return self.__axis.controller.axis_settings.convert_func.get(setting_name)
 
+    @property
     def config_settings(self):
-        return self.__axis.controller.axis_settings.config_settings()
+        return self.__axis.controller.axis_settings.config_settings
+
+    def register_channels_callbacks(self):
+        for chan in self._beacon_channels.values():
+            chan.register_callback(chan._setting_update_cb)
+
+    def unregister_channels_callbacks(self):
+        for chan in self._beacon_channels.values():
+            chan.unregister_callback(chan._setting_update_cb)
+
+    def _create_channel(self, setting_name):
+        chan_name = "axis.%s.%s" % (self.__axis.name, setting_name)
+        chan = Channel(chan_name)
+        cb = functools.partial(
+            setting_update_from_channel, setting_name=setting_name, axis=self.__axis
+        )
+        chan._setting_update_cb = cb
+        return chan
+
+    def init_channels(self):
+        self._beacon_channels.clear()
+        for setting_name in self.__axis.controller.axis_settings.setting_names:
+            self._beacon_channels[setting_name] = self._create_channel(setting_name)
+        self.register_channels_callbacks()
+
+    def _get_setting_or_config_value(self, name, default=None):
+        # return setting or config parameter
+        converter = self.convert_func(name)
+        value = self.get(name)
+        if value is None:
+            value = self.__axis.config.get(name, converter, default=default)
+        return value
+
+    def check_config_settings(self):
+        axis = self.__axis
+        props = dict(
+            inspect.getmembers(axis.__class__, lambda o: isinstance(o, property))
+        )
+        config_settings = []
+        for setting_name in self.config_settings:
+            # check if setting is in config
+            value = axis.config.get(setting_name)
+            if value is None:
+                raise RuntimeError(
+                    "Axis %s: missing configuration key '%s`"
+                    % (self.name, setting_name)
+                )
+            if setting_name == "steps_per_unit":
+                # steps_per_unit is read-only
+                continue
+            config_settings.append(setting_name)
+            # check if setting has a method to initialize (set) its value,
+            # without actually executing the property
+            try:
+                assert callable(props[setting_name].fset)
+            except AssertionError:
+                raise RuntimeError(
+                    "Axis %s: missing method '%s` to set setting value"
+                    % (axis.name, setting_name)
+                )
+        return config_settings
+
+    def init(self):
+        """ Initialize settings
+
+        "config settings" are those that **must** be in YML config like
+        steps per unit, acceleration and velocity ; otherwise settings
+        can optionally be present in the config file.
+        Config settings must have a property setter on the Axis object.
+        "persistent settings" are stored in redis, like position for example;
+        in any case, when a setting is set its value is emitted via a
+        dedicated channel.
+        Some settings can be both config+persistent (like velocity) or
+        none (like state, which is only emitted when it changes, but not stored
+        at all)
+        """
+        axis = self.__axis
+
+        config_settings = self.check_config_settings()
+        config_steps_per_unit = axis.config.get("steps_per_unit", float)
+
+        if axis.no_offset:
+            sign = 1
+            offset = 0
+        else:
+            sign = self._get_setting_or_config_value("sign", 1)
+            offset = self._get_setting_or_config_value("offset", 0)
+        self.set("sign", sign)
+        self.set("offset", offset)
+
+        self.set("backlash", self._get_setting_or_config_value("backlash", 0))
+
+        low_limit_dial = self._get_setting_or_config_value("low_limit")
+        high_limit_dial = self._get_setting_or_config_value("high_limit")
+
+        if config_steps_per_unit:
+            cval = config_steps_per_unit
+            rval = self._hash.raw_get("steps_per_unit")
+            # Record steps_per_unit
+            if rval is None:
+                self.set("steps_per_unit", cval)
+            else:
+                rval = float(rval)
+                if cval != rval:
+                    ratio = rval / cval
+                    new_dial = axis.dial * ratio
+
+                    self.set("steps_per_unit", cval)
+                    if not axis.no_offset:
+                        # calculate offset so user pos stays the same
+                        self.set("offset", axis.position - axis.sign * new_dial)
+                    self.set("dial_position", new_dial)
+
+                    if math.copysign(rval, cval) != rval:
+                        ll, hl = low_limit_dial, high_limit_dial
+                        low_limit_dial, high_limit_dial = -hl, -ll
+
+        self.set("low_limit", low_limit_dial)
+        self.set("high_limit", high_limit_dial)
+
+        for setting_name in config_settings:
+            value = self._get_setting_or_config_value(setting_name)
+            setattr(axis, setting_name, value)
 
     def set(self, setting_name, value):
         if setting_name == "state":
             if self.__state == value:
                 return
             self.__state = value
-        """
-        * set setting
-        * send event
-        * write
-        """
+
         axis = self.__axis
         axis_settings = axis.controller.axis_settings
         if setting_name not in axis_settings.setting_names:
@@ -135,9 +261,9 @@ class AxisSettings:
         else:
             if axis_settings.persistent_setting[setting_name]:
                 with KillMask():
-                    axis.settings._hash[setting_name] = value
+                    self._hash[setting_name] = value
 
-            axis._beacon_channels[setting_name].value = value
+            self._beacon_channels[setting_name].value = value
 
         event.send(axis, "internal_" + setting_name, value)
         try:
@@ -151,45 +277,42 @@ class AxisSettings:
         disabled_settings = self._disabled_settings
 
         if setting_name not in axis_settings.setting_names:
-            raise ValueError(
-                "No setting '%s` for axis '%s`" % (setting_name, axis.name)
-            )
+            raise NameError("No setting '%s` for axis '%s`" % (setting_name, axis.name))
 
         if setting_name in disabled_settings.names:
             return disabled_settings.config_dict.get(setting_name)
+
+        if axis_settings.persistent_setting[setting_name]:
+            with KillMask():
+                value = self._hash.get(setting_name)
         else:
-            if axis_settings.persistent_setting[setting_name]:
-                with KillMask():
-                    value = axis.settings._hash.get(setting_name)
+            chan = self._beacon_channels.get(setting_name)
+            if chan:
+                value = chan.value
             else:
-                chan = axis._beacon_channels.get(setting_name)
-                if chan:
-                    value = chan.value
-                else:
-                    value = None
-
-            if value is not None:
-                convert_func = self.convert_func(setting_name)
-                if convert_func is not None:
-                    value = convert_func(value)
-            return value
+                value = None
+
+        if value is not None:
+            convert_func = self.convert_func(setting_name)
+            if convert_func is not None:
+                value = convert_func(value)
+        return value
 
     def clear(self, setting_name):
-        axis = self.__axis
-        axis_settings = axis.controller.axis_settings
         disabled_settings = self._disabled_settings
 
         if setting_name in disabled_settings.names:
             disabled_settings.config_dict[setting_name] = None
         else:
-            axis.settings._hash[setting_name] = None
+            self._hash[setting_name] = None
             # reset beacon channel, if it is there
             try:
-                channel = axis._beacon_channels[setting_name]
+                channel = self._beacon_channels[setting_name]
             except KeyError:
                 pass
             else:
-                channel.value = channel.default_value
+                channel.close()
+                self._beacon_channels[setting_name] = self._create_channel(setting_name)
 
     def disable_cache(self, setting_name, flag=True):
         """
diff --git a/bliss/controllers/motor.py b/bliss/controllers/motor.py
index 43d4e655bb..985c178a6a 100644
--- a/bliss/controllers/motor.py
+++ b/bliss/controllers/motor.py
@@ -7,14 +7,8 @@
 
 import math
 import numpy
-import inspect
-import functools
 from bliss.common.motor_config import StaticConfig
-from bliss.common.motor_settings import (
-    ControllerAxisSettings,
-    setting_update_from_channel,
-    floatOrNone,
-)
+from bliss.common.motor_settings import ControllerAxisSettings, floatOrNone
 from bliss.common.axis import Trajectory
 from bliss.common.motor_group import Group, TrajectoryGroup
 from bliss.common import event
@@ -26,18 +20,6 @@ from bliss.config import settings
 from gevent import lock
 
 
-# apply settings or config parameters
-def get_setting_or_config_value(axis, name):
-    converter = axis.settings.convert_func(name)
-    value = axis.settings.get(name)
-    if value is None:
-        try:
-            value = axis.config.get(name, converter)
-        except Exception:
-            return None
-    return value
-
-
 class Controller:
     """
     Motor controller base class
@@ -171,10 +153,10 @@ class Controller:
     def _initialize_axis(self, axis, *args, **kwargs):
         """
         """
-        if self.__initialized_axis[axis]:
-            return
-
         with self.__lock:
+            if self.__initialized_axis[axis]:
+                return
+
             # Initialize controller hardware only once.
             if not self.__initialized_hw.value:
                 self.initialize_hardware()
@@ -193,90 +175,15 @@ class Controller:
                 axis_initialized = self.__initialized_hw_axis[axis]
                 if not axis_initialized.value:
                     self.initialize_hardware_axis(axis)
+                    axis.settings.check_config_settings()
+                    axis.settings.init()  # get settings, from config or from cache, and apply to hardware
                     axis_initialized.value = 1
 
-                    self._init_settings(axis)
             except BaseException:
                 # Failed to initialize
                 self.__initialized_axis[axis] = False
                 raise
 
-    def _init_settings(self, axis):
-        """ Initialize hardware with settings
-        """
-        props = dict(
-            inspect.getmembers(axis.__class__, lambda o: isinstance(o, property))
-        )
-
-        sign = get_setting_or_config_value(axis, "sign")
-        if sign is None or axis.no_offset:
-            sign = 1
-        axis.settings.set("sign", sign)
-
-        offset = get_setting_or_config_value(axis, "offset")
-        if offset is None or axis.no_offset:
-            offset = 0
-        axis.settings.set("offset", offset)
-
-        backlash = get_setting_or_config_value(axis, "backlash")
-        if backlash is None:
-            backlash = 0
-        axis.backlash = backlash
-
-        low_limit_dial = get_setting_or_config_value(axis, "low_limit")
-        high_limit_dial = get_setting_or_config_value(axis, "high_limit")
-        axis.dial_limits = low_limit_dial, high_limit_dial
-
-        for setting_name in axis.settings.config_settings():
-            # check if setting is in config
-            if axis.config.get(setting_name) is None:
-                raise RuntimeError(
-                    "Axis %s: missing configuration key '%s`"
-                    % (axis.name, setting_name)
-                )
-            # check if setting has a method to initialize (set) its value,
-            # without actually executing the property
-            try:
-                props[setting_name].fset
-            except AttributeError:
-                raise RuntimeError(
-                    "Axis %s: missing method '%s` to set setting value"
-                    % (axis.name, setting_name)
-                )
-
-        for setting_name in axis.settings.config_settings():
-            if setting_name == "steps_per_unit":
-                cval = float(axis.config.get(setting_name))
-                rval = axis.settings._hash.raw_get(setting_name)
-                # Record steps_per_unit
-                if rval is None:
-                    axis.settings.set(setting_name, cval)
-                    continue
-                else:
-                    rval = float(rval)
-                if cval != rval:
-                    ratio = rval / cval
-                    new_dial = axis.dial * ratio
-
-                    axis.settings.set("steps_per_unit", cval)
-                    if not axis.no_offset:
-                        # calculate offset so user pos stays the same
-                        axis.settings.set(
-                            "offset", axis.position - axis.sign * new_dial
-                        )
-                    else:
-                        axis.settings.set("offset", 0)
-                    axis.settings.set("dial_position", new_dial)
-
-                    if math.copysign(rval, cval) != rval:
-                        ll = axis.settings.get("low_limit")
-                        hl = axis.settings.get("high_limit")
-                        axis.settings.set("low_limit", -hl)
-                        axis.settings.set("high_limit", -ll)
-            else:
-                value = get_setting_or_config_value(axis, setting_name)
-                setattr(axis, setting_name, value)
-
     def get_axis(self, axis_name):
         axis = self._axes.get(axis_name)
         if axis is None:  # create it
@@ -303,23 +210,13 @@ class Controller:
                 # reference axis
                 return
 
-            axis._beacon_channels.clear()
-
-            for setting_name in axis.settings:
-                setting_value = get_setting_or_config_value(axis, setting_name)
-                chan_name = "axis.%s.%s" % (axis.name, setting_name)
-                cb = functools.partial(
-                    setting_update_from_channel, setting_name=setting_name, axis=axis
-                )
-                chan = Channel(chan_name, default_value=setting_value, callback=cb)
-                chan._setting_update_cb = cb
-                axis._beacon_channels[setting_name] = chan
-
             if axis.controller is self:
                 axis_initialized = Cache(axis, "initialized", default_value=0)
                 self.__initialized_hw_axis[axis] = axis_initialized
                 self.__initialized_axis[axis] = False
+
             self._add_axis(axis)
+
         return axis
 
     def _add_axis(self, axis):
diff --git a/bliss/tango/servers/axis_ds.py b/bliss/tango/servers/axis_ds.py
index 5d1b3c4343..cedabc1162 100644
--- a/bliss/tango/servers/axis_ds.py
+++ b/bliss/tango/servers/axis_ds.py
@@ -1442,7 +1442,7 @@ def __create_tango_axis_class(axis):
         % axis.name
     )
 
-    for setting_name in axis.settings:
+    for setting_name in axis.settings.setting_names:
         if setting_name in [
             "velocity",
             "position",
@@ -1456,7 +1456,7 @@ def __create_tango_axis_class(axis):
         ]:
             elog.debug(" BlissAxisManager.py -- std SETTING %s " % (setting_name))
         else:
-            _setting_type = axis.controller.axis_settings.convert_func[setting_name]
+            _setting_type = axis.settings.convert_func(setting_name)
             _attr_type = types_conv_tab[_setting_type]
             elog.debug(
                 " BlissAxisManager.py -- adds SETTING %s as %s attribute"
-- 
GitLab


From d8098b6836fb779775b1a44ceee17b35582ff599 Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Thu, 3 Sep 2020 15:47:58 +0200
Subject: [PATCH 16/19] channels: do not update values when closing

Prevent errors in CI
---
 bliss/config/channels.py | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/bliss/config/channels.py b/bliss/config/channels.py
index 949941e9ed..e7510d83d1 100644
--- a/bliss/config/channels.py
+++ b/bliss/config/channels.py
@@ -115,15 +115,22 @@ class Bus(AdvancedInstantiationInterface):
         self._send_task = gevent.spawn(self._send)
         self._send_event = gevent.event.Event()
 
+        self.__closing = False
+
     # Close
 
+    @property
+    def closing(self):
+        return self.__closing
+
     def close(self):
-        for channel in list(self._channels.values()):
-            channel.close()
+        self.__closing = True
         if self._send_task:
             self._send_task.kill()
         if self._listen_task:
             self._listen_task.kill()
+        for channel in list(self._channels.values()):
+            channel.close()
 
     @classmethod
     def clear_cache(cls):
@@ -455,7 +462,8 @@ class Channel(AdvancedInstantiationInterface):
                 reply_value = self._default_value
 
             # Set the value
-            self._set_raw_value(reply_value)
+            if not self._bus.closing:
+                self._set_raw_value(reply_value)
 
             # Unregister task if everything went smoothly
             self._query_task = None
-- 
GitLab


From ae2804955eedccdbcd26f4deabc7da48295c1b6e Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Thu, 3 Sep 2020 17:10:48 +0200
Subject: [PATCH 17/19] tests: add test for invalid configuration

---
 bliss/common/motor_settings.py             |  2 +-
 tests/motors/test_axis.py                  | 22 ++++++++++++++++++++++
 tests/test_configuration/motors/mockup.yml |  3 +++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/bliss/common/motor_settings.py b/bliss/common/motor_settings.py
index a6277ffb70..76db27b65c 100644
--- a/bliss/common/motor_settings.py
+++ b/bliss/common/motor_settings.py
@@ -156,7 +156,7 @@ class AxisSettings:
             if value is None:
                 raise RuntimeError(
                     "Axis %s: missing configuration key '%s`"
-                    % (self.name, setting_name)
+                    % (axis.name, setting_name)
                 )
             if setting_name == "steps_per_unit":
                 # steps_per_unit is read-only
diff --git a/tests/motors/test_axis.py b/tests/motors/test_axis.py
index b550a6d310..afd6110471 100644
--- a/tests/motors/test_axis.py
+++ b/tests/motors/test_axis.py
@@ -1023,3 +1023,25 @@ def test_no_settings_offset(beacon):
     nsa.offset = 1
     assert nsa.position == pytest.approx(1)
     assert nsa.dial == pytest.approx(0)
+
+
+def test_invalid_config(beacon):
+    invalid_cfg = beacon.get_config("invalid_cfg_axis")
+
+    invalid_mot = beacon.get("invalid_cfg_axis")
+    with pytest.raises(RuntimeError) as exc:
+        invalid_mot.position  # lazy init
+    assert "velocity" in str(exc.value)
+
+    invalid_cfg["velocity"] = 1
+    with pytest.raises(RuntimeError) as exc:
+        invalid_mot.position  # lazy init
+    assert "acceleration" in str(exc.value)
+
+    invalid_cfg["acceleration"] = 1
+    with pytest.raises(RuntimeError) as exc:
+        invalid_mot.position  # lazy init
+    assert "steps_per_unit" in str(exc.value)
+
+    invalid_cfg["steps_per_unit"] = 1
+    assert invalid_mot.position == 0
diff --git a/tests/test_configuration/motors/mockup.yml b/tests/test_configuration/motors/mockup.yml
index 9cc1378fc5..4b23f3f6e1 100644
--- a/tests/test_configuration/motors/mockup.yml
+++ b/tests/test_configuration/motors/mockup.yml
@@ -247,6 +247,9 @@ controller:
       steps_per_unit: 10
       acceleration: 100
       velocity: 1e9
+- class: mockup
+  axes:
+    - name: invalid_cfg_axis
 - class: EnergyWavelength
   axes:
     - name: $mono
-- 
GitLab


From 7fbfd0b6814a51787be8e508ce63c0b58104c21b Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Fri, 4 Sep 2020 15:16:00 +0200
Subject: [PATCH 18/19] motor_config: rename StaticConfig => MotorConfig

---
 bliss/common/axis.py         | 6 +++---
 bliss/common/encoder.py      | 4 ++--
 bliss/common/motor_config.py | 2 +-
 bliss/controllers/motor.py   | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/bliss/common/axis.py b/bliss/common/axis.py
index 1684a8eaf6..d38a0df630 100644
--- a/bliss/common/axis.py
+++ b/bliss/common/axis.py
@@ -12,7 +12,7 @@ and :class:`~bliss.common.axis.GroupMove`)
 """
 from bliss import global_map
 from bliss.common.cleanup import capture_exceptions
-from bliss.common.motor_config import StaticConfig
+from bliss.common.motor_config import MotorConfig
 from bliss.common.motor_settings import AxisSettings
 from bliss.common import event
 from bliss.common.greenlet_utils import protect_from_one_kill
@@ -672,7 +672,7 @@ class Axis:
         self.__encoder = config.get("encoder")
         if self.__encoder is not None:
             self.__encoder.axis = self
-        self.__config = StaticConfig(config)
+        self.__config = MotorConfig(config)
         self.__settings = AxisSettings(self)
         self._init_config_properties()
         self.__no_offset = False
@@ -742,7 +742,7 @@ class Axis:
 
     @property
     def config(self):
-        """Reference to the :class:`~bliss.common.motor_config.StaticConfig`"""
+        """Reference to the :class:`~bliss.common.motor_config.MotorConfig`"""
         return self.__config
 
     @property
diff --git a/bliss/common/encoder.py b/bliss/common/encoder.py
index 10d228d9f7..cdea3c3787 100644
--- a/bliss/common/encoder.py
+++ b/bliss/common/encoder.py
@@ -5,7 +5,7 @@
 # Copyright (c) 2015-2020 Beamline Control Unit, ESRF
 # Distributed under the GNU LGPLv3. See LICENSE for more info.
 
-from bliss.common.motor_config import StaticConfig
+from bliss.common.motor_config import MotorConfig
 from bliss.common.counter import SamplingCounter
 from bliss.controllers import counter
 from bliss.common import event
@@ -37,7 +37,7 @@ class Encoder:
         self._counter_controller.create_counter(
             SamplingCounter, "position", unit=config.get("unit")
         )
-        self.__config = StaticConfig(config)
+        self.__config = MotorConfig(config)
         self.__axis_ref = None
 
     @property
diff --git a/bliss/common/motor_config.py b/bliss/common/motor_config.py
index cd7acf3a38..6ae67815fa 100644
--- a/bliss/common/motor_config.py
+++ b/bliss/common/motor_config.py
@@ -10,7 +10,7 @@ from bliss.common.utils import Null
 import time
 
 
-class StaticConfig:
+class MotorConfig:
     def __init__(self, config_node):
         self.__config_dict = config_node
         self.__config_has_changed = False
diff --git a/bliss/controllers/motor.py b/bliss/controllers/motor.py
index 985c178a6a..8eba6af9a1 100644
--- a/bliss/controllers/motor.py
+++ b/bliss/controllers/motor.py
@@ -7,7 +7,7 @@
 
 import math
 import numpy
-from bliss.common.motor_config import StaticConfig
+from bliss.common.motor_config import MotorConfig
 from bliss.common.motor_settings import ControllerAxisSettings, floatOrNone
 from bliss.common.axis import Trajectory
 from bliss.common.motor_group import Group, TrajectoryGroup
@@ -30,7 +30,7 @@ class Controller:
 
     def __init__(self, name, config, axes, encoders, shutters, switches):
         self.__name = name
-        self.__config = StaticConfig(config)
+        self.__config = MotorConfig(config)
         self.__initialized_hw = Cache(self, "initialized", default_value=False)
         self.__initialized_hw_axis = dict()
         self.__initialized_encoder = dict()
-- 
GitLab


From 0114cd2137318c16558c9a32390521d1810c2004 Mon Sep 17 00:00:00 2001
From: Matias Guijarro <guijarro@esrf.fr>
Date: Fri, 4 Sep 2020 16:11:14 +0200
Subject: [PATCH 19/19] requirements update

---
 requirements-test-conda.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/requirements-test-conda.txt b/requirements-test-conda.txt
index d4896bd792..a9fbf063a7 100644
--- a/requirements-test-conda.txt
+++ b/requirements-test-conda.txt
@@ -6,7 +6,8 @@ pytest == 5.3.5
 pytest-cov
 pytest-mock
 lima-core == 1.9.4
-lima-camera-simulator-tango
+lima-camera-simulator >= 1.9.2
+lima-camera-simulator-tango >= 1.9.2
 lima-tango-server
 scipy
 black == 18.6b4
-- 
GitLab