-
Notifications
You must be signed in to change notification settings - Fork 4
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
Move camera configurations close to camera classes #74
Conversation
Before:
import camera_configurations
import camera_factory
...
if camera_selection == 0:
config = camera_configurations.ConfigCamera0()
elif camera_selection == 1:
config = camera_configurations.ConfigCamera1()
elif ...
...
camera_device = camera_factory(..., config) After:
import camera0
import camera1
...
import camera_factory
...
if camera_selection == 0:
config = camera0.ConfigCamera0()
elif camera_selection == 1:
config = camera1.ConfigCamera1()
elif ...
...
camera_device = camera_factory(..., config) |
9f88f1a
to
43542cb
Compare
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.
Reviewed
modules/camera/camera_picamera2.py
Outdated
timeout: Getting image timeout in seconds. | ||
|
||
exposure_time: Microseconds. | ||
analogue_gain: |
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.
A part of ISO, analogueGain * DigitialGain * 100 = ISO. From 0 to suggested max is 16, but can go up to 64.
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.
I'm not sure I understand the 2nd part. The acceptable range of values is 0 to 64, but the suggested range to use is 0 to 16 ? Why is the default argument 64 then?
modules/camera/camera_picamera2.py
Outdated
|
||
if self.maybe_lens_position is not None: | ||
camera_controls["LensPosition"] = self.maybe_lens_position | ||
camera_controls["AfMode"] = picamera2.controls.AfModeEnum.Manual |
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.
I think this needs an import: from libcamera import controls
, then it's just controls.AfModeEnum...
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.
Done.
modules/camera/camera_picamera2.py
Outdated
camera_controls["AfMode"] = picamera2.controls.AfModeEnum.Manual | ||
else: | ||
camera_controls["LensPosition"] = 0.0 | ||
camera_controls["AfMode"] = picamera2.controls.AfModeEnum.Auto |
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.
Same with this one
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.
Done.
modules/camera/camera_picamera2.py
Outdated
exposure_time: Microseconds. | ||
analogue_gain: | ||
contrast: 0.0 to 32.0 . 0.0 is no contrast, 1.0 is normal contrast, higher is more contrast. | ||
lens_position: Position of the lens is dioptres (reciprocal of metres: 1/m ). |
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.
(0 means infinity)
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.
Done.
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.
Approved
Improve locality of code (LOC) by moving the configuration classes close to the camera classes. The tradeoff is that users are required to import all camera classes for configurations. A possible mitigation is to generate the configuration from a common data structure (e.g. dictionary from YAML).