[{"id":31605,"web_url":"https://patchwork.libcamera.org/comment/31605/","msgid":"<20241007221229.GE30699@pendragon.ideasonboard.com>","date":"2024-10-07T22:12:29","subject":"Re: [PATCH v2 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":"Hi Stefan,\n\nThank you for the patch.\n\nOn Mon, Oct 07, 2024 at 11:54:07AM +0200, Stefan Klug wrote:\n> For flexible debugging it is helpful to minimize the roundtrip time.\n> This script parses the sourcetree and looks for usages of\n\ns/sourcetree/source tree/\n\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> Signed-off-by: Stefan Klug \n> \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 | 165 ++++++++++++++++++++++++++++++++++++\n> 1 file changed, 165 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..c5c4570ffd00\n> --- /dev/null\n> +++ b/utils/gen-debug-controls.py\n> @@ -0,0 +1,165 @@\n> +#!/usr/bin/env python3\n> +# SPDX-License-Identifier: GPL-2.0-or-later\n> +# Copyright (C) 2024, Ideas on Board 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 ruyaml\n> +except:\n> + try:\n> + import ruamel.yaml as ruyaml\n\nDo we want to try both ruyaml and ruamel.yaml ? If their behaviour is\nidentical then I'd use the latter only as it's packaged by Debian. If\ntheir behaviour is different, then this is calling for trouble as one of\nthe two will se less testing. I'd thus just try ruamel.yaml.\n\n> + except:\n> + logger.error(\n> + f'Failed to import ruyaml. Please install the ruyaml or 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> + # possibly followed by template param\n> + r'(?:\\<((?:[^)(])*)\\>)?'\n> + # referencing a debug control\n> + r'\\(\\s*controls::debug::(\\w+)\\s*,'\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 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\nI think you can drop the trailing comma.\n\n> + args = 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\n # Create a map of names in the existing yaml for easier updating.\n\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 comments etc.)\n\n # Remove elements from controls without recreating the list (to keep\n # comments etc.).\n\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 copyright header\n> + f.write((\"# SPDX-License-Identifier: LGPL-2.1-or-later\\n\"\n> + \"#\\n\"\n> + \"# This file is 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 146CBBE080\n\tfor ;\n\tMon, 7 Oct 2024 22:12:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0F5F563537;\n\tTue, 8 Oct 2024 00:12:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0328B62C91\n\tfor ;\n\tTue, 8 Oct 2024 00:12:35 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [132.205.230.14])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5AE042E0;\n\tTue, 8 Oct 2024 00:10:59 +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=\"N+j63IAP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728339059;\n\tbh=qeiV31Lf5iA3jCz/eX5y7wAo7rnGh/Sc9ZrSAiDhoQ0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=N+j63IAPKlwwPbN+u5Tr2y8D8dnsAD2VzPlHE+EziYHGAkxrhBt6//p4BLDve/K2g\n\tjK9BdpDSQp1W9K/tp3uqmyBoaz2N5e2D6DnvipwWZOq6SnlzQLji2CbISIXKtdsnOA\n\thrFmvRiBg25NF5D0uyPFYyWc5Gw9h22YPikb5SbI=","Date":"Tue, 8 Oct 2024 01:12:29 +0300","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 3/7] utils: Add script to generate\n\tcontrol_ids_debug.yaml","Message-ID":"<20241007221229.GE30699@pendragon.ideasonboard.com>","References":"<20241007095425.211158-1-stefan.klug@ideasonboard.com>\n\t<20241007095425.211158-4-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241007095425.211158-4-stefan.klug@ideasonboard.com>","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":31615,"web_url":"https://patchwork.libcamera.org/comment/31615/","msgid":"","date":"2024-10-08T10:43:56","subject":"Re: [PATCH v2 3/7] utils: Add script to generate\n\tcontrol_ids_debug.yaml","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"On Tue, Oct 08, 2024 at 01:12:29AM +0300, Laurent Pinchart wrote:\n> Hi Stefan,\n> \n> Thank you for the patch.\n> \n> On Mon, Oct 07, 2024 at 11:54:07AM +0200, Stefan Klug wrote:\n> > For flexible debugging it is helpful to minimize the roundtrip time.\n> > This script parses the sourcetree and looks for usages of\n> \n> s/sourcetree/source tree/\n> \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> > Signed-off-by: Stefan Klug \n> > \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 | 165 ++++++++++++++++++++++++++++++++++++\n> > 1 file changed, 165 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..c5c4570ffd00\n> > --- /dev/null\n> > +++ b/utils/gen-debug-controls.py\n> > @@ -0,0 +1,165 @@\n> > +#!/usr/bin/env python3\n> > +# SPDX-License-Identifier: GPL-2.0-or-later\n> > +# Copyright (C) 2024, Ideas on Board 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 ruyaml\n> > +except:\n> > + try:\n> > + import ruamel.yaml as ruyaml\n> \n> Do we want to try both ruyaml and ruamel.yaml ? If their behaviour is\n> identical then I'd use the latter only as it's packaged by Debian. If\n> their behaviour is different, then this is calling for trouble as one of\n> the two will se less testing. I'd thus just try ruamel.yaml.\n\nIt is a bit of an awkward situation. ruamel.yaml is still listed as\nbeta, ruyaml as stable. As ruyaml is the fork that seems to have more\ntraction I would consider ruamel.yaml to be a dead end. debian sid and\ntrixie package ruyaml. As followup on my comment in the series v1\nthread: I tried to switch to pyyaml. It is really bad, as it reorders\nthe yaml file and \"vendor\" ends at the bottom.\n\nI can go back to the ruamel if you like, but it seems the wrong\ndirection to me. In the end it doesn't matter and the benefit of ruamel\nis the broad availability.\n\n> \n> > + except:\n> > + logger.error(\n> > + f'Failed to import ruyaml. Please install the ruyaml or 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> > + # possibly followed by template param\n> > + r'(?:\\<((?:[^)(])*)\\>)?'\n> > + # referencing a debug control\n> > + r'\\(\\s*controls::debug::(\\w+)\\s*,'\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 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> \n> I think you can drop the trailing comma.\n\nI'm sure I fixed that and the args below... now for sure.\n\n> \n> > + args = 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> \n> # Create a map of names in the existing yaml for easier updating.\n\nack\n\n> \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 comments etc.)\n> \n> # Remove elements from controls without recreating the list (to keep\n> # comments etc.).\n\nack\n\n> \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 copyright header\n> > + f.write((\"# SPDX-License-Identifier: LGPL-2.1-or-later\\n\"\n> > + \"#\\n\"\n> > + \"# This file is 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> -- \n> Regards,\n> \n> Laurent Pinchart","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 EC95CBE080\n\tfor ;\n\tTue, 8 Oct 2024 10:44:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B0505618CB;\n\tTue, 8 Oct 2024 12:44:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EC05C618C9\n\tfor ;\n\tTue, 8 Oct 2024 12:43:59 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:d:f30b:aa60:fabf])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3F3E53D6;\n\tTue, 8 Oct 2024 12:42:23 +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=\"KPmnNr2z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728384143;\n\tbh=aBVgO4MOETUlaNho0fg8MZQSSMPp3iFEMoHCEMsoCVk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KPmnNr2zVKVI9Oxr41YZdQyIWOyFwyzuMFGZ7GDv9ZebZM+7xFdWaLUSRBhZaB9gB\n\tF7jzJTDLibuVHMUHr7nzhkVqSbZC0Pu2b9XOL2rka9LoQUSSdwuh6ZShjfz8Lmzowd\n\taAAcm6z5s38xaf/+wsvyBZUOlxTX/7puv5OLMhuY=","Date":"Tue, 8 Oct 2024 12:43:56 +0200","From":"Stefan Klug ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 3/7] utils: Add script to generate\n\tcontrol_ids_debug.yaml","Message-ID":"","References":"<20241007095425.211158-1-stefan.klug@ideasonboard.com>\n\t<20241007095425.211158-4-stefan.klug@ideasonboard.com>\n\t<20241007221229.GE30699@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241007221229.GE30699@pendragon.ideasonboard.com>","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\" "}}]