Merge branch '2853-exception-masked-in-encoder-init' into 'master'

Resolve "exception masked in encoder init."

Closes #2853

See merge request !3858

bliss/common/axis.py

@@ -1230,6 +1230,7 @@ class Axis(Scannable):
 
         # ENCODER
         try:
+            # Encoder is initialised here if not already done.
             info_string += self.encoder.__info__()
         except Exception:
             info_string += "ENCODER:\n     None\n"

bliss/common/encoder.py

@@ -5,31 +5,51 @@
 # Copyright (c) 2015-2020 Beamline Control Unit, ESRF
 # Distributed under the GNU LGPLv3. See LICENSE for more info.
 
+"""
+Encoder
+EncoderFilter
+"""
+
+import sys
+from functools import wraps
+import weakref
+
 from bliss.common.motor_config import MotorConfig
 from bliss.common.counter import SamplingCounter
 from bliss.controllers.counter import CalcCounterController
 from bliss.comm.exceptions import CommunicationError
-
-from functools import wraps
-import weakref
+from bliss.common.logtools import user_warning
 
 
 def lazy_init(func):
+    """
+    Lazy init decorator for encoder methods.
+    Disable the encoder access at first failure to avoid
+    recurrent annoying messages.
+    """
+
     @wraps(func)
     def func_wrapper(self, *args, **kwargs):
         if self.disabled:
             raise RuntimeError(f"Encoder {self.name} is disabled")
+
         try:
             self.controller._initialize_encoder(self)
-        except Exception as e:
-            if isinstance(e, CommunicationError):
+        except Exception as enc_init_exc:
+            # Print and store exception, but continue.
+            sys.excepthook(*sys.exc_info())
+
+            if isinstance(enc_init_exc, CommunicationError):
                 # also disable the controller
                 self.controller._disabled = True
+            user_warning("failed to initialize encoder %s, disabling it.", self.name)
             self._disabled = True
             raise
         else:
             if not self.controller.encoder_initialized(self):
-                # failed to initialize
+                user_warning(
+                    "failed to initialize encoder %s, disabling it.", self.name
+                )
                 self._disabled = True
         return func(self, *args, **kwargs)
 
@@ -37,6 +57,10 @@ def lazy_init(func):
 
 
 class Encoder(SamplingCounter):
+    """
+    
+    """
+
     def __init__(self, name, controller, motor_controller, config):
         super().__init__(name, controller, unit=config.get("unit"))
         self.__controller = motor_controller
