Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moved the configuration from the .env file to beamline-configuration.yml and ui.yml #1139

Merged
merged 7 commits into from
Nov 23, 2023

Conversation

marcus-oscarsson
Copy link
Member

@marcus-oscarsson marcus-oscarsson commented Nov 22, 2023

Moved the configuration from the .env file to beamline-configuration.yml and ui.yml. At the same time improved the handling of the data model for ui.yml.

Each section can now have its own data model, which is needed since the server check objects types as UIComponentModel for the actual HardwareObject correspondens, leading to the following error message for the camera_setup section in ui.yml:

image

Needs PR #821 on mxcubecore to work

__root__: Dict[str, UIPropertiesModel]
sample_view: UIPropertiesModel
beamline_setup: UIPropertiesModel
camera_setup: UICameraConfigModel
Copy link
Member Author

Choose a reason for hiding this comment

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

The UIComponentModel is checked against the HardwareObject it configures to avoid mistakes. However the this caused a warning for the objects in the camera_setup` section

@@ -11,3 +11,20 @@ class SimpleNameValue(BaseModel):
class AppSettingsModel(BaseModel):
mode: ModeEnum = Field(ModeEnum.OSC, description="MXCuBE mode SSX or OSC")
version: str = Field("", description="MXCuBE version")
mesh_result_format: str = Field(
Copy link
Member Author

Choose a reason for hiding this comment

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

Most of the setting from the .env file was moved to general application settings, configured via beamline-config.yaml

@@ -78,76 +78,81 @@ run_processing_parallel: true
run_number: 1
click_centring_num_clicks: 3

mesh_result_format: PNG
Copy link
Member Author

Choose a reason for hiding this comment

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

New configuration added here

attribute: diffractometer.sampx
role: focus
step: 0.1
precision: 3
suffix: mm

sample_view_video_controls:
Copy link
Member Author

Choose a reason for hiding this comment

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

The option to display the focus control on the video display was moved to ui.yml and the same option was added to all controls (in case it would be needed)

Copy link
Contributor

Choose a reason for hiding this comment

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

are you thinking on potentially hide the 3 click, zoom. etc?

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly, I thought it was better to simply generalize the visibility of these controls, as there already was one that could be hidden/shown. Its less likely for some controls but it is useful for a few especially for SSX

Copy link
Contributor

@meguiraun meguiraun left a comment

Choose a reason for hiding this comment

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

nice and useful addition! I left ew minor comments/questions

tack,

"PNG", description="Format of mesh result for display"
)
use_native_mesh: bool = Field(
True, description=" Use the native mesh feature available, true by default"
Copy link
Contributor

Choose a reason for hiding this comment

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

please remind me what this one meant... this would be the md3 command? no workflow?

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly, so its basically just to disable the native mesh method so that its not available to the users. Its useful for instance for us as we have the workflow meshes, and it would mean that there are two kinds of meshes and that is of course a bit confusing. So this option is simply to disable the native one and favor the workflow one.

Copy link
Contributor

Choose a reason for hiding this comment

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

good, less confusion is always good :)

attribute: diffractometer.sampx
role: focus
step: 0.1
precision: 3
suffix: mm

sample_view_video_controls:
Copy link
Contributor

Choose a reason for hiding this comment

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

are you thinking on potentially hide the 3 click, zoom. etc?

}

getControlAvailability(name) {
const avilable = find(
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

Copy link
Member Author

Choose a reason for hiding this comment

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

:), thanks

},
) || { show: false };

return available.show;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd move the fallback here: return available?.show || false

Copy link
Member Author

Choose a reason for hiding this comment

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

oh, yes thats nice :)

@marcus-oscarsson marcus-oscarsson marked this pull request as ready for review November 23, 2023 12:43
@marcus-oscarsson
Copy link
Member Author

Thanks for the input everybody ! :)

@marcus-oscarsson marcus-oscarsson merged commit a2d53e1 into develop Nov 23, 2023
12 checks passed
@marcus-oscarsson marcus-oscarsson deleted the mo-moved-config branch November 23, 2023 14:24
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