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

Add securuity group and extra parameters #788

Merged
merged 4 commits into from
Jun 6, 2023

Conversation

FreddieAkeroyd
Copy link
Member

Description of work

Add your own description here

To test

See ISISComputingGroup/IBEX#7821

Acceptance criteria

List the acceptance criteria for the PR


Code Review

Functional Tests

  • IOC responds correctly in:
    • Devsim mode
    • Recsim mode
    • Real device, if available
  • Supplementary IOCs (..._0n where n>1) run correctly
  • Log files do not report undefined macros (serach for macLib: macro to find instances of macLib: macro [macro name] is undefined...

Final steps

  • Update the IOC submodule in the main EPICS repo. See Git workflow page for details.
  • Reviewer has merged the associated PR for the release notes

@pull-request-size pull-request-size bot added size/L and removed size/M labels May 25, 2023
@@ -2,4 +2,5 @@ record(motor, "$(P)$(M)") {
field(PCOF, "$(PCOF)")
field(HVEL, "$(HVEL)")
field(RDBD, "$(RDBD)")
field(ASG, "WASL0")
Copy link
Contributor

Choose a reason for hiding this comment

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

everything including the SPs and home/stop buttons are disabled for me - do I need some sort of access control file?

Copy link
Contributor

Choose a reason for hiding this comment

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

was being stupid, accessSecurity submodule was out of date

Copy link
Contributor

@rerpha rerpha Jun 5, 2023

Choose a reason for hiding this comment

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

I can still change OFF, SET, FOFF, CNEN and BDST - probably don't want to do at least the first one?i'm not sure we ever set BDST to anything other than 0.

Copy link
Member Author

@FreddieAkeroyd FreddieAkeroyd Jun 6, 2023

Choose a reason for hiding this comment

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

We do set BSDT on some Galil systems, in fact SANS2D tank might be one of them, but probably won't set it on a mclennan as it has its own backup steps controller parameter. My concern with protecting OFF/SET is don't reflectometer scientists use it - home to neutrons and then define that as zero? This is more thinking of using this more widely, OFF should ideally be protected on a Mclennan as it is a macro, but we are limited in access control levels so we may need to leave it unprotected. The key things to protect here, and the driver for the ticket, was to avoid confusion when electronics/mechatronics group are trying to fault find a mclennan that doesn't move (properly). So they are likely to try and change ERES/MRES/UEIP/ACCL/VELO etc as part of this so they need to be protected to avoid confusion. I feel they are unlikely to try and change OFF in this case, though in an ideal world we would protect it for a mclennan

Copy link
Member Author

Choose a reason for hiding this comment

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

While looking at this is did however make me realise we need to protect the status update button too ISISComputingGroup/IBEX#7837

@rerpha rerpha merged commit 799fe73 into master Jun 6, 2023
@rerpha rerpha deleted the Ticket7821_mclennan_additions branch June 6, 2023 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants