Make "detector mode" in the task dialogs configurable from the backend

[alessandro-olivo]

This PR allows customizing the "detector modes" shown in the drop-down menu in the "Task" dialogs (i.e. DataCollection, Helical, Mesh, Characterisation). The list of modes comes from the backend, in particular you can configure them in the beamline_config.yml file or in the detector.yml. What is configured in detector.yml will be used as the default value.

HowTo:

Edit the beamline_config.yml file like this:

...
default_acquisition_parameters:
    default:
      ...
      helical:
        # Defaults for helical scan. Missing values are taken from default
        number_of_images: 100
+       detector_mode_list: ["0", "C18", "C2"]
...

and/or add this line <roi_mode_list>["0", "C18", "C12", "C2"]</roi_mode_list> to the detector.yml

The example above is a snippet of the demo/beamline_config.yml,

Notes

If the detector_mode_list parameter is not specified in the configuration file, the dropdown bar will display the default modes defined in the detector.yml (see related PR mxcube/mxcubecore#1297 )

If the detector_modes parameter is defined as an empty list the dropdown bar won't be displayed

Requirements:

⚠️ This PR depends on mxcubecore mxcube/mxcubecore#1297 being merged first in order to function correctly.

[marcus-oscarsson]

Now I see what you like to do, very nice !

[marcus-oscarsson]

should this be detector_roi_mode then or is there also a detector_mode ? :)

[alessandro-olivo]

Actually, I didn't notice the detector_roi_mode mxcubecore/model/queue_model_objects.py and I have no clue if this refers to what is selected in the "Detector mode".

In any case the attributes is a string type but a list is needed in order to populate the dropdown bar

[alessandro-olivo]

I'll check the code to clarify who is who...

[marcus-oscarsson]

I see, but the actual value used and passed to the collection routine will be contained in detector_mode, It will be one of the values in detector_modes right ?

I'm being quite picky but I often wind the trailing s to be quite confusing. Its often easier to add _list or something.

As I understand this, detector_modes is the list of ROI's available and the value passed is detector_mode ?

[alessandro-olivo]

I see, but the actual value used and passed to the collection routine will be contained in detector_mode, It will be one of the values in detector_modes right ?

Yes!

I'm being quite picky but I often wind the trailing s to be quite confusing. Its often easier to add _list or something.

The '_list' suffix helps make the meaning clearer, so I’ll change it! :)

As I understand this, detector_modes is the list of ROI's available and the value passed is detector_mode ?

Actually, we're not really using these "detector modes". Like you, I'd guess that the detector_mode refers to a specific ROI.
But there is something wired (at least to me) I can't find the mapping detector_mode --> detector_mode_roi in the code.

[marcus-oscarsson]

We are currently not using detector_roi_mode but I think it might be interesting for the future. My question was the same. I can't see any mapping between detector_mode and detector_mode_roi either :). I think you can simply call it detector_mode_roi and detector_mode_list. I think detector_mode_roi will get passed down to collect as it already exists.

[alessandro-olivo]

I added the same changes to the other Tasks (Mesh, Characterization, ..)

⚠️⚠️⚠️ This PR mxcube/mxcubecore#1297 has to be merged first

...I'll take care of bumping the mxcubecore version ;)

[marcus-oscarsson]

Perfect well done !, Ill be super happy to merge this one :) I just made one small comment on mxcube/mxcubecore#1297, maybe the ROI mode list can be read from configuration ?

[alessandro-olivo]

@marcus-oscarsson Ready to be merged, if you could give it a quick look, I’d really appreciate it :)

[alessandro-olivo]

Thanks for the review @walesch-yan, you are right!
I will propose another change, that is, remove the "?" protection, which I should have thrown away in the last review, since actually there should not be a case in which that parameter is not defined.

I'd propose this change:

{this.props.initialValues.detector_mode_list.length > 0 && (
    <SelectField
         ....
    />
}

[walesch-yan]

You are right. I somehow overlooked that it is always initialized with an empty list at least.
Thanks a lot for the changes and the overall work @alessandro-olivo. I do have one more request, though. I think it makes sense to change the demo config parameters in demo.yaml as well. That way, we keep consistent configurations for both versions.

[alessandro-olivo]

True, it makes sense, from now on, I'll take care of updating the yaml configuration files as well 😄

[walesch-yan]

Thanks @alessandro-olivo, this looks good to me