| Message ID | 20240805143654.20870-6-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Laurent Pinchart (2024-08-05 15:36:41) > The *::Private classes are part of the internal API, as their name > implies. They are defined in internal headers, but implemented in the > same source file as their public counterparts. This will cause Doxygen > to complain about missing class definition when splitting the public and > internal API documents, as the internal headers won't be parsed by > Doxygen for the public API documentation. > > Marking the classes with \internal isn't enough. The directive prevents > the documentation block from being included in the output, but this Does \private or \privatesection do anything here? or does that still lead to the same issue as it will just mark them as 'private' and still expect to find the definitions? > occurs at the generation stage, after the documentation blocks are > parsed. Fix this by completely hidding the implementation of the > *::Private classes from Doxygen using preprocessor conditional > compilation. To do so, introduce a new macro, __DOXYGEN_PUBLIC__, that > will be defined for the public API documentation only. It's a shame to be adding ifdefery. But understandable if this is the best way forwards. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/camera.cpp | 2 ++ > src/libcamera/camera_manager.cpp | 2 ++ > src/libcamera/framebuffer.cpp | 2 ++ > src/libcamera/request.cpp | 2 ++ > 4 files changed, 8 insertions(+) > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 3c8f30d54401..aca466c9ba72 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -565,6 +565,7 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF > * \brief The vector of stream configurations > */ > > +#ifndef __DOXYGEN_PUBLIC__ > /** > * \class Camera::Private > * \brief Base class for camera private data > @@ -725,6 +726,7 @@ void Camera::Private::setState(State state) > { > state_.store(state, std::memory_order_release); > } > +#endif /* __DOXYGEN_PUBLIC__ */ > > /** > * \class Camera > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp > index 5a21132a08c2..fa44e33a135d 100644 > --- a/src/libcamera/camera_manager.cpp > +++ b/src/libcamera/camera_manager.cpp > @@ -35,6 +35,7 @@ namespace libcamera { > > LOG_DEFINE_CATEGORY(Camera) > > +#ifndef __DOXYGEN_PUBLIC__ > CameraManager::Private::Private() > : initialized_(false) > { > @@ -249,6 +250,7 @@ void CameraManager::Private::removeCamera(std::shared_ptr<Camera> camera) > CameraManager *const o = LIBCAMERA_O_PTR(); > o->cameraRemoved.emit(camera); > } > +#endif /* __DOXYGEN_PUBLIC__ */ > > /** > * \class CameraManager > diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp > index f39db1223432..826848f75a56 100644 > --- a/src/libcamera/framebuffer.cpp > +++ b/src/libcamera/framebuffer.cpp > @@ -107,6 +107,7 @@ LOG_DEFINE_CATEGORY(Buffer) > * \return The array of per-plane metadata > */ > > +#ifndef __DOXYGEN_PUBLIC__ > /** > * \class FrameBuffer::Private > * \brief Base class for FrameBuffer private data > @@ -209,6 +210,7 @@ FrameBuffer::Private::~Private() > * \brief Retrieve the dynamic metadata > * \return Dynamic metadata for the frame contained in the buffer > */ > +#endif /* __DOXYGEN_PUBLIC__ */ > > /** > * \class FrameBuffer > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > index 24fa3a57312e..8c56ed30d0c4 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -38,6 +38,7 @@ namespace libcamera { > > LOG_DEFINE_CATEGORY(Request) > > +#ifndef __DOXYGEN_PUBLIC__ > /** > * \class Request::Private > * \brief Request private data > @@ -306,6 +307,7 @@ void Request::Private::timeout() > > emitPrepareCompleted(); > } > +#endif /* __DOXYGEN_PUBLIC__ */ > > /** > * \enum Request::Status > -- > Regards, > > Laurent Pinchart >
Hi Laurent On 05/08/2024 15:36, Laurent Pinchart wrote: > The *::Private classes are part of the internal API, as their name > implies. They are defined in internal headers, but implemented in the > same source file as their public counterparts. This will cause Doxygen > to complain about missing class definition when splitting the public and > internal API documents, as the internal headers won't be parsed by > Doxygen for the public API documentation. > > Marking the classes with \internal isn't enough. The directive prevents > the documentation block from being included in the output, but this > occurs at the generation stage, after the documentation blocks are > parsed. Fix this by completely hidding the implementation of the > *::Private classes from Doxygen using preprocessor conditional > compilation. To do so, introduce a new macro, __DOXYGEN_PUBLIC__, that > will be defined for the public API documentation only. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> This is fine too - I think I included them because I couldn't figure out a doxygen-only way of excluding them...but this way works. Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > src/libcamera/camera.cpp | 2 ++ > src/libcamera/camera_manager.cpp | 2 ++ > src/libcamera/framebuffer.cpp | 2 ++ > src/libcamera/request.cpp | 2 ++ > 4 files changed, 8 insertions(+) > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 3c8f30d54401..aca466c9ba72 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -565,6 +565,7 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF > * \brief The vector of stream configurations > */ > > +#ifndef __DOXYGEN_PUBLIC__ > /** > * \class Camera::Private > * \brief Base class for camera private data > @@ -725,6 +726,7 @@ void Camera::Private::setState(State state) > { > state_.store(state, std::memory_order_release); > } > +#endif /* __DOXYGEN_PUBLIC__ */ > > /** > * \class Camera > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp > index 5a21132a08c2..fa44e33a135d 100644 > --- a/src/libcamera/camera_manager.cpp > +++ b/src/libcamera/camera_manager.cpp > @@ -35,6 +35,7 @@ namespace libcamera { > > LOG_DEFINE_CATEGORY(Camera) > > +#ifndef __DOXYGEN_PUBLIC__ > CameraManager::Private::Private() > : initialized_(false) > { > @@ -249,6 +250,7 @@ void CameraManager::Private::removeCamera(std::shared_ptr<Camera> camera) > CameraManager *const o = LIBCAMERA_O_PTR(); > o->cameraRemoved.emit(camera); > } > +#endif /* __DOXYGEN_PUBLIC__ */ > > /** > * \class CameraManager > diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp > index f39db1223432..826848f75a56 100644 > --- a/src/libcamera/framebuffer.cpp > +++ b/src/libcamera/framebuffer.cpp > @@ -107,6 +107,7 @@ LOG_DEFINE_CATEGORY(Buffer) > * \return The array of per-plane metadata > */ > > +#ifndef __DOXYGEN_PUBLIC__ > /** > * \class FrameBuffer::Private > * \brief Base class for FrameBuffer private data > @@ -209,6 +210,7 @@ FrameBuffer::Private::~Private() > * \brief Retrieve the dynamic metadata > * \return Dynamic metadata for the frame contained in the buffer > */ > +#endif /* __DOXYGEN_PUBLIC__ */ > > /** > * \class FrameBuffer > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > index 24fa3a57312e..8c56ed30d0c4 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -38,6 +38,7 @@ namespace libcamera { > > LOG_DEFINE_CATEGORY(Request) > > +#ifndef __DOXYGEN_PUBLIC__ > /** > * \class Request::Private > * \brief Request private data > @@ -306,6 +307,7 @@ void Request::Private::timeout() > > emitPrepareCompleted(); > } > +#endif /* __DOXYGEN_PUBLIC__ */ > > /** > * \enum Request::Status
On Wed, Aug 07, 2024 at 06:16:38AM +0100, Kieran Bingham wrote: > Quoting Laurent Pinchart (2024-08-05 15:36:41) > > The *::Private classes are part of the internal API, as their name > > implies. They are defined in internal headers, but implemented in the > > same source file as their public counterparts. This will cause Doxygen > > to complain about missing class definition when splitting the public and > > internal API documents, as the internal headers won't be parsed by > > Doxygen for the public API documentation. > > > > Marking the classes with \internal isn't enough. The directive prevents > > the documentation block from being included in the output, but this > > Does \private or \privatesection do anything here? or does that still > lead to the same issue as it will just mark them as 'private' and still > expect to find the definitions? \private is for private members in the C++ sense. I don't see how it could be used here. > > occurs at the generation stage, after the documentation blocks are > > parsed. Fix this by completely hidding the implementation of the > > *::Private classes from Doxygen using preprocessor conditional > > compilation. To do so, introduce a new macro, __DOXYGEN_PUBLIC__, that > > will be defined for the public API documentation only. > > It's a shame to be adding ifdefery. But understandable if this is the > best way forwards. I don't like it much either, but it's the best option I could find. We have few public classes with Private data, so I think I can live with this. Going forward, once we'll have a C API, Private will disappear anyway. > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/libcamera/camera.cpp | 2 ++ > > src/libcamera/camera_manager.cpp | 2 ++ > > src/libcamera/framebuffer.cpp | 2 ++ > > src/libcamera/request.cpp | 2 ++ > > 4 files changed, 8 insertions(+) > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > index 3c8f30d54401..aca466c9ba72 100644 > > --- a/src/libcamera/camera.cpp > > +++ b/src/libcamera/camera.cpp > > @@ -565,6 +565,7 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF > > * \brief The vector of stream configurations > > */ > > > > +#ifndef __DOXYGEN_PUBLIC__ > > /** > > * \class Camera::Private > > * \brief Base class for camera private data > > @@ -725,6 +726,7 @@ void Camera::Private::setState(State state) > > { > > state_.store(state, std::memory_order_release); > > } > > +#endif /* __DOXYGEN_PUBLIC__ */ > > > > /** > > * \class Camera > > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp > > index 5a21132a08c2..fa44e33a135d 100644 > > --- a/src/libcamera/camera_manager.cpp > > +++ b/src/libcamera/camera_manager.cpp > > @@ -35,6 +35,7 @@ namespace libcamera { > > > > LOG_DEFINE_CATEGORY(Camera) > > > > +#ifndef __DOXYGEN_PUBLIC__ > > CameraManager::Private::Private() > > : initialized_(false) > > { > > @@ -249,6 +250,7 @@ void CameraManager::Private::removeCamera(std::shared_ptr<Camera> camera) > > CameraManager *const o = LIBCAMERA_O_PTR(); > > o->cameraRemoved.emit(camera); > > } > > +#endif /* __DOXYGEN_PUBLIC__ */ > > > > /** > > * \class CameraManager > > diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp > > index f39db1223432..826848f75a56 100644 > > --- a/src/libcamera/framebuffer.cpp > > +++ b/src/libcamera/framebuffer.cpp > > @@ -107,6 +107,7 @@ LOG_DEFINE_CATEGORY(Buffer) > > * \return The array of per-plane metadata > > */ > > > > +#ifndef __DOXYGEN_PUBLIC__ > > /** > > * \class FrameBuffer::Private > > * \brief Base class for FrameBuffer private data > > @@ -209,6 +210,7 @@ FrameBuffer::Private::~Private() > > * \brief Retrieve the dynamic metadata > > * \return Dynamic metadata for the frame contained in the buffer > > */ > > +#endif /* __DOXYGEN_PUBLIC__ */ > > > > /** > > * \class FrameBuffer > > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > > index 24fa3a57312e..8c56ed30d0c4 100644 > > --- a/src/libcamera/request.cpp > > +++ b/src/libcamera/request.cpp > > @@ -38,6 +38,7 @@ namespace libcamera { > > > > LOG_DEFINE_CATEGORY(Request) > > > > +#ifndef __DOXYGEN_PUBLIC__ > > /** > > * \class Request::Private > > * \brief Request private data > > @@ -306,6 +307,7 @@ void Request::Private::timeout() > > > > emitPrepareCompleted(); > > } > > +#endif /* __DOXYGEN_PUBLIC__ */ > > > > /** > > * \enum Request::Status
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 3c8f30d54401..aca466c9ba72 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -565,6 +565,7 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF * \brief The vector of stream configurations */ +#ifndef __DOXYGEN_PUBLIC__ /** * \class Camera::Private * \brief Base class for camera private data @@ -725,6 +726,7 @@ void Camera::Private::setState(State state) { state_.store(state, std::memory_order_release); } +#endif /* __DOXYGEN_PUBLIC__ */ /** * \class Camera diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 5a21132a08c2..fa44e33a135d 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -35,6 +35,7 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Camera) +#ifndef __DOXYGEN_PUBLIC__ CameraManager::Private::Private() : initialized_(false) { @@ -249,6 +250,7 @@ void CameraManager::Private::removeCamera(std::shared_ptr<Camera> camera) CameraManager *const o = LIBCAMERA_O_PTR(); o->cameraRemoved.emit(camera); } +#endif /* __DOXYGEN_PUBLIC__ */ /** * \class CameraManager diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp index f39db1223432..826848f75a56 100644 --- a/src/libcamera/framebuffer.cpp +++ b/src/libcamera/framebuffer.cpp @@ -107,6 +107,7 @@ LOG_DEFINE_CATEGORY(Buffer) * \return The array of per-plane metadata */ +#ifndef __DOXYGEN_PUBLIC__ /** * \class FrameBuffer::Private * \brief Base class for FrameBuffer private data @@ -209,6 +210,7 @@ FrameBuffer::Private::~Private() * \brief Retrieve the dynamic metadata * \return Dynamic metadata for the frame contained in the buffer */ +#endif /* __DOXYGEN_PUBLIC__ */ /** * \class FrameBuffer diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 24fa3a57312e..8c56ed30d0c4 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -38,6 +38,7 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Request) +#ifndef __DOXYGEN_PUBLIC__ /** * \class Request::Private * \brief Request private data @@ -306,6 +307,7 @@ void Request::Private::timeout() emitPrepareCompleted(); } +#endif /* __DOXYGEN_PUBLIC__ */ /** * \enum Request::Status
The *::Private classes are part of the internal API, as their name implies. They are defined in internal headers, but implemented in the same source file as their public counterparts. This will cause Doxygen to complain about missing class definition when splitting the public and internal API documents, as the internal headers won't be parsed by Doxygen for the public API documentation. Marking the classes with \internal isn't enough. The directive prevents the documentation block from being included in the output, but this occurs at the generation stage, after the documentation blocks are parsed. Fix this by completely hidding the implementation of the *::Private classes from Doxygen using preprocessor conditional compilation. To do so, introduce a new macro, __DOXYGEN_PUBLIC__, that will be defined for the public API documentation only. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/camera.cpp | 2 ++ src/libcamera/camera_manager.cpp | 2 ++ src/libcamera/framebuffer.cpp | 2 ++ src/libcamera/request.cpp | 2 ++ 4 files changed, 8 insertions(+)