diff --git a/bapsf_motion/actors/axis_.py b/bapsf_motion/actors/axis_.py index a99185c4..9b2ef50e 100644 --- a/bapsf_motion/actors/axis_.py +++ b/bapsf_motion/actors/axis_.py @@ -132,9 +132,16 @@ def run(self, auto_run: bool = True, force_run: bool = True): if isinstance(self.motor, Motor): self.motor.run(auto_run=auto_run, force_run=force_run) - def terminate(self, delay_loop_stop=False): - self.motor.terminate(delay_loop_stop=True) - super().terminate(delay_loop_stop=delay_loop_stop) + def terminate( + self, + delay_loop_stop: bool = False, + disconnect_signals: bool = False, + ): + self.motor.terminate(delay_loop_stop=True, disconnect_signals=disconnect_signals) + super().terminate( + delay_loop_stop=delay_loop_stop, + disconnect_signals=disconnect_signals, + ) def _spawn_motor(self, ip, motor_settings: Optional[dict] = None): self.logger.debug("Spawning Motor") diff --git a/bapsf_motion/actors/base.py b/bapsf_motion/actors/base.py index 3494630e..89c88d68 100644 --- a/bapsf_motion/actors/base.py +++ b/bapsf_motion/actors/base.py @@ -314,7 +314,11 @@ def run(self, auto_run: bool = True, force_run: bool = True): self._thread = threading.Thread(target=self._loop.run_forever) self._thread.start() - def terminate(self, delay_loop_stop=False): + def terminate( + self, + delay_loop_stop: bool = False, + disconnect_signals: bool = False, + ): r""" Stop the actor's `event loop`_\ . All actor tasks will be cancelled, the connection to the motor will be shutdown, and @@ -322,11 +326,16 @@ def terminate(self, delay_loop_stop=False): Parameters ---------- - delay_loop_stop: bool - If `True`, then do NOT stop the `event loop`_\ . In this - case it is assumed the calling functionality is managing - additional tasks in the event loop, and it is up to that - functionality to stop the loop. (DEFAULT: `False`) + delay_loop_stop: `bool` + (DEFAULT: `False`) If `True`, then do NOT stop the + `event loop`_\ . In this case it is assumed the calling + functionality is managing additional tasks in the event + loop, and it is up to that functionality to stop the loop. + + disconnect_signals: `bool` + (DEFAULT: `False`) If `True`, then disconnect any signal + when terminating the actor. If `False`, then signals + are NOT disconnected, but are block. """ for task in list(self.tasks): self.loop.call_soon_threadsafe(task.cancel) diff --git a/bapsf_motion/actors/drive_.py b/bapsf_motion/actors/drive_.py index 7291b8d0..52a66896 100644 --- a/bapsf_motion/actors/drive_.py +++ b/bapsf_motion/actors/drive_.py @@ -286,11 +286,18 @@ def position(self) -> u.Quantity: return pos * self.axes[0].units - def terminate(self, delay_loop_stop=False): + def terminate( + self, + delay_loop_stop: bool = False, + disconnect_signals: bool = False, + ): for ax in self.axes: - ax.terminate(delay_loop_stop=True) + ax.terminate(delay_loop_stop=True, disconnect_signals=disconnect_signals) - super().terminate(delay_loop_stop=delay_loop_stop) + super().terminate( + delay_loop_stop=delay_loop_stop, + disconnect_signals=disconnect_signals, + ) def send_command(self, command: str, *args, axis: int | None = None): """ diff --git a/bapsf_motion/actors/manager_.py b/bapsf_motion/actors/manager_.py index 72c79aa2..7ab6c265 100644 --- a/bapsf_motion/actors/manager_.py +++ b/bapsf_motion/actors/manager_.py @@ -346,10 +346,17 @@ def config(self) -> RunManagerConfig: config.__doc__ = EventActor.config.__doc__ - def terminate(self, delay_loop_stop=False): + def terminate( + self, + delay_loop_stop: bool = False, + disconnect_signals: bool = False, + ): for mg in self.mgs.values(): - mg.terminate(delay_loop_stop=True) - super().terminate(delay_loop_stop=delay_loop_stop) + mg.terminate(delay_loop_stop=True, disconnect_signals=disconnect_signals) + super().terminate( + delay_loop_stop=delay_loop_stop, + disconnect_signals=disconnect_signals, + ) def _spawn_motion_group(self, config: Dict[str, Any]) -> MotionGroup: return MotionGroup( diff --git a/bapsf_motion/actors/motion_group_.py b/bapsf_motion/actors/motion_group_.py index df58d87c..e6c1f442 100644 --- a/bapsf_motion/actors/motion_group_.py +++ b/bapsf_motion/actors/motion_group_.py @@ -864,10 +864,20 @@ def _spawn_transform( ) return self._transform - def terminate(self, delay_loop_stop=False): + def terminate( + self, + delay_loop_stop: bool = False, + disconnect_signals: bool = True, + ): if self.drive is not None: - self.drive.terminate(delay_loop_stop=True) - super().terminate(delay_loop_stop=delay_loop_stop) + self.drive.terminate( + delay_loop_stop=True, + disconnect_signals=disconnect_signals, + ) + super().terminate( + delay_loop_stop=delay_loop_stop, + disconnect_signals=disconnect_signals, + ) @property def config(self) -> "MotionGroupConfig": diff --git a/bapsf_motion/actors/motor_.py b/bapsf_motion/actors/motor_.py index a1ba5537..ae06bf89 100644 --- a/bapsf_motion/actors/motor_.py +++ b/bapsf_motion/actors/motor_.py @@ -199,6 +199,14 @@ def __init__(self): self._movement_started = SimpleSignal() self._status_changed = SimpleSignal() + self._signal_names = ( + "connection_established", + "connection_lost", + "movement_finished", + "movement_started", + "status_changed", + ) + @property def connection_established(self) -> SimpleSignal: """ @@ -238,6 +246,28 @@ def status_changed(self) -> SimpleSignal: `~Motor.status` is changes.""" return self._status_changed + def _get_signal(self, name: str) -> SimpleSignal: + return getattr(self, name) + + def set_blocking(self, block: bool): + """Block or unblock the all signals.""" + + for name in self._signal_names: + signal = self._get_signal(name) + signal.set_blocking(block) + + def disconnect(self, func: Callable): + """Disconnect the callback/handler ``func`` from all signals.""" + for name in self._signal_names: + signal = getattr(self, name) + signal.disconnect(func) + + def disconnect_all(self): + """Disconnect all callbacks/handlers from all signals.""" + for name in self._signal_names: + signal = getattr(self, name) + signal.disconnect_all() + class Motor(EventActor): """ @@ -767,6 +797,7 @@ def run(self, auto_run: bool = True, force_run: bool = True): self._initialize_tasks() super().run(auto_run=auto_run) + self.signals.set_blocking(False) @property def connected(self): @@ -1888,19 +1919,26 @@ async def _heartbeat(self): old_HR = heartrate await asyncio.sleep(heartrate) - def terminate(self, delay_loop_stop=False): + def terminate( + self, + delay_loop_stop: bool = False, + disconnect_signals: bool = False, + ): self.logger.info("Terminating motor") - # disconnect all signals before terminating - self.signals.status_changed.disconnect_all() - self.signals.movement_started.disconnect_all() - self.signals.movement_finished.disconnect_all() + # handle signals + self.signals.set_blocking(True) + if disconnect_signals: + self.signals.disconnect_all() if not self.terminated and self.connected: self.stop() self.disable() - super().terminate(delay_loop_stop=delay_loop_stop) + super().terminate( + delay_loop_stop=delay_loop_stop, + disconnect_signals=disconnect_signals, + ) self._heartbeat_task = None try: diff --git a/bapsf_motion/gui/configure/configure_.py b/bapsf_motion/gui/configure/configure_.py index 3b539e7e..2d074cfc 100644 --- a/bapsf_motion/gui/configure/configure_.py +++ b/bapsf_motion/gui/configure/configure_.py @@ -529,7 +529,7 @@ def rm(self, new_rm): if not isinstance(new_rm, RunManager): return elif isinstance(self._rm, RunManager): - self._rm.terminate() + self._rm.terminate(disconnect_signals=True) self._rm = new_rm @@ -546,7 +546,7 @@ def _config_changed_handler(self): def replace_rm(self, config): if isinstance(self.rm, RunManager): - self.rm.terminate() + self.rm.terminate(disconnect_signals=True) self.logger.info(f"Replacing the run manager with new config: {config}.") _rm = RunManager(config=config, auto_run=True, build_mode=True) @@ -600,7 +600,7 @@ def update_display_mg_list(self): self.logger.info(f"Adding to MG List - {label}") _icon = ( qta.icon(icon_name_dict["window-close"], color="red") - if mg.terminated + if mg.terminated or not mg.connected else qta.icon(icon_name_dict["check-circle"], color="green") ) # type: QIcon _item = QListWidgetItem( @@ -631,7 +631,7 @@ def _motion_group_modify_existing(self): mg = self.rm.mgs[key] if not mg.terminated: - mg.terminate(delay_loop_stop=True) + mg.terminate(delay_loop_stop=True, disconnect_signals=True) self._mg_being_modified = mg self._spawn_mg_widget(mg) @@ -753,7 +753,7 @@ def _spawn_mg_widget(self, mg: MotionGroup = None): # terminate RunManager so we can avoid communication issue during # MotionGroup configuration if isinstance(self.rm, RunManager) and not self.rm.terminated: - self.rm.terminate() + self.rm.terminate(disconnect_signals=True) self._mg_widget = MGWidget( mg_config=config, @@ -921,7 +921,7 @@ def closeEvent(self, event: "QCloseEvent") -> None: self.configChanged.disconnect() if isinstance(self.rm, RunManager) and not self.rm.terminated: - self.rm.terminate() + self.rm.terminate(disconnect_signals=True) self.rm = None if isinstance(self._mg_widget, MGWidget): diff --git a/bapsf_motion/gui/configure/controllers.py b/bapsf_motion/gui/configure/controllers.py index 213a5abf..6787fdd7 100644 --- a/bapsf_motion/gui/configure/controllers.py +++ b/bapsf_motion/gui/configure/controllers.py @@ -999,7 +999,6 @@ def unlink_motion_group(self): acw.setVisible(visible) - # self.mg.terminate(delay_loop_stop=True) self._mg = None self._mspace_drive_polarity = None self.setEnabled(False) @@ -1034,6 +1033,7 @@ def enable_motion_buttons(self): def _drive_connection_lost(self): self.mg.drive.stop() self.setEnabled(False) + self.driveStatusChanged.emit() @Slot() def _drive_connection_established(self): @@ -1042,6 +1042,7 @@ def _drive_connection_established(self): if self.mg.drive.connected: self.setEnabled(True) + self.driveStatusChanged.emit() @Slot(int) def _drive_movement_started(self, axis_index): diff --git a/bapsf_motion/gui/configure/drive_overlay.py b/bapsf_motion/gui/configure/drive_overlay.py index a3145ed0..72d18dc8 100644 --- a/bapsf_motion/gui/configure/drive_overlay.py +++ b/bapsf_motion/gui/configure/drive_overlay.py @@ -539,7 +539,7 @@ def axis_config(self, config): if isinstance(self.axis, Axis): # configuration has changed and is different from current axis actor self.blockSignals(True) - self.axis.terminate(delay_loop_stop=True) + self.axis.terminate(delay_loop_stop=True, disconnect_signals=True) self.axis = None self.blockSignals(False) @@ -634,7 +634,7 @@ def _change_ip_address(self): config = self.axis_config config["ip"] = new_ip if isinstance(self.axis, Axis): - self.axis.terminate(delay_loop_stop=True) + self.axis.terminate(delay_loop_stop=True, disconnect_signals=True) self.axis = None self.axis_config = config @@ -719,7 +719,7 @@ def _spawn_axis(self) -> Union[Axis, None]: self.logger.info("Spawning Axis.") if isinstance(self.axis, Axis): self.blockSignals(True) - self.axis.terminate(delay_loop_stop=True) + self.axis.terminate(delay_loop_stop=True, disconnect_signals=True) self.axis = None self.blockSignals(False) @@ -833,7 +833,7 @@ def _set_motor_current(self, current: float | int): return if isinstance(self.axis, Axis): - self.axis.terminate(delay_loop_stop=True) + self.axis.terminate(delay_loop_stop=True, disconnect_signals=True) self.axis = None axis_config = self.axis_config.copy() @@ -851,7 +851,7 @@ def _set_motor_speed(self, speed: float | int): return if isinstance(self.axis, Axis): - self.axis.terminate(delay_loop_stop=True) + self.axis.terminate(delay_loop_stop=True, disconnect_signals=True) self.axis = None axis_config = self.axis_config.copy() @@ -869,7 +869,7 @@ def _set_limit_mode(self, limit_mode: int): return if isinstance(self.axis, Axis): - self.axis.terminate(delay_loop_stop=True) + self.axis.terminate(delay_loop_stop=True, disconnect_signals=True) self.axis = None axis_config = self.axis_config.copy() @@ -888,7 +888,7 @@ def closeEvent(self, event): pass if isinstance(self.axis, Axis) and not self.axis.terminated: - self.axis.terminate(delay_loop_stop=True) + self.axis.terminate(delay_loop_stop=True, disconnect_signals=True) loop_safe_stop(self.axis_loop) @@ -946,13 +946,16 @@ def __init__(self, mg: MotionGroup, parent: "mgw.MGWidget" = None): # initialize drive configuration _drive_config = None if isinstance(self.mg, MotionGroup) and isinstance(self.mg.drive, Drive): - self.mg.drive.terminate(delay_loop_stop=True) + self.mg.drive.terminate(delay_loop_stop=True, disconnect_signals=False) _drive_config = _deepcopy_dict(self.mg.drive.config) + elif not isinstance(parent, mgw.MGWidget): pass + elif parent.drive_dropdown.currentText != "Custom Drive": index = parent.drive_dropdown.currentIndex() _drive_config = _deepcopy_dict(parent.drive_defaults[index][1]) + elif "drive" in parent._initial_mg_config: _drive_config = _deepcopy_dict(parent._initial_mg_config["drive"]) @@ -1119,19 +1122,27 @@ def _validate_drive(self): # 5. The drive is instantiable Drive() self.logger.info("Validating drive.") - if not all([isinstance(axw.axis, Axis) for axw in self.axis_widgets]): - self.logger.warning("Drive is not valid since not all axes are configured.") - self._change_validation_state(False) - return - elif not all([axw.axis.connected for axw in self.axis_widgets]): - self.logger.warning("Drive is not valid since not all axes are online.") - self._change_validation_state(False) - return - elif self.dr_name_widget.text() == "": + if self.dr_name_widget.text() == "": self.logger.warning("Drive is not valid, it needs a name.") self._change_validation_state(False) return + for axw in self.axis_widgets: + if not isinstance(axw.axis, Axis): + self.logger.warning( + "Drive is not valid since not ALL axes are configured." + ) + self._change_validation_state(False) + return + + if axw.axis.terminated or not axw.axis.connected: + axw.axis.run() + + if not axw.axis.connected: + self.logger.warning("Drive is not valid since not all axes are online.") + self._change_validation_state(False) + return + # TODO: NEED AN HANDLER THAT ENSURES NO OTHER MOTION GROUP USES # A DRIVE WITH THE SAME IPs # for handler in self._drive_handlers: @@ -1174,13 +1185,14 @@ def _spawn_axis_widget(self, name): def _spawn_drive(self, config=None): self.logger.info(f"Spawning Drive. {self.drive_config}") if isinstance(self.drive, Drive): - self.drive.terminate(delay_loop_stop=True) + self.drive.terminate(delay_loop_stop=True, disconnect_signals=True) self._set_drive(None) for axw in self.axis_widgets: if axw.axis is None: continue - axw.axis.terminate(delay_loop_stop=True) + + axw.axis.terminate(delay_loop_stop=True, disconnect_signals=True) config = config if config is not None else self.drive_config try: @@ -1195,7 +1207,7 @@ def _spawn_drive(self, config=None): # we do NOT want the drive actor to be running, since the # AxisConfigWidgets will have running Axis actors # - drive.terminate(delay_loop_stop=True) + drive.terminate(delay_loop_stop=True, disconnect_signals=True) # update Axis actors for ii, ax in enumerate(drive.axes): @@ -1217,12 +1229,12 @@ def _spawn_drive(self, config=None): def _safe_return_config_emit(self, config: Dict[str, Any]): self.configChanged.disconnect() if isinstance(self.drive, Drive) and not self.drive.terminated: - self.drive.terminate(delay_loop_stop=True) + self.drive.terminate(delay_loop_stop=True, disconnect_signals=True) self._set_drive(None) for axw in self.axis_widgets: axw.configChanged.disconnect() - axw.axis.terminate(delay_loop_stop=True) + axw.axis.terminate(delay_loop_stop=True, disconnect_signals=True) axw.axis = None axw.close() diff --git a/bapsf_motion/gui/configure/motion_group_widget.py b/bapsf_motion/gui/configure/motion_group_widget.py index a345402f..fb1b39bc 100644 --- a/bapsf_motion/gui/configure/motion_group_widget.py +++ b/bapsf_motion/gui/configure/motion_group_widget.py @@ -564,7 +564,9 @@ def _connect_signals(self): self.drive_control_widget.targetPositionChanged.connect( self._handle_drive_control_target_position_changed ) - self.drive_control_widget.driveStatusChanged.connect(self.update_position_in_plot) + self.drive_control_widget.driveStatusChanged.connect( + self._handle_drive_status_changed + ) self.done_btn.clicked.connect(self.return_and_close) self.discard_btn.clicked.connect(self.discard_close) @@ -1260,8 +1262,7 @@ def _popup_drive_configuration(self): # LostConnection dialog. self.set_disable_for_popup() - if isinstance(self.mg, MotionGroup): - self.mg.terminate(delay_loop_stop=True) + self._terminate_mg(delay_loop_stop=True, disconnect_signals=False) self._overlay_setup(DriveConfigOverlay(self.mg, parent=self)) @@ -1310,6 +1311,24 @@ def _overlay_close(self): self._overlay_widget = None self._overlay_shown = False + def _terminate_mg( + self, + delay_loop_stop: bool = False, + disconnect_signals: bool = False, + ): + mg = self.mg + if not isinstance(mg, MotionGroup): + return + + drive = mg.drive + if not isinstance(drive, Drive): + return + + mg.terminate( + delay_loop_stop=delay_loop_stop, + disconnect_signals=disconnect_signals, + ) + def resizeEvent(self, event: QResizeEvent): if self._overlay_shown: self._overlay_widget.resize(event.size()) @@ -1446,6 +1465,20 @@ def _handle_transform_overlay_close(self, config: Dict[str, Any]): self._change_transform(config) + @Slot() + def _handle_drive_status_changed(self): + valid = self._validate_drive() + + if not valid: + self.done_btn.setEnabled(False) + return + + if valid and not self.done_btn.isEnabled(): + self.configChanged.emit() + return + + self.update_position_in_plot() + @Slot() def _handle_movement_started(self): self.disable_config_controls_for_motion() @@ -1649,8 +1682,7 @@ def _spawn_motion_group(self): if isinstance(self.mg, MotionGroup): self.logger.info("Terminating Motion Group for re-spawn.") - self.mg.terminate(delay_loop_stop=True) - # self._set_mg(None) + self._terminate_mg(delay_loop_stop=True, disconnect_signals=True) self._mg = None mg = None @@ -1667,7 +1699,7 @@ def _spawn_motion_group(self): exc_info=err, ) try: - mg.terminate(delay_loop_stop=True) + mg.terminate(delay_loop_stop=True, disconnect_signals=True) except AttributeError: pass @@ -1798,6 +1830,8 @@ def _validate_motion_group_name(self) -> bool: return True def _validate_drive(self) -> bool: + self.logger.info("Validating drive") + self.drive_btn.setToolTip("") if not isinstance(self.mg, MotionGroup) or not isinstance(self.mg.drive, Drive): @@ -1821,6 +1855,24 @@ def _validate_drive(self) -> bool: ) return False + if not self.mg.drive.connected: + self.drive_btn.set_invalid() + self.drive_control_widget.setEnabled(False) + self.done_btn.setEnabled(False) + + not_connected = {} + for ax in self.mg.drive.axes: + if ax.connected: + continue + + not_connected[ax.name] = ax.ip + + self.drive_btn.setToolTip( + "Drive is not fully connected to the motors. The " + f"following motors are NOT connected {not_connected}." + ) + return False + shared_ips = [] for ax in self.mg.drive.axes: if ax.ip in self._deployed_restrictions["ips"]: @@ -1937,7 +1989,7 @@ def return_and_close(self): # disable the Drive control widget, so we do not risk creating # extra events while terminating self.drive_control_widget.setEnabled(False) - self.mg.terminate(delay_loop_stop=True) + self._terminate_mg(delay_loop_stop=True, disconnect_signals=True) self.returnConfig.emit(index, config) self.close() @@ -1980,7 +2032,7 @@ def discard_close(self): # disable the Drive control widget, so we do not risk creating # extra events while terminating self.drive_control_widget.setEnabled(False) - self.mg.terminate(delay_loop_stop=True) + self._terminate_mg(delay_loop_stop=True, disconnect_signals=True) self.returnConfig.emit(-1, {}) self.close() @@ -1996,8 +2048,7 @@ def closeEvent(self, event: QCloseEvent): # extra events while terminating self.drive_control_widget.setEnabled(False) - if isinstance(self.mg, MotionGroup) and not self.mg.terminated: - self.mg.terminate(delay_loop_stop=True) + self._terminate_mg(delay_loop_stop=True, disconnect_signals=True) if self._overlay_widget is not None: self._overlay_widget.close()