[{"id":22578,"web_url":"https://patchwork.libcamera.org/comment/22578/","msgid":"","date":"2022-04-05T16:24:22","subject":"Re: [libcamera-devel] [PATCH v2 2/5] generate fixed- and\n\tvariable-sized Span Controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Christian,\n\nThank you for the patch.\n\nOn Tue, Apr 05, 2022 at 01:42:12AM +0100, Christian Rauch via libcamera-devel wrote:\n> This defines Controls with a 'size' as either variable-sized Span or as\n> fixed-sized Span.\n> \n> Signed-off-by: Christian Rauch \n> ---\n> utils/gen-controls.py | 33 +++++++++++++++++++++++----------\n> 1 file changed, 23 insertions(+), 10 deletions(-)\n> \n> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> index 3f99b5e2..4cadf160 100755\n> --- a/utils/gen-controls.py\n> +++ b/utils/gen-controls.py\n> @@ -7,6 +7,7 @@\n> # gen-controls.py - Generate control definitions from YAML\n> \n> import argparse\n> +from math import prod\n\nI'd prefer\n\nimport math\n\nand math.prod() below, to avoid future namespace clashes.\n\n> import string\n> import sys\n> import yaml\n> @@ -22,6 +23,26 @@ def format_description(description):\n> return '\\n'.join([(line and ' * ' or ' *') + line for line in description])\n> \n> \n> +def get_ctrl_type_format(ctrl):\n\nThis should be named get_ctrl_type() as it returns the control type.\n\n> + ctrl_type = ctrl['type']\n> + ctrl_size_arr = ctrl.get('size')\n> + ctrl_is_span = ctrl_size_arr is not None\n> + if ctrl_is_span:\n> + ctrl_span_size = prod(ctrl_size_arr) if len(ctrl_size_arr) > 0 else None\n\nYou can move this to the \"elif ctrl_is_span\" below.\n\n> +\n> + if ctrl_type == 'string':\n> + return 'std::string'\n> + elif ctrl_is_span:\n> + if ctrl_span_size:\n> + # fixed-sized Span\n> + return 'Span'.format(ctrl_type, ctrl_span_size)\n\nWe're increasingly standardizing on the f'...' string formatting method,\nwhich here would be\n\n return f'Span'\n\n> + else:\n> + # variable-sized Span\n> + return 'Span'.format(ctrl_type)\n\nSame here.\n\n> + else:\n> + return ctrl_type\n> +\n> +\n> def generate_cpp(controls):\n> enum_doc_start_template = string.Template('''/**\n> * \\\\enum ${name}Enum\n> @@ -50,11 +71,7 @@ ${description}\n> name, ctrl = ctrl.popitem()\n> id_name = snake_case(name).upper()\n> \n> - ctrl_type = ctrl['type']\n> - if ctrl_type == 'string':\n> - ctrl_type = 'std::string'\n> - elif ctrl.get('size'):\n> - ctrl_type = 'Span' % ctrl_type\n> + ctrl_type = get_ctrl_type_format(ctrl)\n> \n> info = {\n> 'name': name,\n> @@ -135,11 +152,7 @@ def generate_h(controls):\n> \n> ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n> \n> - ctrl_type = ctrl['type']\n> - if ctrl_type == 'string':\n> - ctrl_type = 'std::string'\n> - elif ctrl.get('size'):\n> - ctrl_type = 'Span' % ctrl_type\n> + ctrl_type = get_ctrl_type_format(ctrl)\n\nNice change overall :-)\n\n> \n> info = {\n> 'name': name,","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 63E81C3256\n\tfor ;\n\tTue, 5 Apr 2022 16:24:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DA8C265642;\n\tTue, 5 Apr 2022 18:24:26 +0200 (CEST)","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 8D19D604BB\n\tfor ;\n\tTue, 5 Apr 2022 18:24:25 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0F38C5D;\n\tTue, 5 Apr 2022 18:24:25 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649175866;\n\tbh=4YBp0gxA8V9ArsP8+RyDYVATf3RlXCgZaw6N1BIWE6c=;\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=1R7lHpWWTzcdqaL/ajfaxGhE2AKiBCbqnUUb15BysrHBeIEbVASepsHs7jSYibxL5\n\tuXwrQfGxeDKFV5n1NfWJXk7/9ysOFO2+VF1yJfbbuk3djZRuZfC5d8Hh2Hx367XLFF\n\t9Do39qJ+BCa4lF0aNQTubKQ/PKzc1MZCYYgW5HsIHs5bPCce/XhV82c4JqRX4LSPoo\n\t4MPylRZgcZ3lb24MVfIY02dDA6WkRZVcvqB/p5erH0ovA8RLaTFHFNHs4nUEhVes3w\n\t2+hMEczCRj4BGqDeaMNzXns6hWM4lyeOdYBm03Quwk37/eHdJppmdEXHvsuxBaxTN7\n\tRLT5yNdUkmEAg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649175865;\n\tbh=4YBp0gxA8V9ArsP8+RyDYVATf3RlXCgZaw6N1BIWE6c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bfbWrq5HL00RvrEOegHzr+joGjrDTm9//cBcbBkm+UwMgX4wzIYxQbXVzA4LSnqNT\n\tAs1+bf1X18O0dLyrh7NPkkncKrYx9fECGs4zbirCOsMppHIO/2s41nKagELPMd5hoa\n\t3oy4W/8s08zqViFKpnnO3nLc68KWbj9gDqocfp+I="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"bfbWrq5H\"; dkim-atps=neutral","Date":"Tue, 5 Apr 2022 19:24:22 +0300","To":"Christian Rauch ","Message-ID":"","References":"<20220405004215.86340-1-Rauch.Christian@gmx.de>\n\t<20220405004215.86340-3-Rauch.Christian@gmx.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220405004215.86340-3-Rauch.Christian@gmx.de>","Subject":"Re: [libcamera-devel] [PATCH v2 2/5] generate fixed- and\n\tvariable-sized Span Controls","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":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]