[{"id":18667,"web_url":"https://patchwork.libcamera.org/comment/18667/","msgid":"<20210810095531.GV2167@pyrite.rasen.tech>","date":"2021-08-10T09:55:31","subject":"Re: [libcamera-devel] [PATCH 2/4] ipa: vimc: Rename IPA trace enums","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Umang,\n\nOn Fri, Aug 06, 2021 at 03:44:07PM +0530, Umang Jain wrote:\n> To avoid the conflict of naming with IPAOperations in future,\n> rename trace enum to IPATrace*.\n\nWhat kind of conflicts might we have? I'm not seeing the future that\nyou're seeing :S\n\n\nPaul\n\n> \n> This commit does not introduce any functional changes.\n> \n> Signed-off-by: Umang Jain \n> ---\n> include/libcamera/ipa/vimc.mojom | 10 +++++-----\n> src/ipa/vimc/vimc.cpp | 10 +++++-----\n> test/ipa/ipa_interface_test.cpp | 18 +++++++++---------\n> 3 files changed, 19 insertions(+), 19 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom\n> index ee66353d..99b6412b 100644\n> --- a/include/libcamera/ipa/vimc.mojom\n> +++ b/include/libcamera/ipa/vimc.mojom\n> @@ -10,11 +10,11 @@ import \"include/libcamera/ipa/core.mojom\";\n> \n> const string VimcIPAFIFOPath = \"/tmp/libcamera_ipa_vimc_fifo\";\n> \n> -enum IPAOperationCode {\n> -\tIPAOperationNone,\n> -\tIPAOperationInit,\n> -\tIPAOperationStart,\n> -\tIPAOperationStop,\n> +enum IPATraceCode {\n> +\tIPATraceNone,\n> +\tIPATraceInit,\n> +\tIPATraceStart,\n> +\tIPATraceStop,\n> };\n> \n> interface IPAVimcInterface {\n> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n> index fb134084..54d9086a 100644\n> --- a/src/ipa/vimc/vimc.cpp\n> +++ b/src/ipa/vimc/vimc.cpp\n> @@ -40,7 +40,7 @@ public:\n> \n> private:\n> \tvoid initTrace();\n> -\tvoid trace(enum ipa::vimc::IPAOperationCode operation);\n> +\tvoid trace(enum ipa::vimc::IPATraceCode operation);\n> \n> \tint fd_;\n> };\n> @@ -59,7 +59,7 @@ IPAVimc::~IPAVimc()\n> \n> int IPAVimc::init(const IPASettings &settings)\n> {\n> -\ttrace(ipa::vimc::IPAOperationInit);\n> +\ttrace(ipa::vimc::IPATraceInit);\n> \n> \tLOG(IPAVimc, Debug)\n> \t\t<< \"initializing vimc IPA with configuration file \"\n> @@ -76,7 +76,7 @@ int IPAVimc::init(const IPASettings &settings)\n> \n> int IPAVimc::start()\n> {\n> -\ttrace(ipa::vimc::IPAOperationStart);\n> +\ttrace(ipa::vimc::IPATraceStart);\n> \n> \tLOG(IPAVimc, Debug) << \"start vimc IPA!\";\n> \n> @@ -85,7 +85,7 @@ int IPAVimc::start()\n> \n> void IPAVimc::stop()\n> {\n> -\ttrace(ipa::vimc::IPAOperationStop);\n> +\ttrace(ipa::vimc::IPATraceStop);\n> \n> \tLOG(IPAVimc, Debug) << \"stop vimc IPA!\";\n> }\n> @@ -117,7 +117,7 @@ void IPAVimc::initTrace()\n> \tfd_ = ret;\n> }\n> \n> -void IPAVimc::trace(enum ipa::vimc::IPAOperationCode operation)\n> +void IPAVimc::trace(enum ipa::vimc::IPATraceCode operation)\n> {\n> \tif (fd_ < 0)\n> \t\treturn;\n> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n> index ee9f2651..2f30b26a 100644\n> --- a/test/ipa/ipa_interface_test.cpp\n> +++ b/test/ipa/ipa_interface_test.cpp\n> @@ -34,7 +34,7 @@ class IPAInterfaceTest : public Test, public Object\n> {\n> public:\n> \tIPAInterfaceTest()\n> -\t\t: trace_(ipa::vimc::IPAOperationNone), notifier_(nullptr), fd_(-1)\n> +\t\t: trace_(ipa::vimc::IPATraceNone), notifier_(nullptr), fd_(-1)\n> \t{\n> \t}\n> \n> @@ -112,10 +112,10 @@ protected:\n> \t\t}\n> \n> \t\ttimer.start(1000);\n> -\t\twhile (timer.isRunning() && trace_ != ipa::vimc::IPAOperationInit)\n> +\t\twhile (timer.isRunning() && trace_ != ipa::vimc::IPATraceInit)\n> \t\t\tdispatcher->processEvents();\n> \n> -\t\tif (trace_ != ipa::vimc::IPAOperationInit) {\n> +\t\tif (trace_ != ipa::vimc::IPATraceInit) {\n> \t\t\tcerr << \"Failed to test IPA initialization sequence\"\n> \t\t\t << endl;\n> \t\t\treturn TestFail;\n> @@ -124,10 +124,10 @@ protected:\n> \t\t/* Test start of IPA module. */\n> \t\tipa_->start();\n> \t\ttimer.start(1000);\n> -\t\twhile (timer.isRunning() && trace_ != ipa::vimc::IPAOperationStart)\n> +\t\twhile (timer.isRunning() && trace_ != ipa::vimc::IPATraceStart)\n> \t\t\tdispatcher->processEvents();\n> \n> -\t\tif (trace_ != ipa::vimc::IPAOperationStart) {\n> +\t\tif (trace_ != ipa::vimc::IPATraceStart) {\n> \t\t\tcerr << \"Failed to test IPA start sequence\" << endl;\n> \t\t\treturn TestFail;\n> \t\t}\n> @@ -135,10 +135,10 @@ protected:\n> \t\t/* Test stop of IPA module. */\n> \t\tipa_->stop();\n> \t\ttimer.start(1000);\n> -\t\twhile (timer.isRunning() && trace_ != ipa::vimc::IPAOperationStop)\n> +\t\twhile (timer.isRunning() && trace_ != ipa::vimc::IPATraceStop)\n> \t\t\tdispatcher->processEvents();\n> \n> -\t\tif (trace_ != ipa::vimc::IPAOperationStop) {\n> +\t\tif (trace_ != ipa::vimc::IPATraceStop) {\n> \t\t\tcerr << \"Failed to test IPA stop sequence\" << endl;\n> \t\t\treturn TestFail;\n> \t\t}\n> @@ -161,7 +161,7 @@ private:\n> \t\t\tcerr << \"Failed to read from IPA test FIFO at '\"\n> \t\t\t << ipa::vimc::VimcIPAFIFOPath << \"': \" << strerror(ret)\n> \t\t\t << endl;\n> -\t\t\ttrace_ = ipa::vimc::IPAOperationNone;\n> +\t\t\ttrace_ = ipa::vimc::IPATraceNone;\n> \t\t}\n> \t}\n> \n> @@ -170,7 +170,7 @@ private:\n> \tstd::shared_ptr pipe_;\n> \tstd::unique_ptr ipa_;\n> \tstd::unique_ptr ipaManager_;\n> -\tenum ipa::vimc::IPAOperationCode trace_;\n> +\tenum ipa::vimc::IPATraceCode trace_;\n> \tEventNotifier *notifier_;\n> \tint fd_;\n> };\n> -- \n> 2.31.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 56147C3240\n\tfor ;\n\tTue, 10 Aug 2021 09:55:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DA036687FA;\n\tTue, 10 Aug 2021 11:55:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 88F22687DE\n\tfor ;\n\tTue, 10 Aug 2021 11:55:38 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 03BF03F0;\n\tTue, 10 Aug 2021 11:55:36 +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=\"KWTJK7W7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628589338;\n\tbh=K0tHP0SGTKXaLSw6Y3HiKLGxMtGF9fCWgJAwG7xVarg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KWTJK7W7ClU2cPBeS15tUK08wAvE8wvHCOkColGEzDyp5ethDkx1KKPiSJkvhImb1\n\tHDFuf+IRBZz+5GD8+lftM5/2QtAtIWJQOgdVDveQLB2fiG0CSB+1VpDsWqcWSPE4YP\n\tyPxu80nFHZap0Ie5Q52TbI0Oan4BonOSeyPxZpug=","Date":"Tue, 10 Aug 2021 18:55:31 +0900","From":"paul.elder@ideasonboard.com","To":"Umang Jain ","Message-ID":"<20210810095531.GV2167@pyrite.rasen.tech>","References":"<20210806101409.324645-1-umang.jain@ideasonboard.com>\n\t<20210806101409.324645-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210806101409.324645-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/4] ipa: vimc: Rename IPA trace enums","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18669,"web_url":"https://patchwork.libcamera.org/comment/18669/","msgid":"","date":"2021-08-10T10:16:50","subject":"Re: [libcamera-devel] [PATCH 2/4] ipa: vimc: Rename IPA trace enums","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Paul,\n\nOn 8/10/21 3:25 PM, paul.elder@ideasonboard.com wrote:\n> Hi Umang,\n>\n> On Fri, Aug 06, 2021 at 03:44:07PM +0530, Umang Jain wrote:\n>> To avoid the conflict of naming with IPAOperations in future,\n>> rename trace enum to IPATrace*.\n> What kind of conflicts might we have? I'm not seeing the future that\n> you're seeing :S\nPatch 4/4 might put things in context, but probably will be dropped as \npart of re-work.\n\nAnyway I think IPATraceCode is a more suitable naming for tracing codes.\n\n\n>\n>\n> Paul\n>\n>> This commit does not introduce any functional changes.\n>>\n>> Signed-off-by: Umang Jain \n>> ---\n>> include/libcamera/ipa/vimc.mojom | 10 +++++-----\n>> src/ipa/vimc/vimc.cpp | 10 +++++-----\n>> test/ipa/ipa_interface_test.cpp | 18 +++++++++---------\n>> 3 files changed, 19 insertions(+), 19 deletions(-)\n>>\n>> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom\n>> index ee66353d..99b6412b 100644\n>> --- a/include/libcamera/ipa/vimc.mojom\n>> +++ b/include/libcamera/ipa/vimc.mojom\n>> @@ -10,11 +10,11 @@ import \"include/libcamera/ipa/core.mojom\";\n>> \n>> const string VimcIPAFIFOPath = \"/tmp/libcamera_ipa_vimc_fifo\";\n>> \n>> -enum IPAOperationCode {\n>> -\tIPAOperationNone,\n>> -\tIPAOperationInit,\n>> -\tIPAOperationStart,\n>> -\tIPAOperationStop,\n>> +enum IPATraceCode {\n>> +\tIPATraceNone,\n>> +\tIPATraceInit,\n>> +\tIPATraceStart,\n>> +\tIPATraceStop,\n>> };\n>> \n>> interface IPAVimcInterface {\n>> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n>> index fb134084..54d9086a 100644\n>> --- a/src/ipa/vimc/vimc.cpp\n>> +++ b/src/ipa/vimc/vimc.cpp\n>> @@ -40,7 +40,7 @@ public:\n>> \n>> private:\n>> \tvoid initTrace();\n>> -\tvoid trace(enum ipa::vimc::IPAOperationCode operation);\n>> +\tvoid trace(enum ipa::vimc::IPATraceCode operation);\n>> \n>> \tint fd_;\n>> };\n>> @@ -59,7 +59,7 @@ IPAVimc::~IPAVimc()\n>> \n>> int IPAVimc::init(const IPASettings &settings)\n>> {\n>> -\ttrace(ipa::vimc::IPAOperationInit);\n>> +\ttrace(ipa::vimc::IPATraceInit);\n>> \n>> \tLOG(IPAVimc, Debug)\n>> \t\t<< \"initializing vimc IPA with configuration file \"\n>> @@ -76,7 +76,7 @@ int IPAVimc::init(const IPASettings &settings)\n>> \n>> int IPAVimc::start()\n>> {\n>> -\ttrace(ipa::vimc::IPAOperationStart);\n>> +\ttrace(ipa::vimc::IPATraceStart);\n>> \n>> \tLOG(IPAVimc, Debug) << \"start vimc IPA!\";\n>> \n>> @@ -85,7 +85,7 @@ int IPAVimc::start()\n>> \n>> void IPAVimc::stop()\n>> {\n>> -\ttrace(ipa::vimc::IPAOperationStop);\n>> +\ttrace(ipa::vimc::IPATraceStop);\n>> \n>> \tLOG(IPAVimc, Debug) << \"stop vimc IPA!\";\n>> }\n>> @@ -117,7 +117,7 @@ void IPAVimc::initTrace()\n>> \tfd_ = ret;\n>> }\n>> \n>> -void IPAVimc::trace(enum ipa::vimc::IPAOperationCode operation)\n>> +void IPAVimc::trace(enum ipa::vimc::IPATraceCode operation)\n>> {\n>> \tif (fd_ < 0)\n>> \t\treturn;\n>> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n>> index ee9f2651..2f30b26a 100644\n>> --- a/test/ipa/ipa_interface_test.cpp\n>> +++ b/test/ipa/ipa_interface_test.cpp\n>> @@ -34,7 +34,7 @@ class IPAInterfaceTest : public Test, public Object\n>> {\n>> public:\n>> \tIPAInterfaceTest()\n>> -\t\t: trace_(ipa::vimc::IPAOperationNone), notifier_(nullptr), fd_(-1)\n>> +\t\t: trace_(ipa::vimc::IPATraceNone), notifier_(nullptr), fd_(-1)\n>> \t{\n>> \t}\n>> \n>> @@ -112,10 +112,10 @@ protected:\n>> \t\t}\n>> \n>> \t\ttimer.start(1000);\n>> -\t\twhile (timer.isRunning() && trace_ != ipa::vimc::IPAOperationInit)\n>> +\t\twhile (timer.isRunning() && trace_ != ipa::vimc::IPATraceInit)\n>> \t\t\tdispatcher->processEvents();\n>> \n>> -\t\tif (trace_ != ipa::vimc::IPAOperationInit) {\n>> +\t\tif (trace_ != ipa::vimc::IPATraceInit) {\n>> \t\t\tcerr << \"Failed to test IPA initialization sequence\"\n>> \t\t\t << endl;\n>> \t\t\treturn TestFail;\n>> @@ -124,10 +124,10 @@ protected:\n>> \t\t/* Test start of IPA module. */\n>> \t\tipa_->start();\n>> \t\ttimer.start(1000);\n>> -\t\twhile (timer.isRunning() && trace_ != ipa::vimc::IPAOperationStart)\n>> +\t\twhile (timer.isRunning() && trace_ != ipa::vimc::IPATraceStart)\n>> \t\t\tdispatcher->processEvents();\n>> \n>> -\t\tif (trace_ != ipa::vimc::IPAOperationStart) {\n>> +\t\tif (trace_ != ipa::vimc::IPATraceStart) {\n>> \t\t\tcerr << \"Failed to test IPA start sequence\" << endl;\n>> \t\t\treturn TestFail;\n>> \t\t}\n>> @@ -135,10 +135,10 @@ protected:\n>> \t\t/* Test stop of IPA module. */\n>> \t\tipa_->stop();\n>> \t\ttimer.start(1000);\n>> -\t\twhile (timer.isRunning() && trace_ != ipa::vimc::IPAOperationStop)\n>> +\t\twhile (timer.isRunning() && trace_ != ipa::vimc::IPATraceStop)\n>> \t\t\tdispatcher->processEvents();\n>> \n>> -\t\tif (trace_ != ipa::vimc::IPAOperationStop) {\n>> +\t\tif (trace_ != ipa::vimc::IPATraceStop) {\n>> \t\t\tcerr << \"Failed to test IPA stop sequence\" << endl;\n>> \t\t\treturn TestFail;\n>> \t\t}\n>> @@ -161,7 +161,7 @@ private:\n>> \t\t\tcerr << \"Failed to read from IPA test FIFO at '\"\n>> \t\t\t << ipa::vimc::VimcIPAFIFOPath << \"': \" << strerror(ret)\n>> \t\t\t << endl;\n>> -\t\t\ttrace_ = ipa::vimc::IPAOperationNone;\n>> +\t\t\ttrace_ = ipa::vimc::IPATraceNone;\n>> \t\t}\n>> \t}\n>> \n>> @@ -170,7 +170,7 @@ private:\n>> \tstd::shared_ptr pipe_;\n>> \tstd::unique_ptr ipa_;\n>> \tstd::unique_ptr ipaManager_;\n>> -\tenum ipa::vimc::IPAOperationCode trace_;\n>> +\tenum ipa::vimc::IPATraceCode trace_;\n>> \tEventNotifier *notifier_;\n>> \tint fd_;\n>> };\n>> -- \n>> 2.31.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 0969DBD87D\n\tfor ;\n\tTue, 10 Aug 2021 10:16:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84A0A68822;\n\tTue, 10 Aug 2021 12:16:57 +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 B5D28687F0\n\tfor ;\n\tTue, 10 Aug 2021 12:16:55 +0200 (CEST)","from [192.168.1.104] (unknown [103.238.109.8])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BFAD23F0;\n\tTue, 10 Aug 2021 12:16:54 +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=\"X7gjDOaz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628590615;\n\tbh=7hQIM+jnNeYRIECc6VbaQ4fRHskm1gAI1TDcw6FPpHQ=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=X7gjDOazrfdvGjQ27WdVj6GDKDrLrPV44IMxs3ZuhKCR6QV+oUPKaspnQCjvhWeyw\n\tvGjrJYeTWGwNdI3iqr9dCTgULQHiANIfIjEHXjpV5ri0XKzk4sLb6vyZ6dG+wdtauy\n\tbTBNh/6EKaUK0JkkBzza7//IQzb1I0ceIDXGWs3E=","To":"paul.elder@ideasonboard.com","References":"<20210806101409.324645-1-umang.jain@ideasonboard.com>\n\t<20210806101409.324645-3-umang.jain@ideasonboard.com>\n\t<20210810095531.GV2167@pyrite.rasen.tech>","From":"Umang Jain ","Message-ID":"","Date":"Tue, 10 Aug 2021 15:46:50 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20210810095531.GV2167@pyrite.rasen.tech>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 2/4] ipa: vimc: Rename IPA trace enums","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]