Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mxcubecore/HardwareObjects/GenericDiffractometer.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ class GenericDiffractometer(HardwareObject):
"""

CENTRING_MOTORS_NAME = [
"phi",
"omega",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GenericDiffractometer should be replaced by AbstractDiffractometer.

"phiz",
"phiy",
"sampx",
Expand Down
157 changes: 63 additions & 94 deletions mxcubecore/HardwareObjects/QtGraphicsManager.py

Large diffs are not rendered by default.

17 changes: 10 additions & 7 deletions mxcubecore/HardwareObjects/SampleView.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ def get_positions(self) -> dict[str, float]:
Returns:
Centring motor positions as {role: position}
"""
motors_dict = {}
for key, val in self.centring_motors.items():
motors_dict.update({key: val.motor.get_value()})
motors_dict = dict(
(key, val.get_value()) for key, val in self.centring_motors.items()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(key, val.get_value()) for key, val in self.centring_motors.items()
(key, val.motor.get_value()) for key, val in self.centring_motors.items()

Do you really need to change this for the sake of a one line less? 😄

)
return motors_dict

def get_centred_point_from_coord(self, x, y, return_by_names=None):
Expand Down Expand Up @@ -246,7 +246,7 @@ def motor_positions_to_screen(
inv_rot_matrix,
)