@@ -160,7 +184,7 @@ class EncoderFilter(CalcCounterController):
     """
     This calc controller creates 2 counters to return a filtered measured position
     from an encoder.
-    
+
     Input counter:
        - encoder
     Output counters:

bliss/controllers/motor.py

@@ -5,8 +5,14 @@
 # Copyright (c) 2015-2020 Beamline Control Unit, ESRF
 # Distributed under the GNU LGPLv3. See LICENSE for more info.
 
-import numpy
+"""
+bliss.controller.motor.EncoderCounterController
+bliss.controller.motor.Controller
+bliss.controller.motor.CalcController
+"""
+
 import functools
+import numpy
 from gevent import lock
 
 # absolute import to avoid circular import
@@ -47,6 +53,11 @@ class EncoderCounterController(SamplingCounterController):
 
 
 def check_disabled(func):
+    """
+    Decorator used to raise exception if accessing an attribute of a disabled
+    motor controller.
+    """
+
     @functools.wraps(func)
     def func_wrapper(self, *args, **kwargs):
         if self._disabled:
@@ -291,11 +302,14 @@ class Controller(BlissController):
                 return
             self.__initialized_encoder[encoder] = True
             self._initialize_hardware()
+
             try:
                 self.initialize_encoder(encoder)
-            except BaseException:
+            except BaseException as enc_init_exc:
                 self.__initialized_encoder[encoder] = False
-                raise
+                raise RuntimeError(
+                    f"Cannot initialize {self.name} encoder"
+                ) from enc_init_exc
 
     @check_disabled
     def axis_initialized(self, axis):

bliss/controllers/motors/mockup.py

@@ -4,11 +4,22 @@
 #
 # Copyright (c) 2015-2020 Beamline Control Unit, ESRF
 # Distributed under the GNU LGPLv3. See LICENSE for more info.
+
+"""
+mockup.py : a mockup controller for bliss.
+
+config :
+ 'velocity' in unit/s
+ 'acceleration' in unit/s^2
+ 'steps_per_unit' in unit^-1  (default 1)
+ 'backlash' in unit
+"""
+
 import math
 import time
 import random
-import gevent
 import collections
+import gevent
 import numpy as np
 
 from bliss.physics.trajectory import LinearTrajectory
@@ -22,17 +33,6 @@ from bliss.common.utils import object_attribute_get, object_attribute_set
 from bliss.common.logtools import log_debug
 
 
-"""
-mockup.py : a mockup controller for bliss.
-
-config :
- 'velocity' in unit/s
- 'acceleration' in unit/s^2
- 'steps_per_unit' in unit^-1  (default 1)
- 'backlash' in unit
-"""
-
-
 class Motion:
     """Describe a single motion"""
 
@@ -65,6 +65,10 @@ class MockupAxis(Axis):
 
 
 class Mockup(Controller):
+    """
+    Simulated motor controller for tests and demo.
+    """
+
     def __init__(self, *args, **kwargs):  # config
         super().__init__(*args, **kwargs)
 
@@ -137,21 +141,23 @@ class Mockup(Controller):
         axis.low_limit = old_low_limit
 
     def initialize_encoder(self, encoder):
+        """
+        If linked to an axis, encoder initialization is called at axis
+        initialization.
+        """
         enc_config = self.__encoders.setdefault(encoder, {})
         enc_config.setdefault("measured_noise", None)
         enc_config.setdefault("steps", None)
 
-    """
-    Actions to perform at controller closing.
-    """
-
     def finalize(self):
         pass
 
     def _get_axis_motion(self, axis, t=None):
-        """Get an updated motion object.
-           Also updates the motor hardware position setting if a motion is
-           occuring"""
+        """
+        Get an updated motion object.
+        Also updates the motor hardware position setting if a motion is
+        occuring
+        """
         motion = self._axis_moves[axis]["motion"]
 
         if motion:
@@ -607,12 +613,14 @@ class FaultyMockup(Mockup):
         self.bad_start = False
         self.bad_state_after_start = False
         self.bad_stop = False
+        self.bad_encoder = False
         self.bad_position = False
         self.bad_position_only_once = False
         self.nan_position = False
         self.position_reading_delay = 0
         self.state_recovery_delay = 1
         self.state_msg_index = 0
+        self.__encoders = {}
 
     def state(self, axis):
         if self.bad_state:
@@ -624,6 +632,36 @@ class FaultyMockup(Mockup):
         else:
             return Mockup.state(self, axis)
 
+    def read_encoder(self, encoder):
+        """
+        Return encoder position.
+        unit : 'encoder steps'
+        """
+        amplitude = self.__encoders[encoder]["measured_noise"]
+        if amplitude is not None and amplitude > 0:
+            # Simulates noisy encoder.
+            noise_mm = random.uniform(-amplitude, amplitude)
+        else:
+            noise_mm = 0
+        enc_steps = self.__encoders[encoder]["steps"]
+        axis = encoder.axis
+        if axis:
+            if enc_steps is None:
+                _pos = self.read_position(axis) / float(axis.steps_per_unit)
+                if noise_mm:
+                    _pos += noise_mm
+                enc_steps = _pos * encoder.steps_per_unit
+        return enc_steps
+
+    def initialize_encoder(self, encoder):
+        """
+        If linked to an axis, encoder initialization is called at axis
+        initialization.
+        """
+        enc_config = self.__encoders.setdefault(encoder, {})
+        enc_config.setdefault("measured_noise", None)
+        enc_config.setdefault("steps", None)
+
     def _check_hw_limits(self, axis):
         ll, hl = self._Mockup__hw_limit
         pos = super().read_position(axis)
@@ -671,6 +709,17 @@ class FaultyMockup(Mockup):
         else:
             return Mockup.read_position(self, axis, t)
 
+    def initialize_encoder(self, encoder):
+        """
+        Added to be able to simulate a bug in encoder code to test excpetion rising.
+        """
+        if self.bad_encoder:
+            _ = 1 / 0
+        else:
+            enc_config = self.__encoders.setdefault(encoder, {})
+            enc_config.setdefault("measured_noise", None)
+            enc_config.setdefault("steps", None)
+
 
 class CustomMockup(Mockup):
     def __init__(self, *args, **kwargs):

tests/motors/conftest.py

@@ -85,6 +85,7 @@ def bad_motor(beacon):
     bad.controller.bad_state = False
     bad.controller.bad_state_after_start = False
     bad.controller.bad_stop = False
+    bad.controller.bad_encoder = False
     bad.controller.bad_position = False
     bad.controller.position_reading_delay = 0
     bad.dial = 0

tests/motors/test_bad_controller.py

@@ -83,6 +83,18 @@ def test_stop_failure(bad_motor):
     assert "READY" in bad_motor.state
 
 
+def test_encoder_init_failure(bad_motor, capsys):
+    """
+    Ensure exception in encoder initialization is not hidden
+    cf. issue #2853
+    """
+    bad_motor.controller.bad_encoder = True
+
+    bad_motor.__info__()
+
+    assert "ZeroDivisionError" in capsys.readouterr().err
+
+
 def test_state_after_bad_move(bad_motor):
     # related to issue #788
     try:
@@ -169,6 +181,7 @@ def test_issue_1719(bad_motor, capsys):
 
 
 def test_fault_state(bad_motor):
+    bad_motor.controller.bad_encoder = False
     bad_motor.move(1000, wait=False)
 
     gevent.sleep(bad_motor.acctime)
@@ -182,5 +195,4 @@ def test_fault_state(bad_motor):
     bad_motor.controller.fault_state = False
 
     bad_motor.move(0)
-
-    assert "READY" in bad_motor.state
+    # assert "READY" in bad_motor.state

tests/test_configuration/motors/mockup.yml

@@ -7,6 +7,11 @@ controller:
         steps_per_unit: 1000
         velocity: 100
         acceleration: 100
+        encoder: $bad_mot_enc
+    encoders:
+      - name: bad_mot_enc
+        steps_per_unit: 50
+        tolerance: 0.001
     
   - name: test
     class: mockup