Skip to content

Conversation

alessandro-olivo
Copy link
Contributor

@alessandro-olivo alessandro-olivo commented Aug 1, 2025

Thanks to @walesch-yan for pointing out the issue suggested by the title. Here's a short video explaining it: Screencast from 2025-07-28 17-29-10.webm

This bug was introduced by me 😅 with this PR #1723

I decided to use the detector_mode_list parameter directly from acq_parameters, instead of the one from taskData, since it's not really task data, at least it is not something that can be changed like the detector_roi_mode.
This list is more like the limits of the range of the motors.

@alessandro-olivo
Copy link
Contributor Author

For PR like this, should I also open a related issue? Just asking to understand the best practice. :)

@alessandro-olivo alessandro-olivo force-pushed the ao-fix-detector_mode_list branch from 7b7cf45 to e191160 Compare August 1, 2025 16:11
@fabcor-maxiv
Copy link
Contributor

For PR like this, should I also open a related issue? Just asking to understand the best practice. :)

From my point of view, this is not necessary to open an issue ticket, since you already have the pull request with the fix.

Copy link
Collaborator

@walesch-yan walesch-yan left a comment

Choose a reason for hiding this comment

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

Looks good to me! Indeed taking the list from the defaultparameters makes more ense, as you suggest.

Regarding the question about opening an issue: I agree with @fabcor-maxiv, as you already have a solution for it. I think creating an issue would be rather something you should do when you find a problem, that you would like to have the opinion of others on it or you think it might take some time to implement a solution and want others to be aware of it resp. prevent a scenario where multiple people work on the same issue
Long story short: I believe the way you handled this is perfectly fine by shortly restating the problem in the description and opening a seperate issue is not necessary :)

…ide a task item in the queue (DataCollection view)
@alessandro-olivo alessandro-olivo force-pushed the ao-fix-detector_mode_list branch from e191160 to 0859bd9 Compare August 4, 2025 09:39
@alessandro-olivo alessandro-olivo merged commit f1e9570 into mxcube:develop Aug 4, 2025
12 checks passed
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.

3 participants