Base image time stamp off epicsTS not startTime

[jsouter]

We noticed that all image timestamps coming from an ADPilatus acquisition were identical, and identified that ADSimDetector, ADPilatus and others (will check) have this logic. Have changed the time stamp update to resemble the EPICS mode timestamping from ADVimba.

[coretl]

@MarkRivers is this a suitable change for us to make in all the areaDetector drivers we can find?

[MarkRivers]

ADPilatus does have a problem because there is an inner loop for multiple images, and startTime is only updated in the outer loop. https://github.com/areaDetector/ADPilatus/blob/7b50bed1de60d5cd9c802ee2cded6d79b8b2acf2/pilatusApp/src/pilatusDetector.cpp#L1156

However, I don't think that ADSimDetector really has a logic problem, because startTime is updated in the loop for every image. https://github.com/areaDetector/ADSimDetector/blob/14750dd31daf0880a56b809168b0cccc5f3949bf/simDetectorApp/src/simDetector.cpp#L783 This is the output of camonitor looking at pImage->timeStamp from a SimDetector plugin. Note that it is updating correctly.

(base) [epics@corvette ~]$ camonitor -e10 13SIM1:Pva1:TimeStamp_RBV
13SIM1:Pva1:TimeStamp_RBV      2023-11-13 11:46:12.586475 1.0687455725e+09
13SIM1:Pva1:TimeStamp_RBV      2023-11-13 11:46:12.686648 1.0687455726e+09
13SIM1:Pva1:TimeStamp_RBV      2023-11-13 11:46:12.786772 1.0687455727e+09
13SIM1:Pva1:TimeStamp_RBV      2023-11-13 11:46:12.887006 1.0687455728e+09
13SIM1:Pva1:TimeStamp_RBV      2023-11-13 11:46:12.987000 1.0687455729e+09
13SIM1:Pva1:TimeStamp_RBV      2023-11-13 11:46:13.087133 1.0687455730e+09
13SIM1:Pva1:TimeStamp_RBV      2023-11-13 11:46:13.187363 1.0687455731e+09
13SIM1:Pva1:TimeStamp_RBV      2023-11-13 11:46:13.287342 1.0687455732e+09
13SIM1:Pva1:TimeStamp_RBV      2023-11-13 11:46:13.387419 1.0687455733e+09
13SIM1:Pva1:TimeStamp_RBV      2023-11-13 11:46:13.487541 1.0687455734e+09

Remember that pImage->timeStamp is not always the same as pImage->epicsTS. pImage->timeStamp can be based on the time returned from the camera hardware. pImage->epicsTS is always the EPICS timestamp when the driver received the image.

In the case where the vendor driver cannot return a timestamp then I agree that we should be consistent and always create pImage->timeStamp from pImage->epicsTS, even in cases like ADSimDetector where the current logic works.

[coretl]

However, I don't think that ADSimDetector really has a logic problem, because startTime is updated in the loop for every image.

Good point, I hadn't spotted this...

In the case where the vendor driver cannot return a timestamp then I agree that we should be consistent and always create pImage->timeStamp from pImage->epicsTS, even in cases like ADSimDetector where the current logic works.

Excellent, thank you

@jsouter please can you do a similar PR for ADPilatus and then look through all the other detectors in this org and make PRs for those with the same problem?

[jsouter]

Yep, I will have a look through them.

[jsouter]

Have a few I've made a note of to double check but I can't see any obvious timestamp issues in the other repos. Will close this and make a PR for ADPilatus, thanks both!

[coretl]

Will close this and make a PR for ADPilatus

I thought from @MarkRivers reply that we should merge this SimDetector change as well as making PRs for ADPilatus and anything else that uses locally generated timeStamps (rather than from hardware) so that it matches epicsTS...

I can't see any obvious timestamp issues in the other repos

https://github.com/search?q=org%3AareaDetector+%22-%3EtimeStamp+%3D+startTime.secPastEpoch%22&type=code

Looks like there are lots in the same class as SimDetector, timeStamp will update on each frame, but not exactly match epicsTS as they are generated a few lines of code apart...

[jsouter]

Ah, right you are, I'll have another look, but yes, possibly no other cases where the timestamp should remain static for the entire acquisition at least, may have to open another PR as was a bit hasty in closing my branches!