[libcamera-devel,1/2] licamera: controls: Drop unnecessary template qualifiers in documentation

16693 diff mbox series

The doxygen document blocks of various ControlList function qualify
functions with full template and return type specification. This isn't
needed, and the extra verbosity makes the documentation blocks more
difficult to read. Drop the template qualifiers and return types. The
generated documentation is not affected.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/controls.cpp | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)


base-commit: 62e32042aee4042b6a931bc27a6ad11384b521f5

Hi Laurent

For all the 4/2 patches

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>


On Wed, Jul 20, 2022 at 12:02:57AM +0300, Laurent Pinchart via libcamera-devel wrote:
> The doxygen document blocks of various ControlList function qualify
> functions with full template and return type specification. This isn't
> needed, and the extra verbosity makes the documentation blocks more
> difficult to read. Drop the template qualifiers and return types. The
> generated documentation is not affected.

I wonder if we shouldn't make ControlList a d-pointer class and have
the numerical-id interface available to the library only..

>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/controls.cpp | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 03ac6345247c..3fd535f204b2 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -954,7 +954,7 @@ bool ControlList::contains(unsigned int id) const
>  }
>
>  /**
> - * \fn template<typename T> T ControlList::get(const Control<T> &ctrl) const
> + * \fn ControlList::get(const Control<T> &ctrl) const
>   * \brief Get the value of control \a ctrl
>   * \param[in] ctrl The control
>   *
> @@ -969,7 +969,7 @@ bool ControlList::contains(unsigned int id) const
>   */
>
>  /**
> - * \fn template<typename T, typename V> void ControlList::set(const Control<T> &ctrl, const V &value)
> + * \fn ControlList::set(const Control<T> &ctrl, const V &value)
>   * \brief Set the control \a ctrl value to \a value
>   * \param[in] ctrl The control
>   * \param[in] value The control value
> @@ -983,8 +983,7 @@ bool ControlList::contains(unsigned int id) const
>   */
>
>  /**
> - * \fn template<typename T, typename V> \
> - * void ControlList::set(const Control<T> &ctrl, const std::initializer_list<V> &value)
> + * \fn ControlList::set(const Control<T> &ctrl, const std::initializer_list<V> &value)
>   * \copydoc ControlList::set(const Control<T> &ctrl, const V &value)
>   */
>
>
> base-commit: 62e32042aee4042b6a931bc27a6ad11384b521f5
> --
> Regards,
>
> Laurent Pinchart
>

Hi Jacopo,

On Wed, Jul 20, 2022 at 11:17:50AM +0200, Jacopo Mondi wrote:
> Hi Laurent
> 
> For all the 4/2 patches
> 
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> On Wed, Jul 20, 2022 at 12:02:57AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > The doxygen document blocks of various ControlList function qualify
> > functions with full template and return type specification. This isn't
> > needed, and the extra verbosity makes the documentation blocks more
> > difficult to read. Drop the template qualifiers and return types. The
> > generated documentation is not affected.
> 
> I wonder if we shouldn't make ControlList a d-pointer class and have
> the numerical-id interface available to the library only..

I'm actually thinking about reworking ControlList to be a wrapper around
a buffer that stores all the control values. This would make
serialization much faster, and would also avoid lots of dynamic
allocations. I don't know yet which form this would take though, I
haven't really thought about the design.

> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/controls.cpp | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > index 03ac6345247c..3fd535f204b2 100644
> > --- a/src/libcamera/controls.cpp
> > +++ b/src/libcamera/controls.cpp
> > @@ -954,7 +954,7 @@ bool ControlList::contains(unsigned int id) const
> >  }
> >
> >  /**
> > - * \fn template<typename T> T ControlList::get(const Control<T> &ctrl) const
> > + * \fn ControlList::get(const Control<T> &ctrl) const
> >   * \brief Get the value of control \a ctrl
> >   * \param[in] ctrl The control
> >   *
> > @@ -969,7 +969,7 @@ bool ControlList::contains(unsigned int id) const
> >   */
> >
> >  /**
> > - * \fn template<typename T, typename V> void ControlList::set(const Control<T> &ctrl, const V &value)
> > + * \fn ControlList::set(const Control<T> &ctrl, const V &value)
> >   * \brief Set the control \a ctrl value to \a value
> >   * \param[in] ctrl The control
> >   * \param[in] value The control value
> > @@ -983,8 +983,7 @@ bool ControlList::contains(unsigned int id) const
> >   */
> >
> >  /**
> > - * \fn template<typename T, typename V> \
> > - * void ControlList::set(const Control<T> &ctrl, const std::initializer_list<V> &value)
> > + * \fn ControlList::set(const Control<T> &ctrl, const std::initializer_list<V> &value)
> >   * \copydoc ControlList::set(const Control<T> &ctrl, const V &value)
> >   */
> >
> >
> > base-commit: 62e32042aee4042b6a931bc27a6ad11384b521f5

Hi Laurent,

Thank you for the fixes

On 7/20/22 02:32, Laurent Pinchart via libcamera-devel wrote:
> The doxygen document blocks of various ControlList function qualify
> functions with full template and return type specification. This isn't
> needed, and the extra verbosity makes the documentation blocks more
> difficult to read. Drop the template qualifiers and return types. The
> generated documentation is not affected.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

For all the patches attached in the thread,

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

> ---
>   src/libcamera/controls.cpp | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 03ac6345247c..3fd535f204b2 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -954,7 +954,7 @@ bool ControlList::contains(unsigned int id) const
>   }
>   
>   /**
> - * \fn template<typename T> T ControlList::get(const Control<T> &ctrl) const
> + * \fn ControlList::get(const Control<T> &ctrl) const
>    * \brief Get the value of control \a ctrl
>    * \param[in] ctrl The control
>    *
> @@ -969,7 +969,7 @@ bool ControlList::contains(unsigned int id) const
>    */
>   
>   /**
> - * \fn template<typename T, typename V> void ControlList::set(const Control<T> &ctrl, const V &value)
> + * \fn ControlList::set(const Control<T> &ctrl, const V &value)
>    * \brief Set the control \a ctrl value to \a value
>    * \param[in] ctrl The control
>    * \param[in] value The control value
> @@ -983,8 +983,7 @@ bool ControlList::contains(unsigned int id) const
>    */
>   
>   /**
> - * \fn template<typename T, typename V> \
> - * void ControlList::set(const Control<T> &ctrl, const std::initializer_list<V> &value)
> + * \fn ControlList::set(const Control<T> &ctrl, const std::initializer_list<V> &value)
>    * \copydoc ControlList::set(const Control<T> &ctrl, const V &value)
>    */
>   
>
> base-commit: 62e32042aee4042b6a931bc27a6ad11384b521f5