Add FLIR camera device. #242
Conversation
Tested on local machine.
Also add config_id to metadata.
| '14bit': self.instrument_lib.AdcBitDepth_Bit14} | ||
|
|
||
| def _open(self): | ||
| serial_number = CONFIG_INI.get(self.config_id, 'serial_number') |
There was a problem hiding this comment.
[revision] We don't ref CONFIG_INI from catkit so serial_number will need to be passed in.
| # Camera was not running. We can safely ignore. | ||
| pass | ||
| else: | ||
| self.log.error(f'PySpin error: {e.errorcode}') |
There was a problem hiding this comment.
[revision?] Do we not want to raise this?
| self.cam.DeInit() | ||
| self.cam = None | ||
| self.cam_list.Clear() | ||
|
|
||
| self._destroy_system() |
There was a problem hiding this comment.
[revision] These look like they should be in a finally block, perhaps even two:
...
finally:
self.cam.DeInit() # <----- Can this raise?
finally:
self.cam = None
self.cam_list.Clear()
self._destroy_system()| from catkit.interfaces.Camera import Camera | ||
| from catkit.catkit_types import MetaDataEntry, units, quantity | ||
| import catkit.util | ||
| from catkit.config import CONFIG_INI |
There was a problem hiding this comment.
[revision] Remove this, please, we don't ref CONFIG_INI from catkit.[revision] We don't ref CONFIG_INI from catkit.
| self.pixel_format_enum = {'mono8': self.instrument_lib.PixelFormat_Mono8, | ||
| 'mono12p': self.instrument_lib.PixelFormat_Mono12p, | ||
| 'mono16': self.instrument_lib.PixelFormat_Mono16} | ||
|
|
||
| self.adc_bit_depth_enum = {'8bit': self.instrument_lib.AdcBitDepth_Bit8, | ||
| '10bit': self.instrument_lib.AdcBitDepth_Bit10, | ||
| '12bit': self.instrument_lib.AdcBitDepth_Bit12, | ||
| '14bit': self.instrument_lib.AdcBitDepth_Bit14} |
There was a problem hiding this comment.
[revision] If we're gonna mangle "enum" semantics into the name, then I'd rather see these as actual enums, please (or remove "enum" from the attr name):
self.pixel_format_enum = Enum("pixel_format_enum", {'MONO8': self.instrument_lib.PixelFormat_Mono8,
'MONO12_P': self.instrument_lib.PixelFormat_Mono12p,
'MONO16': self.instrument_lib.PixelFormat_Mono16})The reason is that the access pattern between a dict and an enum is quite different.
There was a problem hiding this comment.
In case you read this and make alterations to un-name-mangle the "enum" before reading on, I would read on. I really get the impression that you just wanna use an Enum.
| if not type(exposure_time) in [int, float]: | ||
| exposure_time_us = exposure_time.to(units.microsecond).m | ||
| else: | ||
| exposure_time_us = exposure_time | ||
|
|
||
| self.exposure_time = exposure_time_us |
There was a problem hiding this comment.
[revision] Use isinstance and not type for this, please.
[revision] Conditional assignment is preferred here, please, or at least dedupe exposure_time_us.
self.exposure_time = exposure_time.to(units.microsecond).m if isinstance(exposure_time, (int, float)) else exposure_timeor
self.exposure_time = exposure_time
if not isinstance(self.exposure_time, (int, float)):
self.exposure_time = self.exposure_time.to(units.microsecond).m| else: | ||
| meta_data.append(extra_metadata) | ||
|
|
||
| # try-finally shuts down the camera if the generator is stopped. |
There was a problem hiding this comment.
Does EndAcquisition() really "shut down" the camera?
There was a problem hiding this comment.
i.e., would _open() need to be re-called?
| while frame_count < num_exposures: | ||
| try: | ||
| # Wait for image with timeout (in ms). | ||
| image_result = self.cam.GetNextImage(100) |
There was a problem hiding this comment.
[revision] Let's hoist the timeout to an instance param, please. Or perhaps a class attr if there's a reason to.
| if e.errorcode == -1011: | ||
| # The timeout was triggered. Nothing to worry about. | ||
| continue | ||
| elif e.errorcode == -1010: |
There was a problem hiding this comment.
[revision] Let's make an enum/macro for these error codes, please.
| if self.pixel_format == 'mono8': | ||
| pixel_format = self.instrument_lib.PixelFormat_Mono8 | ||
| else: | ||
| pixel_format = self.instrument_lib.PixelFormat_Mono16 |
There was a problem hiding this comment.
[revision] I could be wrong but this entire thing seems very circular and cries for an enum, you've implemented a similar mapping more than once now.
I would make an enum and then just assign a member to self.pixel_format and be done with it.
jamienoss
added
the
progress-stalled
This PR does not contain an emulator for a FLIR camera. It can therefore not be used with the HiCAT simulator. This will be done in a separate PR.