[{"id":367,"web_url":"https://patchwork.libcamera.org/comment/367/","msgid":"<20190116151113.GE6484@bigcity.dyn.berto.se>","date":"2019-01-16T15:11:13","subject":"Re: [libcamera-devel] [PATCH v2 1/5] test: list-cameras: Make test\n\toutput more verbose","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your work.\n\nOn 2019-01-16 14:59:45 +0100, Jacopo Mondi wrote:\n> Make the list-cameras test a little more verbose to better describe\n> failures. While at there use the Test class defined TestStatus value as\n> test exit codes, and skip the test if no camera gets registred.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> test/list-cameras.cpp | 41 +++++++++++++++++++++++++++++++++++------\n> 1 file changed, 35 insertions(+), 6 deletions(-)\n> \n> diff --git a/test/list-cameras.cpp b/test/list-cameras.cpp\n> index e2026c9..c9dc199 100644\n> --- a/test/list-cameras.cpp\n> +++ b/test/list-cameras.cpp\n> @@ -7,6 +7,7 @@\n> \n> #include \n> \n> +#include \n> #include \n> \n> #include \"test.h\"\n> @@ -14,27 +15,55 @@\n> using namespace std;\n> using namespace libcamera;\n> \n> +/*\n> + * List all cameras registered in the system, using the CameraManager.\n> + *\n> + * In order for the test to run successfully, a pipeline handler supporting\n> + * the current test platform should be available in the library.\n> + * Libcamera provides a platform-agnostic pipeline handler for the 'vimc'\n> + * virtual media device, which can be used for testing purposes.\n> + *\n> + * The test tries to list all cameras registered in the system, if no\n> + * camera is found the test is skipped. If the test gets skipped on a\n> + * platform where a pipeline handler is known to be available, an error\n> + * in camera enumeration might get unnoticed.\n\ns/get unnoticed/go unnoticed/\n\nWith this fix\n\nReviewed-by: Niklas Söderlund \n\nNice update of the TC!\n\n> + */\n> class ListTest : public Test\n> {\n> protected:\n> \tint init()\n> \t{\n> \t\tcm = CameraManager::instance();\n> -\t\tcm->start();\n> +\n> +\t\tint ret = cm->start();\n> +\t\tif (ret) {\n> +\t\t\tcerr << \"Failed to start the CameraManager\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> \n> \t\treturn 0;\n> \t}\n> \n> \tint run()\n> \t{\n> -\t\tunsigned int count = 0;\n> +\t\tvector cameraList = cm->list();\n> +\t\tif (cameraList.empty()) {\n> +\t\t\tcerr << \"No cameras registered in the system: test skip\" << endl\n> +\t\t\t << \"This might be expected if no pipeline handler supports the testing platform\" << endl;\n> +\t\t\treturn TestSkip;\n> +\t\t}\n> +\n> +\t\tfor (auto name : cameraList) {\n> +\t\t\tCamera *cam = cm->get(name);\n> +\t\t\tif (!cam) {\n> +\t\t\t\tcerr << \"Failed to get camera '\" << name << \"' by name\" << endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> \n> -\t\tfor (auto name : cm->list()) {\n> -\t\t\tcout << \"- \" << name << endl;\n> -\t\t\tcount++;\n> +\t\t\tcout << \"Found camera '\" << cam->name() << \"'\" << endl;\n> \t\t}\n> \n> -\t\treturn count ? 0 : -ENODEV;\n> +\t\treturn TestPass;\n> \t}\n> \n> \tvoid cleanup()\n> -- \n> 2.20.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D0C1960B2D\n\tfor ;\n\tWed, 16 Jan 2019 16:11:15 +0100 (CET)","by mail-lj1-x244.google.com with SMTP id v15-v6so5683885ljh.13\n\tfor ;\n\tWed, 16 Jan 2019 07:11:15 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\ty24-v6sm1071069ljd.20.2019.01.16.07.11.13\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 16 Jan 2019 07:11:14 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=qn8PvVt+ZH1XcG/RdQv76tXZHFiJED5txWxqH+N28fo=;\n\tb=FiC2UBqi2Eo+R0xxvdNrIBOQt4ynvyme/SK0oyKrsdnK0ZPKt2t7gDk3JccdLKpRsc\n\tgHK/LSbvSDKFcKZEdUZKFhjmyK8nuYsSyV6MxtgHxAEj0b8NR1P2dbnppHB/1qYxlezr\n\tAIoX/esQ9ZvW7FpvYhI4PggCQRCLY/PtxQ8lKAmH/XH2gvTo8J5HaXnTH+F/XR7G/FeV\n\txJSs8RKwco1iSppt3YpQBCBPrTp/2wvGcL3O9/qmywKcm8JDtqcTCx925JNWAJxrq370\n\tA72vRKBRHkyRG9I/PZxOYyWCjoZunrxbR1yhSyNFsLHJU2cRzBCbprUgJNjuMGuiMKWX\n\tyv/g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=qn8PvVt+ZH1XcG/RdQv76tXZHFiJED5txWxqH+N28fo=;\n\tb=WlG1D8IStd5Xw5W5xjUJxdI4sJmp8jNLgL/mJmC4W2gtBwAnI57c2s9BYb2C4+QvxO\n\t6QjrwxSaAU+a5xqWtvPE+7tEQ4ovTh0TOqCKidOuBDLHCy8wxSQxEhSQqrF4zqF/1M2d\n\t/4rA83dNb9UAIhZCTd/24SSaOQgAWoFOkLX+g6czK1yT+peY+RaX3Yowxg4ajsd39q+z\n\tkbXeM9BM2LLY5ze4XWoDvGMVsV6KcuGCkZbwv4s1/RbiYTOZ2msvZJNCkhlEV7gc1l5K\n\tlFpNyVPYM1BAbxJePrGdlnPXWJFa2n1bAdnSdXtEUWrcCqq9whQPC7d3aeatGxDoLUj9\n\t5bpw==","X-Gm-Message-State":"AJcUukfYxamJb0dqSTSqshODsaa2G8+th3R4WOQFr4EY/bJK7zizuEOw\n\t6T8dTV9NAKLtHWVCORoQyKHF+DzIHnw=","X-Google-Smtp-Source":"ALg8bN79x4tSlepUL1Tk7P1Xh3WRvIP0pU65GLX6ZtV5aupPcFtTjPAHKu0zq9VLYMdZw1QoATaA7w==","X-Received":"by 2002:a2e:b00a:: with SMTP id\n\ty10-v6mr6794224ljk.109.1547651474965; \n\tWed, 16 Jan 2019 07:11:14 -0800 (PST)","Date":"Wed, 16 Jan 2019 16:11:13 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190116151113.GE6484@bigcity.dyn.berto.se>","References":"<20190116135949.2097-1-jacopo@jmondi.org>\n\t<20190116135949.2097-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190116135949.2097-2-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 1/5] test: list-cameras: Make test\n\toutput more verbose","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 16 Jan 2019 15:11:16 -0000"}},{"id":433,"web_url":"https://patchwork.libcamera.org/comment/433/","msgid":"<20190121090956.GB4420@pendragon.ideasonboard.com>","date":"2019-01-21T09:09:56","subject":"Re: [libcamera-devel] [PATCH v2 1/5] test: list-cameras: Make test\n\toutput more verbose","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Wed, Jan 16, 2019 at 02:59:45PM +0100, Jacopo Mondi wrote:\n> Make the list-cameras test a little more verbose to better describe\n> failures. While at there use the Test class defined TestStatus value as\n> test exit codes, and skip the test if no camera gets registred.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> test/list-cameras.cpp | 41 +++++++++++++++++++++++++++++++++++------\n> 1 file changed, 35 insertions(+), 6 deletions(-)\n> \n> diff --git a/test/list-cameras.cpp b/test/list-cameras.cpp\n> index e2026c9..c9dc199 100644\n> --- a/test/list-cameras.cpp\n> +++ b/test/list-cameras.cpp\n> @@ -7,6 +7,7 @@\n> \n> #include \n> \n> +#include \n> #include \n> \n> #include \"test.h\"\n> @@ -14,27 +15,55 @@\n> using namespace std;\n> using namespace libcamera;\n> \n> +/*\n> + * List all cameras registered in the system, using the CameraManager.\n> + *\n> + * In order for the test to run successfully, a pipeline handler supporting\n> + * the current test platform should be available in the library.\n> + * Libcamera provides a platform-agnostic pipeline handler for the 'vimc'\n> + * virtual media device, which can be used for testing purposes.\n> + *\n> + * The test tries to list all cameras registered in the system, if no\n> + * camera is found the test is skipped. If the test gets skipped on a\n> + * platform where a pipeline handler is known to be available, an error\n> + * in camera enumeration might get unnoticed.\n> + */\n> class ListTest : public Test\n> {\n> protected:\n> \tint init()\n> \t{\n> \t\tcm = CameraManager::instance();\n> -\t\tcm->start();\n> +\n> +\t\tint ret = cm->start();\n> +\t\tif (ret) {\n> +\t\t\tcerr << \"Failed to start the CameraManager\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> \n> \t\treturn 0;\n> \t}\n> \n> \tint run()\n> \t{\n> -\t\tunsigned int count = 0;\n> +\t\tvector cameraList = cm->list();\n> +\t\tif (cameraList.empty()) {\n> +\t\t\tcerr << \"No cameras registered in the system: test skip\" << endl\n> +\t\t\t << \"This might be expected if no pipeline handler supports the testing platform\" << endl;\n> +\t\t\treturn TestSkip;\n\nAs commented before, I have some doubts on the value of this test,\nespecially aftere rebasing it on the lifetime management series, as it\nwill then only return TestSkip or TestPass. A test that can't fail has\nlimited use :-) I think listing cameras should be part of a libcamera\ncommand line tool, not a test case.\n\nI wont block this patch, but please be aware that I plan to remove the\nlist-cameras test.\n\n> +\t\t}\n> +\n> +\t\tfor (auto name : cameraList) {\n> +\t\t\tCamera *cam = cm->get(name);\n> +\t\t\tif (!cam) {\n> +\t\t\t\tcerr << \"Failed to get camera '\" << name << \"' by name\" << endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> \n> -\t\tfor (auto name : cm->list()) {\n> -\t\t\tcout << \"- \" << name << endl;\n> -\t\t\tcount++;\n> +\t\t\tcout << \"Found camera '\" << cam->name() << \"'\" << endl;\n> \t\t}\n> \n> -\t\treturn count ? 0 : -ENODEV;\n> +\t\treturn TestPass;\n> \t}\n> \n> \tvoid cleanup()","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E7C2660B21\n\tfor ;\n\tMon, 21 Jan 2019 10:09:56 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 65A1041;\n\tMon, 21 Jan 2019 10:09:56 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548061796;\n\tbh=q7q25W++CDzkmeW587qeP7WSpbDHUpP33mgUkexV0PE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YJStLN2Ua2gSgYGk6/W2IzthJQs40u45gmcEaw3l/xXe9QzIDSCPpt28wyAv5anvO\n\tFCM7cLq491EwThyqr0PCh5erNag3lPYhMmlYZ4ceSHJJaQm49+k4sJP5tXiL2/h8oJ\n\tSDzXZTpWlAj/JIjgCFIq/P9/oR/SMJVXCLoBMrkI=","Date":"Mon, 21 Jan 2019 11:09:56 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190121090956.GB4420@pendragon.ideasonboard.com>","References":"<20190116135949.2097-1-jacopo@jmondi.org>\n\t<20190116135949.2097-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190116135949.2097-2-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 1/5] test: list-cameras: Make test\n\toutput more verbose","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 21 Jan 2019 09:09:57 -0000"}},{"id":434,"web_url":"https://patchwork.libcamera.org/comment/434/","msgid":"<20190121091541.xwdg6hciikz33wb3@uno.localdomain>","date":"2019-01-21T09:15:41","subject":"Re: [libcamera-devel] [PATCH v2 1/5] test: list-cameras: Make test\n\toutput more verbose","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Jan 21, 2019 at 11:09:56AM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Wed, Jan 16, 2019 at 02:59:45PM +0100, Jacopo Mondi wrote:\n> > Make the list-cameras test a little more verbose to better describe\n> > failures. While at there use the Test class defined TestStatus value as\n> > test exit codes, and skip the test if no camera gets registred.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > test/list-cameras.cpp | 41 +++++++++++++++++++++++++++++++++++------\n> > 1 file changed, 35 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/test/list-cameras.cpp b/test/list-cameras.cpp\n> > index e2026c9..c9dc199 100644\n> > --- a/test/list-cameras.cpp\n> > +++ b/test/list-cameras.cpp\n> > @@ -7,6 +7,7 @@\n> >\n> > #include \n> >\n> > +#include \n> > #include \n> >\n> > #include \"test.h\"\n> > @@ -14,27 +15,55 @@\n> > using namespace std;\n> > using namespace libcamera;\n> >\n> > +/*\n> > + * List all cameras registered in the system, using the CameraManager.\n> > + *\n> > + * In order for the test to run successfully, a pipeline handler supporting\n> > + * the current test platform should be available in the library.\n> > + * Libcamera provides a platform-agnostic pipeline handler for the 'vimc'\n> > + * virtual media device, which can be used for testing purposes.\n> > + *\n> > + * The test tries to list all cameras registered in the system, if no\n> > + * camera is found the test is skipped. If the test gets skipped on a\n> > + * platform where a pipeline handler is known to be available, an error\n> > + * in camera enumeration might get unnoticed.\n> > + */\n> > class ListTest : public Test\n> > {\n> > protected:\n> > \tint init()\n> > \t{\n> > \t\tcm = CameraManager::instance();\n> > -\t\tcm->start();\n> > +\n> > +\t\tint ret = cm->start();\n> > +\t\tif (ret) {\n> > +\t\t\tcerr << \"Failed to start the CameraManager\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> >\n> > \t\treturn 0;\n> > \t}\n> >\n> > \tint run()\n> > \t{\n> > -\t\tunsigned int count = 0;\n> > +\t\tvector cameraList = cm->list();\n> > +\t\tif (cameraList.empty()) {\n> > +\t\t\tcerr << \"No cameras registered in the system: test skip\" << endl\n> > +\t\t\t << \"This might be expected if no pipeline handler supports the testing platform\" << endl;\n> > +\t\t\treturn TestSkip;\n>\n> As commented before, I have some doubts on the value of this test,\n> especially aftere rebasing it on the lifetime management series, as it\n> will then only return TestSkip or TestPass. A test that can't fail has\n> limited use :-) I think listing cameras should be part of a libcamera\n> command line tool, not a test case.\n>\n> I wont block this patch, but please be aware that I plan to remove the\n> list-cameras test.\n>\n\nThis was my understanding as well, that's why I have sent an IPU3\npipeline specific test. I'm fine with this being turned into a\nutility, and encourage pipeline specific tests to verify enumeration\nand matching works as expected.\n\nThanks\n j\n\n> > +\t\t}\n> > +\n> > +\t\tfor (auto name : cameraList) {\n> > +\t\t\tCamera *cam = cm->get(name);\n> > +\t\t\tif (!cam) {\n> > +\t\t\t\tcerr << \"Failed to get camera '\" << name << \"' by name\" << endl;\n> > +\t\t\t\treturn TestFail;\n> > +\t\t\t}\n> >\n> > -\t\tfor (auto name : cm->list()) {\n> > -\t\t\tcout << \"- \" << name << endl;\n> > -\t\t\tcount++;\n> > +\t\t\tcout << \"Found camera '\" << cam->name() << \"'\" << endl;\n> > \t\t}\n> >\n> > -\t\treturn count ? 0 : -ENODEV;\n> > +\t\treturn TestPass;\n> > \t}\n> >\n> > \tvoid cleanup()\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DB26C60B21\n\tfor ;\n\tMon, 21 Jan 2019 10:15:29 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 65CDF1BF20D;\n\tMon, 21 Jan 2019 09:15:29 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Mon, 21 Jan 2019 10:15:41 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190121091541.xwdg6hciikz33wb3@uno.localdomain>","References":"<20190116135949.2097-1-jacopo@jmondi.org>\n\t<20190116135949.2097-2-jacopo@jmondi.org>\n\t<20190121090956.GB4420@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"qokfkkmfejkr2wla\"","Content-Disposition":"inline","In-Reply-To":"<20190121090956.GB4420@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 1/5] test: list-cameras: Make test\n\toutput more verbose","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 21 Jan 2019 09:15:30 -0000"}}]