Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/sci cam sign flip #124

Draft
wants to merge 13 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions catkit2/services/flir_camera/flir_camera.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ def make_property_helper(name, read_only=False):
self.height = self.config['height']
self.offset_x = self.config['offset_x']
self.offset_y = self.config['offset_y']
self.rot90 = self.config.get('rot90', False)
self.flip_x = self.config.get('flip_x', False)
self.flip_y = self.config.get('flip_y', False)

self.pixel_format = self.config['pixel_format']
self.adc_bit_depth = self.config['adc_bit_depth']
Expand All @@ -144,6 +147,9 @@ def make_property_helper(name, read_only=False):
make_property_helper('height')
make_property_helper('offset_x')
make_property_helper('offset_y')
make_property_helper('rot90', read_only=True)
make_property_helper('flip_x', read_only=True)
make_property_helper('flip_y', read_only=True)

make_property_helper('sensor_width', read_only=True)
make_property_helper('sensor_height', read_only=True)
Expand Down Expand Up @@ -186,11 +192,18 @@ def acquisition_loop(self):
pixel_format = PySpin.PixelFormat_Mono16

# Make sure the data stream has the right size and datatype.
has_correct_parameters = np.allclose(self.images.shape, [self.height, self.width])
if self.rot90:
has_correct_parameters = np.allclose(self.images.shape, [self.width, self.height])
else:
has_correct_parameters = np.allclose(self.images.shape, [self.height, self.width])

has_correct_parameters = has_correct_parameters and (self.images.dtype == pixel_dtype)

if not has_correct_parameters:
self.images.update_parameters(pixel_dtype, [self.height, self.width], self.NUM_FRAMES_IN_BUFFER)
if self.rot90:
self.images.update_parameters(pixel_dtype, [self.width, self.height], self.NUM_FRAMES_IN_BUFFER)
else:
self.images.update_parameters(pixel_dtype, [self.height, self.width], self.NUM_FRAMES_IN_BUFFER)

self.cam.BeginAcquisition()
self.is_acquiring.submit_data(np.array([1], dtype='int8'))
Expand All @@ -214,7 +227,7 @@ def acquisition_loop(self):
continue

img = image_result.Convert(pixel_format).GetNDArray().astype(pixel_dtype, copy=False)

img = self.rot_flip_image(img)
# Submit image to datastream.
self.images.submit_data(img)

Expand Down Expand Up @@ -263,6 +276,17 @@ def get_temperature(self):
with self.mutex:
return self.cam.DeviceTemperature.GetValue()

def rot_flip_image(self, img):
# rotation needs to happen first
if self.rot90:
img = np.rot90(img)
if self.flip_x:
img = np.flipud(img)
if self.flip_y:
img = np.fliplr(img)
return img
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does not result in C Contiguous array - working on it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is expected and desired. This is what makes it fast to run multiple of these operations in a row.



if __name__ == '__main__':
service = FlirCamera()
service.run()
124 changes: 117 additions & 7 deletions catkit2/services/zwo_camera/zwo_camera.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,25 @@ def open(self):
# Set image format to be RAW16, although camera is only 12-bit.
self.camera.set_image_type(zwoasi.ASI_IMG_RAW16)

# Set device values from config file (set width and height before offsets)
# Set device values from config file.
offset_x = self.config.get('offset_x', 0)
offset_y = self.config.get('offset_y', 0)
self.rot90 = self.config.get('rot90', False)
self.flip_x = self.config.get('flip_x', False)
self.flip_y = self.config.get('flip_y', False)

self.width = self.config.get('width', self.sensor_width - offset_x)
self.height = self.config.get('height', self.sensor_height - offset_y)
Comment on lines 126 to 127
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't the width/height properties need to be changed as well? To exchange width/height that is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if rot90 is True I updated the code so that the width and height properties are swapped. This could cause issues though if the get_camera_offset function hasn't been called yet (since it needs to know the previous origin location) so I also added an in-line comment to warn agains this.

self.offset_x = offset_x
self.offset_y = offset_y

self.offset_x, self.offset_y = self.get_camera_offset(offset_x, offset_y)

# Note that get_camera_offset must be called before updating the values of self.width and self.height.
if self.rot90:
# If rotating by 90 degrees swap the values for width and height.
w = self.width
h = self.height
self.width = h
self.height = w

self.gain = self.config.get('gain', 0)
self.exposure_time_step_size = self.config.get('exposure_time_step_size', 1)
Expand All @@ -133,7 +144,13 @@ def open(self):

# Create datastreams
# Use the full sensor size here to always allocate enough shared memory.
self.images = self.make_data_stream('images', 'float32', [self.sensor_height, self.sensor_width], self.NUM_FRAMES_IN_BUFFER)
# If the sensor is rotated by 90 degrees, make sure to flip the axes in the datastream
if self.rot90:
self.images = self.make_data_stream('images', 'float32', [self.sensor_width, self.sensor_height],
self.NUM_FRAMES_IN_BUFFER)
else:
self.images = self.make_data_stream('images', 'float32', [self.sensor_height, self.sensor_width],
self.NUM_FRAMES_IN_BUFFER)
self.temperature = self.make_data_stream('temperature', 'float64', [1], self.NUM_FRAMES_IN_BUFFER)

