Prevent negative clicks during manual centering and display a UI message #1649

alessandro-olivo · 2025-05-15T13:44:43Z

The feature/fix allows handling user clicks that happen while a manual centering step is still in progress. A message is shown on the UI to inform the user.

The issue

After the first manual centering click, if the user clicks again before the first step is completed, a message appears on the UI and the number of available clicks is reduced by one. If the user keeps clicking, the click count can go negative, as shown in the screenshot:

This behavior can be tested adding a time.sleep(3) in the manual_centring method of the DiffractometerMockup HWO

    def manual_centring(self):
        """
        Descript. :
        """
        for click in range(3):
            self.user_clicked_event = AsyncResult()
            #self.waiting_for_click = True
            x, y = self.user_clicked_event.get()
            if click < 2:
+                time.sleep(3)
                self.motor_hwobj_dict["phi"].set_value_relative(90)
        self.last_centred_position[0] = x
        self.last_centred_position[1] = y
        centred_pos_dir = self._get_random_centring_position()
        return centred_pos_dir

The proposed change

In mxcubeweb/core/components/sampleview.py

    def centring_handle_click(self, x, y):
        if HWR.beamline.diffractometer.current_centring_procedure:
+            try:
                HWR.beamline.diffractometer.imageClicked(x, y, x, y)
                self.centring_click()
+            except Exception:
+                return {"clicksLeft": -1}
        else:
            if not self.centring_clicks_left():
                self.centring_reset_click_count()

The change allows, raising an Exception from .imageClicked(...), to inform the frontend that the centering step is still in progress. If the frontend receives a negative click (i.e. "-1") it will show the following message:

Test the feature:

Just apply the following changes in the DiffractometerMockup:

...
+from gevent.lock import Semaphore

from mxcubecore import HardwareRepository as HWR
from mxcubecore.HardwareObjects.GenericDiffractometer import (
    GenericDiffractometer,
    PhaseEnum,
)


class DiffractometerMockup(GenericDiffractometer):
...

    def init(self):
        """
        Descript. :
        """
        # self.image_width = 100
        # self.image_height = 100

        GenericDiffractometer.init(self)
+      self.waiting_for_click = False
+      self.click_lock = Semaphore()
        self.x_calib = 0.000444

...
    
+    def image_clicked(self, x, y, xi=None, yi=None):
+        with self.click_lock:
+            if self.waiting_for_click:
+                self.waiting_for_click = False
+                self.user_clicked_event.set((x, y))
+            else:
+                self.log.warning("User click but the last centring operation"
+                                              " was still in progress")
+                raise RuntimeError("Last centring operation is still in "
+                                                   "progress, click will be ignored")

    def manual_centring(self):
        """
        Descript. :
        """
        for click in range(3):
            self.user_clicked_event = AsyncResult()
+          self.waiting_for_click = True
            x, y = self.user_clicked_event.get()
            if click < 2:
+              time.sleep(3)
                self.motor_hwobj_dict["phi"].set_value_relative(90)

...

Thoughts

Maybe the Exception could be a more specific User-defined Exception ? Should we use user-defined Exceptions in mxcube ?
Is there a better solution ? Maybe with a new signal o something like that ?

…feature-diffr_is_moving" in the mxcubecore repository is mandatory for this commit)

…notification in the UI

axelboc

I'm surprised we hadn't caught this bug sooner. 😱 Good catch! 🎣 It could explain why we were having issues with this in the Cypress tests...

As before, as a safeguard, I would prefer if the back-end returned a 4xx error to the front-end (which would then display that error via showErrorPanel). Then, to improve the user experience, I would find a way to not dispatch the click action while the 3-click centring process is in progress. As you hinted at, this would require a WebSocket message, a Redux action, and a new flag in the state.

marcus-oscarsson · 2025-05-15T14:19:51Z

Very nice :), it would be great with error from the backend as well as @axelboc pointed out.

marcus-oscarsson · 2025-05-16T06:49:17Z

What you suggest is very nice and I think you did an excellent work wit this PR :)

Maybe the Exception could be a more specific User-defined Exception ? Should we use user-defined Exceptions in mxcube ?

Yes, it would perhaps be preferable, or at least to use a more specific exception
As pointed out above this exception could then be handled on the web side and a 400
error could be returned.

Is there a better solution ? Maybe with a new signal o something like that ?

You mean, instead of returning or raising an exception to send a signal and websocket with the result instead ?

That could also work very well. I'm not sure it would make much of a difference for this kind of short interaction.
We are not risking any timeout or similar as each click is handled separately. The logic you suggest above is IMO
somewhat easier to follow and to handle.

axelboc · 2025-05-16T07:07:28Z

I'm not sure it would make much of a difference for this kind of short interaction.

Yeah, you're right, it'd probably be a bit over-engineered. So keeping the 4xx error response in mind, I think that means basically catching the error and showing that "Please wait" message instead of dispatching showErrorPanel, right?

marcus-oscarsson · 2025-05-16T08:07:20Z

Yes, thats what I thought

marcus-oscarsson · 2025-05-19T07:17:14Z

I guess this would also need the necessary changes to MiniDiff and GenericDiffractometer. Have you tested how this behaves without the changes you suggest to GenericDiffractometer ?

alessandro-olivo · 2025-05-19T16:49:57Z

Actually I tried only with DiffractometerMockup, but I can test also for the HWOs you proposed

alessandro-olivo added 2 commits May 15, 2025 12:57

Avoid negative clicks counting during centring operation (branch "ao-…

c20e294

…feature-diffr_is_moving" in the mxcubecore repository is mandatory for this commit)

Prevent user clicks during manual centering movements, and display a …

7120fa8

…notification in the UI

axelboc reviewed May 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent negative clicks during manual centering and display a UI message #1649

Prevent negative clicks during manual centering and display a UI message #1649

Prevent negative clicks during manual centering and display a UI message #1649

Are you sure you want to change the base?

Prevent negative clicks during manual centering and display a UI message #1649

Conversation

The issue

The proposed change

Test the feature:

Thoughts

Choose a reason for hiding this comment