-
Notifications
You must be signed in to change notification settings - Fork 0
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
base: develop
Are you sure you want to change the base?
Changes from all commits
d77cc6e
80eccb1
5f191c2
c57a239
9929bff
d4cd568
04976f7
ac33100
e611147
dedbc6b
4533d5d
cc977fe
f1affe5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
@@ -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) | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
@@ -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() | ||
|
@@ -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. | ||
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd construct the matrix with There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.