self.is_acquiring = self.make_data_stream('is_acquiring', 'int8', [1], self.NUM_FRAMES_IN_BUFFER)
Expand Down Expand Up @@ -162,6 +179,9 @@ def setter(val):
make_property_helper('height', requires_stopped_acquisition=True)
make_property_helper('offset_x')
make_property_helper('offset_y')
make_property_helper('rot90', read_only=True)
make_property_helper('flip_x', read_only=True)
make_property_helper('flip_y', read_only=True)
Comment on lines +182 to +184
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need these as properties? Or can we have them in the CameraProxy object instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have an intuition for what the pros and cons of having them in the CameraProxy vs. here would be. I agree though they don't necessarily have to be properties since they're only used in the rot_flip_image and get_camera_offset functions and could easily be key word arguments in there

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To give some context, I'm not talking about Python properties. I'm talking about catkit2 properties, which is what the make_property_helper() function is creating. Those properties are available from the outside, ie. from client-side. Communication is done via TCP, which is not necessary. Since they are read-only and they are never meant to be changed/changable, their value can be purely gotten from the config file. Therefore, no communication between ServiceProxy and Service needs to take place, making it much easier to get the right value straight from the config in the ServiceProxy.


make_property_helper('sensor_width', read_only=True)
make_property_helper('sensor_height', read_only=True)
Expand All @@ -186,10 +206,16 @@ def close(self):

def acquisition_loop(self):
# Make sure the data stream has the right size and datatype.
has_correct_parameters = np.allclose(self.images.shape, [self.height, self.width])
if self.rot90:
has_correct_parameters = np.allclose(self.images.shape, [self.width, self.height])
else:
has_correct_parameters = np.allclose(self.images.shape, [self.height, self.width])

if not has_correct_parameters:
self.images.update_parameters('float32', [self.height, self.width], self.NUM_FRAMES_IN_BUFFER)
if self.rot90:
self.images.update_parameters('float32', [self.width, self.height], self.NUM_FRAMES_IN_BUFFER)
else:
self.images.update_parameters('float32', [self.height, self.width], self.NUM_FRAMES_IN_BUFFER)

# Start acquisition.
self.camera.start_video_capture()
Expand All @@ -200,7 +226,8 @@ def acquisition_loop(self):
try:
while self.should_be_acquiring.is_set() and not self.should_shut_down:
img = self.camera.capture_video_frame(timeout=timeout)

# make sure image has proper rotation and flips for the camera
img = self.rot_flip_image(img)
self.images.submit_data(img.astype('float32'))
finally:
# Stop acquisition.
Expand Down Expand Up @@ -316,6 +343,89 @@ def offset_y(self):
def offset_y(self, offset_y):
self.camera.set_roi_start_position(self.offset_x, offset_y)

def rot_flip_image(self, img):
# rotation needs to happen first
if self.rot90:
img = np.rot90(img)
if self.flip_x:
img = np.flipud(img)
if self.flip_y:
img = np.fliplr(img)
return np.ascontiguousarray(img)

def get_camera_offset(self, x, y):
"""Convert relative camera offsets given by the user to absolute offsets in camera array coordinates.

This is done by performing the following procedure:
1.) Translate the origin to the center of the ROI.
2.) if rot90 is True, rotate 90 degrees counter-clockwise about the center of the ROI
3.) Translate the origin back to the upper left fo the ROI.
4.) If flip_x is True, reflect in x.
5.) If flip_y is True, reflect in y.

Parameters
----------
x: float
The x-offset coordinate
y: float
The y-offset coordinate
Returns
-------
new_x, new_y: float, float
The transformed x and y coordinates for their location in camera array coordinates. If there is no rotation
or flip in x or y, then this returns the same x, y values that are input.
"""
# Define the translation matrix T to get to the center of the ROI.
T = np.zeros((3, 3))
np.fill_diagonal(T, 1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd construct the matrix with np.eye(3) instead rather than creating them as zeros and then filling the diagonal with ones. The same for the other matrices.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! I forgot np.eye existed - updated that code for all the matrices where I wanted diagonals filled

T[0][-1] = -self.width / 2
T[1][-1] = -self.height / 2

# Initialize rotation matrix R.
R = np.zeros((3, 3))

# Initialize x reflection matrix, X. Defaults to unity matrix.
X = np.eye(3, 3)

# Initialize y reflection matrix, Y. Defaults to unity matrix.
Y = np.eye(3, 3)

if self.rot90:
# Define rotation matrix.
R[0][1] = -1
R[1][0] = 1
R[2][2] = 1
else:
# Default to unity matrix.
np.fill_diagonal(R, 1)

if self.flip_x:
# Define x reflection matrix.
X[0][0] = -1

if self.flip_y:
# Define y reflection matrix.
Y[1][1] = -1
Comment on lines +402 to +408
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't the flips also need translation matrices, since the flips are reflections around the centerlines of the images?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! moved the final translation to after the x and y flips


# Define translation matrix back so that the origin is in the upper left as expected.
T_back = np.eye(3, 3)

if self.rot90:
# Want to come back to new origin for which the height/width dimensions will be flipped if rotated.
T_back[0][-1] = self.height / 2
T_back[1][-1] = self.width / 2
else:
T_back[0][-1] = self.width / 2
T_back[1][-1] = self.height / 2

# Perform the dot product. First flip in x to establish top left origin, then translate to ROI center, rotate,
# translate back to origin, flip in x, and finally flip in y.
coords = [x, y, 1]
new_coords = np.linalg.multi_dot([T_back, Y, X, R, T, coords])

return new_coords[0], new_coords[1]


if __name__ == '__main__':
service = ZwoCamera()
service.run()
Loading