[libcamera-devel,v2,0/5] Generate docs from mojom files
mbox series

Message ID 20210525095218.1237140-1-paul.elder@ideasonboard.com
Headers show
Series
  • Generate docs from mojom files
Related show

Message

Paul Elder May 25, 2021, 9:52 a.m. UTC
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/5) simply extracts all comments that look like:
/**
anything
*/
and outputs that, along with a header and namespace libcamera {} to a
cpp files.

The rest of the patches plumb the documentation extractor and generation
through meson and core.mojom.

Not much has changed in v2, except that the dependent patch series has
been pushed to master so this series has been rebased on that.

Paul Elder (5):
  utils: ipc: Add script to extract doxygen docs from mojom files
  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 |   7 +-
 src/libcamera/ipa/meson.build     |  18 ++-
 utils/ipc/extract-docs.py         |  74 +++++++++++
 utils/ipc/meson.build             |   2 +
 6 files changed, 287 insertions(+), 20 deletions(-)
 create mode 100755 utils/ipc/extract-docs.py

Comments

Laurent Pinchart May 25, 2021, 10:15 a.m. UTC | #1
Hi Paul,

Thank you for the patch.

On Tue, May 25, 2021 at 06:52:15PM +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>
> 
> ---
> Changes in v2:
> - simplify the conversion
> ---
>  include/libcamera/ipa/meson.build | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
> index eca4e9ee..729483ab 100644
> --- a/include/libcamera/ipa/meson.build
> +++ b/include/libcamera/ipa/meson.build
> @@ -81,7 +81,7 @@ foreach file : ipa_mojom_files
>      endif
>  
>      # {pipeline}.mojom-module
> -    mojom = custom_target(file.split('.')[0] + '_mojom_module',
> +    mojom = custom_target(name + '_mojom_module',

A bit of a drive-by fix ? You may want to mention it in the commit
message.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>                            input : file,
>                            output : file + '-module',
>                            depends : ipa_mojom_core,
> @@ -143,3 +143,5 @@ foreach file : ipa_mojom_files
>  
>      libcamera_generated_ipa_headers += [header, serializer, proxy_header]
>  endforeach
> +
> +ipa_mojom_files = files(ipa_mojom_files)
Umang Jain May 26, 2021, 1:30 p.m. UTC | #2
Hi Paul

On 5/25/21 3:22 PM, 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 729483ab..81fb69f0 100644
> --- a/include/libcamera/ipa/meson.build
> +++ b/include/libcamera/ipa/meson.build
> @@ -145,3 +145,6 @@ foreach file : ipa_mojom_files
>   endforeach
>   
>   ipa_mojom_files = files(ipa_mojom_files)
> +
> +# Pass this to the documentation generator in src/libcamera/ipa
> +ipa_mojom_files += files(['core.mojom'])
nitpick: I would setup this as:

ipa_mojom_files += files([
     'core.mojom',
])

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Umang Jain May 26, 2021, 1:31 p.m. UTC | #3
On 5/25/21 3:22 PM, 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>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@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 560b2fdd..44695240 100644
> --- a/src/libcamera/ipa/meson.build
> +++ b/src/libcamera/ipa/meson.build
> @@ -1,5 +1,17 @@
>   # SPDX-License-Identifier: CC0-1.0
>   
> -libcamera_ipa_interfaces = files([
> -    'core_ipa_interface.cpp',
> -])
> +libcamera_ipa_interfaces = []
> +
> +foreach file : ipa_mojom_files
> +    name = '@0@'.format(file).split('/')[-1].split('.')[0]
> +
> +    # {pipeline}_ipa_interface.cpp
> +    libcamera_ipa_interfaces += \
> +        custom_target(name + '_ipa_interface_cpp',
> +                      input : file,
> +                      output : name + '_ipa_interface.cpp',
> +                      command : [
> +                          mojom_docs_extractor,
> +                          '-o', '@OUTPUT@', '@INPUT@'
> +                      ])
> +endforeach
Umang Jain May 26, 2021, 1:36 p.m. UTC | #4
Hi Paul

On 5/26/21 7:00 PM, Umang Jain wrote:
> Hi Paul
>
> On 5/25/21 3:22 PM, 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 729483ab..81fb69f0 100644
>> --- a/include/libcamera/ipa/meson.build
>> +++ b/include/libcamera/ipa/meson.build
>> @@ -145,3 +145,6 @@ foreach file : ipa_mojom_files
>>   endforeach
>>     ipa_mojom_files = files(ipa_mojom_files)
>> +
>> +# Pass this to the documentation generator in src/libcamera/ipa
>> +ipa_mojom_files += files(['core.mojom'])
> nitpick: I would setup this as:
>
> ipa_mojom_files += files([
>     'core.mojom',
> ])
Didn't realize it's only core.mojom :S, Looks good as per the patch.
>
> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
>