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

Ticket6244 - Adding OPI for 3he fridge system #1266

Merged
merged 7 commits into from
Mar 19, 2021

Conversation

rerpha
Copy link
Contributor

@rerpha rerpha commented Mar 11, 2021

Description of work

Adds a device screen for the 3he fridge system on musr.

Ticket

ISISComputingGroup/IBEX#6244

Acceptance criteria

See ticket

Unit tests

None

System tests

None

Documentation

Highlight and provide a link to any additions or changes to the documentation, if applicable. The aim is provide information to help the reviewer


Code Review

  • Is the code of an acceptable quality?
  • Do the changes function as described and is it robust?
  • Is there associated PR for the release notes?

Final Steps

Copy link
Contributor

@John-Holt-Tessella John-Holt-Tessella left a comment

Choose a reason for hiding this comment

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

  • You need a PV_ROOT set
  • Background should be grey not white; if you want the diagram to be white put the white behind the diagram
  • All labels should be in sentence case with colons
  • Can you run OPI checks (non-strict) and check all errors, e.g. graph error
  • Check parent macros has ITC and HELIOX in
  • Put a bit of space between the graph and the box so they don't overlap
  • Graph heart beats are on heliox not HLX
  • Why is there a script on the opi that doesn't seem to do anything
  • Are we expecting animation on the picture as valves close and open?

base/uk.ac.stfc.isis.ibex.opis/resources/opi_info.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@JamesKingWork JamesKingWork left a comment

Choose a reason for hiding this comment

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

Looks good

@JamesKingWork JamesKingWork merged commit 77f9c6d into master Mar 19, 2021
@JamesKingWork JamesKingWork deleted the Ticket6244_3he_fridge_opi branch March 19, 2021 12:31
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