[{"id":26009,"web_url":"https://patchwork.libcamera.org/comment/26009/","msgid":"","date":"2022-12-07T00:54:05","subject":"Re: [libcamera-devel] [PATCH] Expose the Request Sequence Number in\n\tPython Bindings","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Matthew,\n\nThank you for the patch.\n\nOn Tue, Dec 06, 2022 at 03:29:54PM -0800, Matthew Goodman via libcamera-devel wrote:\n> This is a one line change to expose the Request object's sequence number on the\n> pybind11 surfaces for use in upstream applications.\n> \n> I'm new to this email based approach (let me know if I am doing this right),\n> but the associated PR is here:\n> https://github.com/kbingham/libcamera/pull/61\n\nYou're doing a fairly good job overall :-) A few comments:\n\n- This last paragraph in the commit message shouldn't be part of the\n commit message itself (it shouldn't be recorded in the git history),\n so it should go after the --- line (see [*]).\n\n- Please start the subject line with the correct prefix. You can use git\n log on the file or directory you're modifying to have an idea of what\n the right prefix is.\n\n- Kieran likes to point to https://cbea.ms/git-commit/ as a good source\n of information regarding how to write commit messages (you can ignore\n the \"Limit the subject line to 50 characters\" rule, that's not\n achievable when adding prefixes). This should tell you that the commit\n message should explain *why* the change is useful, not just what it\n does.\n\n- Please don't post HTML e-mails, especially for patches, that messes up\n the formatting and makes it impossible to apply them correctly. I\n recomment using git-send-email to send patches, it will avoid lots of\n issues with regular e-mail clients (web interfaces are particularly\n ill-suited, they are pretty much guaranteed to mess things up). We\n have a guide to assist configuration of git-send-email if you've never\n used it, you can find it at\n https://libcamera.org/contributing.html#submitting-patches.\n\nThe code change itself looks good :-)\n\n> Signed-off-by: Matthew Goodman \n> ---\n\n[*] here\n\n> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> index 75947889..d14e18e2 100644\n> --- a/src/py/libcamera/py_main.cpp\n> +++ b/src/py/libcamera/py_main.cpp\n> @@ -335,6 +335,7 @@ PYBIND11_MODULE(_libcamera, m)\n>                 .def_property_readonly(\"status\", &Request::status)\n>                 .def_property_readonly(\"buffers\", &Request::buffers)\n>                 .def_property_readonly(\"cookie\", &Request::cookie)\n> +               .def_property_readonly(\"sequence\", &Request::sequence)\n>                 .def_property_readonly(\"has_pending_buffers\", &\n> Request::hasPendingBuffers)\n>                 .def(\"set_control\", [](Request &self, const ControlId &id,\n> py::object value) {\n>                         self.controls().set(id.id(), pyToControlValue(value,\n> id.type()));","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 CFDE9BE08B\n\tfor ;\n\tWed, 7 Dec 2022 00:54:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0B7446333F;\n\tWed, 7 Dec 2022 01:54:10 +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 8CA0D61F22\n\tfor ;\n\tWed, 7 Dec 2022 01:54:08 +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 D55843D7;\n\tWed, 7 Dec 2022 01:54:07 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1670374450;\n\tbh=QDczx0ONgnf+hyDjKAnzu1XGm5oX42Bk7lVbZ48r4u4=;\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=TSxYJ87Hd/vZMbOFr9Isdg8Cn98i4D9kq0Ezwn3UgvoFYj1AFumJkx4/guhnZ6R2s\n\tjcclB+Uhq0ZqRhofW7cg0n9LpyMlnkyc27FkUdeyGbiKzTz9cfTGPrhWZJIyajGms8\n\tZEMlcqdp/R7jNnL6VXi4J34Q+s2UZefU/PuwEmyTM/zwaqt9UhYaeI3DlRN+ik694X\n\tOm+yz0A8qE9/sxghpY6e91ZmHV0XGeUWBLElP1yucH5peZhjRDAZUg/Bsv/CuwoOQW\n\tzjrVrSSwBpOUqOqc3R0xe4V6DuO1w/NvLpFaOJdLINzgI+tXqbaBQ21ZrqtbqgV+3p\n\teB3ZMdujTj0Ew==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1670374448;\n\tbh=QDczx0ONgnf+hyDjKAnzu1XGm5oX42Bk7lVbZ48r4u4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XMrhsT/FswMxQVfGpXD6/ClCS80CLxpI04Hosfd31U3+Hs2/OAtgR8RI3gxB8QJP+\n\t0d6Ci8diqOEy62sLCj9WiScpgIWHE2AwHqoiIRuk8wSkaRMyGYU68TuEvxZY7qQanV\n\tHMJujUEJLumOy8ssHeRlLnPR/Tsqo16nEf/N8vsg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"XMrhsT/F\"; dkim-atps=neutral","Date":"Wed, 7 Dec 2022 02:54:05 +0200","To":"Matthew Goodman ","Message-ID":"","References":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] Expose the Request Sequence Number in\n\tPython Bindings","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\" "}},{"id":26017,"web_url":"https://patchwork.libcamera.org/comment/26017/","msgid":"<167041228458.9133.4209551127710622126@Monstersaurus>","date":"2022-12-07T11:24:44","subject":"Re: [libcamera-devel] [PATCH] Expose the Request Sequence Number in\n\tPython Bindings","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Matthew,\n\nQuoting Laurent Pinchart via libcamera-devel (2022-12-07 00:54:05)\n> Hi Matthew,\n> \n> Thank you for the patch.\n> \n> On Tue, Dec 06, 2022 at 03:29:54PM -0800, Matthew Goodman via libcamera-devel wrote:\n> > This is a one line change to expose the Request object's sequence number on the\n> > pybind11 surfaces for use in upstream applications.\n> > \n> > I'm new to this email based approach (let me know if I am doing this right),\n> > but the associated PR is here:\n> > https://github.com/kbingham/libcamera/pull/61\n> \n> You're doing a fairly good job overall :-) A few comments:\n> \n> - This last paragraph in the commit message shouldn't be part of the\n> commit message itself (it shouldn't be recorded in the git history),\n> so it should go after the --- line (see [*]).\n> \n> - Please start the subject line with the correct prefix. You can use git\n> log on the file or directory you're modifying to have an idea of what\n> the right prefix is.\n> \n> - Kieran likes to point to https://cbea.ms/git-commit/ as a good source\n> of information regarding how to write commit messages (you can ignore\n> the \"Limit the subject line to 50 characters\" rule, that's not\n> achievable when adding prefixes). This should tell you that the commit\n> message should explain *why* the change is useful, not just what it\n> does.\n\nYeah ... ;-)\n\nAt least something like this:\n\n\"\"\"\nThe python bindings are missing the ability to read the sequence number\nof the Request object from the public API.\n\nExpose the objects sequence number on the pybind11 surfaces to support\napplications reading this value.\n\"\"\"\n\nAnd for the change from me,\n\n\nReviewed-by: Kieran Bingham \n\nYou can add Review by tags that are given to you directly to any follow\nup patches, so please add this text above your Signed-off: line.\n\n> \n> - Please don't post HTML e-mails, especially for patches, that messes up\n> the formatting and makes it impossible to apply them correctly. I\n> recomment using git-send-email to send patches, it will avoid lots of\n\nAs this is python code, perhaps Tomi could give the second review (on\nv2) - and then we can merge this.\n\n\n> issues with regular e-mail clients (web interfaces are particularly\n> ill-suited, they are pretty much guaranteed to mess things up). We\n> have a guide to assist configuration of git-send-email if you've never\n> used it, you can find it at\n> https://libcamera.org/contributing.html#submitting-patches.\n> \n> The code change itself looks good :-)\n> \n\nAgreed. I wish the sort order of these defs was the same as the public\ninterface though, but that's not something to fix in this patch.\n\n--\nKieran\n\n\n> > Signed-off-by: Matthew Goodman \n> > ---\n> \n> [*] here\n> \n> > diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> > index 75947889..d14e18e2 100644\n> > --- a/src/py/libcamera/py_main.cpp\n> > +++ b/src/py/libcamera/py_main.cpp\n> > @@ -335,6 +335,7 @@ PYBIND11_MODULE(_libcamera, m)\n> >                 .def_property_readonly(\"status\", &Request::status)\n> >                 .def_property_readonly(\"buffers\", &Request::buffers)\n> >                 .def_property_readonly(\"cookie\", &Request::cookie)\n> > +               .def_property_readonly(\"sequence\", &Request::sequence)\n> >                 .def_property_readonly(\"has_pending_buffers\", &\n> > Request::hasPendingBuffers)\n> >                 .def(\"set_control\", [](Request &self, const ControlId &id,\n> > py::object value) {\n> >                         self.controls().set(id.id(), pyToControlValue(value,\n> > id.type()));\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 B145FBE08B\n\tfor ;\n\tWed, 7 Dec 2022 11:24:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 225C563335;\n\tWed, 7 Dec 2022 12:24:49 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2499863335\n\tfor ;\n\tWed, 7 Dec 2022 12:24:48 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 80B7E87B;\n\tWed, 7 Dec 2022 12:24:47 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1670412289;\n\tbh=dFCssISHqgZWzH57LjL5EhjAOn6T4dp9fLIKo7b55Rk=;\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=3Gd3RbSt2oM7yj4ixiFJ9oSYtlYWtxj53Mw6OEdQMWRwzF+TFgxIcYuBieDLSzRmW\n\tLjJxYEOnMxQZWx7xWHErAmryrK5r/985fe2CRrJbYOBA21l7s6qu3juP1zyesI2Opp\n\tsMN+anFJzEP21/ZkE3menlfOtiecCKnleU9Dad8xVmfYnhZd/vwiwb5NCO9g9yZVEX\n\torzrQof1HfvmomQOkbsKu5c8tPLZbGRkoDCEQDg4X4Z+iv83Xuicmzb4CfKCW54kQg\n\tNe3S29fCLBEVNG/C4aHUq2cdzQuRZDaJdrA7xmTqCIkpnherBUD8U7HWwTFHzGlFZT\n\tv4BFx3ry4nDgw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1670412287;\n\tbh=dFCssISHqgZWzH57LjL5EhjAOn6T4dp9fLIKo7b55Rk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Jxmea/m9lDpD4MGyBj9ITcv/DhRT52FupCPivmGfUNDUtpp+MMWwmK01bsI+E1hvt\n\tnaQXfs0tth6+PKB5/YBy4IM4pujQdIZsM+yoWBiUbLXL7oHV4L5dPXI9nQYw6QH51e\n\tCuK5jzNQv7lBXvBGXxfmdtbVlXenRcBOcIr9EEok="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Jxmea/m9\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"\n\t","To":"Laurent Pinchart ,\n\tLaurent Pinchart via libcamera-devel\n\t, \n\tMatthew Goodman ","Date":"Wed, 07 Dec 2022 11:24:44 +0000","Message-ID":"<167041228458.9133.4209551127710622126@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] Expose the Request Sequence Number in\n\tPython Bindings","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\" "}}]