From f2c6dd0b24226524a2fdb3286babc218db27d535 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20L=C3=B3pez=20S=C3=A1nchez?= Date: Tue, 1 Jun 2021 08:32:00 +0000 Subject: [PATCH 01/27] add method get_listeners --- src/sardana/sardanaevent.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/sardana/sardanaevent.py b/src/sardana/sardanaevent.py index 1348affdb8..f0e5e6150f 100644 --- a/src/sardana/sardanaevent.py +++ b/src/sardana/sardanaevent.py @@ -105,6 +105,9 @@ def has_listeners(self): return False return len(self._listeners) > 0 + def get_listeners(self): + return self._listeners + def fire_event(self, event_type, event_value, listeners=None): self.flush_queue() self._fire_event(event_type, event_value, listeners=listeners) From 0848e135a9e7a14f81376d529c9c7f962bacaa10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20L=C3=B3pez=20S=C3=A1nchez?= Date: Tue, 1 Jun 2021 08:35:30 +0000 Subject: [PATCH 02/27] add check for dependent elements before delete a element, --- src/sardana/pool/pool.py | 7 +++++++ src/sardana/pool/poolelement.py | 12 ++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/sardana/pool/pool.py b/src/sardana/pool/pool.py index 832d81504e..d88392d23b 100644 --- a/src/sardana/pool/pool.py +++ b/src/sardana/pool/pool.py @@ -40,6 +40,7 @@ from sardana import InvalidId, ElementType, TYPE_ACQUIRABLE_ELEMENTS, \ TYPE_PSEUDO_ELEMENTS, TYPE_PHYSICAL_ELEMENTS, TYPE_MOVEABLE_ELEMENTS +from sardana.pool.poolbaseelement import PoolBaseElement from sardana.sardanamanager import SardanaElementManager, SardanaIDManager from sardana.sardanamodulemanager import ModuleManager from sardana.sardanaevent import EventType @@ -557,6 +558,12 @@ def delete_element(self, name): raise Exception("There is no element with name '%s'" % name) elem_type = elem.get_type() + + dependent_elements = elem.get_dependent_elements() + if len(dependent_elements) > 0: + raise Exception("The element {} can't be deleted because {} depend" + " on it.".format(name, ", ".join(dependent_elements))) + if elem_type == ElementType.Controller: if len(elem.get_elements()) > 0: raise Exception("Cannot delete controller with elements. " diff --git a/src/sardana/pool/poolelement.py b/src/sardana/pool/poolelement.py index 2574c9fe4a..959a593e98 100644 --- a/src/sardana/pool/poolelement.py +++ b/src/sardana/pool/poolelement.py @@ -88,6 +88,18 @@ def set_action_cache(self, action_cache): def get_source(self): return "{0}/{1}".format(self.full_name, self.get_default_acquisition_channel()) + def get_dependent_elements(self): + pool_base_elements = [] + for listener in self.get_listeners(): + listener = listener() + if isinstance(listener.__self__, PoolBaseElement): + pool_base_elements.append(listener.__self__.get_name()) + + return pool_base_elements + + def has_dependent_elements(self): + return len(self.get_dependent_elements()) > 0 + # -------------------------------------------------------------------------- # instrument # -------------------------------------------------------------------------- From d498b1f1a424f273962d0472cc314a3719aa6d01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20L=C3=B3pez=20S=C3=A1nchez?= Date: Thu, 3 Jun 2021 12:18:36 +0200 Subject: [PATCH 03/27] fix remove pydevd debugger --- src/sardana/tango/pool/Pool.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sardana/tango/pool/Pool.py b/src/sardana/tango/pool/Pool.py index 54c0e29934..37b39252bb 100644 --- a/src/sardana/tango/pool/Pool.py +++ b/src/sardana/tango/pool/Pool.py @@ -538,7 +538,8 @@ def _create_single_element(self, kwargs): self._check_element(name, full_name) util = PyTango.Util.instance() - + import pydevd + pydevd.settrace(suspend=False, trace_only_current_thread=True) def create_element_cb(device_name): try: db = util.get_database() From 3e9fce89dd288d244fa8719968761e2e9305d398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20L=C3=B3pez=20S=C3=A1nchez?= Date: Thu, 3 Jun 2021 12:22:38 +0200 Subject: [PATCH 04/27] improve dependent elements error message When delete a element that depens of a motor group a better message for the user is printed. --- src/sardana/pool/pool.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/sardana/pool/pool.py b/src/sardana/pool/pool.py index d88392d23b..7f8c88b02c 100644 --- a/src/sardana/pool/pool.py +++ b/src/sardana/pool/pool.py @@ -558,11 +558,13 @@ def delete_element(self, name): raise Exception("There is no element with name '%s'" % name) elem_type = elem.get_type() - + dependent_elements = elem.get_dependent_elements() if len(dependent_elements) > 0: raise Exception("The element {} can't be deleted because {} depend" - " on it.".format(name, ", ".join(dependent_elements))) + " on it. '_mg_ms_*' are motor groups, use any tango client" + " (e.g. Jive) to delete them and restart server." + .format(name, ", ".join(dependent_elements))) if elem_type == ElementType.Controller: if len(elem.get_elements()) > 0: @@ -590,6 +592,7 @@ def delete_element(self, name): if hasattr(elem, "get_controller"): elem.set_deleted(True) + def create_instrument(self, full_name, klass_name, id=None): is_root = full_name.count('/') == 1 From 510f4a311dd956c3788bb61bc09837799fc50bcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20L=C3=B3pez=20S=C3=A1nchez?= Date: Thu, 3 Jun 2021 12:28:41 +0200 Subject: [PATCH 05/27] improve get_dependent_elements now it takes into account if the listeners are from an object instance --- src/sardana/pool/poolbaseelement.py | 3 +++ src/sardana/pool/poolelement.py | 9 ++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/sardana/pool/poolbaseelement.py b/src/sardana/pool/poolbaseelement.py index 68d3e88813..79148f40dc 100644 --- a/src/sardana/pool/poolbaseelement.py +++ b/src/sardana/pool/poolbaseelement.py @@ -99,6 +99,9 @@ def serialize(self, *args, **kwargs): ret = PoolObject.serialize(self, *args, **kwargs) return ret + def get_dependent_elements(self): + return [] + # -------------------------------------------------------------------------- # simulation mode # -------------------------------------------------------------------------- diff --git a/src/sardana/pool/poolelement.py b/src/sardana/pool/poolelement.py index 959a593e98..5bc342d9e4 100644 --- a/src/sardana/pool/poolelement.py +++ b/src/sardana/pool/poolelement.py @@ -91,9 +91,12 @@ def get_source(self): def get_dependent_elements(self): pool_base_elements = [] for listener in self.get_listeners(): - listener = listener() - if isinstance(listener.__self__, PoolBaseElement): - pool_base_elements.append(listener.__self__.get_name()) + try: + dependent_elem = listener().__self__ + except AttributeError: + continue + if isinstance(dependent_elem, PoolBaseElement): + pool_base_elements.append(dependent_elem.name) return pool_base_elements From b28a6fd56d4a43696ad6934dd0b8dca295e24b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20L=C3=B3pez=20S=C3=A1nchez?= Date: Thu, 3 Jun 2021 11:04:45 +0000 Subject: [PATCH 06/27] clarifies message of dependent elements --- src/sardana/pool/pool.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/sardana/pool/pool.py b/src/sardana/pool/pool.py index 7f8c88b02c..73935b229f 100644 --- a/src/sardana/pool/pool.py +++ b/src/sardana/pool/pool.py @@ -562,8 +562,9 @@ def delete_element(self, name): dependent_elements = elem.get_dependent_elements() if len(dependent_elements) > 0: raise Exception("The element {} can't be deleted because {} depend" - " on it. '_mg_ms_*' are motor groups, use any tango client" - " (e.g. Jive) to delete them and restart server." + " on it. \n\nIf the name of the dependent element starts with" + "'_mg_ms_*' it means that are motor groups, use any tango " + "client (e.g. Jive) to delete them and restart the server." .format(name, ", ".join(dependent_elements))) if elem_type == ElementType.Controller: From 35cb86a4006723f00054c5bebbe4d35199541a7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20L=C3=B3pez=20S=C3=A1nchez?= Date: Thu, 3 Jun 2021 15:28:15 +0000 Subject: [PATCH 07/27] remove pydevd --- src/sardana/tango/pool/Pool.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/sardana/tango/pool/Pool.py b/src/sardana/tango/pool/Pool.py index 37b39252bb..56d32eebaf 100644 --- a/src/sardana/tango/pool/Pool.py +++ b/src/sardana/tango/pool/Pool.py @@ -538,8 +538,6 @@ def _create_single_element(self, kwargs): self._check_element(name, full_name) util = PyTango.Util.instance() - import pydevd - pydevd.settrace(suspend=False, trace_only_current_thread=True) def create_element_cb(device_name): try: db = util.get_database() From bdf0b53266eee4bcb6d652c1150d0c0b62e0292a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20L=C3=B3pez=20S=C3=A1nchez?= Date: Tue, 15 Jun 2021 10:13:56 +0200 Subject: [PATCH 08/27] add weakref to element in add_pseudo_elements --- src/sardana/pool/poolbasechannel.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/sardana/pool/poolbasechannel.py b/src/sardana/pool/poolbasechannel.py index f35370881c..9ed1a2c82a 100644 --- a/src/sardana/pool/poolbasechannel.py +++ b/src/sardana/pool/poolbasechannel.py @@ -40,6 +40,7 @@ ControllerConfiguration from sardana.sardanaevent import EventType from sardana.pool import AcqSynch, AcqMode +import weakref class ValueBuffer(SardanaBuffer): @@ -106,6 +107,7 @@ class PoolBaseChannel(PoolElement): def __init__(self, **kwargs): PoolElement.__init__(self, **kwargs) + self._value = self.ValueAttributeClass(self, listeners=self.on_change) self._value_buffer = self.ValueBufferClass(self, listeners=self.on_change) @@ -150,7 +152,7 @@ def add_pseudo_element(self, element): """ if not self.has_pseudo_elements(): self.get_value_buffer().persistent = True - self._pseudo_elements.append(element) + self._pseudo_elements.append(weakref.ref(element)) def remove_pseudo_element(self, element): """Removes pseudo element e.g. pseudo counters that this channel From 2b9359db297c34d9487c153a61ba776d70c6d05f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20L=C3=B3pez=20S=C3=A1nchez?= Date: Tue, 15 Jun 2021 10:16:47 +0200 Subject: [PATCH 09/27] set to None all the posible referrers --- src/sardana/tango/pool/Controller.py | 1 + src/sardana/tango/pool/MeasurementGroup.py | 1 + src/sardana/tango/pool/PseudoCounter.py | 3 +++ src/sardana/tango/pool/PseudoMotor.py | 1 + 4 files changed, 6 insertions(+) diff --git a/src/sardana/tango/pool/Controller.py b/src/sardana/tango/pool/Controller.py index d721999234..c8c3dafec6 100644 --- a/src/sardana/tango/pool/Controller.py +++ b/src/sardana/tango/pool/Controller.py @@ -72,6 +72,7 @@ def set_ctrl(self, ctrl): @DebugIt() def delete_device(self): PoolDevice.delete_device(self) + self.ctrl = None @DebugIt() def init_device(self): diff --git a/src/sardana/tango/pool/MeasurementGroup.py b/src/sardana/tango/pool/MeasurementGroup.py index 0225d826ef..30bf7f45e8 100644 --- a/src/sardana/tango/pool/MeasurementGroup.py +++ b/src/sardana/tango/pool/MeasurementGroup.py @@ -68,6 +68,7 @@ def delete_device(self): mg = self.measurement_group if mg is not None: mg.remove_listener(self.on_measurement_group_changed) + self.measurement_group = None @DebugIt() def init_device(self): diff --git a/src/sardana/tango/pool/PseudoCounter.py b/src/sardana/tango/pool/PseudoCounter.py index 9293d9cf6b..32269d1772 100644 --- a/src/sardana/tango/pool/PseudoCounter.py +++ b/src/sardana/tango/pool/PseudoCounter.py @@ -71,6 +71,9 @@ def delete_device(self): pseudo_counter = self.pseudo_counter if pseudo_counter is not None: pseudo_counter.remove_listener(self.on_pseudo_counter_changed) + self.element = None + self.pseudo_counter = None + self.ctrl = None @DebugIt() def init_device(self): diff --git a/src/sardana/tango/pool/PseudoMotor.py b/src/sardana/tango/pool/PseudoMotor.py index 562b36b162..5e6220aa18 100644 --- a/src/sardana/tango/pool/PseudoMotor.py +++ b/src/sardana/tango/pool/PseudoMotor.py @@ -73,6 +73,7 @@ def delete_device(self): pseudo_motor = self.pseudo_motor if pseudo_motor is not None: pseudo_motor.remove_listener(self.on_pseudo_motor_changed) + self.pseudo_motor = None @DebugIt() def init_device(self): From 9929c105ab474120190c20e227f0fd157422ff54 Mon Sep 17 00:00:00 2001 From: zreszela Date: Thu, 17 Jun 2021 14:15:59 +0200 Subject: [PATCH 10/27] style: fix PEP8 errors --- src/sardana/pool/pool.py | 9 ++++----- src/sardana/pool/poolbasechannel.py | 5 +++-- src/sardana/tango/pool/Pool.py | 1 + 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/sardana/pool/pool.py b/src/sardana/pool/pool.py index 73935b229f..df6fee5051 100644 --- a/src/sardana/pool/pool.py +++ b/src/sardana/pool/pool.py @@ -40,7 +40,6 @@ from sardana import InvalidId, ElementType, TYPE_ACQUIRABLE_ELEMENTS, \ TYPE_PSEUDO_ELEMENTS, TYPE_PHYSICAL_ELEMENTS, TYPE_MOVEABLE_ELEMENTS -from sardana.pool.poolbaseelement import PoolBaseElement from sardana.sardanamanager import SardanaElementManager, SardanaIDManager from sardana.sardanamodulemanager import ModuleManager from sardana.sardanaevent import EventType @@ -561,11 +560,12 @@ def delete_element(self, name): dependent_elements = elem.get_dependent_elements() if len(dependent_elements) > 0: - raise Exception("The element {} can't be deleted because {} depend" - " on it. \n\nIf the name of the dependent element starts with" + raise Exception( + "The element {} can't be deleted because {} depend on it." + "\n\nIf the name of the dependent element starts with" "'_mg_ms_*' it means that are motor groups, use any tango " "client (e.g. Jive) to delete them and restart the server." - .format(name, ", ".join(dependent_elements))) + .format(name, ", ".join(dependent_elements))) if elem_type == ElementType.Controller: if len(elem.get_elements()) > 0: @@ -593,7 +593,6 @@ def delete_element(self, name): if hasattr(elem, "get_controller"): elem.set_deleted(True) - def create_instrument(self, full_name, klass_name, id=None): is_root = full_name.count('/') == 1 diff --git a/src/sardana/pool/poolbasechannel.py b/src/sardana/pool/poolbasechannel.py index e038d7df0d..2c9b76096e 100644 --- a/src/sardana/pool/poolbasechannel.py +++ b/src/sardana/pool/poolbasechannel.py @@ -30,6 +30,8 @@ __docformat__ = 'restructuredtext' +import weakref + from sardana.sardanadefs import AttrQuality, ElementType from sardana.sardanaattribute import SardanaAttribute from sardana.sardanabuffer import SardanaBuffer @@ -40,7 +42,7 @@ ControllerConfiguration from sardana.sardanaevent import EventType from sardana.pool import AcqSynch, AcqMode -import weakref + class ValueBuffer(SardanaBuffer): @@ -107,7 +109,6 @@ class PoolBaseChannel(PoolElement): def __init__(self, **kwargs): PoolElement.__init__(self, **kwargs) - self._value = self.ValueAttributeClass(self, listeners=self.on_change) self._value_buffer = self.ValueBufferClass(self, listeners=self.on_change) diff --git a/src/sardana/tango/pool/Pool.py b/src/sardana/tango/pool/Pool.py index 56d32eebaf..54c0e29934 100644 --- a/src/sardana/tango/pool/Pool.py +++ b/src/sardana/tango/pool/Pool.py @@ -538,6 +538,7 @@ def _create_single_element(self, kwargs): self._check_element(name, full_name) util = PyTango.Util.instance() + def create_element_cb(device_name): try: db = util.get_database() From 011e1a4503b145cbf8dddca16fd7fb120d4dc247 Mon Sep 17 00:00:00 2001 From: zreszela Date: Thu, 17 Jun 2021 14:17:23 +0200 Subject: [PATCH 11/27] refactor: rename internal variable in get_dependent_elements() --- src/sardana/pool/poolelement.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/sardana/pool/poolelement.py b/src/sardana/pool/poolelement.py index 5bc342d9e4..73f8d9b880 100644 --- a/src/sardana/pool/poolelement.py +++ b/src/sardana/pool/poolelement.py @@ -89,16 +89,16 @@ def get_source(self): return "{0}/{1}".format(self.full_name, self.get_default_acquisition_channel()) def get_dependent_elements(self): - pool_base_elements = [] + dependent_elements = [] for listener in self.get_listeners(): try: - dependent_elem = listener().__self__ + elem = listener().__self__ except AttributeError: continue - if isinstance(dependent_elem, PoolBaseElement): - pool_base_elements.append(dependent_elem.name) + if isinstance(elem, PoolBaseElement): + dependent_elements.append(elem.name) - return pool_base_elements + return dependent_elements def has_dependent_elements(self): return len(self.get_dependent_elements()) > 0 From 45695172b025f421f78ee3c08a84bd5dd5aa20b1 Mon Sep 17 00:00:00 2001 From: zreszela Date: Thu, 17 Jun 2021 14:19:53 +0200 Subject: [PATCH 12/27] fix: pseudo elements weakref usage - Dereference weakrefs stored in PoolBaseChannel._pseudo_elements. - Fix PoolBaseChannel.remove_pseudo_element() - Fix docstring of PoolBaseChannel.get_pseudo_elements() --- src/sardana/pool/poolacquisition.py | 2 +- src/sardana/pool/poolbasechannel.py | 21 ++++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/sardana/pool/poolacquisition.py b/src/sardana/pool/poolacquisition.py index 4f7e287d25..02a1d53425 100644 --- a/src/sardana/pool/poolacquisition.py +++ b/src/sardana/pool/poolacquisition.py @@ -561,7 +561,7 @@ def run(self, *args, **kwargs): # clean also the pseudo counters, even the ones that do not # participate directly in the acquisition for pseudo_elem in elem.get_pseudo_elements(): - pseudo_elem.clear_value_buffer() + pseudo_elem().clear_value_buffer() if self._hw_acq_args is not None: self._hw_acq._wait() diff --git a/src/sardana/pool/poolbasechannel.py b/src/sardana/pool/poolbasechannel.py index 2c9b76096e..6f4cdfe6a1 100644 --- a/src/sardana/pool/poolbasechannel.py +++ b/src/sardana/pool/poolbasechannel.py @@ -56,7 +56,7 @@ def is_value_required(self, idx): :rtype: bool """ for element in self.obj.get_pseudo_elements(): - if element.get_value_buffer().next_idx <= idx: + if element().get_value_buffer().next_idx <= idx: return True return False @@ -138,8 +138,8 @@ def get_pseudo_elements(self): """Returns list of pseudo elements e.g. pseudo counters that this channel belongs to. - :return: pseudo elements - :rtype: seq<:class:`~sardana.pool.poolpseudocounter.PoolPseudoCounter`> + :return: weak references to pseudo elements + :rtype: seq<:class:`weakref.ref`> """ return self._pseudo_elements @@ -163,10 +163,17 @@ def remove_pseudo_element(self, element): :type element: :class:`~sardana.pool.poolpseudocounter.PoolPseudoCounter` """ - - self._pseudo_elements.remove(element) - if not self.has_pseudo_elements(): - self.get_value_buffer().persistent = False + for pseudo_element in self._pseudo_elements: + if pseudo_element() == element: + self._pseudo_elements.remove(element) + if not self.has_pseudo_elements(): + self.get_value_buffer().persistent = False + break + else: + raise ValueError( + "{} is not a pseudo element of {}".format( + element.name, self.name) + ) def get_value_attribute(self): """Returns the value attribute object for this experiment channel From f22a614cad2d854035697dbbb579047ebf9f5a0c Mon Sep 17 00:00:00 2001 From: zreszela Date: Thu, 17 Jun 2021 14:32:18 +0200 Subject: [PATCH 13/27] refactor: do not delete references to element and ctrl in PseudoCounter Performing more tests revealed that these references are not problematic in order to remove the pseudocounter core object correctly. Finally do not delete them. --- src/sardana/tango/pool/PseudoCounter.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/sardana/tango/pool/PseudoCounter.py b/src/sardana/tango/pool/PseudoCounter.py index 32269d1772..edbcf5ff12 100644 --- a/src/sardana/tango/pool/PseudoCounter.py +++ b/src/sardana/tango/pool/PseudoCounter.py @@ -51,6 +51,9 @@ def __init__(self, dclass, name): PoolExpChannelDevice.__init__(self, dclass, name) self._first_read_cache = False + def __del__(self): + print("PseudoCounter.__del__") + def init(self, name): PoolExpChannelDevice.init(self, name) @@ -71,9 +74,7 @@ def delete_device(self): pseudo_counter = self.pseudo_counter if pseudo_counter is not None: pseudo_counter.remove_listener(self.on_pseudo_counter_changed) - self.element = None self.pseudo_counter = None - self.ctrl = None @DebugIt() def init_device(self): From 7701e4c8ed2a0ef8139f083dbc11ec7c3a3b97ae Mon Sep 17 00:00:00 2001 From: zreszela Date: Thu, 17 Jun 2021 14:43:15 +0200 Subject: [PATCH 14/27] fix: delete reference to core object in MotorGroup Seems like PyTango/Tango keeps alive Device objects after deleting them (to be verified). These objects keep reference to the Sardana core objects, making it impossible to delete them and affecting the dependency relations. Delete reference to the motor_group in order to avoid this problem. --- src/sardana/tango/pool/MotorGroup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sardana/tango/pool/MotorGroup.py b/src/sardana/tango/pool/MotorGroup.py index fdf63a6c66..5478a4e0b0 100644 --- a/src/sardana/tango/pool/MotorGroup.py +++ b/src/sardana/tango/pool/MotorGroup.py @@ -70,6 +70,7 @@ def delete_device(self): motor_group = self.motor_group if motor_group is not None: motor_group.remove_listener(self.on_motor_group_changed) + self.motor_group = None @DebugIt() def init_device(self): From 0ba422c5a2e8c243ba598023f37491f71983890c Mon Sep 17 00:00:00 2001 From: zreszela Date: Thu, 17 Jun 2021 14:48:34 +0200 Subject: [PATCH 15/27] refactor: give better example in exception message --- src/sardana/pool/pool.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/sardana/pool/pool.py b/src/sardana/pool/pool.py index df6fee5051..505104ead5 100644 --- a/src/sardana/pool/pool.py +++ b/src/sardana/pool/pool.py @@ -562,9 +562,10 @@ def delete_element(self, name): if len(dependent_elements) > 0: raise Exception( "The element {} can't be deleted because {} depend on it." - "\n\nIf the name of the dependent element starts with" - "'_mg_ms_*' it means that are motor groups, use any tango " - "client (e.g. Jive) to delete them and restart the server." + "\n\nIf the name of the dependent element starts with " + "'_mg_ms_*' it means that are motor groups, execute " + "DeleteElement() command on the Pool e.g. " + "Pool_demo1_1.DeleteElement('_mg_ms_20671_1') in Spock." .format(name, ", ".join(dependent_elements))) if elem_type == ElementType.Controller: From 8efa6ac9cbf7fdaa39940a8f22c1347f2a877beb Mon Sep 17 00:00:00 2001 From: zreszela Date: Thu, 17 Jun 2021 21:58:05 +0200 Subject: [PATCH 16/27] refactor: remove __del__ introduced for debugging purposes --- src/sardana/tango/pool/PseudoCounter.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/sardana/tango/pool/PseudoCounter.py b/src/sardana/tango/pool/PseudoCounter.py index edbcf5ff12..f2220c30cc 100644 --- a/src/sardana/tango/pool/PseudoCounter.py +++ b/src/sardana/tango/pool/PseudoCounter.py @@ -51,9 +51,6 @@ def __init__(self, dclass, name): PoolExpChannelDevice.__init__(self, dclass, name) self._first_read_cache = False - def __del__(self): - print("PseudoCounter.__del__") - def init(self, name): PoolExpChannelDevice.init(self, name) From 45f38e3a5e0e4d784a1b576fb110733a245f99a4 Mon Sep 17 00:00:00 2001 From: zreszela Date: Thu, 17 Jun 2021 21:59:44 +0200 Subject: [PATCH 17/27] fix: remove weakref from the list in remove_psuedo_element() --- src/sardana/pool/poolbasechannel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sardana/pool/poolbasechannel.py b/src/sardana/pool/poolbasechannel.py index 6f4cdfe6a1..2bb5dab1c8 100644 --- a/src/sardana/pool/poolbasechannel.py +++ b/src/sardana/pool/poolbasechannel.py @@ -165,7 +165,7 @@ def remove_pseudo_element(self, element): """ for pseudo_element in self._pseudo_elements: if pseudo_element() == element: - self._pseudo_elements.remove(element) + self._pseudo_elements.remove(pseudo_element) if not self.has_pseudo_elements(): self.get_value_buffer().persistent = False break From c9599df2936a3456caa707a6db1442ee2f182bcb Mon Sep 17 00:00:00 2001 From: zreszela Date: Tue, 29 Jun 2021 15:41:55 +0200 Subject: [PATCH 18/27] fix: do not pas meas grp as "head" kwarg to acquisition.prepare() "head" kwarg is not used by the acquisition actions and it ends up as strong reference in the acquisition sub-actions. This leads to a cycle reference of the measurement group and the acquisition actions causing problems when discovering dependent elements when deleting an element. --- src/sardana/pool/poolmeasurementgroup.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/sardana/pool/poolmeasurementgroup.py b/src/sardana/pool/poolmeasurementgroup.py index b3b131f662..b94c4444c4 100644 --- a/src/sardana/pool/poolmeasurementgroup.py +++ b/src/sardana/pool/poolmeasurementgroup.py @@ -1345,16 +1345,13 @@ def prepare(self): value = self._get_value() self._pending_starts = self.nb_starts - kwargs = {'head': self} - self.acquisition.prepare(self.configuration, self.acquisition_mode, value, self._synch_description, self._moveable_obj, self.sw_synch_initial_domain, - self.nb_starts, - **kwargs) + self.nb_starts) def start_acquisition(self, value=None): """Start measurement. From e9a89000dcfec0121b943db21945d715d8451d6b Mon Sep 17 00:00:00 2001 From: zreszela Date: Tue, 29 Jun 2021 15:45:47 +0200 Subject: [PATCH 19/27] fix: set PoolController.operator as weakref Measurement group becomes an operator of the controller during an acquisition and remains as operator after. This creates an additional strong reference to the measurement group making it impossible to really delete causing troubles when discovering dependent elements on an element deletion. Use weakref.ref instead. --- src/sardana/pool/poolcontroller.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sardana/pool/poolcontroller.py b/src/sardana/pool/poolcontroller.py index ad3bb10360..1e204ce59c 100644 --- a/src/sardana/pool/poolcontroller.py +++ b/src/sardana/pool/poolcontroller.py @@ -442,10 +442,10 @@ def set_operator(self, operator): :param operator: the new operator object :type operator: object""" - self._operator = operator + self._operator = weakref.ref(operator) def get_operator(self): - return self._operator + return self._operator() operator = property(fget=get_operator, fset=set_operator, doc="current controller operator") From ff95a1ef6e3282056d775b4727b58f00ec0c2d57 Mon Sep 17 00:00:00 2001 From: zreszela Date: Tue, 29 Jun 2021 16:16:51 +0200 Subject: [PATCH 20/27] test: fix order of deleting elements in tearDown() When deleting element, no dependent element can exist. First delete pseudo element and pseudo controller, and then the physical ones. --- src/sardana/tango/pool/test/base_sartest.py | 80 ++++++++++++--------- 1 file changed, 46 insertions(+), 34 deletions(-) diff --git a/src/sardana/tango/pool/test/base_sartest.py b/src/sardana/tango/pool/test/base_sartest.py index 28ff386303..89811c525f 100644 --- a/src/sardana/tango/pool/test/base_sartest.py +++ b/src/sardana/tango/pool/test/base_sartest.py @@ -102,6 +102,8 @@ def setUp(self, pool_properties=None): self.ctrl_list = [] self.elem_list = [] + self.pseudo_ctrl_list = [] + self.pseudo_elem_list = [] try: # physical controllers and elements for sar_type, lib, cls, prefix, postfix, nelem in self.cls_list: @@ -145,58 +147,68 @@ def setUp(self, pool_properties=None): print(e) msg = 'Impossible to create ctrl: "%s"' % (ctrl_name) raise Exception('Aborting SartestTestCase: %s' % (msg)) - self.ctrl_list.append(ctrl_name) + self.pseudo_ctrl_list.append(ctrl_name) for role in roles: elem = role.split("=")[1] if elem not in self.elem_list: - self.elem_list.append(elem) + self.pseudo_elem_list.append(elem) except Exception as e: # force tearDown in order to eliminate the Pool BasePoolTestCase.tearDown(self) print(e) + def _delete_elem(self, elem_name): + # Cleanup eventual taurus devices. This is especially important + # if the sardana-taurus extensions are in use since this + # devices are created and destroyed within the testsuite. + # Persisting taurus device may react on API_EventTimeouts, enabled + # polling, etc. + if elem_name in self.f.tango_alias_devs: + _cleanup_device(elem_name) + try: + if os.name != "nt": + self.pool.DeleteElement(elem_name) + print(elem_name) + except Exception as e: + print(e) + self.dirty_elems.append(elem_name) + + def _delete_ctrl(self, ctrl_name): + # Cleanup eventual taurus devices. This is especially important + # if the sardana-taurus extensions are in use since this + # devices are created and destroyed within the testsuite. + # Persisting taurus device may react on API_EventTimeouts, enabled + # polling, etc. + if ctrl_name in self.f.tango_alias_devs: + _cleanup_device(ctrl_name) + try: + if os.name != "nt": + self.pool.DeleteElement(ctrl_name) + print(ctrl_name) + except: + self.dirty_ctrls.append(ctrl_name) + def tearDown(self): """Remove the elements and the controllers """ - dirty_elems = [] - dirty_ctrls = [] - f = taurus.Factory() + self.dirty_elems = [] + self.dirty_ctrls = [] + self.f = taurus.Factory() + for elem_name in self.pseudo_elem_list: + self._delete_elem(elem_name) + for ctrl_name in self.pseudo_ctrl_list: + self._delete_ctrl(ctrl_name) for elem_name in self.elem_list: - # Cleanup eventual taurus devices. This is especially important - # if the sardana-taurus extensions are in use since this - # devices are created and destroyed within the testsuite. - # Persisting taurus device may react on API_EventTimeouts, enabled - # polling, etc. - if elem_name in f.tango_alias_devs: - _cleanup_device(elem_name) - try: - if os.name != "nt": - self.pool.DeleteElement(elem_name) - except Exception as e: - print(e) - dirty_elems.append(elem_name) - + self._delete_elem(elem_name) for ctrl_name in self.ctrl_list: - # Cleanup eventual taurus devices. This is especially important - # if the sardana-taurus extensions are in use since this - # devices are created and destroyed within the testsuite. - # Persisting taurus device may react on API_EventTimeouts, enabled - # polling, etc. - if ctrl_name in f.tango_alias_devs: - _cleanup_device(ctrl_name) - try: - if os.name != "nt": - self.pool.DeleteElement(ctrl_name) - except: - dirty_ctrls.append(ctrl_name) - + self._delete_ctrl(ctrl_name) _cleanup_device(self.pool_name) BasePoolTestCase.tearDown(self) - if dirty_elems or dirty_ctrls: + if self.dirty_elems or self.dirty_ctrls: msg = "Cleanup failed. Database may be left dirty." + \ - "\n\tCtrls : %s\n\tElems : %s" % (dirty_ctrls, dirty_elems) + "\n\tCtrls : %s\n\tElems : %s" % (self.dirty_ctrls, self.dirty_elems) raise Exception(msg) From cf2851d34a92d36e38071c1b05cff9602c4111c0 Mon Sep 17 00:00:00 2001 From: zreszela Date: Tue, 29 Jun 2021 16:26:16 +0200 Subject: [PATCH 21/27] test: delete motor groups created in some tests When deleting an element no dependent elements can exist. Delete the motor groups before the physical motors are deleted. --- src/sardana/macroserver/macros/test/test_scanct.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/sardana/macroserver/macros/test/test_scanct.py b/src/sardana/macroserver/macros/test/test_scanct.py index 702013cadb..999d70ec99 100644 --- a/src/sardana/macroserver/macros/test/test_scanct.py +++ b/src/sardana/macroserver/macros/test/test_scanct.py @@ -24,6 +24,7 @@ ############################################################################## """Tests for continuous scans (ct-like)""" +import json import time import PyTango import unittest @@ -333,6 +334,8 @@ def macro_stops(self, meas_config, macro_params, wait_timeout=None, ScanctTest.check_stopped(self) def tearDown(self): + for mg in self.pool.MotorGroupList: + self.pool.DeleteElement(json.loads(mg)['name']) ScanctTest.tearDown(self) unittest.TestCase.tearDown(self) @@ -388,5 +391,7 @@ def macro_stops(self, meas_config, macro_params, wait_timeout=None, ScanctTest.check_stopped(self) def tearDown(self): + for mg in self.pool.MotorGroupList: + self.pool.DeleteElement(json.loads(mg)['name']) ScanctTest.tearDown(self) unittest.TestCase.tearDown(self) From 97a96ee6126391ae9b9ba33e7da3fae1a6f13e27 Mon Sep 17 00:00:00 2001 From: zreszela Date: Tue, 29 Jun 2021 17:08:49 +0200 Subject: [PATCH 22/27] fix: gc.collect() pseudo counters with cycle references Pseudo counters must have cycle references with physical elements. This causes problems when deleting elements after prior deletion of pseudo counters. This was discovered with the following tests: - sardana/macroserver/macros/test/test_scanct.py::AscanctTest::test_ascanct_macro_runs_4 - sardana/macroserver/macros/test/test_scanct.py::AscanctTest::test_ascanct_macro_stops - sardana/macroserver/macros/test/test_scanct.py::A2scanctTest::test_a2scanct_macro_runs - sardana/macroserver/macros/test/test_scanct.py::MeshctTest::test_meshct_macro_runs - sardana/tango/pool/test/test_measurementgroup.py::TangoAcquisitionTestCase::test_meas_cont_acquisition_2 - sardana/tango/pool/test/test_measurementgroup.py::TangoAcquisitionTestCase::test_stop_meas_cont_acquisition_3 - sardana/taurus/core/tango/sardana/test/test_measgrpstress.py::TestStressMeasurementGroup::test_stress_count_2 - sardana/taurus/core/tango/sardana/test/test_measgrpstress.py::TestStressMeasurementGroup::test_stress_count_3 - sardana/taurus/core/tango/sardana/test/test_measgrpstress.py::TestStressMeasurementGroup::test_stress_count_4 - sardana/taurus/core/tango/sardana/test/test_measgrpstress.py::TestStressMeasurementGroup::test_stress_count_6 Force gc.collect() to avoid this problem. --- src/sardana/pool/pool.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/sardana/pool/pool.py b/src/sardana/pool/pool.py index 505104ead5..500e78e6bf 100644 --- a/src/sardana/pool/pool.py +++ b/src/sardana/pool/pool.py @@ -32,6 +32,7 @@ __docformat__ = 'restructuredtext' +import gc import os.path import logging.handlers @@ -557,7 +558,9 @@ def delete_element(self, name): raise Exception("There is no element with name '%s'" % name) elem_type = elem.get_type() - + dependent_elements = elem.get_dependent_elements() + if len(dependent_elements) > 0: + gc.collect() dependent_elements = elem.get_dependent_elements() if len(dependent_elements) > 0: raise Exception( From 146edfaf9c690e51e3f193af8d22b1571299e985 Mon Sep 17 00:00:00 2001 From: zreszela Date: Tue, 20 Jul 2021 08:27:49 +0200 Subject: [PATCH 23/27] refactor: change return type of get_dependent_elements() get_dependent_elements() returns only element name. Change the return value to return the dependent elements objects in order to extend use cases of this method e.g. allow to check the type of the returned dependent elements. --- src/sardana/pool/pool.py | 3 ++- src/sardana/pool/poolelement.py | 10 +++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/sardana/pool/pool.py b/src/sardana/pool/pool.py index 500e78e6bf..eb8f7c4507 100644 --- a/src/sardana/pool/pool.py +++ b/src/sardana/pool/pool.py @@ -563,13 +563,14 @@ def delete_element(self, name): gc.collect() dependent_elements = elem.get_dependent_elements() if len(dependent_elements) > 0: + names = [elem.name for elem in dependent_elements] raise Exception( "The element {} can't be deleted because {} depend on it." "\n\nIf the name of the dependent element starts with " "'_mg_ms_*' it means that are motor groups, execute " "DeleteElement() command on the Pool e.g. " "Pool_demo1_1.DeleteElement('_mg_ms_20671_1') in Spock." - .format(name, ", ".join(dependent_elements))) + .format(name, ", ".join(names))) if elem_type == ElementType.Controller: if len(elem.get_elements()) > 0: diff --git a/src/sardana/pool/poolelement.py b/src/sardana/pool/poolelement.py index 73f8d9b880..d4706bcb20 100644 --- a/src/sardana/pool/poolelement.py +++ b/src/sardana/pool/poolelement.py @@ -89,6 +89,14 @@ def get_source(self): return "{0}/{1}".format(self.full_name, self.get_default_acquisition_channel()) def get_dependent_elements(self): + """Get elements which depend on this element. + + Get elements e.g. pseudo elements or groups, which depend on this + element. + + :return: dependent elements + :rtype: seq + """ dependent_elements = [] for listener in self.get_listeners(): try: @@ -96,7 +104,7 @@ def get_dependent_elements(self): except AttributeError: continue if isinstance(elem, PoolBaseElement): - dependent_elements.append(elem.name) + dependent_elements.append(elem) return dependent_elements From b055e770c3c1648411d4fbffb2f7c2f4e4acab93 Mon Sep 17 00:00:00 2001 From: zreszela Date: Tue, 20 Jul 2021 08:49:27 +0200 Subject: [PATCH 24/27] call gc.collect() only in case of PseudoCounters gc.collect() workaround is tried if any kind of dependent element resides in the pool. Call it only in the case of pseudo counters. --- src/sardana/pool/pool.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/sardana/pool/pool.py b/src/sardana/pool/pool.py index eb8f7c4507..2d9519ccfe 100644 --- a/src/sardana/pool/pool.py +++ b/src/sardana/pool/pool.py @@ -557,10 +557,15 @@ def delete_element(self, name): except: raise Exception("There is no element with name '%s'" % name) - elem_type = elem.get_type() + # TODO: most probably due to some cycle-reference the psuedo counter + # objects are not being deleted when undefining them, as a workaround + # try to delete them with gc.collect() dependent_elements = elem.get_dependent_elements() - if len(dependent_elements) > 0: - gc.collect() + for dependent_element in dependent_elements: + if dependent_element.get_type() == ElementType.PseudoCounter: + gc.collect() + break + dependent_elements = elem.get_dependent_elements() if len(dependent_elements) > 0: names = [elem.name for elem in dependent_elements] @@ -571,7 +576,8 @@ def delete_element(self, name): "DeleteElement() command on the Pool e.g. " "Pool_demo1_1.DeleteElement('_mg_ms_20671_1') in Spock." .format(name, ", ".join(names))) - + + elem_type = elem.get_type() if elem_type == ElementType.Controller: if len(elem.get_elements()) > 0: raise Exception("Cannot delete controller with elements. " From 6ac7aa6729b5c886781d2d8096e56279fc3a2338 Mon Sep 17 00:00:00 2001 From: zreszela Date: Wed, 21 Jul 2021 11:45:52 +0200 Subject: [PATCH 25/27] fix: meas group with undefined disabled channel Disabled channels in the measurement group are considered as not members of the group. They neither appear in "element list" attribute nor the measurement group listens to their events. Hence it is possible to undefine an experimental channel which is disabled. This may lead to a situation when we try to enable again an undefined channel. Consider this case by not accepting a configuration which contains an undefined element. Also allow to acquire with a measurement group which contains and undefined and disabled channel. --- src/sardana/pool/poolmeasurementgroup.py | 13 +++++++++++-- src/sardana/taurus/core/tango/sardana/pool.py | 2 ++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/sardana/pool/poolmeasurementgroup.py b/src/sardana/pool/poolmeasurementgroup.py index a773aedc2d..c335b16f76 100644 --- a/src/sardana/pool/poolmeasurementgroup.py +++ b/src/sardana/pool/poolmeasurementgroup.py @@ -800,7 +800,12 @@ def set_configuration_from_user(self, cfg): params['pool'] = pool channel = PoolExternalObject(**params) else: - channel = pool.get_element_by_full_name(ch_name) + try: + channel = pool.get_element_by_full_name(ch_name) + except KeyError: + raise ValueError( + '{} is not defined'.format(ch_data['name'])) + ch_data = self._fill_channel_data(channel, ch_data) user_config_channel[ch_name] = ch_data ch_item = ChannelConfiguration(channel, ch_data) @@ -958,7 +963,11 @@ def set_configuration_from_user(self, cfg): for conf_synch in conf_synch_ctrl.get_channels(enabled=True): user_elem_ids_list.append(conf_synch.id) self._parent.set_user_element_ids(user_elem_ids_list) - + # force assignment of user elements to the measurement group + # in order to update the list of dependent elements (listeners) + # of the element e.g. to prevent undefinition of an element added + # to the measurement group. + self._parent.get_user_elements() self.changed = True def _fill_channel_data(self, channel, channel_data): diff --git a/src/sardana/taurus/core/tango/sardana/pool.py b/src/sardana/taurus/core/tango/sardana/pool.py index 45a5e7821a..6483fc6263 100644 --- a/src/sardana/taurus/core/tango/sardana/pool.py +++ b/src/sardana/taurus/core/tango/sardana/pool.py @@ -1525,6 +1525,8 @@ def _build(self): tg_attr_validator = TangoAttributeNameValidator() for channel_name, channel_data in list(self.channels.items()): cache[channel_name] = None + if not channel_data.get("enabled", True): + continue data_source = channel_data['source'] params = tg_attr_validator.getUriGroups(data_source) if params is None: From 7f84584334e1f29ac5e5f8478e2a4d8a869e5b22 Mon Sep 17 00:00:00 2001 From: zreszela Date: Thu, 22 Jul 2021 17:17:16 +0200 Subject: [PATCH 26/27] add has_dependent_elements() to PoolBaseElement PoolElement implements get_dependent_elements() and has_dependent_elements() while PoolBaseElement only implements get_dependent_elements(). Implement has_dependent_elements() also for the PoolBaseElement to be able to use both methods on any type of Sardana element. --- src/sardana/pool/poolbaseelement.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/sardana/pool/poolbaseelement.py b/src/sardana/pool/poolbaseelement.py index 4b236d64e3..9b5346ed36 100644 --- a/src/sardana/pool/poolbaseelement.py +++ b/src/sardana/pool/poolbaseelement.py @@ -103,6 +103,9 @@ def serialize(self, *args, **kwargs): def get_dependent_elements(self): return [] + def has_dependent_elements(self): + return False + # -------------------------------------------------------------------------- # simulation mode # -------------------------------------------------------------------------- From 563e299c68ff15f3f29d50858b911a775f74f6fd Mon Sep 17 00:00:00 2001 From: zreszela Date: Thu, 22 Jul 2021 17:23:04 +0200 Subject: [PATCH 27/27] fix: gc.collect() of dependent elements cycle-reference may exist between an element and a traceback stored in SardanaValue or SardanaAttribute as a consequence of getting sys.exc_info(). Keeping a list of dependent elements in the scope of delete_element() makes it impossible to gc.collect() to collect the cycled objects. Use has_dependent_elements() in order to determine if there are dependent elements and then call the gc.collect(). Also, do not call it conditionally only for the pseudo counters but for any element type - the cycle may be created for between any alement and a traceback. --- src/sardana/pool/pool.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/sardana/pool/pool.py b/src/sardana/pool/pool.py index fc9d7be3f6..e3759adf82 100644 --- a/src/sardana/pool/pool.py +++ b/src/sardana/pool/pool.py @@ -560,15 +560,12 @@ def delete_element(self, name): elem = self.get_element(full_name=name) except: raise Exception("There is no element with name '%s'" % name) - - # TODO: most probably due to some cycle-reference the psuedo counter - # objects are not being deleted when undefining them, as a workaround - # try to delete them with gc.collect() - dependent_elements = elem.get_dependent_elements() - for dependent_element in dependent_elements: - if dependent_element.get_type() == ElementType.PseudoCounter: - gc.collect() - break + + # cycle-reference may exist between the element and a traceback + # stored in SardanaValue or SardanaAttribute as a consequence of + # getting sys.exc_info() - try to delete them with gc.collect() + if elem.has_dependent_elements(): + gc.collect() dependent_elements = elem.get_dependent_elements() if len(dependent_elements) > 0: