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

[Jacopo Mondi]

Hi Niklas,

On Wed, Aug 28, 2019 at 11:58:16AM +0200, Niklas Söderlund wrote:
> 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?

I was just reading ipa_module.c and that's what I've found there
/**

 * \file ipa_module.h
 * \brief Image Processing Algorithm module
 */

/**
 * \file ipa_module_info.h
 * \brief Image Processing Algorithm module information
 */

It's not a new thing then

Thanks
   j
>
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190828/027f3700/attachment.sig>