[PATCH v5 02/18] libcamera: Make all internal headers visible to Doxygen

Laurent Pinchart laurent.pinchart at ideasonboard.com
Wed Aug 7 14:50:33 CEST 2024


On Wed, Aug 07, 2024 at 05:59:16AM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2024-08-06 14:39:38)
> > On Tue, Aug 06, 2024 at 02:34:56PM +0100, Daniel Scally wrote:
> > > On 05/08/2024 15:36, Laurent Pinchart wrote:
> > > > Two classes that have both public and internal headers, namely Camera
> > > > and Request, make only their public header visible to Doxygen through a
> > > > file directive. Fix them.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > 
> > > Reviewed-by: Daniel Scally <dan.scally at ideasonboard.com>
> > > 
> > > How did you notice this, out of curiosity? I never saw any Doxygen messages about it.
> > 
> > Just by chance, when looking at the four classes that have public and
> > internal headers of the same name, I realized there was a discrepancy.
> > 
> > > > ---
> > > >   src/libcamera/camera.cpp  | 5 +++++
> > > >   src/libcamera/request.cpp | 5 +++++
> > > >   2 files changed, 10 insertions(+)
> > > >
> > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > > > index 67f3490133b2..f89510ea0472 100644
> > > > --- a/src/libcamera/camera.cpp
> > > > +++ b/src/libcamera/camera.cpp
> > > > @@ -117,6 +117,11 @@
> > > >    * of view is affected by the pipeline.
> > > >    */
> > > >   
> > > > +/**
> > > > + * \file libcamera/internal/camera.h
> > > > + * \brief Internal camera device handling
> > > > + */
> 
> So the file directives cause doxygen to incoud
> 
> Based on https://www.doxygen.nl/manual/commands.html#cmdfile the file
> directive only applies to the comment block it's in as I understand it.
> So this is 'only' adding a brief to the internal headers? Or is it doing
> more like actually pulling the file into the documentation?

I don't remember exactly why the file directive is needed, I just recall
it is for some reason that matters :-)

> Either way, it seems consistent, and meets our coding style requirement
> "Every documented header file shall have a \file documentation block in
> the .cpp source file."
>
> Out of curiousity - what happens if the path is not unique? Is there a
> search order? Is a file in the same location as the .cpp chosen first?

Doxygen complains in that case.

> A quick check highlights :
> 
> kbingham at Monstersaurus:~/iob/libcamera/libcamera-clean$ gg "\\\\file " | sed 's/.*file //' | sort | uniq -c | grep "2 "
>       2 agc.h
>       2 awb.h
>       2 blc.h
>       2 delayed_controls.h
>       2 ipa_context.h
> 
> Does any action need to be taken on those files? 

At least one of each isn't part of the documentation build, so we should
be fine for now.

> Hang on - I'm on 2/18 - I should probably check the rest of the series
> before sending myself on rabbit holes ... ;-)
> 
> Anyway, for this patch 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > > > +
> > > >   namespace libcamera {
> > > >   
> > > >   LOG_DECLARE_CATEGORY(Camera)
> > > > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> > > > index cfb451e908da..fdf12c1e9088 100644
> > > > --- a/src/libcamera/request.cpp
> > > > +++ b/src/libcamera/request.cpp
> > > > @@ -28,6 +28,11 @@
> > > >    * \brief Describes a frame capture request to be processed by a camera
> > > >    */
> > > >   
> > > > +/**
> > > > + * \file libcamera/internal/request.h
> > > > + * \brief Internal support for request handling
> > > > + */
> > > > +
> > > >   namespace libcamera {
> > > >   
> > > >   LOG_DEFINE_CATEGORY(Request)

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list