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

MAXIV: update Machine Info HWO #974

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

elmjag
Copy link
Contributor

@elmjag elmjag commented Jul 22, 2024

Updated MachInfo hardware object, for use at MAXIV BioMAX and MicroMAX beamlines.

Based on older BioMAX HWO, updated to follow the new AbstractMachineInfo API and refactored.

Copy link
Member

@beteva beteva left a comment

Choose a reason for hiding this comment

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

Hi @elmjag, nice to have provided tests.
This is your specific HO, but I was wondering, as it calls tango devices, is there a reason you do not use TangoMachineInfo?

@elmjag
Copy link
Contributor Author

elmjag commented Jul 29, 2024

Not sure, this implementation is based on old version of MachineInfo HWO from BioMAX.

I did not knew that TangoMachineInfo existed. I'll take a look if we can use that instead.

@elmjag elmjag marked this pull request as draft July 29, 2024 08:16
@elmjag
Copy link
Contributor Author

elmjag commented Jul 29, 2024

I have looked at TangoMachineInfo now. The TangoMachineInfo is designed to read all machine info topics from one specified tango device.

Here at MAXIV the topics are provided by multiple tango devices. We also need to do some parsing for a topic, thus we'll need to have a custom MachineInfo HWO anyway.

I feel that trying to use TangoMachineInfo in our case will make the implementation more complicated compared to how it's done in this PR.

@elmjag elmjag marked this pull request as ready for review July 29, 2024 14:12
values['lifetime']
values['topup_remaining']
"""
def catch_errs(func):
Copy link
Member

Choose a reason for hiding this comment

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

:), I would have simply called it catch_errors but its matter of taste I guess and it is after all site specific code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, I have changed it to catch_errors

MachInfo hardware objects are used to obtain information from the accelerator
control system.
# how often we refresh machine info
REFRESH_PERIOD_SEC = 30
Copy link
Member

Choose a reason for hiding this comment

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

You could also easily make it configurable via get_property but perhaps not desired in your case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel it's not worth the hassle in this case. We have not changed this value on a very long time.

@marcus-oscarsson
Copy link
Member

Looks good to me, Ill let you rebase and merge :), you (of-course) do as you like with the comments

New MachInfo hardware object, for using at BioMAX and MicroMAX.

Based on older HWO used at BioMAX, but updated to follow the
new AbstractMachineInfo API.
@elmjag
Copy link
Contributor Author

elmjag commented Jul 30, 2024

Looks good to me, Ill let you rebase and merge :), you (of-course) do as you like with the comments

I have re-based, but I don't have the commit bit to do the merge. 😞

@marcus-oscarsson marcus-oscarsson merged commit f6f88d9 into mxcube:develop Jul 30, 2024
8 checks passed
@marcus-oscarsson
Copy link
Member

Oh, I think it should be ok now :)

@elmjag elmjag deleted the maxiv-mach-info branch July 30, 2024 08:56
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