Add two records: CreateDirectories and DirectoryExists. Automatically create directories based on this PV like in AD #102
Conversation
|
Ok, I think this is ready for review. All tests are passing + tests added for new conditions, and I also tested with a real PandA, everything behaves as expected. |
There was a problem hiding this comment.
I have a couple of changes that I think should be made, and then a lot of nitpicking comments.
I also am not the person to give the final approval to this, as I'm somewhat detached from this project these days, so please wait on @evalott100 to give final approval.
| await caput( | ||
| hdf5_test_prefix + ":CreateDirectory", | ||
| create_depth, | ||
| wait=True, | ||
| ) | ||
| await caput( | ||
| hdf5_test_prefix + ":HDFDirectory", | ||
| target_path, | ||
| datatype=DBR_CHAR_STR, | ||
| wait=True, | ||
| ) | ||
| exists = await caget(hdf5_test_prefix + ":DirectoryExists") |
There was a problem hiding this comment.
It would be nice to add timeout=TIMEOUT to all these ca* calls, just to ensure a known maximum termination time. I do note we haven't been particularly good about always doing it in other tests.
|
Is the directory depth here absolute rather than relative to how much of the requested path already exists? i.e. if directory |
|
The dir depth can be either or, depending on the sign - this is the same as the behavior of the same record in Basically, you can consider that the value represents the index in the path split into directories below which directories are permitted to be created by the IOC. So in your example of From the areaDetector file plugin docs:
|
|
@evalott100 Could we get this merged and included in a new release? I'd like to make some dependent changes in ophyd-async to add the option to use this feature. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #102 +/- ##
==========================================
+ Coverage 90.87% 90.97% +0.10%
==========================================
Files 8 8
Lines 1227 1285 +58
Branches 194 206 +12
==========================================
+ Hits 1115 1169 +54
- Misses 74 78 +4
Partials 38 38 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Nice work! This will be really useful. I'm happy to release once it's merged.
It'd maybe be good to add docs on this: https://pandablocks.github.io/PandABlocks-ioc/main/how-to/capture-hdf.html
A copy of the areaDetector description of CreateDirectory should suffice.
| if create_dir_depth < 0: | ||
| max_dirs_to_create = abs(create_dir_depth) | ||
| elif create_dir_depth > len(new_path.parents): | ||
| max_dirs_to_create = 0 |
There was a problem hiding this comment.
Need a test covering line 561
|
Run |
Marking as draft until I can write tests and perform a full test with a real PandA system, but this works in my initial testing.
Designed to work just like the AD
CreateDirectorysystem: https://github.com/areaDetector/ADCore/blob/631fb284dc06d52b9286e62418a80f7f3a7a05e2/docs/ADCore/NDPluginFile.rst?plain=1#L57-L64The
DirectoryExistsrecord is also meant to behave the same as in areaDetector, and prevent attempting to write the HDF file if the target does not exist or is not writable by the user running the IOC process.