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

IVU control window #717

Merged
merged 7 commits into from
Oct 25, 2024
Merged

IVU control window #717

merged 7 commits into from
Oct 25, 2024

Conversation

RafaelLyra8
Copy link
Contributor

@RafaelLyra8 RafaelLyra8 commented Oct 17, 2024

IVU Control Window.

image
image

@Gabrielrezende-asc
Copy link

Gabrielrezende-asc commented Oct 18, 2024

Nice @RafaelLyra8 ! Maybe we could change the tag "Change Kparam" to "Start Movement", once the same button starts pitch and center movements.
Also, I think we can add the following protection systems in this window:

  • When you try to disable center mode or pitch mode with a center offset or pitch offset different from zero, the undulator automatically sets these PVs setpoints to zero and moves the undulator; only then are the pitch mode or center mode disabled.
  • When you try to change the Kparam with pitch mode and center mode enabled, a message telling the user to disable these modes appears on the screen.
    I think it would be great to have this protection system by now, once our movement tests showed that the undulator control system is not robust when combining different movements. After the upgrades in the PLC code, we can remove these protections.

What do you think, @fernandohds564 , @xresende, @RafaelLyra8 ?

@fernandohds564
Copy link
Contributor

Nice @RafaelLyra8 ! Maybe we could change the tag "Change Kparam" to "Start Movement", once the same button starts pitch and center movements. Also, I think we can add the following protection systems in this window:

* When you try to disable center mode or pitch mode with a center offset or pitch offset different from zero, the undulator automatically sets these PVs setpoints to zero and moves the undulator; only then are the pitch mode or center mode disabled.

* When you try to change the Kparam with pitch mode and center mode enabled, a message telling the user to disable these modes appears on the screen.
  I think it would be great to have this protection system by now, once our movement tests showed that the undulator control system is not robust when combining different movements.  After the upgrades in the PLC code, we can remove these protections.

What do you think, @fernandohds564 , @xresende, @RafaelLyra8 ?

I don't know if I understood correctly @Gabrielrezende-asc. Can we talk offline in person?

@xresende
Copy link
Contributor

Nice @RafaelLyra8 ! Maybe we could change the tag "Change Kparam" to "Start Movement", once the same button starts pitch and center movements. Also, I think we can add the following protection systems in this window:

  • When you try to disable center mode or pitch mode with a center offset or pitch offset different from zero, the undulator automatically sets these PVs setpoints to zero and moves the undulator; only then are the pitch mode or center mode disabled.
  • When you try to change the Kparam with pitch mode and center mode enabled, a message telling the user to disable these modes appears on the screen.
    I think it would be great to have this protection system by now, once our movement tests showed that the undulator control system is not robust when combining different movements. After the upgrades in the PLC code, we can remove these protections.

What do you think, @fernandohds564 , @xresende, @RafaelLyra8 ?

another option is for the window just to complain that the requirements are not met, instead of acting on the IVU on behalf of the user. The window could have a warnning pop-up suggesting how to proceed, instead of acting. I think this is safer behaviour.

@RafaelLyra8 RafaelLyra8 merged commit e6ecbc3 into master Oct 25, 2024
1 check passed
@anacso17 anacso17 deleted the ivu_control branch October 30, 2024 15:52
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