[libcamera-devel] [PATCH v2 2/6] controls: Update argument handling for controls generation scripts
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Nov 23 10:57:30 CET 2023
Hi Naush,
Thank you for the patch.
On Tue, Nov 21, 2023 at 02:53:46PM +0000, Naushir Patuck via libcamera-devel wrote:
> The template file to the gen-controls.py and gen-py-controls.py is now
> passed in through the '-t' command line argument instead of being a
> positional argument. This will allow multiple input files to be
> provided to the scripts in a future commit.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> include/libcamera/meson.build | 7 ++++---
> src/libcamera/meson.build | 7 ++++---
> src/py/libcamera/gen-py-controls.py | 2 +-
> src/py/libcamera/meson.build | 18 ++++++++----------
> utils/gen-controls.py | 2 +-
> 5 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 2c8c0258c95e..5fb772e6dd14 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -41,12 +41,13 @@ control_source_files = {
> control_headers = []
>
> foreach header, mode : control_source_files
> - input_files = files('../../src/libcamera/' + header +'.yaml', header + '.h.in')
> + input_files = files('../../src/libcamera/' + header +'.yaml')
> + template_file = files(header + '.h.in')
> control_headers += custom_target(header + '_h',
> input : input_files,
> output : header + '.h',
> - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',
> - '--mode', mode],
> + command : [gen_controls, '-o', '@OUTPUT@',
> + '--mode', mode, '-t', template_file, '@INPUT@'],
> install : true,
> install_dir : libcamera_headers_install_dir)
> endforeach
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index e49bf850b355..05ee38daf22b 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -128,12 +128,13 @@ endif
> control_sources = []
>
> foreach source, mode : control_source_files
> - input_files = files(source +'.yaml', source + '.cpp.in')
> + input_files = files(source +'.yaml')
> + template_file = files(source + '.cpp.in')
Hmmm... I think we have a problem here. The custom_target() input
argument is used to track dependencies, and cause the target to be
rebuild if any of the dependencies has changed. Passing the template
file through '-t' is fine, but dropping it from the inputs means that
dependencies won't be tracked properly. You can test this by modifying
the template file (without touching anything else) and seeing if all the
source files get rebuilt.
> control_sources += custom_target(source + '_cpp',
> input : input_files,
> output : source + '.cpp',
> - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',
> - '--mode', mode])
> + command : [gen_controls, '-o', '@OUTPUT@',
> + '--mode', mode, '-t', template_file, '@INPUT@'])
> endforeach
>
> libcamera_sources += control_sources
> diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py
> index 8f14cdafe313..ea28f0139f23 100755
> --- a/src/py/libcamera/gen-py-controls.py
> +++ b/src/py/libcamera/gen-py-controls.py
> @@ -93,7 +93,7 @@ def main(argv):
> help='Output file name. Defaults to standard output if not specified.')
> parser.add_argument('input', type=str,
> help='Input file name.')
> - parser.add_argument('template', type=str,
> + parser.add_argument('-t', dest='template', type=str, required=True,
> help='Template file name.')
While at it, could you make the arguments sorted alphabetically ? As we
have '--mode', I would make the template argument
parser.add_argument('--template', '-', dest='template', type=str,
required=True, help='Template file name')
and change '--mode' to '--mode', '-m' for consistency.
A "while at it" comment in the commit message is fine with me, unless
you insist splitting it in a separate patch :-)
> parser.add_argument('--mode', type=str, required=True,
> help='Mode is either "controls" or "properties"')
> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
> index f58c7198ee9e..1c3ea1843ac0 100644
> --- a/src/py/libcamera/meson.build
> +++ b/src/py/libcamera/meson.build
> @@ -28,29 +28,27 @@ pycamera_sources = files([
>
> # Generate controls
>
> -gen_py_controls_input_files = files([
> - '../../libcamera/control_ids.yaml',
> - 'py_controls_generated.cpp.in',
> -])
> +gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')
> +gen_py_controls_template = files('py_controls_generated.cpp.in')
>
> gen_py_controls = files('gen-py-controls.py')
>
> pycamera_sources += custom_target('py_gen_controls',
> input : gen_py_controls_input_files,
> output : ['py_controls_generated.cpp'],
> - command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@', '@INPUT@'])
> + command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@',
> + '-t', gen_py_controls_template, '@INPUT@'])
>
> # Generate properties
>
> -gen_py_property_enums_input_files = files([
> - '../../libcamera/property_ids.yaml',
> - 'py_properties_generated.cpp.in',
> -])
> +gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')
> +gen_py_properties_template = files('py_properties_generated.cpp.in')
>
> pycamera_sources += custom_target('py_gen_properties',
> input : gen_py_property_enums_input_files,
> output : ['py_properties_generated.cpp'],
> - command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@', '@INPUT@'])
> + command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@',
> + '-t', gen_py_properties_template, '@INPUT@'])
>
> # Generate formats
>
> diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> index 8f2f8fdb02c3..8af33d29cd07 100755
> --- a/utils/gen-controls.py
> +++ b/utils/gen-controls.py
> @@ -361,7 +361,7 @@ def main(argv):
> help='Output file name. Defaults to standard output if not specified.')
> parser.add_argument('input', type=str,
> help='Input file name.')
> - parser.add_argument('template', type=str,
> + parser.add_argument('-t', dest='template', type=str, required=True,
> help='Template file name.')
Same comment as above.
> parser.add_argument('--mode', type=str, required=True, choices=['controls', 'properties'],
> help='Mode of operation')
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list