I24 serial: commissioning fixes #645
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #645 +/- ##
==========================================
+ Coverage 82.64% 85.00% +2.35%
==========================================
Files 98 98
Lines 6551 6548 -3
==========================================
+ Hits 5414 5566 +152
+ Misses 1137 982 -155
|
noemifrisina
requested review from
rtuck99,
DominicOram,
dperl-dls and
olliesilvester
| @@ -224,20 +222,20 @@ def generate_dcid( | |||
| ) | |||
| resp.raise_for_status() | |||
| self.dcid = resp.json()["dataCollectionId"] | |||
| logger.info("Generated DCID %s", self.dcid) | |||
| SSX_LOGGER.info("Generated DCID %s", self.dcid) | |||
| except requests.HTTPError as e: | |||
There was a problem hiding this comment.
I think we should have some tests for this file
There was a problem hiding this comment.
Adding them in #627 , which is actually changing the code in this file so while I may add them here they would go away in a couple of hours?
| @@ -467,7 +471,7 @@ def start_i24( | |||
| SSX_LOGGER.info("Using Eiger detector") | |||
|
|
|||
| SSX_LOGGER.debug(f"Creating the directory for the collection in {filepath}.") | |||
| Path(filepath).mkdir(parents=True) | |||
| Path(filepath).mkdir(parents=True, exist_ok=True) | |||
|
|
|||
There was a problem hiding this comment.
why do we only create the directory for the eiger and not the pilatus?
There was a problem hiding this comment.
Because the Pilatus creates its own directories, while the Eiger doesn't, so there's no need to explicitly create it. Until a few weeks ago - when we started running as i24detector on procserv - this one line was actually triggering a one image collection from the Pilatus just to get the directory....
| caput(pv.eiger_wavelength, caget(pv.dcm_lambda)) | ||
| caput(pv.eiger_omegaincr, 0.0) | ||
| caput(pv.eiger_beamx, 1605.7) | ||
| caput(pv.eiger_beamy, 1702.7) | ||
| caput(pv.eiger_beamx, 1600.0) |
There was a problem hiding this comment.
I think these should be defined as global constants at the top of the file rather than buried in the code here.
There was a problem hiding this comment.
This is being done in another PR, see
Fixes from commissioning on 13/11/24
cs_makerin order to allow for the motors to finish moving (see I24 serial: cs_maker should wait for the motors to be finished moving before setting the next pmac_string #646 )