CaltechOpticalObservatories / camera-interface

Detector controller interface server supporting Archon and ARC (aka "Leach") controllers.
7 stars 1 forks source link

conditional longexposure #24

Closed astronomerdave closed 3 months ago

astronomerdave commented 3 months ago

disable longexposure if feature not supported by ACF

scizen9 commented 3 months ago

Implementing the longexposure command in the ACF file is quite simple, involving only three lines of code. Perhaps we should just make it a requirement? Just a thought.

astronomerdave commented 3 months ago

Making it a requirement might be worth considering; it might be worth talking with some people that write the waveforms (though currently that is pretty much only Tim). But how frequently do you think projects will want to switch back and forth between different units? Until you tried to use it last week I had never encountered anyone using longexposure except for when Tim needed to take both the hour-long dark current measurements in the lab, AND take short exposures.

I think in practice, an instrument is either going to measure their exposure times in milliseconds or seconds and never change the units used. For example, NGPS is using long exposures, on the order of tens of minutes, possibly an hour, and does so using units of seconds. The more I think about this, the more I am liking the notion of a configurable exposure time unit, sec/msec, set in the config file, but allowing it to be overridden with the longexposure command for the cases like Tim's.

astronomerdave commented 3 months ago

Incidentally, this change makes me wonder if we should handle the different kinds of exposures differently, because I had to make the exact same change in three different places now, with the introduction of the hexpose and video functions. I realize that might be easier said than done, and is just a general comment, not specific to this PR.

scizen9 commented 3 months ago

Yes, let's think about a way to make the units a configuration parameter. I'll close this PR for now, but keep the branch in case we decide we need to change units quickly. I will also close my issue on this and make a new issue requesting that the units be a config parameter.

scizen9 commented 3 months ago

Let's close this for now. It can be re-opened later.

astronomerdave commented 3 months ago

renamed branch from dhale -> conditional-longexposure