| Message ID | 20210524084029.1179881-1-paul.elder@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Mon, May 24, 2021 at 05:40:28PM +0900, Paul Elder wrote: > We don't want to generate the same functional files for core.mojom as > the other mojom files, but we do want to generate the documentation cpp > files. Add core.mojom to the mojom files list after the main generation > is complete, so that the documentation generator can pick it up. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/ipa/meson.build | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build > index c7fa5cd7..fc791b2f 100644 > --- a/include/libcamera/ipa/meson.build > +++ b/include/libcamera/ipa/meson.build > @@ -143,3 +143,6 @@ foreach file : ipa_mojom_files > > libcamera_generated_ipa_headers += [header, serializer, proxy_header] > endforeach > + > +# Pass this to the documentation generator in src/libcamera/ipa > +ipa_mojom_files += files(['core.mojom'])
Hi Paul, Thank you for the patch. On Mon, May 24, 2021 at 05:40:29PM +0900, Paul Elder wrote: > Plumb meson to build the cpp files from the mojom files for the purpose > of containing the documentation for the IPA interfaces. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/libcamera/ipa/meson.build | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/ipa/meson.build b/src/libcamera/ipa/meson.build > index 0a16d197..0d649eed 100644 > --- a/src/libcamera/ipa/meson.build > +++ b/src/libcamera/ipa/meson.build > @@ -1,5 +1,17 @@ > # SPDX-License-Identifier: LGPL-2.1-or-later > > -libcamera_ipa_interfaces = files([ > - 'core_ipa_interface.cpp', > -]) > +libcamera_ipa_interfaces = [] > + > +foreach file : ipa_mojom_files > + name = '@0@'.format(file).split('/')[-1].split('.')[0] A bit of a hack, but there's no simple way around it that I can think of. One option would be to use @BASENAME@ below. The second split()[] could also become a .replace('.mojom', ''). > + > + # {pipeline}_ipa_interface.cpp > + libcamera_ipa_interfaces += \ > + custom_target(name + '_ipa_interface_cpp', This would become custom_targer('@BASENAME@_ipa_interface_cpp', > + input : file, > + output : name + '_ipa_interface.cpp', Similarly here. Untested, and possibly not better. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + command : [ > + mojom_docs_extractor, > + '-o', '@OUTPUT@', '@INPUT@' > + ]) > +endforeach
Hi Paul, Thank you for the patch. On Mon, May 24, 2021 at 05:40:26PM +0900, Paul Elder wrote: > Use meson's files() to list the mojom files instead of the file names > directly. This is so that we can still access the files from > src/libcamera/ipa/meson.build later for building documentation cpp > files from the mojom files. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > include/libcamera/ipa/meson.build | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build > index eca4e9ee..c7fa5cd7 100644 > --- a/include/libcamera/ipa/meson.build > +++ b/include/libcamera/ipa/meson.build > @@ -58,12 +58,12 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h', > './' +'@INPUT@' > ]) > > -ipa_mojom_files = [ > +ipa_mojom_files = files([ > 'ipu3.mojom', > 'raspberrypi.mojom', > 'rkisp1.mojom', > 'vimc.mojom', > -] > +]) > > ipa_mojoms = [] > > @@ -74,16 +74,16 @@ ipa_mojoms = [] > # TODO Define per-pipeline ControlInfoMap with yaml? > > foreach file : ipa_mojom_files > - name = file.split('.')[0] > + name = '@0@'.format(file).split('/')[-1].split('.')[0] > > if name not in pipelines > continue > endif > > # {pipeline}.mojom-module > - mojom = custom_target(file.split('.')[0] + '_mojom_module', > + mojom = custom_target(name + '_mojom_module', > input : file, > - output : file + '-module', > + output : name + '.mojom-module', > depends : ipa_mojom_core, > command : [ > mojom_parser, You could also just do ipa_mojom_files = files(ipa_mojom_files) below this loop.
Hi Laurent, On Mon, May 24, 2021 at 04:28:27PM +0300, Laurent Pinchart wrote: > Hi Paul, > > Thank you for the patch. > > On Mon, May 24, 2021 at 05:40:29PM +0900, Paul Elder wrote: > > Plumb meson to build the cpp files from the mojom files for the purpose > > of containing the documentation for the IPA interfaces. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > src/libcamera/ipa/meson.build | 18 +++++++++++++++--- > > 1 file changed, 15 insertions(+), 3 deletions(-) > > > > diff --git a/src/libcamera/ipa/meson.build b/src/libcamera/ipa/meson.build > > index 0a16d197..0d649eed 100644 > > --- a/src/libcamera/ipa/meson.build > > +++ b/src/libcamera/ipa/meson.build > > @@ -1,5 +1,17 @@ > > # SPDX-License-Identifier: LGPL-2.1-or-later > > > > -libcamera_ipa_interfaces = files([ > > - 'core_ipa_interface.cpp', > > -]) > > +libcamera_ipa_interfaces = [] > > + > > +foreach file : ipa_mojom_files > > + name = '@0@'.format(file).split('/')[-1].split('.')[0] > > A bit of a hack, but there's no simple way around it that I can think Yeah :/ > of. One option would be to use @BASENAME@ below. The second split()[] ERROR: Tried to create target "@BASENAME@_ipa_interface_cpp", but a target of that name already exists. > could also become a .replace('.mojom', ''). ERROR: Unknown method "replace" for a string. > > > + > > + # {pipeline}_ipa_interface.cpp > > + libcamera_ipa_interfaces += \ > > + custom_target(name + '_ipa_interface_cpp', > > This would become > > custom_targer('@BASENAME@_ipa_interface_cpp', > > > + input : file, > > + output : name + '_ipa_interface.cpp', > > Similarly here. Untested, and possibly not better. I think I'll keep what I have here :/ > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks, Paul > > > + command : [ > > + mojom_docs_extractor, > > + '-o', '@OUTPUT@', '@INPUT@' > > + ]) > > +endforeach
This patch series depends on v3 of "External IPU3 IPA Support". This patch series enables generating documentation cpp files from mojom files directly, so that we no longer need to manually put the comments in cpp files directly. The script (patch 1/6) simply extracts all comments that look like: /** anything */ and outputs that, along with a header and namespace libcamera {} to a cpp files. Patch 2/6 blocks struct constructors from being parsed by doxygen. The rest of the patches plumb the documentation extractor and generation through meson and core.mojom. Paul Elder (6): utils: ipc: Add script to extract doxygen docs from mojom files utils: ipc: Prevent struct constructors from being parsed by doxygen meson: ipa: Use files() to locate the mojom files ipa: core: Move documentation from cpp file back into the mojom file meson: ipa: Pass core.mojom to the docs generator meson: ipa: Build documentation cpp files from mojom files Documentation/guides/ipa.rst | 6 + include/libcamera/ipa/core.mojom | 200 +++++++++++++-- include/libcamera/ipa/meson.build | 13 +- src/libcamera/ipa/core_ipa_interface.cpp | 237 ------------------ src/libcamera/ipa/meson.build | 18 +- utils/ipc/extract-docs.py | 77 ++++++ .../definition_functions.tmpl | 3 + utils/ipc/meson.build | 2 + 8 files changed, 295 insertions(+), 261 deletions(-) delete mode 100644 src/libcamera/ipa/core_ipa_interface.cpp create mode 100755 utils/ipc/extract-docs.py