Fix timestamp checks for PCO cameras #525
Conversation
This also ignores the index of the first frame and checks only if all subsequent frames are correctly allignes
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #525 +/- ##
=======================================
Coverage 88.38% 88.38%
=======================================
Files 121 121
Lines 8317 8319 +2
=======================================
+ Hits 7351 7353 +2
Misses 966 966
☔ View full report in Codecov by Sentry. |
concert/experiments/addons.py
Outdated
| @@ -409,6 +412,7 @@ async def _check_timestamp(self, producer): | |||
| raise PCOTimestampCheckError("Not all images contained timestamps.") | |||
|
|
|||
|
|
|||
|
|
|||
| self._timestamp_enabled = await self.get_timestamp() in ['both', 'binary'] | ||
| await super().start_readout() | ||
| self._timestamp_enabled = (await self.get_timestamp_mode()).value_name in ['UCA_PCO_CAMERA_TIMESTAMP_BINARY', 'UCA_PCO_CAMERA_TIMESTAMP_BOTH'] | ||
| await UcaCamera.start_readout(self) |
There was a problem hiding this comment.
Why move away from super()?
There was a problem hiding this comment.
Still, why did you change this?
| if img.metadata['frame_number'] != i + 1: | ||
| if i == 0: | ||
| first_frame = img.metadata['frame_number'] | ||
| if img.metadata['frame_number'] != i + first_frame: |
There was a problem hiding this comment.
Is it safe to rely on the first frame index being 1? All PCO's behave the same way for sure?
| @@ -388,6 +388,7 @@ async def _check_timestamp(self, producer): | |||
| self.timestamp_missing = False | |||
| i = 0 | |||
| last_acquisition = await self._experiment.acquisitions[-1].get_state() == "running" | |||
There was a problem hiding this comment.
I know this isn't new but it's really a hack, we should improve this in the future.
There was a problem hiding this comment.
I mean what if we raise an exception even if this is the first acquisition? If you want the timestamp and it's not there than it makes sense, doesn't it?
| self._timestamp_enabled = await self.get_timestamp() in ['both', 'binary'] | ||
| await super().start_readout() | ||
| self._timestamp_enabled = (await self.get_timestamp_mode()).value_name in ['UCA_PCO_CAMERA_TIMESTAMP_BINARY', 'UCA_PCO_CAMERA_TIMESTAMP_BOTH'] | ||
| await UcaCamera.start_readout(self) |
There was a problem hiding this comment.
Still, why did you change this?
Fixes timestamp checks for PCO cameras.
This also ignores the index of the first frame and checks only if all subsequent frames are correctly aligned