[{"id":28177,"web_url":"https://patchwork.libcamera.org/comment/28177/","msgid":"","date":"2023-11-27T15:38:44","subject":"Re: [libcamera-devel] [PATCH v3 3/7] build: controls: Rework how\n\tcontrols and properties are generated","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Fri, Nov 24, 2023 at 12:37:09PM +0000, Naushir Patuck via libcamera-devel wrote:\n> Add support for using separate YAML files for controls and properties\n> generation. The mapping of vendor/pipeline handler to control file is\n> done through the controls_map variable in include/libcamera/meson.build.\n>\n> This simplifies management of vendor control definitions and avoids\n> possible merge conflicts when changing the control_ids.yaml file for\n> core and draft controls. With this change, libcamera and draft controls\n> and properties files are designated the 'libcamera' vendor tag.\n>\n> In this change, we also rename control_ids.yaml -> control_ids_core.yaml\n> and property_ids.yaml -> property_ids_core.yaml to designate these as\n> core libcamera controls.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> Documentation/guides/pipeline-handler.rst | 8 +--\n> include/libcamera/meson.build | 52 +++++++++++++++----\n> meson.build | 2 +\n> ...control_ids.yaml => control_ids_core.yaml} | 0\n> src/libcamera/meson.build | 21 ++++++--\n> ...operty_ids.yaml => property_ids_core.yaml} | 0\n> src/py/libcamera/gen-py-controls.py | 10 ++--\n> src/py/libcamera/meson.build | 12 ++++-\n> utils/gen-controls.py | 13 +++--\n> 9 files changed, 88 insertions(+), 30 deletions(-)\n> rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (100%)\n> rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (100%)\n>\n> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> index 10b9c75c2a7f..66d428a19c4f 100644\n> --- a/Documentation/guides/pipeline-handler.rst\n> +++ b/Documentation/guides/pipeline-handler.rst\n> @@ -587,12 +587,12 @@ immutable properties of the ``Camera`` device.\n>\n> The libcamera controls and properties are defined in YAML form which is\n> processed to automatically generate documentation and interfaces. Controls are\n> -defined by the src/libcamera/`control_ids.yaml`_ file and camera properties\n> -are defined by src/libcamera/`properties_ids.yaml`_.\n> +defined by the src/libcamera/`control_ids_core.yaml`_ file and camera properties\n> +are defined by src/libcamera/`properties_ids_core.yaml`_.\n>\n> .. _controls framework: https://libcamera.org/api-html/controls_8h.html\n> -.. _control_ids.yaml: https://libcamera.org/api-html/control__ids_8h.html\n> -.. _properties_ids.yaml: https://libcamera.org/api-html/property__ids_8h.html\n> +.. _control_ids_core.yaml: https://libcamera.org/api-html/control__ids_8h.html\n> +.. _properties_ids_core.yaml: https://libcamera.org/api-html/property__ids_8h.html\n>\n> Pipeline handlers can optionally register the list of controls an application\n> can set as well as a list of immutable camera properties. Being both\n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 5fb772e6dd14..a763c8ff4344 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -32,22 +32,56 @@ install_headers(libcamera_public_headers,\n>\n> libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir\n>\n> -# control_ids.h and property_ids.h and associated modes\n> -control_source_files = {\n> - 'control_ids': 'controls',\n> - 'property_ids': 'properties',\n> +controls_map = {\n> + 'controls': {\n> + 'core': 'control_ids_core.yaml',\n> + },\n> +\n> + 'properties': {\n> + 'core': 'property_ids_core.yaml',\n> + }\n> }\n>\n> control_headers = []\n> +controls_files = []\n> +properties_files = []\n> +\n> +foreach mode, entry : controls_map\n> + files_list = []\n> + input_files = []\n> + foreach vendor, header : entry\n> + if vendor != 'core' and vendor != 'draft'\n> + if vendor not in pipelines\n> + message('vendor not in pipelines')\n\nThis gives\n\nMessage: vendor not in pipelines\nMessage: vendor not in pipelines\n\nin the meson output log, that's not really useful imo\n\n> + continue\n> + endif\n> + endif\n> +\n> + if header in files_list\n> + message('header in files list')\n> + continue\n> + endif\n> +\n> + files_list += header\n> + input_files += files('../../src/libcamera/' + header)\n> + endforeach\n> +\n> + outfile = ''\n> + if mode == 'controls'\n> + outfile = 'control_ids.h'\n> + controls_files += files_list\n> + else\n> + outfile = 'property_ids.h'\n> + properties_files += files_list\n> + endif\n>\n> -foreach header, mode : control_source_files\n> - input_files = files('../../src/libcamera/' + header +'.yaml')\n> - template_file = files(header + '.h.in')\n> + template_file = files(outfile + '.in')\n> control_headers += custom_target(header + '_h',\n> input : input_files,\n> - output : header + '.h',\n> + output : outfile,\n> command : [gen_controls, '-o', '@OUTPUT@',\n> - '--mode', mode, '-t', template_file, '@INPUT@'],\n> + '--mode', mode, '-t', template_file,\n> + '@INPUT@'],\n> install : true,\n> install_dir : libcamera_headers_install_dir)\n> endforeach\n> diff --git a/meson.build b/meson.build\n> index e9a1c7e360ce..ee57cb780149 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -267,6 +267,8 @@ py_mod.find_installation('python3', modules : py_modules)\n> summary({\n> 'Enabled pipelines': pipelines,\n> 'Enabled IPA modules': enabled_ipa_names,\n> + 'Controls files': controls_files,\n> + 'Properties files': properties_files,\n\n Controls files : control_ids_draft.yaml\n control_ids_core.yaml\n Properties files : property_ids_draft.yaml\n property_ids_core.yaml\nThis (not from this patch but with the whole series applied) is more\nuseful indeed!\n\n> 'Hotplug support': libudev.found(),\n> 'Tracing support': tracing_enabled,\n> 'Android support': android_enabled,\n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids_core.yaml\n> similarity index 100%\n> rename from src/libcamera/control_ids.yaml\n> rename to src/libcamera/control_ids_core.yaml\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 05ee38daf22b..6d9902e6ffd1 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -127,12 +127,23 @@ endif\n>\n> control_sources = []\n>\n> -foreach source, mode : control_source_files\n> - input_files = files(source +'.yaml')\n> - template_file = files(source + '.cpp.in')\n> - control_sources += custom_target(source + '_cpp',\n> +controls_mode_files = {\n> + 'controls' : controls_files,\n> + 'properties' : properties_files,\n> +}\n> +\n> +foreach mode, input_files : controls_mode_files\n> + input_files = files(input_files)\n> +\n> + if mode == 'controls'\n> + template_file = files('control_ids.cpp.in')\n> + else\n> + template_file = files('property_ids.cpp.in')\n> + endif\n> +\n> + control_sources += custom_target(mode + '_cpp',\n> input : input_files,\n> - output : source + '.cpp',\n> + output : mode + '_ids.cpp',\n> command : [gen_controls, '-o', '@OUTPUT@',\n> '--mode', mode, '-t', template_file, '@INPUT@'])\n> endforeach\n> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids_core.yaml\n> similarity index 100%\n> rename from src/libcamera/property_ids.yaml\n> rename to src/libcamera/property_ids_core.yaml\n> diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> index cfcfd4d16acf..8ae8d5126e39 100755\n> --- a/src/py/libcamera/gen-py-controls.py\n> +++ b/src/py/libcamera/gen-py-controls.py\n> @@ -95,7 +95,7 @@ def main(argv):\n> help='Output file name. Defaults to standard output if not specified.')\n> parser.add_argument('--template', '-t', type=str, required=True,\n> help='Template file name.')\n> - parser.add_argument('input', type=str,\n> + parser.add_argument('input', type=str, nargs='+',\n> help='Input file name.')\n> args = parser.parse_args(argv[1:])\n>\n> @@ -103,11 +103,11 @@ def main(argv):\n> print(f'Invalid mode option \"{args.mode}\"', file=sys.stderr)\n> return -1\n>\n> - data = open(args.input, 'rb').read()\n> -\n> controls = {}\n> - vendor = yaml.safe_load(data)['vendor']\n> - controls[vendor] = yaml.safe_load(data)['controls']\n> + for input in args.input:\n> + data = open(input, 'rb').read()\n> + vendor = yaml.safe_load(data)['vendor']\n> + controls[vendor] = yaml.safe_load(data)['controls']\n>\n> data = generate_py(controls, args.mode)\n>\n> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> index 1c3ea1843ac0..31af63ec0dc6 100644\n> --- a/src/py/libcamera/meson.build\n> +++ b/src/py/libcamera/meson.build\n> @@ -28,11 +28,15 @@ pycamera_sources = files([\n>\n> # Generate controls\n>\n> -gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')\n> +gen_py_controls_input_files = []\n> gen_py_controls_template = files('py_controls_generated.cpp.in')\n>\n> gen_py_controls = files('gen-py-controls.py')\n>\n> +foreach file : controls_files\n> + gen_py_controls_input_files += files('../../libcamera/' + file)\n> +endforeach\n> +\n> pycamera_sources += custom_target('py_gen_controls',\n> input : gen_py_controls_input_files,\n> output : ['py_controls_generated.cpp'],\n> @@ -41,9 +45,13 @@ pycamera_sources += custom_target('py_gen_controls',\n>\n> # Generate properties\n>\n> -gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> +gen_py_property_enums_input_files = []\n> gen_py_properties_template = files('py_properties_generated.cpp.in')\n>\n> +foreach file : properties_files\n> + gen_py_property_enums_input_files += files('../../libcamera/' + file)\n> +endforeach\n> +\n> pycamera_sources += custom_target('py_gen_properties',\n> input : gen_py_property_enums_input_files,\n> output : ['py_properties_generated.cpp'],\n> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> index 6680ecf84acb..30c58f35473c 100755\n> --- a/utils/gen-controls.py\n> +++ b/utils/gen-controls.py\n> @@ -12,6 +12,7 @@ import operator\n> import string\n> import sys\n> import yaml\n> +import os\n>\n>\n> class ControlEnum(object):\n> @@ -339,15 +340,17 @@ def main(argv):\n> help='Output file name. Defaults to standard output if not specified.')\n> parser.add_argument('--template', '-t', dest='template', type=str, required=True,\n> help='Template file name.')\n> - parser.add_argument('input', type=str,\n> + parser.add_argument('input', type=str, nargs='+',\n> help='Input file name.')\n>\n> args = parser.parse_args(argv[1:])\n>\n> - data = open(args.input, 'rb').read()\n> - vendor = yaml.safe_load(data)['vendor']\n> - controls = yaml.safe_load(data)['controls']\n> - controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]\n> + controls = []\n> + for input in args.input:\n> + data = open(input, 'rb').read()\n> + vendor = yaml.safe_load(data)['vendor']\n> + ctrls = yaml.safe_load(data)['controls']\n> + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n\nHow does thi work in Python, should data be closed ?\n\nAnyway, this all looks good, with the above 'message' dropped\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> if args.template.endswith('.cpp.in'):\n> data = generate_cpp(controls)\n> --\n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EF9B7BDE6B\n\tfor ;\n\tMon, 27 Nov 2023 15:38:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 42E5F629BC;\n\tMon, 27 Nov 2023 16:38:49 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E491D629B6\n\tfor ;\n\tMon, 27 Nov 2023 16:38:47 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 313F22B6;\n\tMon, 27 Nov 2023 16:38:13 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1701099529;\n\tbh=lcDbjk6PXawchshRef9veOc9XrwGEk5OHAFQD/cSrfk=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=VWmjMkW3D9GGmGes5vVcVJkHKewS8IP636udkpy/01G90i7Be0odvZTKr1+6eZGuH\n\tkIB1t4B12Oyg6hkF4dH6xut+QZFlUgrp83f+4TVRHUyycWqB6lwzwS/TlGszUQzu4F\n\t80FB2NylXwbU3XLSe/1hiCnrUvHOJ6yLQUrlOws3EcIQ6rWJwa5/Bpx9pyej6M+g2C\n\tnRb+MnuixfzqeWYlPRrAxNq8nGsDYcWqT4s1ezavRU0YnxhkqpwuaOtIYK0ohilaF1\n\t9qZiIRpRZb5xtVeJflR/FPG02rEjLFwqlck7JT8TyNloljcFsbgTwmCYTGvD2LRItt\n\tpbi9oaMqzmcGA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1701099493;\n\tbh=lcDbjk6PXawchshRef9veOc9XrwGEk5OHAFQD/cSrfk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TfinPQzYX5CyEOn7+PSTB0Hne6V46qkhQUgSDrgYG4cuT5KCRXAKyHKsaZVR+FBnT\n\touzsdUsq4YTtilz+lTe/GJo/F/JH6m1IakT/vUD8jM1iKQUObJcWglTlYwL7YAegkL\n\tBlkI3AbR41YTXwaQrbwEcinJUO6wBklZuv9GLY9k="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"TfinPQzY\"; dkim-atps=neutral","Date":"Mon, 27 Nov 2023 16:38:44 +0100","To":"Naushir Patuck ","Message-ID":"","References":"<20231124123713.22519-1-naush@raspberrypi.com>\n\t<20231124123713.22519-4-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231124123713.22519-4-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v3 3/7] build: controls: Rework how\n\tcontrols and properties are generated","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28178,"web_url":"https://patchwork.libcamera.org/comment/28178/","msgid":"","date":"2023-11-27T15:45:35","subject":"Re: [libcamera-devel] [PATCH v3 3/7] build: controls: Rework how\n\tcontrols and properties are generated","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nThank you for the feedback.\n\nOn Mon, 27 Nov 2023 at 15:38, Jacopo Mondi\n wrote:\n>\n> Hi Naush\n>\n> On Fri, Nov 24, 2023 at 12:37:09PM +0000, Naushir Patuck via libcamera-devel wrote:\n> > Add support for using separate YAML files for controls and properties\n> > generation. The mapping of vendor/pipeline handler to control file is\n> > done through the controls_map variable in include/libcamera/meson.build.\n> >\n> > This simplifies management of vendor control definitions and avoids\n> > possible merge conflicts when changing the control_ids.yaml file for\n> > core and draft controls. With this change, libcamera and draft controls\n> > and properties files are designated the 'libcamera' vendor tag.\n> >\n> > In this change, we also rename control_ids.yaml -> control_ids_core.yaml\n> > and property_ids.yaml -> property_ids_core.yaml to designate these as\n> > core libcamera controls.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > Documentation/guides/pipeline-handler.rst | 8 +--\n> > include/libcamera/meson.build | 52 +++++++++++++++----\n> > meson.build | 2 +\n> > ...control_ids.yaml => control_ids_core.yaml} | 0\n> > src/libcamera/meson.build | 21 ++++++--\n> > ...operty_ids.yaml => property_ids_core.yaml} | 0\n> > src/py/libcamera/gen-py-controls.py | 10 ++--\n> > src/py/libcamera/meson.build | 12 ++++-\n> > utils/gen-controls.py | 13 +++--\n> > 9 files changed, 88 insertions(+), 30 deletions(-)\n> > rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (100%)\n> > rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (100%)\n> >\n> > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> > index 10b9c75c2a7f..66d428a19c4f 100644\n> > --- a/Documentation/guides/pipeline-handler.rst\n> > +++ b/Documentation/guides/pipeline-handler.rst\n> > @@ -587,12 +587,12 @@ immutable properties of the ``Camera`` device.\n> >\n> > The libcamera controls and properties are defined in YAML form which is\n> > processed to automatically generate documentation and interfaces. Controls are\n> > -defined by the src/libcamera/`control_ids.yaml`_ file and camera properties\n> > -are defined by src/libcamera/`properties_ids.yaml`_.\n> > +defined by the src/libcamera/`control_ids_core.yaml`_ file and camera properties\n> > +are defined by src/libcamera/`properties_ids_core.yaml`_.\n> >\n> > .. _controls framework: https://libcamera.org/api-html/controls_8h.html\n> > -.. _control_ids.yaml: https://libcamera.org/api-html/control__ids_8h.html\n> > -.. _properties_ids.yaml: https://libcamera.org/api-html/property__ids_8h.html\n> > +.. _control_ids_core.yaml: https://libcamera.org/api-html/control__ids_8h.html\n> > +.. _properties_ids_core.yaml: https://libcamera.org/api-html/property__ids_8h.html\n> >\n> > Pipeline handlers can optionally register the list of controls an application\n> > can set as well as a list of immutable camera properties. Being both\n> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> > index 5fb772e6dd14..a763c8ff4344 100644\n> > --- a/include/libcamera/meson.build\n> > +++ b/include/libcamera/meson.build\n> > @@ -32,22 +32,56 @@ install_headers(libcamera_public_headers,\n> >\n> > libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir\n> >\n> > -# control_ids.h and property_ids.h and associated modes\n> > -control_source_files = {\n> > - 'control_ids': 'controls',\n> > - 'property_ids': 'properties',\n> > +controls_map = {\n> > + 'controls': {\n> > + 'core': 'control_ids_core.yaml',\n> > + },\n> > +\n> > + 'properties': {\n> > + 'core': 'property_ids_core.yaml',\n> > + }\n> > }\n> >\n> > control_headers = []\n> > +controls_files = []\n> > +properties_files = []\n> > +\n> > +foreach mode, entry : controls_map\n> > + files_list = []\n> > + input_files = []\n> > + foreach vendor, header : entry\n> > + if vendor != 'core' and vendor != 'draft'\n> > + if vendor not in pipelines\n> > + message('vendor not in pipelines')\n>\n> This gives\n>\n> Message: vendor not in pipelines\n> Message: vendor not in pipelines\n>\n> in the meson output log, that's not really useful imo\n\nIndeed. This was my debug message and should really not be in there!\n\n>\n> > + continue\n> > + endif\n> > + endif\n> > +\n> > + if header in files_list\n> > + message('header in files list')\n> > + continue\n> > + endif\n> > +\n> > + files_list += header\n> > + input_files += files('../../src/libcamera/' + header)\n> > + endforeach\n> > +\n> > + outfile = ''\n> > + if mode == 'controls'\n> > + outfile = 'control_ids.h'\n> > + controls_files += files_list\n> > + else\n> > + outfile = 'property_ids.h'\n> > + properties_files += files_list\n> > + endif\n> >\n> > -foreach header, mode : control_source_files\n> > - input_files = files('../../src/libcamera/' + header +'.yaml')\n> > - template_file = files(header + '.h.in')\n> > + template_file = files(outfile + '.in')\n> > control_headers += custom_target(header + '_h',\n> > input : input_files,\n> > - output : header + '.h',\n> > + output : outfile,\n> > command : [gen_controls, '-o', '@OUTPUT@',\n> > - '--mode', mode, '-t', template_file, '@INPUT@'],\n> > + '--mode', mode, '-t', template_file,\n> > + '@INPUT@'],\n> > install : true,\n> > install_dir : libcamera_headers_install_dir)\n> > endforeach\n> > diff --git a/meson.build b/meson.build\n> > index e9a1c7e360ce..ee57cb780149 100644\n> > --- a/meson.build\n> > +++ b/meson.build\n> > @@ -267,6 +267,8 @@ py_mod.find_installation('python3', modules : py_modules)\n> > summary({\n> > 'Enabled pipelines': pipelines,\n> > 'Enabled IPA modules': enabled_ipa_names,\n> > + 'Controls files': controls_files,\n> > + 'Properties files': properties_files,\n>\n> Controls files : control_ids_draft.yaml\n> control_ids_core.yaml\n> Properties files : property_ids_draft.yaml\n> property_ids_core.yaml\n> This (not from this patch but with the whole series applied) is more\n> useful indeed!\n>\n> > 'Hotplug support': libudev.found(),\n> > 'Tracing support': tracing_enabled,\n> > 'Android support': android_enabled,\n> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids_core.yaml\n> > similarity index 100%\n> > rename from src/libcamera/control_ids.yaml\n> > rename to src/libcamera/control_ids_core.yaml\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 05ee38daf22b..6d9902e6ffd1 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -127,12 +127,23 @@ endif\n> >\n> > control_sources = []\n> >\n> > -foreach source, mode : control_source_files\n> > - input_files = files(source +'.yaml')\n> > - template_file = files(source + '.cpp.in')\n> > - control_sources += custom_target(source + '_cpp',\n> > +controls_mode_files = {\n> > + 'controls' : controls_files,\n> > + 'properties' : properties_files,\n> > +}\n> > +\n> > +foreach mode, input_files : controls_mode_files\n> > + input_files = files(input_files)\n> > +\n> > + if mode == 'controls'\n> > + template_file = files('control_ids.cpp.in')\n> > + else\n> > + template_file = files('property_ids.cpp.in')\n> > + endif\n> > +\n> > + control_sources += custom_target(mode + '_cpp',\n> > input : input_files,\n> > - output : source + '.cpp',\n> > + output : mode + '_ids.cpp',\n> > command : [gen_controls, '-o', '@OUTPUT@',\n> > '--mode', mode, '-t', template_file, '@INPUT@'])\n> > endforeach\n> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids_core.yaml\n> > similarity index 100%\n> > rename from src/libcamera/property_ids.yaml\n> > rename to src/libcamera/property_ids_core.yaml\n> > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > index cfcfd4d16acf..8ae8d5126e39 100755\n> > --- a/src/py/libcamera/gen-py-controls.py\n> > +++ b/src/py/libcamera/gen-py-controls.py\n> > @@ -95,7 +95,7 @@ def main(argv):\n> > help='Output file name. Defaults to standard output if not specified.')\n> > parser.add_argument('--template', '-t', type=str, required=True,\n> > help='Template file name.')\n> > - parser.add_argument('input', type=str,\n> > + parser.add_argument('input', type=str, nargs='+',\n> > help='Input file name.')\n> > args = parser.parse_args(argv[1:])\n> >\n> > @@ -103,11 +103,11 @@ def main(argv):\n> > print(f'Invalid mode option \"{args.mode}\"', file=sys.stderr)\n> > return -1\n> >\n> > - data = open(args.input, 'rb').read()\n> > -\n> > controls = {}\n> > - vendor = yaml.safe_load(data)['vendor']\n> > - controls[vendor] = yaml.safe_load(data)['controls']\n> > + for input in args.input:\n> > + data = open(input, 'rb').read()\n> > + vendor = yaml.safe_load(data)['vendor']\n> > + controls[vendor] = yaml.safe_load(data)['controls']\n> >\n> > data = generate_py(controls, args.mode)\n> >\n> > diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> > index 1c3ea1843ac0..31af63ec0dc6 100644\n> > --- a/src/py/libcamera/meson.build\n> > +++ b/src/py/libcamera/meson.build\n> > @@ -28,11 +28,15 @@ pycamera_sources = files([\n> >\n> > # Generate controls\n> >\n> > -gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')\n> > +gen_py_controls_input_files = []\n> > gen_py_controls_template = files('py_controls_generated.cpp.in')\n> >\n> > gen_py_controls = files('gen-py-controls.py')\n> >\n> > +foreach file : controls_files\n> > + gen_py_controls_input_files += files('../../libcamera/' + file)\n> > +endforeach\n> > +\n> > pycamera_sources += custom_target('py_gen_controls',\n> > input : gen_py_controls_input_files,\n> > output : ['py_controls_generated.cpp'],\n> > @@ -41,9 +45,13 @@ pycamera_sources += custom_target('py_gen_controls',\n> >\n> > # Generate properties\n> >\n> > -gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > +gen_py_property_enums_input_files = []\n> > gen_py_properties_template = files('py_properties_generated.cpp.in')\n> >\n> > +foreach file : properties_files\n> > + gen_py_property_enums_input_files += files('../../libcamera/' + file)\n> > +endforeach\n> > +\n> > pycamera_sources += custom_target('py_gen_properties',\n> > input : gen_py_property_enums_input_files,\n> > output : ['py_properties_generated.cpp'],\n> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > index 6680ecf84acb..30c58f35473c 100755\n> > --- a/utils/gen-controls.py\n> > +++ b/utils/gen-controls.py\n> > @@ -12,6 +12,7 @@ import operator\n> > import string\n> > import sys\n> > import yaml\n> > +import os\n> >\n> >\n> > class ControlEnum(object):\n> > @@ -339,15 +340,17 @@ def main(argv):\n> > help='Output file name. Defaults to standard output if not specified.')\n> > parser.add_argument('--template', '-t', dest='template', type=str, required=True,\n> > help='Template file name.')\n> > - parser.add_argument('input', type=str,\n> > + parser.add_argument('input', type=str, nargs='+',\n> > help='Input file name.')\n> >\n> > args = parser.parse_args(argv[1:])\n> >\n> > - data = open(args.input, 'rb').read()\n> > - vendor = yaml.safe_load(data)['vendor']\n> > - controls = yaml.safe_load(data)['controls']\n> > - controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]\n> > + controls = []\n> > + for input in args.input:\n> > + data = open(input, 'rb').read()\n> > + vendor = yaml.safe_load(data)['vendor']\n> > + ctrls = yaml.safe_load(data)['controls']\n> > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n>\n> How does thi work in Python, should data be closed ?\n\nI think it does get closed since we don't hold onto a reference to the\nopen() return value. But please correct me if that is wrong since I\nwas (blindly) following what the existing code was doing.\n\nRegards,\nNaush\n\n>\n> Anyway, this all looks good, with the above 'message' dropped\n>\n> Reviewed-by: Jacopo Mondi \n>\n> Thanks\n> j\n>\n> >\n> > if args.template.endswith('.cpp.in'):\n> > data = generate_cpp(controls)\n> > --\n> > 2.34.1\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E2C1DC31E9\n\tfor ;\n\tMon, 27 Nov 2023 15:46:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5E7F4629BC;\n\tMon, 27 Nov 2023 16:46:13 +0100 (CET)","from mail-yw1-x112f.google.com (mail-yw1-x112f.google.com\n\t[IPv6:2607:f8b0:4864:20::112f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A95D0629B6\n\tfor ;\n\tMon, 27 Nov 2023 16:46:11 +0100 (CET)","by mail-yw1-x112f.google.com with SMTP id\n\t00721157ae682-5cece20f006so22231297b3.3\n\tfor ;\n\tMon, 27 Nov 2023 07:46:11 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1701099973;\n\tbh=AMdny4gs7QUKDFlVxHAaIf2gmReiuk9joyUJDwH2srw=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=KiHlItylZKsnQqqSNQsbRHvUP+fWaU5m3ZhbT2w+gb02Kjv5T6a2pfrnbERffap5X\n\thNKE2OCDu9Hd89qrfpdbDAG9PpQjgMCrHCHtAkHbW9xCG/3RhPTRZBCK99W1FRguiD\n\t6CBlBdxdjBgkm2jbiouElI7QWbEefK3JdExZZGGLLYrCVs6rMV5LfQv7mcGAZ8yHFG\n\tWNC6FcVeGQHl3LEaaY2ZmdDvvl9JG6YkjpeDvjtITN20ovBg5J7yFK2reIWWcvZZo+\n\tVhxbwrpwyzeDOlmPPlkBDg7aebbaGYB1fq451ygehYNcauB0KFBflAHCsC2KqIy+gm\n\t+3QsMAvvBJWpg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1701099970; x=1701704770;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=hBvxptbmUOUA38R/VbbwzY3k7QtpYNDR8DwChIx47nA=;\n\tb=ekU6Vag/QLaP56RhIx2KMjemCm3pfylqE8p4DTuaK3l2NDU6nCdv+IhnwcGKWtVExS\n\tQBIcgbopq9JI/hQ/lotdeybGbFp1lhhWfrFraFWhgT5lDxhpOIXVbMPAeG2NiifScwDz\n\tReNDsmn7QwfZfxfcaPTatHXuW+VIvN/KhceES1cfDvuUp4uywsXvVjIfFcdcNuIGYF7L\n\tRKf/VVoYbIiO4lU19RTIrzqTq0LhsQDplHtFe5lZ1F4oks7qL3ZmtPggW6iKe8g9SLLH\n\td0hDSAF0ceID4NLvM2urCmBv8mublLJRPiFj/6wfP4Sj4OrY6w6lmwM2qclbB/F1EzPN\n\tt8oQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"ekU6Vag/\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1701099970; x=1701704770;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=hBvxptbmUOUA38R/VbbwzY3k7QtpYNDR8DwChIx47nA=;\n\tb=phgGhXZFUzJZjXu40F4tYk88YD0kEG35aeOHI9NCCRnoCH1WUPBBfKQmD+/12IlJrH\n\tdihZJDbCfSD32Lzpd4XBFsu+DCaUikNl5cNPUND59Zv7eVp6PnJzwnZI/4ZAidrohdh2\n\tdhCXvmv2y0rPMZflZl+iG7d/YD8rCQVYb+fctRWffu+e11eFpRAy5+kujzW78bznfySn\n\tFVo/tb14SxTb4bcCexHwDnmfLlstF4mokfFrPGfOZqyZMvDyUlk3yEVPpfY2baf8urL6\n\tB3pdoYl54kYnC5qrKkUu+lh5HUkPWpRI+BZl7sajpJ+34ZxmC2J9WgEANqM30OwnOwhn\n\txSKA==","X-Gm-Message-State":"AOJu0Ywb713vEocuk0DC1qMEK8JrPV62MSgtUfl4SwGRIBKGY7/POuza\n\tFRJjQQ4Oemu0365yxKSa0v3cDbxUvxXETE3Yj4S3XQ==","X-Google-Smtp-Source":"AGHT+IFCjxv8h7LW4zbsp1IQ76FqcJQ8yx15ON8Qdh1tf3/2C/Gljq9avf20/ZK4zWDHuGkExSoXRzONQ0YjBEmemUc=","X-Received":"by 2002:a0d:ea0b:0:b0:5ca:e2d4:623a with SMTP id\n\tt11-20020a0dea0b000000b005cae2d4623amr11925641ywe.12.1701099970278;\n\tMon, 27 Nov 2023 07:46:10 -0800 (PST)","MIME-Version":"1.0","References":"<20231124123713.22519-1-naush@raspberrypi.com>\n\t<20231124123713.22519-4-naush@raspberrypi.com>\n\t","In-Reply-To":"","Date":"Mon, 27 Nov 2023 15:45:35 +0000","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 3/7] build: controls: Rework how\n\tcontrols and properties are generated","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28205,"web_url":"https://patchwork.libcamera.org/comment/28205/","msgid":"<170118415014.630990.4215388948375627091@ping.linuxembedded.co.uk>","date":"2023-11-28T15:09:10","subject":"Re: [libcamera-devel] [PATCH v3 3/7] build: controls: Rework how\n\tcontrols and properties are generated","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2023-11-27 15:45:35)\n> Hi Jacopo,\n> \n> Thank you for the feedback.\n> \n> On Mon, 27 Nov 2023 at 15:38, Jacopo Mondi\n> wrote:\n> >\n> > Hi Naush\n> >\n> > On Fri, Nov 24, 2023 at 12:37:09PM +0000, Naushir Patuck via libcamera-devel wrote:\n> > > Add support for using separate YAML files for controls and properties\n> > > generation. The mapping of vendor/pipeline handler to control file is\n> > > done through the controls_map variable in include/libcamera/meson.build.\n> > >\n> > > This simplifies management of vendor control definitions and avoids\n> > > possible merge conflicts when changing the control_ids.yaml file for\n> > > core and draft controls. With this change, libcamera and draft controls\n> > > and properties files are designated the 'libcamera' vendor tag.\n> > >\n> > > In this change, we also rename control_ids.yaml -> control_ids_core.yaml\n> > > and property_ids.yaml -> property_ids_core.yaml to designate these as\n> > > core libcamera controls.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > Documentation/guides/pipeline-handler.rst | 8 +--\n> > > include/libcamera/meson.build | 52 +++++++++++++++----\n> > > meson.build | 2 +\n> > > ...control_ids.yaml => control_ids_core.yaml} | 0\n> > > src/libcamera/meson.build | 21 ++++++--\n> > > ...operty_ids.yaml => property_ids_core.yaml} | 0\n> > > src/py/libcamera/gen-py-controls.py | 10 ++--\n> > > src/py/libcamera/meson.build | 12 ++++-\n> > > utils/gen-controls.py | 13 +++--\n> > > 9 files changed, 88 insertions(+), 30 deletions(-)\n> > > rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (100%)\n> > > rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (100%)\n> > >\n> > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> > > index 10b9c75c2a7f..66d428a19c4f 100644\n> > > --- a/Documentation/guides/pipeline-handler.rst\n> > > +++ b/Documentation/guides/pipeline-handler.rst\n> > > @@ -587,12 +587,12 @@ immutable properties of the ``Camera`` device.\n> > >\n> > > The libcamera controls and properties are defined in YAML form which is\n> > > processed to automatically generate documentation and interfaces. Controls are\n> > > -defined by the src/libcamera/`control_ids.yaml`_ file and camera properties\n> > > -are defined by src/libcamera/`properties_ids.yaml`_.\n> > > +defined by the src/libcamera/`control_ids_core.yaml`_ file and camera properties\n> > > +are defined by src/libcamera/`properties_ids_core.yaml`_.\n> > >\n> > > .. _controls framework: https://libcamera.org/api-html/controls_8h.html\n> > > -.. _control_ids.yaml: https://libcamera.org/api-html/control__ids_8h.html\n> > > -.. _properties_ids.yaml: https://libcamera.org/api-html/property__ids_8h.html\n> > > +.. _control_ids_core.yaml: https://libcamera.org/api-html/control__ids_8h.html\n> > > +.. _properties_ids_core.yaml: https://libcamera.org/api-html/property__ids_8h.html\n> > >\n> > > Pipeline handlers can optionally register the list of controls an application\n> > > can set as well as a list of immutable camera properties. Being both\n> > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> > > index 5fb772e6dd14..a763c8ff4344 100644\n> > > --- a/include/libcamera/meson.build\n> > > +++ b/include/libcamera/meson.build\n> > > @@ -32,22 +32,56 @@ install_headers(libcamera_public_headers,\n> > >\n> > > libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir\n> > >\n> > > -# control_ids.h and property_ids.h and associated modes\n> > > -control_source_files = {\n> > > - 'control_ids': 'controls',\n> > > - 'property_ids': 'properties',\n> > > +controls_map = {\n> > > + 'controls': {\n> > > + 'core': 'control_ids_core.yaml',\n> > > + },\n> > > +\n> > > + 'properties': {\n> > > + 'core': 'property_ids_core.yaml',\n> > > + }\n> > > }\n> > >\n> > > control_headers = []\n> > > +controls_files = []\n> > > +properties_files = []\n> > > +\n> > > +foreach mode, entry : controls_map\n> > > + files_list = []\n> > > + input_files = []\n> > > + foreach vendor, header : entry\n> > > + if vendor != 'core' and vendor != 'draft'\n> > > + if vendor not in pipelines\n> > > + message('vendor not in pipelines')\n> >\n> > This gives\n> >\n> > Message: vendor not in pipelines\n> > Message: vendor not in pipelines\n> >\n> > in the meson output log, that's not really useful imo\n> \n> Indeed. This was my debug message and should really not be in there!\n> \n> >\n> > > + continue\n> > > + endif\n> > > + endif\n> > > +\n> > > + if header in files_list\n> > > + message('header in files list')\n> > > + continue\n> > > + endif\n> > > +\n> > > + files_list += header\n> > > + input_files += files('../../src/libcamera/' + header)\n> > > + endforeach\n> > > +\n> > > + outfile = ''\n> > > + if mode == 'controls'\n> > > + outfile = 'control_ids.h'\n> > > + controls_files += files_list\n> > > + else\n> > > + outfile = 'property_ids.h'\n> > > + properties_files += files_list\n> > > + endif\n> > >\n> > > -foreach header, mode : control_source_files\n> > > - input_files = files('../../src/libcamera/' + header +'.yaml')\n> > > - template_file = files(header + '.h.in')\n> > > + template_file = files(outfile + '.in')\n> > > control_headers += custom_target(header + '_h',\n> > > input : input_files,\n> > > - output : header + '.h',\n> > > + output : outfile,\n> > > command : [gen_controls, '-o', '@OUTPUT@',\n> > > - '--mode', mode, '-t', template_file, '@INPUT@'],\n> > > + '--mode', mode, '-t', template_file,\n> > > + '@INPUT@'],\n> > > install : true,\n> > > install_dir : libcamera_headers_install_dir)\n> > > endforeach\n> > > diff --git a/meson.build b/meson.build\n> > > index e9a1c7e360ce..ee57cb780149 100644\n> > > --- a/meson.build\n> > > +++ b/meson.build\n> > > @@ -267,6 +267,8 @@ py_mod.find_installation('python3', modules : py_modules)\n> > > summary({\n> > > 'Enabled pipelines': pipelines,\n> > > 'Enabled IPA modules': enabled_ipa_names,\n> > > + 'Controls files': controls_files,\n> > > + 'Properties files': properties_files,\n> >\n> > Controls files : control_ids_draft.yaml\n> > control_ids_core.yaml\n> > Properties files : property_ids_draft.yaml\n> > property_ids_core.yaml\n> > This (not from this patch but with the whole series applied) is more\n> > useful indeed!\n> >\n> > > 'Hotplug support': libudev.found(),\n> > > 'Tracing support': tracing_enabled,\n> > > 'Android support': android_enabled,\n> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids_core.yaml\n> > > similarity index 100%\n> > > rename from src/libcamera/control_ids.yaml\n> > > rename to src/libcamera/control_ids_core.yaml\n> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > index 05ee38daf22b..6d9902e6ffd1 100644\n> > > --- a/src/libcamera/meson.build\n> > > +++ b/src/libcamera/meson.build\n> > > @@ -127,12 +127,23 @@ endif\n> > >\n> > > control_sources = []\n> > >\n> > > -foreach source, mode : control_source_files\n> > > - input_files = files(source +'.yaml')\n> > > - template_file = files(source + '.cpp.in')\n> > > - control_sources += custom_target(source + '_cpp',\n> > > +controls_mode_files = {\n> > > + 'controls' : controls_files,\n> > > + 'properties' : properties_files,\n> > > +}\n> > > +\n> > > +foreach mode, input_files : controls_mode_files\n> > > + input_files = files(input_files)\n> > > +\n> > > + if mode == 'controls'\n> > > + template_file = files('control_ids.cpp.in')\n> > > + else\n> > > + template_file = files('property_ids.cpp.in')\n> > > + endif\n> > > +\n> > > + control_sources += custom_target(mode + '_cpp',\n> > > input : input_files,\n> > > - output : source + '.cpp',\n> > > + output : mode + '_ids.cpp',\n> > > command : [gen_controls, '-o', '@OUTPUT@',\n> > > '--mode', mode, '-t', template_file, '@INPUT@'])\n> > > endforeach\n> > > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids_core.yaml\n> > > similarity index 100%\n> > > rename from src/libcamera/property_ids.yaml\n> > > rename to src/libcamera/property_ids_core.yaml\n> > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > > index cfcfd4d16acf..8ae8d5126e39 100755\n> > > --- a/src/py/libcamera/gen-py-controls.py\n> > > +++ b/src/py/libcamera/gen-py-controls.py\n> > > @@ -95,7 +95,7 @@ def main(argv):\n> > > help='Output file name. Defaults to standard output if not specified.')\n> > > parser.add_argument('--template', '-t', type=str, required=True,\n> > > help='Template file name.')\n> > > - parser.add_argument('input', type=str,\n> > > + parser.add_argument('input', type=str, nargs='+',\n> > > help='Input file name.')\n> > > args = parser.parse_args(argv[1:])\n> > >\n> > > @@ -103,11 +103,11 @@ def main(argv):\n> > > print(f'Invalid mode option \"{args.mode}\"', file=sys.stderr)\n> > > return -1\n> > >\n> > > - data = open(args.input, 'rb').read()\n> > > -\n> > > controls = {}\n> > > - vendor = yaml.safe_load(data)['vendor']\n> > > - controls[vendor] = yaml.safe_load(data)['controls']\n> > > + for input in args.input:\n> > > + data = open(input, 'rb').read()\n> > > + vendor = yaml.safe_load(data)['vendor']\n> > > + controls[vendor] = yaml.safe_load(data)['controls']\n> > >\n> > > data = generate_py(controls, args.mode)\n> > >\n> > > diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> > > index 1c3ea1843ac0..31af63ec0dc6 100644\n> > > --- a/src/py/libcamera/meson.build\n> > > +++ b/src/py/libcamera/meson.build\n> > > @@ -28,11 +28,15 @@ pycamera_sources = files([\n> > >\n> > > # Generate controls\n> > >\n> > > -gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')\n> > > +gen_py_controls_input_files = []\n> > > gen_py_controls_template = files('py_controls_generated.cpp.in')\n> > >\n> > > gen_py_controls = files('gen-py-controls.py')\n> > >\n> > > +foreach file : controls_files\n> > > + gen_py_controls_input_files += files('../../libcamera/' + file)\n> > > +endforeach\n> > > +\n> > > pycamera_sources += custom_target('py_gen_controls',\n> > > input : gen_py_controls_input_files,\n> > > output : ['py_controls_generated.cpp'],\n> > > @@ -41,9 +45,13 @@ pycamera_sources += custom_target('py_gen_controls',\n> > >\n> > > # Generate properties\n> > >\n> > > -gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > > +gen_py_property_enums_input_files = []\n> > > gen_py_properties_template = files('py_properties_generated.cpp.in')\n> > >\n> > > +foreach file : properties_files\n> > > + gen_py_property_enums_input_files += files('../../libcamera/' + file)\n> > > +endforeach\n> > > +\n> > > pycamera_sources += custom_target('py_gen_properties',\n> > > input : gen_py_property_enums_input_files,\n> > > output : ['py_properties_generated.cpp'],\n> > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > > index 6680ecf84acb..30c58f35473c 100755\n> > > --- a/utils/gen-controls.py\n> > > +++ b/utils/gen-controls.py\n> > > @@ -12,6 +12,7 @@ import operator\n> > > import string\n> > > import sys\n> > > import yaml\n> > > +import os\n> > >\n> > >\n> > > class ControlEnum(object):\n> > > @@ -339,15 +340,17 @@ def main(argv):\n> > > help='Output file name. Defaults to standard output if not specified.')\n> > > parser.add_argument('--template', '-t', dest='template', type=str, required=True,\n> > > help='Template file name.')\n> > > - parser.add_argument('input', type=str,\n> > > + parser.add_argument('input', type=str, nargs='+',\n> > > help='Input file name.')\n> > >\n> > > args = parser.parse_args(argv[1:])\n> > >\n> > > - data = open(args.input, 'rb').read()\n> > > - vendor = yaml.safe_load(data)['vendor']\n> > > - controls = yaml.safe_load(data)['controls']\n> > > - controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]\n> > > + controls = []\n> > > + for input in args.input:\n> > > + data = open(input, 'rb').read()\n> > > + vendor = yaml.safe_load(data)['vendor']\n> > > + ctrls = yaml.safe_load(data)['controls']\n> > > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n> >\n> > How does thi work in Python, should data be closed ?\n> \n> I think it does get closed since we don't hold onto a reference to the\n> open() return value. But please correct me if that is wrong since I\n> was (blindly) following what the existing code was doing.\n> \n\nNot a python expert enough here, but I know I often use \"\n\twith open(input, 'rb') as file:\n\t\tdata = file.read\n\netc ... which automatically closes the file at the end of that scope.\n\n\nWith the style above - there is no handle to the opened file - so we\ndon't have anything to call close on ... so it's a bit of refactoring\neither way - unless we can be sure python closes automatically.\n\nBut ... we're certainly going to close when the file exits...\n\nWith that resolved in a suitable way (which could just be confirming\nthat it autocloses if that works) and the debug message removed:\n\n\nReviewed-by: Kieran Bingham \n\n> Regards,\n> Naush\n> \n> >\n> > Anyway, this all looks good, with the above 'message' dropped\n> >\n> > Reviewed-by: Jacopo Mondi \n> >\n> > Thanks\n> > j\n> >\n> > >\n> > > if args.template.endswith('.cpp.in'):\n> > > data = generate_cpp(controls)\n> > > --\n> > > 2.34.1\n> > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 73D54BDE6B\n\tfor ;\n\tTue, 28 Nov 2023 15:09:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D3A9861DA5;\n\tTue, 28 Nov 2023 16:09:14 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3B42061DA2\n\tfor ;\n\tTue, 28 Nov 2023 16:09:13 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B8F719D5;\n\tTue, 28 Nov 2023 16:08:37 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1701184154;\n\tbh=ogikgVJar6Dy0I80Z1vo75+iwKn9STlpyIR1WIXJ4eQ=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=xuyOcvWRcSbGyFYruVG1l+C1TZSEvP3vUhtO9IKJOzhuQh0EWQJY7+3QPX5Nmnck2\n\tFgt+qyZwngIkJmYOO91rnQzcdp6YSOZEB2x+XzM0u1fSvqRhBV05OM6lVUBTdIWRvo\n\tc1oS9Ij8w42gn6Jr8Sl1yetUwXm770c+TIbHeLGQpJqjcEFQWl/dNJWO9GjL+DGKzy\n\tgoSv6UzljXplx8Ph5T20J9TWcLVjy9kjjh4ae/WucJQFyfcZ+8Gj2Qh39iZuS5sOYk\n\tY9RAF+Y0iPum8fACa87P0e+6/GT4ypuMug6SuKXa8QV1nOrtFlAwgwKbyE4Zu5Q0Vj\n\tZgOZ9td2Hv2IA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1701184117;\n\tbh=ogikgVJar6Dy0I80Z1vo75+iwKn9STlpyIR1WIXJ4eQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Grm81uayyy6MmveQrY8lPOSEPJW3e5JmkBRUJ+UNogOxJRvrdFZd0ikhDyz5y9QVd\n\t/dtv+CFoEI11qAiyLIfTPA2G33LIZnZMXW4XtgpneA6Bu4lAfEJ2r7KfYvnbQS+6gv\n\ta+bkMl3a+XiAwLoXSXzIJ1uD+h/tmHgK9s3YiU2k="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Grm81uay\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20231124123713.22519-1-naush@raspberrypi.com>\n\t<20231124123713.22519-4-naush@raspberrypi.com>\n\t\n\t","To":"Jacopo Mondi ,\n\tNaushir Patuck ,\n\tNaushir Patuck via libcamera-devel ","Date":"Tue, 28 Nov 2023 15:09:10 +0000","Message-ID":"<170118415014.630990.4215388948375627091@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 3/7] build: controls: Rework how\n\tcontrols and properties are generated","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28218,"web_url":"https://patchwork.libcamera.org/comment/28218/","msgid":"<20231130090813.GD8402@pendragon.ideasonboard.com>","date":"2023-11-30T09:08:13","subject":"Re: [libcamera-devel] [PATCH v3 3/7] build: controls: Rework how\n\tcontrols and properties are generated","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Nov 28, 2023 at 03:09:10PM +0000, Kieran Bingham via libcamera-devel wrote:\n> Quoting Naushir Patuck via libcamera-devel (2023-11-27 15:45:35)\n> > Hi Jacopo,\n> > \n> > Thank you for the feedback.\n> > \n> > On Mon, 27 Nov 2023 at 15:38, Jacopo Mondi\n> > wrote:\n> > >\n> > > Hi Naush\n> > >\n> > > On Fri, Nov 24, 2023 at 12:37:09PM +0000, Naushir Patuck via libcamera-devel wrote:\n> > > > Add support for using separate YAML files for controls and properties\n> > > > generation. The mapping of vendor/pipeline handler to control file is\n> > > > done through the controls_map variable in include/libcamera/meson.build.\n> > > >\n> > > > This simplifies management of vendor control definitions and avoids\n> > > > possible merge conflicts when changing the control_ids.yaml file for\n> > > > core and draft controls. With this change, libcamera and draft controls\n> > > > and properties files are designated the 'libcamera' vendor tag.\n> > > >\n> > > > In this change, we also rename control_ids.yaml -> control_ids_core.yaml\n> > > > and property_ids.yaml -> property_ids_core.yaml to designate these as\n> > > > core libcamera controls.\n> > > >\n> > > > Signed-off-by: Naushir Patuck \n> > > > ---\n> > > > Documentation/guides/pipeline-handler.rst | 8 +--\n> > > > include/libcamera/meson.build | 52 +++++++++++++++----\n> > > > meson.build | 2 +\n> > > > ...control_ids.yaml => control_ids_core.yaml} | 0\n> > > > src/libcamera/meson.build | 21 ++++++--\n> > > > ...operty_ids.yaml => property_ids_core.yaml} | 0\n> > > > src/py/libcamera/gen-py-controls.py | 10 ++--\n> > > > src/py/libcamera/meson.build | 12 ++++-\n> > > > utils/gen-controls.py | 13 +++--\n> > > > 9 files changed, 88 insertions(+), 30 deletions(-)\n> > > > rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (100%)\n> > > > rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (100%)\n> > > >\n> > > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> > > > index 10b9c75c2a7f..66d428a19c4f 100644\n> > > > --- a/Documentation/guides/pipeline-handler.rst\n> > > > +++ b/Documentation/guides/pipeline-handler.rst\n> > > > @@ -587,12 +587,12 @@ immutable properties of the ``Camera`` device.\n> > > >\n> > > > The libcamera controls and properties are defined in YAML form which is\n> > > > processed to automatically generate documentation and interfaces. Controls are\n> > > > -defined by the src/libcamera/`control_ids.yaml`_ file and camera properties\n> > > > -are defined by src/libcamera/`properties_ids.yaml`_.\n> > > > +defined by the src/libcamera/`control_ids_core.yaml`_ file and camera properties\n> > > > +are defined by src/libcamera/`properties_ids_core.yaml`_.\n> > > >\n> > > > .. _controls framework: https://libcamera.org/api-html/controls_8h.html\n> > > > -.. _control_ids.yaml: https://libcamera.org/api-html/control__ids_8h.html\n> > > > -.. _properties_ids.yaml: https://libcamera.org/api-html/property__ids_8h.html\n> > > > +.. _control_ids_core.yaml: https://libcamera.org/api-html/control__ids_8h.html\n> > > > +.. _properties_ids_core.yaml: https://libcamera.org/api-html/property__ids_8h.html\n> > > >\n> > > > Pipeline handlers can optionally register the list of controls an application\n> > > > can set as well as a list of immutable camera properties. Being both\n> > > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> > > > index 5fb772e6dd14..a763c8ff4344 100644\n> > > > --- a/include/libcamera/meson.build\n> > > > +++ b/include/libcamera/meson.build\n> > > > @@ -32,22 +32,56 @@ install_headers(libcamera_public_headers,\n> > > >\n> > > > libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir\n> > > >\n> > > > -# control_ids.h and property_ids.h and associated modes\n> > > > -control_source_files = {\n> > > > - 'control_ids': 'controls',\n> > > > - 'property_ids': 'properties',\n> > > > +controls_map = {\n> > > > + 'controls': {\n> > > > + 'core': 'control_ids_core.yaml',\n> > > > + },\n> > > > +\n> > > > + 'properties': {\n> > > > + 'core': 'property_ids_core.yaml',\n> > > > + }\n> > > > }\n> > > >\n> > > > control_headers = []\n> > > > +controls_files = []\n> > > > +properties_files = []\n> > > > +\n> > > > +foreach mode, entry : controls_map\n> > > > + files_list = []\n> > > > + input_files = []\n> > > > + foreach vendor, header : entry\n> > > > + if vendor != 'core' and vendor != 'draft'\n> > > > + if vendor not in pipelines\n> > > > + message('vendor not in pipelines')\n> > >\n> > > This gives\n> > >\n> > > Message: vendor not in pipelines\n> > > Message: vendor not in pipelines\n> > >\n> > > in the meson output log, that's not really useful imo\n> > \n> > Indeed. This was my debug message and should really not be in there!\n> > \n> > >\n> > > > + continue\n> > > > + endif\n> > > > + endif\n> > > > +\n> > > > + if header in files_list\n> > > > + message('header in files list')\n> > > > + continue\n> > > > + endif\n> > > > +\n> > > > + files_list += header\n> > > > + input_files += files('../../src/libcamera/' + header)\n> > > > + endforeach\n> > > > +\n> > > > + outfile = ''\n> > > > + if mode == 'controls'\n> > > > + outfile = 'control_ids.h'\n> > > > + controls_files += files_list\n> > > > + else\n> > > > + outfile = 'property_ids.h'\n> > > > + properties_files += files_list\n> > > > + endif\n> > > >\n> > > > -foreach header, mode : control_source_files\n> > > > - input_files = files('../../src/libcamera/' + header +'.yaml')\n> > > > - template_file = files(header + '.h.in')\n> > > > + template_file = files(outfile + '.in')\n> > > > control_headers += custom_target(header + '_h',\n> > > > input : input_files,\n> > > > - output : header + '.h',\n> > > > + output : outfile,\n> > > > command : [gen_controls, '-o', '@OUTPUT@',\n> > > > - '--mode', mode, '-t', template_file, '@INPUT@'],\n> > > > + '--mode', mode, '-t', template_file,\n> > > > + '@INPUT@'],\n> > > > install : true,\n> > > > install_dir : libcamera_headers_install_dir)\n> > > > endforeach\n> > > > diff --git a/meson.build b/meson.build\n> > > > index e9a1c7e360ce..ee57cb780149 100644\n> > > > --- a/meson.build\n> > > > +++ b/meson.build\n> > > > @@ -267,6 +267,8 @@ py_mod.find_installation('python3', modules : py_modules)\n> > > > summary({\n> > > > 'Enabled pipelines': pipelines,\n> > > > 'Enabled IPA modules': enabled_ipa_names,\n> > > > + 'Controls files': controls_files,\n> > > > + 'Properties files': properties_files,\n> > >\n> > > Controls files : control_ids_draft.yaml\n> > > control_ids_core.yaml\n> > > Properties files : property_ids_draft.yaml\n> > > property_ids_core.yaml\n> > > This (not from this patch but with the whole series applied) is more\n> > > useful indeed!\n> > >\n> > > > 'Hotplug support': libudev.found(),\n> > > > 'Tracing support': tracing_enabled,\n> > > > 'Android support': android_enabled,\n> > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids_core.yaml\n> > > > similarity index 100%\n> > > > rename from src/libcamera/control_ids.yaml\n> > > > rename to src/libcamera/control_ids_core.yaml\n> > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > > index 05ee38daf22b..6d9902e6ffd1 100644\n> > > > --- a/src/libcamera/meson.build\n> > > > +++ b/src/libcamera/meson.build\n> > > > @@ -127,12 +127,23 @@ endif\n> > > >\n> > > > control_sources = []\n> > > >\n> > > > -foreach source, mode : control_source_files\n> > > > - input_files = files(source +'.yaml')\n> > > > - template_file = files(source + '.cpp.in')\n> > > > - control_sources += custom_target(source + '_cpp',\n> > > > +controls_mode_files = {\n> > > > + 'controls' : controls_files,\n> > > > + 'properties' : properties_files,\n> > > > +}\n> > > > +\n> > > > +foreach mode, input_files : controls_mode_files\n> > > > + input_files = files(input_files)\n> > > > +\n> > > > + if mode == 'controls'\n> > > > + template_file = files('control_ids.cpp.in')\n> > > > + else\n> > > > + template_file = files('property_ids.cpp.in')\n> > > > + endif\n> > > > +\n> > > > + control_sources += custom_target(mode + '_cpp',\n> > > > input : input_files,\n> > > > - output : source + '.cpp',\n> > > > + output : mode + '_ids.cpp',\n> > > > command : [gen_controls, '-o', '@OUTPUT@',\n> > > > '--mode', mode, '-t', template_file, '@INPUT@'])\n> > > > endforeach\n> > > > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids_core.yaml\n> > > > similarity index 100%\n> > > > rename from src/libcamera/property_ids.yaml\n> > > > rename to src/libcamera/property_ids_core.yaml\n> > > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > > > index cfcfd4d16acf..8ae8d5126e39 100755\n> > > > --- a/src/py/libcamera/gen-py-controls.py\n> > > > +++ b/src/py/libcamera/gen-py-controls.py\n> > > > @@ -95,7 +95,7 @@ def main(argv):\n> > > > help='Output file name. Defaults to standard output if not specified.')\n> > > > parser.add_argument('--template', '-t', type=str, required=True,\n> > > > help='Template file name.')\n> > > > - parser.add_argument('input', type=str,\n> > > > + parser.add_argument('input', type=str, nargs='+',\n> > > > help='Input file name.')\n> > > > args = parser.parse_args(argv[1:])\n> > > >\n> > > > @@ -103,11 +103,11 @@ def main(argv):\n> > > > print(f'Invalid mode option \"{args.mode}\"', file=sys.stderr)\n> > > > return -1\n> > > >\n> > > > - data = open(args.input, 'rb').read()\n> > > > -\n> > > > controls = {}\n> > > > - vendor = yaml.safe_load(data)['vendor']\n> > > > - controls[vendor] = yaml.safe_load(data)['controls']\n> > > > + for input in args.input:\n> > > > + data = open(input, 'rb').read()\n> > > > + vendor = yaml.safe_load(data)['vendor']\n> > > > + controls[vendor] = yaml.safe_load(data)['controls']\n> > > >\n> > > > data = generate_py(controls, args.mode)\n> > > >\n> > > > diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> > > > index 1c3ea1843ac0..31af63ec0dc6 100644\n> > > > --- a/src/py/libcamera/meson.build\n> > > > +++ b/src/py/libcamera/meson.build\n> > > > @@ -28,11 +28,15 @@ pycamera_sources = files([\n> > > >\n> > > > # Generate controls\n> > > >\n> > > > -gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')\n> > > > +gen_py_controls_input_files = []\n> > > > gen_py_controls_template = files('py_controls_generated.cpp.in')\n> > > >\n> > > > gen_py_controls = files('gen-py-controls.py')\n> > > >\n> > > > +foreach file : controls_files\n> > > > + gen_py_controls_input_files += files('../../libcamera/' + file)\n> > > > +endforeach\n> > > > +\n> > > > pycamera_sources += custom_target('py_gen_controls',\n> > > > input : gen_py_controls_input_files,\n> > > > output : ['py_controls_generated.cpp'],\n> > > > @@ -41,9 +45,13 @@ pycamera_sources += custom_target('py_gen_controls',\n> > > >\n> > > > # Generate properties\n> > > >\n> > > > -gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > > > +gen_py_property_enums_input_files = []\n> > > > gen_py_properties_template = files('py_properties_generated.cpp.in')\n> > > >\n> > > > +foreach file : properties_files\n> > > > + gen_py_property_enums_input_files += files('../../libcamera/' + file)\n> > > > +endforeach\n> > > > +\n> > > > pycamera_sources += custom_target('py_gen_properties',\n> > > > input : gen_py_property_enums_input_files,\n> > > > output : ['py_properties_generated.cpp'],\n> > > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > > > index 6680ecf84acb..30c58f35473c 100755\n> > > > --- a/utils/gen-controls.py\n> > > > +++ b/utils/gen-controls.py\n> > > > @@ -12,6 +12,7 @@ import operator\n> > > > import string\n> > > > import sys\n> > > > import yaml\n> > > > +import os\n> > > >\n> > > >\n> > > > class ControlEnum(object):\n> > > > @@ -339,15 +340,17 @@ def main(argv):\n> > > > help='Output file name. Defaults to standard output if not specified.')\n> > > > parser.add_argument('--template', '-t', dest='template', type=str, required=True,\n> > > > help='Template file name.')\n> > > > - parser.add_argument('input', type=str,\n> > > > + parser.add_argument('input', type=str, nargs='+',\n> > > > help='Input file name.')\n> > > >\n> > > > args = parser.parse_args(argv[1:])\n> > > >\n> > > > - data = open(args.input, 'rb').read()\n> > > > - vendor = yaml.safe_load(data)['vendor']\n> > > > - controls = yaml.safe_load(data)['controls']\n> > > > - controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]\n> > > > + controls = []\n> > > > + for input in args.input:\n> > > > + data = open(input, 'rb').read()\n> > > > + vendor = yaml.safe_load(data)['vendor']\n> > > > + ctrls = yaml.safe_load(data)['controls']\n> > > > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n> > >\n> > > How does thi work in Python, should data be closed ?\n> > \n> > I think it does get closed since we don't hold onto a reference to the\n> > open() return value. But please correct me if that is wrong since I\n> > was (blindly) following what the existing code was doing.\n> > \n> \n> Not a python expert enough here, but I know I often use \"\n> \twith open(input, 'rb') as file:\n> \t\tdata = file.read\n> \n> etc ... which automatically closes the file at the end of that scope.\n\nThat's the most pythonic way to handle scoping for a file object indeed,\nwhen explicit management of the scope is needed (i.e. when you need to\nknow exactly when the file will be closed).\n\n> With the style above - there is no handle to the opened file - so we\n> don't have anything to call close on ... so it's a bit of refactoring\n> either way - unless we can be sure python closes automatically.\n\nPython will close the file. The reference count of object returned by\nopen() reaches 0 when the read() function returns, so the file will get\nclosed immediately as far as I understand:\n\n>>> class Foo(object):\n... def __init__(self):\n... print('__init__')\n... def __del__(self):\n... print('__del__')\n...\n>>> f = Foo()\n__init__\n>>> f = None ; print('deleted')\n__del__\ndeleted\n\n> But ... we're certainly going to close when the file exits...\n\nAnd there are few files anyway, so even if the objects stayed around\nuntil a garbage collection pass it wouldn't matter that much.\n\n> With that resolved in a suitable way (which could just be confirming\n> that it autocloses if that works) and the debug message removed:\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> > > Anyway, this all looks good, with the above 'message' dropped\n> > >\n> > > Reviewed-by: Jacopo Mondi \n> > >\n> > > >\n> > > > if args.template.endswith('.cpp.in'):\n> > > > data = generate_cpp(controls)","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 01E46C31E9\n\tfor ;\n\tThu, 30 Nov 2023 09:08:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 27025629C3;\n\tThu, 30 Nov 2023 10:08:09 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3AF9C629BB\n\tfor ;\n\tThu, 30 Nov 2023 10:08:07 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 653706B8;\n\tThu, 30 Nov 2023 10:07:30 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1701335289;\n\tbh=7Drdgd4Jw/GVu3AEGdW5IAllF+gqJab+ABWAksiDMQo=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=VOsgXDHTmSxF1rV3yHQp7RTJNakD6FmJ2lmpaRtBu4KmO6VioDRwMAH62fsTS5txr\n\t6wHEKOeh8CVfYBDQ2OlcEkrKjy6D4DwJGNBFUBAZo0PpICN4Kr9z0zeamTnzCZt357\n\tvoJMlgqiUZqBnrXEjDfiEmGBHkAO27BA9nyuhH2EQThrKzvfXITYn9XV4X71PyH+J0\n\tXMCRnQNqEyKx98rjdItn9ngi2cvVmdsnqe7L1FF2ydbmKohyO3dS2zapv2LN6029O3\n\tp3K9avC9LUN8Ra63aDl6sFJeB3Z6x3CyTR1bCxvZGm666YAahnjwYNzO6Vj5w7pDMO\n\t6UXe7dkfAervA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1701335250;\n\tbh=7Drdgd4Jw/GVu3AEGdW5IAllF+gqJab+ABWAksiDMQo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dKNDAN6mgLs5QqLh0vo561kD7hMqsylU4gumHXt6dzoZYgoo3IKMh1UfS+fM5Ub1J\n\tE+Nm40UE1O5GcyRNyX3iHutBxWYA7Ayd48wnXkYlmZfeukUb+sbpS2N+LPtslidbdj\n\tWj5/WLALGuZ3He11JeNXCujAYCM56e+J/dxeVFYU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"dKNDAN6m\"; dkim-atps=neutral","Date":"Thu, 30 Nov 2023 11:08:13 +0200","To":"Kieran Bingham ","Message-ID":"<20231130090813.GD8402@pendragon.ideasonboard.com>","References":"<20231124123713.22519-1-naush@raspberrypi.com>\n\t<20231124123713.22519-4-naush@raspberrypi.com>\n\t\n\t\n\t<170118415014.630990.4215388948375627091@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<170118415014.630990.4215388948375627091@ping.linuxembedded.co.uk>","Subject":"Re: [libcamera-devel] [PATCH v3 3/7] build: controls: Rework how\n\tcontrols and properties are generated","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"Jacopo Mondi ,\n\tNaushir Patuck via libcamera-devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]