[{"id":31689,"web_url":"https://patchwork.libcamera.org/comment/31689/","msgid":"<172854886383.532453.8997809907252626783@ping.linuxembedded.co.uk>","date":"2024-10-10T08:27:43","subject":"Re: [PATCH v3 3/7] utils: Add script to generate\n\tcontrol_ids_debug.yaml","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2024-10-08 16:29:41)\n> For flexible debugging it is helpful to minimize the roundtrip time.\n> This script parses the source tree and looks for usages of\n> \n> set(controls::debug::Something,\n> \n> and adds (or removes) the controls as necessary from the yaml\n> description. It is meant to be used during development to ease the\n> creation of the correct yaml entries.\n \n\nShould we include this in the build steps for debug builds? or would\nautomating this be problematic? I guess that's where it would then\nintroduce the ruyaml as a build time dependency which perhaps we don't\nwant yet.\n\nAs this is a new script to assist development, it can't introduce\nregressions, and can be worked on on top. So the only thing I care\nspecifically about is that the formatter is clean :-)\n\n\nReviewed-by: Kieran Bingham \n\n\n> Signed-off-by: Stefan Klug \n> \n> ---\n> Changes in v3:\n> - Remove superfluous comma and args variable\n> - Default to ruamel.yaml instead of ruyaml\n> - Small fixes on layout/comments\n> \n> Changes in v2:\n> - Search only until the first comma of the set() call, to allow\n> linebreaks there.\n> - Support ruamel.yaml as fallback\n> - Rename output to ctrl_file\n> - Add \"generated by\" comment in yaml file\n> ---\n> utils/gen-debug-controls.py | 162 ++++++++++++++++++++++++++++++++++++\n> 1 file changed, 162 insertions(+)\n> create mode 100755 utils/gen-debug-controls.py\n> \n> diff --git a/utils/gen-debug-controls.py b/utils/gen-debug-controls.py\n> new file mode 100755\n> index 000000000000..025850731c0b\n> --- /dev/null\n> +++ b/utils/gen-debug-controls.py\n> @@ -0,0 +1,162 @@\n> +#!/usr/bin/env python3\n> +# SPDX-License-Identifier: GPL-2.0-or-later\n> +# Copyright (C) 2024, Google Inc.\n> +#\n> +# Author: Stefan Klug \n> +#\n> +# This script looks for occurrences of the debug metadata controls in the source\n> +# tree and updates src/libcamera/control_ids_debug.yaml accordingly. It is meant\n> +# to be used during development to ease updating of the yaml file while\n> +# debugging.\n> +\n> +import argparse\n> +import logging\n> +import os\n> +import re\n> +import sys\n> +from dataclasses import dataclass\n> +from pathlib import Path\n> +\n> +logger = logging.getLogger(__name__)\n> +logging.basicConfig(level=logging.INFO, format='%(levelname)s: %(message)s')\n> +\n> +try:\n> + import ruamel.yaml as ruyaml\n> +except:\n> + logger.error(\n> + f'Failed to import ruamel.yaml. Please install the ruamel.yaml package.')\n> + sys.exit(1)\n> +\n> +@dataclass\n> +class FoundMatch:\n> + file: os.PathLike\n> + whole_match: str\n> + line: int\n> + type: str\n> + name: str\n> + size: str = None\n> +\n> +\n> +def get_control_name(control):\n> + k = list(control.keys())\n> + if len(k) != 1:\n> + raise Exception(f\"Can't handle control entry with {len(k)} keys\")\n> + return k[0]\n> +\n> +\n> +def find_debug_controls(dir):\n> + extensions = ['.cpp', '.h']\n> + files = [p for p in dir.rglob('*') if p.suffix in extensions]\n> +\n> + # The following regex was tested on\n> + # set>( controls::debug::something , static_cast(var) )\n> + # set<>( controls::debug::something , static_cast(var) )\n> + # set( controls::debug::something , static_cast (var) )\n> + exp = re.compile(r'set' # set function\n> + r'(?:\\<((?:[^)(])*)\\>)?' # followed by a optional template param\n> + r'\\(\\s*controls::debug::(\\w+)\\s*,' # referencing a debug control\n> + )\n> + matches = []\n> + for p in files:\n> + with p.open('r') as f:\n> + for idx, line in enumerate(f):\n> + match = exp.search(line)\n> + if match:\n> + m = FoundMatch(file=p, line=idx, type=match.group(1),\n> + name=match.group(2), whole_match=match.group(0))\n> + if m.type is not None and m.type.startswith('Span'):\n> + # Simple span type detection treating the last word\n> + # inside <> as type.\n> + r = re.match(r'Span<(?:.*\\s+)(.*)>', m.type)\n> + m.type = r.group(1)\n> + m.size = '[n]'\n> + matches.append(m)\n> + return matches\n> +\n> +\n> +def main(argv):\n> + parser = argparse.ArgumentParser(\n> + description='Automatically updates control_ids_debug.yaml')\n> + parser.parse_args(argv[1:])\n> +\n> + yaml = ruyaml.YAML()\n> + root_dir = Path(__file__).resolve().parent.parent\n> + ctrl_file = root_dir.joinpath('src/libcamera/control_ids_debug.yaml')\n> +\n> + matches = find_debug_controls(root_dir.joinpath('src'))\n> +\n> + doc = yaml.load(ctrl_file)\n> +\n> + controls = doc['controls']\n> +\n> + # Create a map of names in the existing yaml for easier updating.\n> + controls_map = {}\n> + for control in controls:\n> + for k, v in control.items():\n> + controls_map[k] = v\n> +\n> + obsolete_names = list(controls_map.keys())\n> +\n> + for m in matches:\n> + if not m.type:\n> + p = m.file.relative_to(Path.cwd(), walk_up=True)\n> + logger.warning(\n> + f'{p}:{m.line + 1}: Failed to deduce type from {m.whole_match} ... skipping')\n> + continue\n> +\n> + p = m.file.relative_to(root_dir)\n> + desc = {'type': m.type,\n> + 'description': f'Debug control {m.name} found in {p}:{m.line}'}\n> + if m.size is not None:\n> + desc['size'] = m.size\n> +\n> + if m.name in controls_map:\n> + # Can't use == for modified check because of the special yaml dicts.\n> + update_needed = False\n> + if list(controls_map[m.name].keys()) != list(desc.keys()):\n> + update_needed = True\n> + else:\n> + for k, v in controls_map[m.name].items():\n> + if v != desc[k]:\n> + update_needed = True\n> + break\n> +\n> + if update_needed:\n> + logger.info(f\"Update control '{m.name}'\")\n> + controls_map[m.name].clear()\n> + controls_map[m.name].update(desc)\n> +\n> + obsolete_names.remove(m.name)\n> + else:\n> + logger.info(f\"Add control '{m.name}'\")\n> + insert_before = len(controls)\n> + for idx, control in enumerate(controls):\n> + if get_control_name(control).lower() > m.name.lower():\n> + insert_before = idx\n> + break\n> + controls.insert(insert_before, {m.name: desc})\n> +\n> + # Remove elements from controls without recreating the list (to keep\n> + # comments etc.).\n> + idx = 0\n> + while idx < len(controls):\n> + name = get_control_name(controls[idx])\n> + if name in obsolete_names:\n> + logger.info(f\"Remove control '{name}'\")\n> + controls.pop(idx)\n> + else:\n> + idx += 1\n> +\n> + with ctrl_file.open('w') as f:\n> + # Ruyaml looses the header.\n> + f.write((\"# SPDX-License-Identifier: LGPL-2.1-or-later\\n\"\n> + \"#\\n\"\n> + \"# This file was generated by utils/gen-debug-controls.py\\n\"\n> + \"#\\n\"))\n> + yaml.dump(doc, f)\n> +\n> + return 0\n> +\n> +\n> +if __name__ == '__main__':\n> + sys.exit(main(sys.argv))\n> -- \n> 2.43.0\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 E9FD6C32EA\n\tfor ;\n\tThu, 10 Oct 2024 08:27:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 07D9765369;\n\tThu, 10 Oct 2024 10:27:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 98A4A63536\n\tfor ;\n\tThu, 10 Oct 2024 10:27:46 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9DB444D4;\n\tThu, 10 Oct 2024 10:26:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"wImLLLXV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728548768;\n\tbh=913/cPknrQhGOMW6QUC1Iy7VdrRWvfvRTDnCgPOOGYA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=wImLLLXV0Ng8VWRVSHUAz7Q3ybIzPzEPOLNqHi+6cX/azZVjDRulMABTDX9EexVd7\n\trEnDE6deZem/JQ6HK2WThI9xU6Q/QTh4oNKIiegFPJ+eBW1YjPmT4QyKSNwqJg6QZk\n\tVjHPjhdCfJvRubu7ToNoTSUJVkALFcn3CX/EjWII=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241008153031.429906-4-stefan.klug@ideasonboard.com>","References":"<20241008153031.429906-1-stefan.klug@ideasonboard.com>\n\t<20241008153031.429906-4-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH v3 3/7] utils: Add script to generate\n\tcontrol_ids_debug.yaml","From":"Kieran Bingham ","Cc":"Stefan Klug ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 10 Oct 2024 09:27:43 +0100","Message-ID":"<172854886383.532453.8997809907252626783@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31711,"web_url":"https://patchwork.libcamera.org/comment/31711/","msgid":"<20241010205752.GO32107@pendragon.ideasonboard.com>","date":"2024-10-10T20:57:52","subject":"Re: [PATCH v3 3/7] utils: Add script to generate\n\tcontrol_ids_debug.yaml","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Oct 10, 2024 at 09:27:43AM +0100, Kieran Bingham wrote:\n> Quoting Stefan Klug (2024-10-08 16:29:41)\n> > For flexible debugging it is helpful to minimize the roundtrip time.\n> > This script parses the source tree and looks for usages of\n> > \n> > set(controls::debug::Something,\n> > \n> > and adds (or removes) the controls as necessary from the yaml\n> > description. It is meant to be used during development to ease the\n> > creation of the correct yaml entries.\n> \n> Should we include this in the build steps for debug builds? or would\n> automating this be problematic? I guess that's where it would then\n> introduce the ruyaml as a build time dependency which perhaps we don't\n> want yet.\n\nIt would create a chicken-and-egg issue. The yaml file is needed to\ncompile libcamera, the script generates it by parsing IPA source files,\nand IPA modules need to link against libcamera. That's why we decide to\nmake it a developer helper script for the time being.\n\n> As this is a new script to assist development, it can't introduce\n> regressions, and can be worked on on top. So the only thing I care\n> specifically about is that the formatter is clean :-)\n> \n> Reviewed-by: Kieran Bingham \n> \n> > Signed-off-by: Stefan Klug \n\nReviewed-by: Laurent Pinchart \n\n> > \n> > ---\n> > Changes in v3:\n> > - Remove superfluous comma and args variable\n> > - Default to ruamel.yaml instead of ruyaml\n> > - Small fixes on layout/comments\n> > \n> > Changes in v2:\n> > - Search only until the first comma of the set() call, to allow\n> > linebreaks there.\n> > - Support ruamel.yaml as fallback\n> > - Rename output to ctrl_file\n> > - Add \"generated by\" comment in yaml file\n> > ---\n> > utils/gen-debug-controls.py | 162 ++++++++++++++++++++++++++++++++++++\n> > 1 file changed, 162 insertions(+)\n> > create mode 100755 utils/gen-debug-controls.py\n> > \n> > diff --git a/utils/gen-debug-controls.py b/utils/gen-debug-controls.py\n> > new file mode 100755\n> > index 000000000000..025850731c0b\n> > --- /dev/null\n> > +++ b/utils/gen-debug-controls.py\n> > @@ -0,0 +1,162 @@\n> > +#!/usr/bin/env python3\n> > +# SPDX-License-Identifier: GPL-2.0-or-later\n> > +# Copyright (C) 2024, Google Inc.\n> > +#\n> > +# Author: Stefan Klug \n> > +#\n> > +# This script looks for occurrences of the debug metadata controls in the source\n> > +# tree and updates src/libcamera/control_ids_debug.yaml accordingly. It is meant\n> > +# to be used during development to ease updating of the yaml file while\n> > +# debugging.\n> > +\n> > +import argparse\n> > +import logging\n> > +import os\n> > +import re\n> > +import sys\n> > +from dataclasses import dataclass\n> > +from pathlib import Path\n> > +\n> > +logger = logging.getLogger(__name__)\n> > +logging.basicConfig(level=logging.INFO, format='%(levelname)s: %(message)s')\n> > +\n> > +try:\n> > + import ruamel.yaml as ruyaml\n> > +except:\n> > + logger.error(\n> > + f'Failed to import ruamel.yaml. Please install the ruamel.yaml package.')\n> > + sys.exit(1)\n> > +\n> > +@dataclass\n> > +class FoundMatch:\n> > + file: os.PathLike\n> > + whole_match: str\n> > + line: int\n> > + type: str\n> > + name: str\n> > + size: str = None\n> > +\n> > +\n> > +def get_control_name(control):\n> > + k = list(control.keys())\n> > + if len(k) != 1:\n> > + raise Exception(f\"Can't handle control entry with {len(k)} keys\")\n> > + return k[0]\n> > +\n> > +\n> > +def find_debug_controls(dir):\n> > + extensions = ['.cpp', '.h']\n> > + files = [p for p in dir.rglob('*') if p.suffix in extensions]\n> > +\n> > + # The following regex was tested on\n> > + # set>( controls::debug::something , static_cast(var) )\n> > + # set<>( controls::debug::something , static_cast(var) )\n> > + # set( controls::debug::something , static_cast (var) )\n> > + exp = re.compile(r'set' # set function\n> > + r'(?:\\<((?:[^)(])*)\\>)?' # followed by a optional template param\n> > + r'\\(\\s*controls::debug::(\\w+)\\s*,' # referencing a debug control\n> > + )\n> > + matches = []\n> > + for p in files:\n> > + with p.open('r') as f:\n> > + for idx, line in enumerate(f):\n> > + match = exp.search(line)\n> > + if match:\n> > + m = FoundMatch(file=p, line=idx, type=match.group(1),\n> > + name=match.group(2), whole_match=match.group(0))\n> > + if m.type is not None and m.type.startswith('Span'):\n> > + # Simple span type detection treating the last word\n> > + # inside <> as type.\n> > + r = re.match(r'Span<(?:.*\\s+)(.*)>', m.type)\n> > + m.type = r.group(1)\n> > + m.size = '[n]'\n> > + matches.append(m)\n> > + return matches\n> > +\n> > +\n> > +def main(argv):\n> > + parser = argparse.ArgumentParser(\n> > + description='Automatically updates control_ids_debug.yaml')\n> > + parser.parse_args(argv[1:])\n> > +\n> > + yaml = ruyaml.YAML()\n> > + root_dir = Path(__file__).resolve().parent.parent\n> > + ctrl_file = root_dir.joinpath('src/libcamera/control_ids_debug.yaml')\n> > +\n> > + matches = find_debug_controls(root_dir.joinpath('src'))\n> > +\n> > + doc = yaml.load(ctrl_file)\n> > +\n> > + controls = doc['controls']\n> > +\n> > + # Create a map of names in the existing yaml for easier updating.\n> > + controls_map = {}\n> > + for control in controls:\n> > + for k, v in control.items():\n> > + controls_map[k] = v\n> > +\n> > + obsolete_names = list(controls_map.keys())\n> > +\n> > + for m in matches:\n> > + if not m.type:\n> > + p = m.file.relative_to(Path.cwd(), walk_up=True)\n> > + logger.warning(\n> > + f'{p}:{m.line + 1}: Failed to deduce type from {m.whole_match} ... skipping')\n> > + continue\n> > +\n> > + p = m.file.relative_to(root_dir)\n> > + desc = {'type': m.type,\n> > + 'description': f'Debug control {m.name} found in {p}:{m.line}'}\n> > + if m.size is not None:\n> > + desc['size'] = m.size\n> > +\n> > + if m.name in controls_map:\n> > + # Can't use == for modified check because of the special yaml dicts.\n> > + update_needed = False\n> > + if list(controls_map[m.name].keys()) != list(desc.keys()):\n> > + update_needed = True\n> > + else:\n> > + for k, v in controls_map[m.name].items():\n> > + if v != desc[k]:\n> > + update_needed = True\n> > + break\n> > +\n> > + if update_needed:\n> > + logger.info(f\"Update control '{m.name}'\")\n> > + controls_map[m.name].clear()\n> > + controls_map[m.name].update(desc)\n> > +\n> > + obsolete_names.remove(m.name)\n> > + else:\n> > + logger.info(f\"Add control '{m.name}'\")\n> > + insert_before = len(controls)\n> > + for idx, control in enumerate(controls):\n> > + if get_control_name(control).lower() > m.name.lower():\n> > + insert_before = idx\n> > + break\n> > + controls.insert(insert_before, {m.name: desc})\n> > +\n> > + # Remove elements from controls without recreating the list (to keep\n> > + # comments etc.).\n> > + idx = 0\n> > + while idx < len(controls):\n> > + name = get_control_name(controls[idx])\n> > + if name in obsolete_names:\n> > + logger.info(f\"Remove control '{name}'\")\n> > + controls.pop(idx)\n> > + else:\n> > + idx += 1\n> > +\n> > + with ctrl_file.open('w') as f:\n> > + # Ruyaml looses the header.\n> > + f.write((\"# SPDX-License-Identifier: LGPL-2.1-or-later\\n\"\n> > + \"#\\n\"\n> > + \"# This file was generated by utils/gen-debug-controls.py\\n\"\n> > + \"#\\n\"))\n> > + yaml.dump(doc, f)\n> > +\n> > + return 0\n> > +\n> > +\n> > +if __name__ == '__main__':\n> > + sys.exit(main(sys.argv))","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 5B831C3260\n\tfor ;\n\tThu, 10 Oct 2024 20:58:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 61A976536B;\n\tThu, 10 Oct 2024 22:57:59 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 179EF6353A\n\tfor ;\n\tThu, 10 Oct 2024 22:57:57 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [132.205.230.15])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8BDB64D4;\n\tThu, 10 Oct 2024 22:56:18 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"uM+JHHDz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728593778;\n\tbh=agCdAVogbpaWNLHFdjcVcr1ouEIYUMYr/ynSpaRkqsY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uM+JHHDzsfIYzSdkoI+wL4mPNYcUNbNnMGHt/Ff86vkm3iUA1U31gNW5gmWUZlcvx\n\taeLAs9cZ4AHFOXnMpQnxm8wTsQia4KJa2ip4I8A50Gu+Q2gpgZLpuH70+I7+yYcsHQ\n\taKA7BT4dGH5FCDaPVcN+ttdOAwp+OkK256WmE9hA=","Date":"Thu, 10 Oct 2024 23:57:52 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 3/7] utils: Add script to generate\n\tcontrol_ids_debug.yaml","Message-ID":"<20241010205752.GO32107@pendragon.ideasonboard.com>","References":"<20241008153031.429906-1-stefan.klug@ideasonboard.com>\n\t<20241008153031.429906-4-stefan.klug@ideasonboard.com>\n\t<172854886383.532453.8997809907252626783@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<172854886383.532453.8997809907252626783@ping.linuxembedded.co.uk>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]