Conversation
|
|
||
|
|
||
| class MerlinController(DetectorController): | ||
| class MerlinController(ADBaseController): |
There was a problem hiding this comment.
I didn't properly check if this is correct but it looked about right
| assert await tetramm.hdf.lazy_open.get_value() | ||
| assert await tetramm.hdf.swmr_mode.get_value() | ||
| assert (await tetramm.hdf.file_template.get_value()) == "%s/%s.h5" | ||
| assert (await tetramm.hdf.file_template.get_value()) == "%s%s.h5" |
There was a problem hiding this comment.
Don't know if this change in syntax could break anything for anyone?
There was a problem hiding this comment.
Looking to try and find where this has even come from
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1014 +/- ##
==========================================
+ Coverage 97.62% 97.67% +0.04%
==========================================
Files 159 159
Lines 6581 6589 +8
==========================================
+ Hits 6425 6436 +11
+ Misses 156 153 -3 ☔ View full report in Codecov by Sentry. |
|
@DiamondJoseph It's pretty much there now but please can you check the linting ins |
src/dodal/beamlines/i13_1.py
Outdated
| bl_prefix=False, | ||
| drv_suffix="CAM:", | ||
| hdf_suffix="HDF5:", | ||
| fileio_suffix="HDF5:", |
There was a problem hiding this comment.
Can we at least be consistent about whether or not we are using HDF5_PREFIX?
There was a problem hiding this comment.
I've adjusted all of the existing beamlines to make use of it- and CAM_SUFFIX/DET_SUFFIX to at least make it clear we have 2 standards and absolutely for sure definitely never any more.
| await merlin.stage() | ||
| await merlin.prepare(one_shot_trigger_info) | ||
| await merlin.controller.arm() | ||
| await merlin._controller.arm() |
There was a problem hiding this comment.
How was this working before?
There was a problem hiding this comment.
i13_1's MerlinController changed inheritance in this PR, I think it lead to the slightly changed syntax here
src/dodal/plans/save_panda.py
Outdated
| return 1 | ||
|
|
||
| if Path(output_file).exists() and not force: | ||
| if Path(file_name).exists() and not force: |
There was a problem hiding this comment.
doesn't this now need to be output directory / file_name?
There was a problem hiding this comment.
Yeah thanks, good catch
| output_file = args.output_file | ||
| file_name = args.file_name | ||
| output_directory = args.output_directory | ||
| force = args.force |
There was a problem hiding this comment.
I think you could just leave the options unchanged and do
p = Path(output_file)
output_directory, file_name = str(p.parent), str(p.name)
then you wouldn't need two parameters. It's a bit strange to make the output directory a mandatory option but the filename optional, normally the output file is the last parameter for most tools
rtuck99
left a comment
rtuck99
left a comment
There was a problem hiding this comment.
Approved with above observations, I leave it to you to decide whether to change back the save_panda options
3 participants
Fixes #1017
DeviceCollector renamed to init_device
sim.demo module renamed to sim
Initialisation arguments for HDF detectors change slightly as a result of added support for non-hdf writers
save_pandautility plan updated to use new ophyd-async saveMerlin now inherits from
ADBaseControllerInstructions to reviewer on how to test:
Checks for reviewer
dodal connect ${BEAMLINE}