chi_angle = math.radians(self.chi_angle)
chi_angle = math.radians(self.chi_angle) if self.chi_angle else 0.0
chi_rot = np.matrix(
[
[math.cos(chi_angle), -math.sin(chi_angle)],
Expand Down Expand Up @@ -276,7 +276,7 @@ def start_manual_centring(self, nb_click: int = 3):
diffr = HWR.beamline.diffractometer
pixels_per_mm = diffr.get_pixels_per_mm()
diffr.wait_status_ready(5)

print(f"self.centring_motors {self.centring_motors}")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please Replace with log if the print is needed

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or remove it.

self.current_centring_procedure = sample_centring.start(
self.centring_motors,
pixels_per_mm[0],
Expand Down Expand Up @@ -354,15 +354,18 @@ def accept_centring(self):
"""Accept the current centred position."""
self.centring_status["valid"] = True
self.centring_status["accepted"] = True
self.emit("centringAccepted", (True, self.get_centring_status()))
args = (True, self.get_centring_status())
self.emit("centringAccepted", args)
self.create_centring_point(*args)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be done in the handler that listens to centringAccepted

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed only the signal emit is needed here.

logging.getLogger("user_level_log").info("Centring successful")

def reject_centring(self):
"""Reject the current centred position."""
if self.current_centring_procedure:
self.current_centring_procedure.kill(block=True)
self.centring_status["valid"] = False
self.emit("centringAccepted", (False, self.get_centring_status()))
args = (False, self.get_centring_status())
self.emit("centringAccepted", args)
Comment on lines +367 to +368

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the reason to define args?

logging.getLogger("user_level_log").info("Centring cancelled")

def cancel_centring(self):
Expand Down
4 changes: 4 additions & 0 deletions mxcubecore/HardwareObjects/abstract/AbstractDiffractometer.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@

from mxcubecore.BaseHardwareObjects import HardwareObject, HardwareObjectState
from mxcubecore import HardwareRepository as HWR
from mxcubecore.model import queue_model_objects


__copyright__ = """ Copyright © by the MXCuBE collaboration """
Expand Down Expand Up @@ -244,6 +245,9 @@ def init(self):
self.connect(_hobj, "valueChanged", _hobj.update_value)
except KeyError:
self.log.warning("Diffractometer: No motors configured")
queue_model_objects.CentredPosition.set_diffractometer_motor_names(
list(self.motors_hwobj_dict)
)
Comment on lines +248 to +250

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything concerning the centring should be now in SampleView.

@rhfogh rhfogh Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the motor names are on the diffractometer, and they are needed in queue_model_objects as the code stands, so this was a quick fix (replacing a faulty fix that was in the PR previously). Still, I think the right conclusion is that this PR is not mature and we need some more thorough refactoring before we can use AbstractDiffractometer with the Qt branch. I shall take this up with Martin as soon as possible - meanwhile this is still WIP.


# nstate (discrete positions) equipment
for role in self.config.nstate_equipment:
Expand Down
3 changes: 3 additions & 0 deletions mxcubecore/HardwareObjects/abstract/AbstractSampleView.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,3 +293,6 @@ def motor_positions_to_screen(self, positions_dict: dict) -> tuple:
"""Get the motor positions according to the calibration"""

return ()

def get_image_scale_list(self):
return [1]
Comment on lines +296 to +298

ghost Sep 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you, please, tell us what is this used for?

11 changes: 10 additions & 1 deletion mxcubecore/HardwareObjects/mockup/DiffractometerMockup.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def init(self):
self.update_state(HardwareObjectState.READY)
for mot in self.motors_hwobj_dict.values():
mot.set_value(random.uniform(0.0, 8.8))
self.omega.set_value(random.uniform(0, 359.9))
self.omega.set_value(random.uniform(0, 360))

ghost Sep 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?


def abort(self):
self.update_state(HardwareObjectState.READY)
Expand Down Expand Up @@ -143,3 +143,12 @@ def _set_phase(self, value: DiffractometerPhase):

def _set_constraint(self, value):
self.current_constraint = value

def use_sample_changer(self):
return True

Comment on lines +146 to +149

ghost Sep 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really needed?

def accept_centring(self):
return True

def user_confirms_centring(self):
return False
Comment on lines +150 to +154

ghost Sep 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything concerning the centring is now in SampleView.

Suggested change
def accept_centring(self):
return True
def user_confirms_centring(self):
return False

1 change: 1 addition & 0 deletions mxcubecore/HardwareObjects/mockup/MotorMockup.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class MotorMockup(ActuatorMockup, AbstractMotor):
def __init__(self, name):
AbstractMotor.__init__(self, name)
self._wrap_range = None
self.direction = 1.0

def init(self):
"""Initialisation method"""
Expand Down
35 changes: 17 additions & 18 deletions mxcubecore/HardwareObjects/sample_centring.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,11 @@ def prepare(centring_motors_dict):

motors_to_move = {}
for m in centring_motors_dict.values():
if m.reference_position is not None:
motors_to_move[m.motor] = m.reference_position
if hasattr(m, "reference_position") and m.reference_position is not None:
motors_to_move[m] = m.reference_position
move_motors(motors_to_move)

SAVED_INITIAL_POSITIONS = {
m.motor: m.motor.get_value() for m in centring_motors_dict.values()
}
SAVED_INITIAL_POSITIONS = {m: m.get_value() for m in centring_motors_dict.values()}

omega = centring_motors_dict["omega"]
phiy = centring_motors_dict["phiy"]
Expand Down Expand Up @@ -281,7 +279,7 @@ def centre_plate1Click(
centred_pos = SAVED_INITIAL_POSITIONS.copy()

centred_pos.update(
{sampx.motor: float(sampx.get_value()), sampy.motor: float(sampy.get_value())}
{sampx: float(sampx.get_value()), sampy: float(sampy.get_value())}
)

return centred_pos
Expand Down Expand Up @@ -352,14 +350,14 @@ def centre_plate(
centred_pos = SAVED_INITIAL_POSITIONS.copy()
centred_pos.update(
{
sampx.motor: float(sampx.get_value() + sampx.direction * dx),
sampy.motor: float(sampy.get_value() + sampy.direction * dy),
phiz.motor: (
sampx: float(sampx.get_value() + sampx.direction * dx),
sampy: float(sampy.get_value() + sampy.direction * dy),
phiz: (
float(phiz.get_value() + phiz.direction * d_vertical[0, 0])
if phiz.__dict__.get("reference_position") is None
else phiz.reference_position
),
phiy.motor: (
phiy: (
float(phiy.get_value() + phiy.direction * d_horizontal[0, 0])
if phiy.__dict__.get("reference_position") is None
else phiy.reference_position
Expand All @@ -383,13 +381,14 @@ def centre_plate(


def ready(motor_list):
logging.getLogger("HWR").info([m.actuator_name for m in motor_list])
logging.getLogger("HWR").info([m.name for m in motor_list])
rstate = [m.is_ready() for m in motor_list]
logging.getLogger("HWR").info(rstate)
return all(rstate)


def wait_ready(motor_positions_dict, timeout=None):
print("motor_positions_dict", motor_positions_dict)
with gevent.Timeout(timeout):
while not ready(motor_positions_dict.keys()):
time.sleep(0.1)
Expand Down Expand Up @@ -443,8 +442,8 @@ def center(
pixelsPerMm_Ver,
beam_xc,
beam_yc,
chi_angle,
n_points,
chi_angle=0.0,
n_points=3,
omega_range=180,
):
global USER_CLICKED_EVENT
Expand Down Expand Up @@ -478,7 +477,7 @@ def center(
raise RuntimeError("Exception while centring")

logging.getLogger("HWR").debug("X=%s,Y=%s", X, Y)
chi_angle = math.radians(chi_angle)
chi_angle = math.radians(chi_angle) if chi_angle else 0.0
chiRotMatrix = numpy.matrix(
[
[math.cos(chi_angle), -math.sin(chi_angle)],
Expand All @@ -503,14 +502,14 @@ def center(
centred_pos = SAVED_INITIAL_POSITIONS.copy()
centred_pos.update(
{
sampx.motor: float(sampx.get_value() + sampx.direction * dx),
sampy.motor: float(sampy.get_value() + sampy.direction * dy),
phiz.motor: (
sampx: float(sampx.get_value() + sampx.direction * dx),

ghost Sep 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this one. The previous code used a Motor object as a key while this code would use CentringMotor object. Would that still be compatible with the calling code ? Neither is good practice but I don't think this is the good moment to change that ?

ghost Sep 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @marcus-oscarsson

sampy: float(sampy.get_value() + sampy.direction * dy),
phiz: (
float(phiz.get_value() + phiz.direction * d_vertical[0, 0])
if phiz.__dict__.get("reference_position") is None
else phiz.reference_position
),
phiy.motor: (
phiy: (
float(phiy.get_value() + phiy.direction * d_horizontal[0, 0])
if phiy.__dict__.get("reference_position") is None
else phiy.reference_position
Expand Down
2 changes: 1 addition & 1 deletion mxcubecore/__version__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@
with open(Path(__file__).resolve().parents[1] / "pyproject.toml", "rb") as f:
pyproject = load_toml(f)
__version__ = pyproject["tool"]["poetry"]["version"]
except OSError:
except (OSError, KeyError):
# File IO error
__version__ = "local"
2 changes: 1 addition & 1 deletion mxcubecore/configuration/mockup/detector-mockup.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<width>3110</width>
<height>3269</height>
<hasShutterless>True</hasShutterless>
<fileSuffix>h5</fileSuffix>
<fileSuffix>cbf</fileSuffix>
<object hwrid="/detector-distance-mockup" role="detector_distance"/>

<roiModes>("0", "C18", "C12", "C2")</roiModes>
Expand Down
4 changes: 2 additions & 2 deletions mxcubecore/configuration/mockup/diffractometer-mockup.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
<object href="/diff-phiz-mockup" role="phiz"/>
<object href="/diff-sampx-mockup" role="sampx"/>
<object href="/diff-sampy-mockup" role="sampy"/>
<object href="/udiff_backlight" role="backlight"/>
<object href="/udiff_frontlight" role="frontlight"/>
<!--object href="/backlightswitch" role="backlight"/-->
<!--object href="/frontlightswitch" role="frontlight"/-->
</motors>
<nstate_equipment>
<object href="/fast-shutter-mockup" role="fshutter"/>
Expand Down
27 changes: 0 additions & 27 deletions mxcubecore/configuration/mockup/gphl/alba_session.xml

This file was deleted.

25 changes: 10 additions & 15 deletions mxcubecore/configuration/mockup/gphl/collect-mockup.xml
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
<object class="Gphl.CollectEmulator">

<!-- <override_data_directories>-->
<!-- If override_data_directories is set, it must contain the three base directories -->
<!-- and may contain raw_folder, process_folder, and archive_folder -->
<base_directory>/scratch_fs1/rhfogh/mxcube_base_dir</base_directory>
<base_process_directory>/scratch_fs1/rhfogh/mxcube_base_dir</base_process_directory>
<base_archive_directory>/scratch_fs1/rhfogh/mxcube_base_dir</base_archive_directory>
<!-- <base_directory>/mnt/scratch/rhfogh/mxcube_data</base_directory>-->
<!-- <base_process_directory>/mnt/scratch/rhfogh/mxcube_data</base_process_directory>-->
<!-- <base_archive_directory>/mnt/scratch/rhfogh/mxcube_data</base_archive_directory>-->
<!-- </override_data_directories>-->
<!--<auto_processing>-->
<!--<program>-->
<!--<executable>none_present_executable</executable>-->
<!--<event>after</event>-->
<!--</program>-->
<!--</auto_processing>-->

<directory_prefix>emulator</directory_prefix>

Expand All @@ -21,8 +17,8 @@
<!-- Simcal background noise level -->
<background>1.0</background>
<!-- Number of rays for simcal ray tracing (2000 minimum for testing, 5000 for use-->
<!-- <n_rays>2001</n_rays>-->
<n_rays>5001</n_rays>
<n_rays>2001</n_rays>
<!--<n_rays>5001</n_rays>-->
<!-- Unit cell orientation mode. 1 is calculate-from-cell, real-space -->
<!-- -1 is from-cell, reciprocal-space. Default is 0 (CELL_AXIS vectors) -->
<orient_mode>1</orient_mode>
Expand All @@ -38,9 +34,8 @@
<memory-pool>6</memory-pool>
<!-- Default is 1.0. Too low makes for 'insufficient spots', -->
<!-- too high for 'insufficient indexed fraction' errors -->
<!-- For now all test hkli files are scaled to work with scale 4.0 -->
<!-- <hkl-scale>1.0</hkl-scale>-->
<hkl-scale>4.0</hkl-scale>
<!-- For now all test hkli files are sdcaled to work with scale 1.0 -->
<hkl-scale>1.0</hkl-scale>
<!-- NB set specifically for 5yav PONLY -->
<!-- <hkl-scale>0.1</hkl-scale>-->
</simcal_options>
Expand Down
26 changes: 0 additions & 26 deletions mxcubecore/configuration/mockup/gphl/detector-mockup.xml

This file was deleted.

6 changes: 3 additions & 3 deletions mxcubecore/configuration/mockup/gphl/gphl-setup.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ configuration:

software_paths:
# Mandatory. Directory with workflow config input, e.g. instrumentation.nml
gphl_beamline_config: gphl_beamline_config
gphl_beamline_config: gphl/gphl_beamline_config

# MANDATORY for CCP4 => $GPHL_CCP4_PATH/bin/ccp4.setup-sh,
# unless already sourced in environment
Expand Down Expand Up @@ -89,6 +89,6 @@ configuration:

# OPTIONAL. simcal *binary* For Mock collection emulation only. Not used by workflow
co.gphl.wf.simcal.bin:
/home/rhfogh/Software/GPhL/nightly_20251106/Files_workflow_TRUNK_alpha-bdg/autoPROC/bin/linux64/simcal
/home/rhfogh/Software/GPhL/nightly_20260519/Files_workflow_TRUNK_alpha-bdg/autoPROC/bin/linux64/simcal
co.gphl.wf.simcal.bdg_licence_dir:
/home/rhfogh/Software/GPhL/nightly_20251106/Files_workflow_TRUNK_alpha-bdg
/home/rhfogh/Software/GPhL/nightly_20260519/Files_workflow_TRUNK_alpha-bdg
Loading
Loading