[libcamera-devel] [PATCH v2 2/7] libcamera: controls: Document control_ids.h

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2019-08-27 11:50:02 +0200, Jacopo Mondi wrote:
> The control identifiers documentation was not generated as they're not
> part of the controls.h file documented in controls.cpp.
> 
> Move documentation for control id to the end of the controls.cpp and
> document the control_ids.h file to have documentation for controls
> properly generated.

I'm a bit debated about this solution. I see why the change is needed 
but I wonder if it's not nicer to create a control_ids.cpp file to hold 
the documentation?

As this is as far as I know the first time we document two header files 
in one cpp file I want to know what others think about this practice.  
I'm not strongly against it, but found myself reading the diff 3 times 
before I understood it ;-)

If the consensus is that we should document multiple header files in one 
cpp file feel free to add (with the typo bellow fixed),

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/controls.cpp | 100 ++++++++++++++++++++-----------------
>  1 file changed, 53 insertions(+), 47 deletions(-)
> 
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 727fdbd9450d..9adc3badc254 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -181,53 +181,6 @@ std::string ControlValue::toString() const
>  	return "<ValueType Error>";
>  }
>  
> -/**
> - * \enum ControlId
> - * \brief Numerical control ID
> - */
> -
> -/**
> - * \var AwbEnable
> - * ControlType: Bool
> - *
> - * Enables or disables the AWB. See also \a libcamera::ControlId::ManualGain
> - */
> -
> -/**
> - * \var Brightness
> - * ControlType: Integer
> - *
> - * Specify a fixed brightness parameter.
> - */
> -
> -/**
> - * \var Contrast
> - * ControlType: Integer
> - *
> - * Specify a fixed contrast parameter.
> - */
> -
> -/**
> - * \var Saturation
> - * ControlType: Integer
> - *
> - * Specify a fixed saturation parameter.
> - */
> -
> -/**
> - * \var ManualExposure
> - * ControlType: Integer
> - *
> - * Specify a fixed exposure time in milli-seconds
> - */
> -
> -/**
> - * \var ManualGain
> - * ControlType: Integer
> - *
> - * Specify a fixed gain parameter
> - */
> -
>  /**
>   * \struct ControlIdentifier
>   * \brief Describe a ControlId with control specific constant meta-data
> @@ -549,4 +502,57 @@ void ControlList::update(const ControlList &other)
>  	}
>  }
>  
> +/**
> + * \file control_ids.h
> + * \brief Definition of numerical identifiers for libcamera control and
> + * properties

s/control/controls/

> + */
> +
> +/**
> + * \enum ControlId
> + * \brief Numerical control ID
> + */
> +
> +/**
> + * \var AwbEnable
> + * ControlType: Bool
> + *
> + * Enables or disables the AWB. See also \a libcamera::ControlId::ManualGain
> + */
> +
> +/**
> + * \var Brightness
> + * ControlType: Integer
> + *
> + * Specify a fixed brightness parameter.
> + */
> +
> +/**
> + * \var Contrast
> + * ControlType: Integer
> + *
> + * Specify a fixed contrast parameter.
> + */
> +
> +/**
> + * \var Saturation
> + * ControlType: Integer
> + *
> + * Specify a fixed saturation parameter.
> + */
> +
> +/**
> + * \var ManualExposure
> + * ControlType: Integer
> + *
> + * Specify a fixed exposure time in milli-seconds
> + */
> +
> +/**
> + * \var ManualGain
> + * ControlType: Integer
> + *
> + * Specify a fixed gain parameter
> + */
> +
>  } /* namespace libcamera */
> -- 
> 2.23.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund