From 84494b82d0ffac7f81cd451427bf44cfd8559042 Mon Sep 17 00:00:00 2001 From: Andy Sweet Date: Wed, 27 Sep 2023 15:00:05 -0700 Subject: [PATCH 1/3] Add initial header comments --- src/acquire-device-kit/device/kit/camera.h | 22 ++++++++++++- .../device/props/camera.h | 32 +++++++++++++++---- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/acquire-device-kit/device/kit/camera.h b/src/acquire-device-kit/device/kit/camera.h index 9f554cc..e21aeb4 100644 --- a/src/acquire-device-kit/device/kit/camera.h +++ b/src/acquire-device-kit/device/kit/camera.h @@ -14,20 +14,40 @@ extern "C" struct Device device; enum DeviceState state; + /// Attempts to set the given properties on the given camera. + /// TODO: this will attempt to set as many properties values + /// as it can. Some properties (e.g. shape and offset) may be + /// coupled - these will be either set atomically or in the + /// case that one value is not compatible with others, not + /// set at all. + /// TODO: should we define a force_set? or define a way to clear + /// a settings cache? + /// TODO: should we return something to indicate which properties + /// were set? Should we define a setget? enum DeviceStatusCode (*set)(struct Camera*, struct CameraProperties* settings); + /// Fills out the given properties from the given camera. enum DeviceStatusCode (*get)(const struct Camera*, struct CameraProperties* settings); + /// Fills out the given property metadata from the given camera. + /// TODO: should each call to the same camera instance fill out the + /// the same values? Or should metadata/capabilities depend on + /// current property values? enum DeviceStatusCode (*get_meta)(const struct Camera*, struct CameraPropertyMetadata* meta); + /// The shape of the next frame that the camera will capture based on its current properties. enum DeviceStatusCode (*get_shape)(const struct Camera*, struct ImageShape* shape); + /// Starts acquiring frames. enum DeviceStatusCode (*start)(struct Camera*); + /// Stops acquiring frames. + /// TODO: does this wait/block until any pending frames have been acquired? enum DeviceStatusCode (*stop)(struct Camera*); - // Fire the software trigger if it's enabled. + /// Fire the software trigger if it's enabled. enum DeviceStatusCode (*execute_trigger)(struct Camera*); + /// Gets the next frame from the camera. enum DeviceStatusCode (*get_frame)(struct Camera*, void* im, size_t* nbytes, diff --git a/src/acquire-device-properties/device/props/camera.h b/src/acquire-device-properties/device/props/camera.h index e5ab537..f04b223 100644 --- a/src/acquire-device-properties/device/props/camera.h +++ b/src/acquire-device-properties/device/props/camera.h @@ -11,17 +11,35 @@ extern "C" { #endif + /// @brief Stores the properties of a camera. + /// @details This can be populated with values from a camera. + /// Or can be filled out to request new values a camera should adopt. struct CameraProperties { + /// @brief The exposure time of a frame in microseconds. + /// @details Acquire assumes that exposure is always manually specified by this period of time. + /// It does not currently support auto-exposure or durations defined by trigger widths. float exposure_time_us; + float line_interval_us; enum Direction readout_direction; + + /// @brief The binning factor, which determines how many pixels in each spatial dimension on the sensor are aggregated to form pixels in an image. + /// @details For example, if we have a sensor of size 1920x1200 and a binning factor of 2, one image should be 960x600. uint8_t binning; + + /// @brief The type of each image pixel or sample. enum SampleType pixel_type; + + /// @brief The offset of the active image region of interest on the sensor from its top-left corner. + /// @details Each value represents a number of aggregated pixels in the frame after binning has been applied. struct camera_properties_offset_s { uint32_t x, y; } offset; + + /// @brief The shape or size of the active region of interest on the sensor. + /// Each value represents a number of aggregated pixels in the frame after binning has been applied. struct camera_properties_shape_s { uint32_t x, y; @@ -36,6 +54,9 @@ extern "C" } output_triggers; }; + /// @brief Stores the metadata about camera properties. + /// @details This is only used to request metadata from a camera, which + /// expresses capabilities of the camera and acceptable property values. struct CameraPropertyMetadata { struct Property exposure_time_us; @@ -56,12 +77,11 @@ extern "C" struct CameraPropertyMetadataDigitalLineMetadata { - /// The number of supported digital IO lines - /// Must be less than 8. + /// @brief The number of supported digital IO lines. Must be less than 8. uint8_t line_count; - /// name[i] is a short, null terminated string naming line i. - /// Support describing up to 8 names for use with triggering. + /// @brief Support describing up to 8 names for use with triggering. + /// @details name[i] is a short, null terminated string naming line i. char names[8][64]; } digital_lines; @@ -69,9 +89,9 @@ extern "C" { struct camera_properties_metadata_trigger_capabilities_s { - /// Bit x is set if line x can be used as a trigger input. + /// @brief Bit x is set if line x can be used as a trigger input. uint8_t input; - /// Bit x is set if line x can be used as a trigger output. + /// @brief Bit x is set if line x can be used as a trigger output. uint8_t output; } acquisition_start, exposure, frame_start; } triggers; From 0c8d2262e2343dd3110f15f10d9d47b6d96245c0 Mon Sep 17 00:00:00 2001 From: Andy Sweet Date: Wed, 15 Nov 2023 16:51:44 -0800 Subject: [PATCH 2/3] Clean up TODOs --- src/acquire-device-kit/device/kit/camera.h | 43 ++++++++++--------- .../device/props/camera.h | 40 ++++++++++------- 2 files changed, 47 insertions(+), 36 deletions(-) diff --git a/src/acquire-device-kit/device/kit/camera.h b/src/acquire-device-kit/device/kit/camera.h index e21aeb4..0db0701 100644 --- a/src/acquire-device-kit/device/kit/camera.h +++ b/src/acquire-device-kit/device/kit/camera.h @@ -9,45 +9,46 @@ extern "C" { #endif + /// @brief Represents and allows control of a camera device. struct Camera { struct Device device; enum DeviceState state; - /// Attempts to set the given properties on the given camera. - /// TODO: this will attempt to set as many properties values - /// as it can. Some properties (e.g. shape and offset) may be - /// coupled - these will be either set atomically or in the - /// case that one value is not compatible with others, not - /// set at all. - /// TODO: should we define a force_set? or define a way to clear - /// a settings cache? - /// TODO: should we return something to indicate which properties - /// were set? Should we define a setget? + /// @brief Attempts to set the given properties on the given camera. + /// @details Setting properties can partially succeed depending on + /// other property values or the state of the camera. + /// If you want to be certain that a specific property was + /// actually used, you should call get afterwards to check + /// that property value. enum DeviceStatusCode (*set)(struct Camera*, struct CameraProperties* settings); - /// Fills out the given properties from the given camera. + + /// @brief Fills out the given properties from the given camera. enum DeviceStatusCode (*get)(const struct Camera*, struct CameraProperties* settings); - /// Fills out the given property metadata from the given camera. - /// TODO: should each call to the same camera instance fill out the - /// the same values? Or should metadata/capabilities depend on - /// current property values? + + /// @brief Fills out the given property metadata from the given camera. + /// @details These metadata or capabilities may depend on the camera's + /// current property values. enum DeviceStatusCode (*get_meta)(const struct Camera*, struct CameraPropertyMetadata* meta); - /// The shape of the next frame that the camera will capture based on its current properties. + + /// @brief The shape of the next frame that the camera will capture. enum DeviceStatusCode (*get_shape)(const struct Camera*, struct ImageShape* shape); - /// Starts acquiring frames. + /// @brief Starts acquiring frames. enum DeviceStatusCode (*start)(struct Camera*); - /// Stops acquiring frames. - /// TODO: does this wait/block until any pending frames have been acquired? + + /// @brief Stops acquiring frames. + /// @details This is not guaranteed to wait or block until any pending + /// frames have been acquired or the camera is actually stopped. enum DeviceStatusCode (*stop)(struct Camera*); - /// Fire the software trigger if it's enabled. + /// @brief Execute the software trigger if it's enabled. enum DeviceStatusCode (*execute_trigger)(struct Camera*); - /// Gets the next frame from the camera. + /// @brief Gets the next frame from the camera. enum DeviceStatusCode (*get_frame)(struct Camera*, void* im, size_t* nbytes, diff --git a/src/acquire-device-properties/device/props/camera.h b/src/acquire-device-properties/device/props/camera.h index f04b223..6000b56 100644 --- a/src/acquire-device-properties/device/props/camera.h +++ b/src/acquire-device-properties/device/props/camera.h @@ -12,11 +12,11 @@ extern "C" #endif /// @brief Stores the properties of a camera. - /// @details This can be populated with values from a camera. - /// Or can be filled out to request new values a camera should adopt. + /// @details Can be populated with values from a camera or + /// can be filled out to define new values that a camera should adopt. struct CameraProperties { - /// @brief The exposure time of a frame in microseconds. + /// @brief Exposure time of a frame in microseconds. /// @details Acquire assumes that exposure is always manually specified by this period of time. /// It does not currently support auto-exposure or durations defined by trigger widths. float exposure_time_us; @@ -24,30 +24,39 @@ extern "C" float line_interval_us; enum Direction readout_direction; - /// @brief The binning factor, which determines how many pixels in each spatial dimension on the sensor are aggregated to form pixels in an image. - /// @details For example, if we have a sensor of size 1920x1200 and a binning factor of 2, one image should be 960x600. + /// @brief Binning or downsample factor. + /// @details Determines how many pixels in each spatial dimension on the + /// sensor are aggregated to form pixels in an image. + /// For example, if we have a sensor of size 1920x1200 and a binning + /// factor of 2, one image should be 960x600. uint8_t binning; - /// @brief The type of each image pixel or sample. + /// @brief Type of each image pixel or sample. enum SampleType pixel_type; - /// @brief The offset of the active image region of interest on the sensor from its top-left corner. - /// @details Each value represents a number of aggregated pixels in the frame after binning has been applied. + /// @brief Offset of the region of interest on the sensor from its top-left corner. + /// @details Each value represents a number of aggregated pixels in the + /// frame after binning has been applied. struct camera_properties_offset_s { uint32_t x, y; } offset; - /// @brief The shape or size of the active region of interest on the sensor. - /// Each value represents a number of aggregated pixels in the frame after binning has been applied. + /// @brief Shape or size of the region of interest on the sensor. + /// @details Each value represents a number of aggregated pixels in the + /// frame after binning has been applied. struct camera_properties_shape_s { uint32_t x, y; } shape; + + /// @brief State of the camera's input triggers. struct camera_properties_input_triggers_s { struct Trigger acquisition_start, frame_start, exposure; } input_triggers; + + /// @brief State of the camera's digital output lines. struct camera_properties_output_triggers_s { struct Trigger exposure, frame_start, trigger_wait; @@ -55,8 +64,8 @@ extern "C" }; /// @brief Stores the metadata about camera properties. - /// @details This is only used to request metadata from a camera, which - /// expresses capabilities of the camera and acceptable property values. + /// @details Only used to request metadata from a camera, which expresses + /// capabilities of the camera and acceptable property values. struct CameraPropertyMetadata { struct Property exposure_time_us; @@ -72,7 +81,8 @@ extern "C" struct Property x, y; } shape; - /// bit field: bit i is 1 if SampleType(i) is supported, 0 otherwise + /// @brief The bits of this value describe the supported types. + /// @details Bit i is 1 if SampleType(i) is supported, 0 otherwise. uint64_t supported_pixel_types; struct CameraPropertyMetadataDigitalLineMetadata @@ -89,9 +99,9 @@ extern "C" { struct camera_properties_metadata_trigger_capabilities_s { - /// @brief Bit x is set if line x can be used as a trigger input. + /// @brief Bit i is set if line i can be used as a trigger input. uint8_t input; - /// @brief Bit x is set if line x can be used as a trigger output. + /// @brief Bit i is set if line i can be used as a trigger output. uint8_t output; } acquisition_start, exposure, frame_start; } triggers; From 5dc53154be8d8199131029e99c6824d2f5e6463e Mon Sep 17 00:00:00 2001 From: Andy Sweet Date: Thu, 14 Dec 2023 09:08:00 -0800 Subject: [PATCH 3/3] Update stop details based on feedback --- src/acquire-device-kit/device/kit/camera.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/acquire-device-kit/device/kit/camera.h b/src/acquire-device-kit/device/kit/camera.h index 0db0701..741e480 100644 --- a/src/acquire-device-kit/device/kit/camera.h +++ b/src/acquire-device-kit/device/kit/camera.h @@ -41,8 +41,10 @@ extern "C" enum DeviceStatusCode (*start)(struct Camera*); /// @brief Stops acquiring frames. - /// @details This is not guaranteed to wait or block until any pending - /// frames have been acquired or the camera is actually stopped. + /// @details This instructs the camera to stop and may block until it + /// actually has stopped acquiring frames. + /// The camera should also be restartable after calling this + /// (i.e. one call of start after one call of stop should succeed). enum DeviceStatusCode (*stop)(struct Camera*); /// @brief Execute the software trigger if it's enabled.