Skip to content

Implement Abstract diffractometer - #1531

Draft
RafaelLyra8 wants to merge 9 commits into
mxcube:developfrom
cnpem:abstract_diff
Draft

Implement Abstract diffractometer#1531
RafaelLyra8 wants to merge 9 commits into
mxcube:developfrom
cnpem:abstract_diff

Conversation

@RafaelLyra8

Copy link
Copy Markdown
Contributor

No description provided.

@beteva beteva left a comment

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.

Well, that was quick. 😃
If you want to use the AbstractDiffractometer, you need to move all the centring methods to a LNLSSampleView, which inherits from SampleView

Comment thread mxcubecore/HardwareObjects/LNLS/LNLSDiffractometer.py
self.current_motor_positions = {}
self.current_phase = DiffractometerPhase.UNKNOWN

def manual_centring(self):

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.

All the centring methods (manual and automatic) can be found in SampleView.py, where it would be more appropriate place for this one. Maybe you can overload the start_manual_centring. This is what the sample_view adapter will execute.

Comment thread mxcubecore/HardwareObjects/LNLS/LNLSDiffractometer.py Outdated
Comment thread mxcubecore/HardwareObjects/LNLS/LNLSDiffractometer.py Outdated
Comment thread mxcubecore/HardwareObjects/LNLS/LNLSDiffractometer.py Outdated
@beteva

beteva commented Mar 17, 2026

Copy link
Copy Markdown
Member

Could you, as well, correct the linting errors. You can run poetry run pre-commit run --all-files in mxcubecore or mxcubeweb root directory

"kappa_phi": 0,
}
self.current_phase = DiffractometerPhase.UNKNOWN
self.set_is_ready(True)

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
self.set_is_ready(True)
self.update_state(self.STATES.READY)

set_is_ready is a deprecated method.

@beteva beteva left a comment

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.

Implementing your own centring procedures is nice. One problem, though, you are not emitting the signals, needed to accept or save the centring point on the GUI. For this you simply need to evoke self.centring_done() and self.accept_centring().
Without the signals, The GUI and the queue would not be informed that centring is executed or finished.

return

def is_ready(self) -> bool:
def wait_status_ready(self, timeout=None):

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.

Well spotted!

Comment thread mxcubecore/HardwareObjects/LNLS/LNLSSampleView.py
Comment thread mxcubecore/HardwareObjects/LNLS/LNLSSampleView.py
Comment thread mxcubecore/HardwareObjects/LNLS/LNLSSampleView.py
Comment thread mxcubecore/HardwareObjects/LNLS/LNLSSampleView.py
@marcus-oscarsson

Copy link
Copy Markdown
Member

There still seems to be some pending suggestions/ideas are we ready to merge this or would you like to have a closer look, @RafaelLyra8 ?

@pedrobenetton
pedrobenetton marked this pull request as draft April 30, 2026 18:29
@pedrobenetton
pedrobenetton requested a review from beteva April 30, 2026 19:31
Comment on lines +92 to +93
def get_centred_point_from_coord(self, x, y, return_by_names=None): # noqa: ARG002
return {"omega": 0, "phiy": 0, "phiz": 0, "sampx": 0, "sampy": 0}

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.

This is a bit strange. Does it mean that you only add it to overload the SampleView one?

@beteva beteva left a comment

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.

Well done.
Only few doubts about the 0 positions of get_centred_point_from_coord, but OK if this is what you need..

@beteva

beteva commented May 4, 2026

Copy link
Copy Markdown
Member

The branch is still WIP. If you are done, you can rebase and merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants