Add the initail PVs required for the xspress3mini#81
Conversation
Codecov Report
@@ Coverage Diff @@
## main #81 +/- ##
==========================================
+ Coverage 87.35% 87.44% +0.08%
==========================================
Files 51 52 +1
Lines 1685 1697 +12
==========================================
+ Hits 1472 1484 +12
Misses 213 213
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
DominicOram
left a comment
DominicOram
left a comment
There was a problem hiding this comment.
Good start, thanks. I think you should remove the pv from all the names, we know they're PVs
| pv_time = Component(EpicsSignalRO, f"SCAS:{1}:TSArrayValue") | ||
| pv_reset_ticks = Component(EpicsSignalRO, f"SCAS:{2}:TSArrayValue") | ||
| pv_reset_count = Component(EpicsSignalRO, f"SCAS:{3}:TSArrayValue") | ||
| pv_all_event = Component(EpicsSignalRO, f"SCAS:{4}:TSArrayValue") | ||
| pv_all_good = Component(EpicsSignalRO, f"SCAS:{5}:TSArrayValue") | ||
| pv_pileup = Component(EpicsSignalRO, f"SCAS:{8}:TSArrayValue") |
There was a problem hiding this comment.
Must: I think these are actually SCA{}:Value_RBV from looking at the synoptic I think you can also just use the numbers themselves rather than an f-string. You're also off by one for each, it should start at 0
| # Assume 6 ROI's per channel and one channel. This might need to be changed | ||
| ROI_1 = Component(Xspress3MiniROI, "MCA_ROI1_") | ||
| ROI_2 = Component(Xspress3MiniROI, "MCA_ROI2_") | ||
| ROI_3 = Component(Xspress3MiniROI, "MCA_ROI3_") | ||
| ROI_4 = Component(Xspress3MiniROI, "MCA_ROI4_") | ||
| ROI_5 = Component(Xspress3MiniROI, "MCA_ROI5_") | ||
| ROI_6 = Component(Xspress3MiniROI, "MCA_ROI6_") |
There was a problem hiding this comment.
Should: I think these are actually the old xspress3 way of doing ROIs, not the mini way so we can remove them
|
|
||
| pv_sca5_update_arrays_mini = Component(EpicsSignalRO, "SCAS:TS:TSAcquire") | ||
|
|
||
| pv_roi_size = Component( |
There was a problem hiding this comment.
Should: Size seems like a poor name here, HLM normally means high limit, which it looks like it is in the synoptic. I would change it to roi_high_limit. I guess it is size if the low limit is 0 but not in general
| ) # Strange as it doesn't use roi variable, but this does happen in gda code. | ||
| pv_roi_llm = Component(EpicsSignal, "SCA5_LLM") | ||
| # GDA code seems dodgy for these two variables. |
There was a problem hiding this comment.
Should: Yh, it is weird but it looks correct to me so I don't think we need these comments
| pv_erase: EpicsSignal = Component(EpicsSignal, "ERASE") | ||
| pv_get_max_num_channels = Component(EpicsSignalRO, "MAX_NUM_CHANNELS_RBV") | ||
|
|
||
| pv_acquire: EpicsSignal = (Component(EpicsSignal, "Acquire"),) |
There was a problem hiding this comment.
Must: I don't think this needs to be a tuple?
| set_trigger_mode_mini: EpicsSignal = Component(EpicsSignal, "TriggerMode") | ||
|
|
||
| pv_get_trig_mode_mini: EpicsSignalRO = Component(EpicsSignalRO, "TriggerMode_RBV") |
There was a problem hiding this comment.
Could: Check out EpicsSignalWithRBV in ophyd this combines PVs like this nicely
DominicOram
changed the title
DominicOram
changed the title
DominicOram
left a comment
DominicOram
left a comment
There was a problem hiding this comment.
Looks good, thanks. For future reference, the PR title gets put in release notes so try and make it friendly for a scientist to read
2 participants
Fixes #71
For now this just adds the PV's used within the GDA script EpicsXspress3MiniDetector.java