[{"id":318,"web_url":"https://patchwork.libcamera.org/comment/318/","msgid":"<1624af7a-8336-9d4a-0a61-16d319455780@ideasonboard.com>","date":"2019-01-15T14:53:50","subject":"Re: [libcamera-devel] [PATCH 1/4] test: list-cameras: Make test\n\toutput more verbose","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 15/01/2019 14:07, 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\ns/registred/registered/\n\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> test/list-cameras.cpp | 52 +++++++++++++++++++++++++++++++++++++------\n> 1 file changed, 45 insertions(+), 7 deletions(-)\n> \n> diff --git a/test/list-cameras.cpp b/test/list-cameras.cpp\n> index e2026c9..b6b0a39 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,64 @@\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 un-noticed.\n\ns/un-noticed/unnoticed/\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> +\t\tif (!cm) {\n> +\t\t\tcerr << \"Failed to get CameraManager instance\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> \n> -\t\treturn 0;\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 TestPass;\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\"\n> +\t\t\t << endl\nWe allow ourselves up to 120 chars per line, I certainly think this endl\ncould go at the end of the previous line,\n\n> +\t\t\t << \"This might be expected if no pipeline handler\"\n> +\t\t\t << \" supports the testing platform\"\n> +\t\t\t << endl;\n\nAnd these three lines can be joined to reach 119 chars. You can even add\na full stop to the end of the sentence :).\n\n\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\n> +\t\t\t\t << \"' by name\" << endl;\n\nI think single quotes saves a lot of escaping ... it seems to make sense\nI think.\n\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 << \"Camera '\" << cam->name()\n> +\t\t\t << \"' registered\" << endl;\n\nI would also have made that a single line for the output. But it looks\nlike it's your preference to break at 80, so as you wish.\n\nI would think string outputs is one place that makes sense to me to take\nadvantage of longer lines.\n\n\n> \t\t}\n> \n> -\t\treturn count ? 0 : -ENODEV;\n> +\t\treturn TestPass;\n> \t}\n> \n> \tvoid cleanup()\n>","headers":{"Return-Path":"","Received":["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 7FDEC60C78\n\tfor ;\n\tTue, 15 Jan 2019 15:53:54 +0100 (CET)","from [192.168.0.21]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B85504F8;\n\tTue, 15 Jan 2019 15:53:53 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1547564033;\n\tbh=n8CdUIvtbjt9Nx31qpSQYm5I7AJvXp5+bjUUlVrAc5A=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=K//oNpNntYE+KJOBMGlDlSSM7N8C7YauRlg0UyLAWjBqBAUsBOrVI0zt0FPsvJFp+\n\tZwZz29ER0NREZRUqlavRJUvBpgZanfxkJOptnEVEm128/epXD5FbDmcRZBSLtGaKZu\n\tGeEtJu8qu01GI41zxg8IpRfBLPn9QyhAGmTxTbdg=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20190115140749.8297-1-jacopo@jmondi.org>\n\t<20190115140749.8297-2-jacopo@jmondi.org>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<1624af7a-8336-9d4a-0a61-16d319455780@ideasonboard.com>","Date":"Tue, 15 Jan 2019 14:53:50 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.2.1","MIME-Version":"1.0","In-Reply-To":"<20190115140749.8297-2-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 1/4] 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":"Tue, 15 Jan 2019 14:53:54 -0000"}},{"id":327,"web_url":"https://patchwork.libcamera.org/comment/327/","msgid":"<7167521.uPQFF21isS@avalon>","date":"2019-01-15T16:08:21","subject":"Re: [libcamera-devel] [PATCH 1/4] 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 Tuesday, 15 January 2019 16:07:46 EET 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 | 52 +++++++++++++++++++++++++++++++++++++------\n> 1 file changed, 45 insertions(+), 7 deletions(-)\n> \n> diff --git a/test/list-cameras.cpp b/test/list-cameras.cpp\n> index e2026c9..b6b0a39 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,64 @@\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 un-noticed.\n> + */\n> class ListTest : public Test\n> {\n> protected:\n> \tint init()\n> \t{\n> \t\tcm = CameraManager::instance();\n> -\t\tcm->start();\n> +\t\tif (!cm) {\n> +\t\t\tcerr << \"Failed to get CameraManager instance\" << endl;\n\nThis can't happen, CameraManager::instance() is guaranteed to succeed.\n\n> +\t\t\treturn TestFail;\n> +\t\t}\n> \n> -\t\treturn 0;\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 TestPass;\n\nSemantically speaking, this doesn't indicate that the test passed. I won't if \nreturn 0 wouldn't be better.\n\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\"\n> +\t\t\t << endl\n> +\t\t\t << \"This might be expected if no pipeline handler\"\n> +\t\t\t << \" supports the testing platform\"\n> +\t\t\t << endl;\n\nWill we support multi-line messages when moving to TAP ?\n\nWhat exactly are we testing here ? If the goal is to test that libcamera \nsuccessfully enumerated cameras, we should instead check that at least one \nsupported media device is available in the init() function, and make this case \na failure.\n\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\n> +\t\t\t\t << \"' 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 << \"Camera '\" << cam->name()\n> +\t\t\t << \"' registered\" << endl;\n\nMaybe \"Found camera '...'\" ? The test doesn't register cameras.\n\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[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9BB7460C86\n\tfor ;\n\tTue, 15 Jan 2019 17:07:05 +0100 (CET)","from avalon.localnet (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2B72A4F8;\n\tTue, 15 Jan 2019 17:07:05 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1547568425;\n\tbh=j2l8kqnlOkOf8VElcCcu8uHDLvrE/rLDB42flgWdxds=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=L2t2dLOAG1EqmeNwaTj6Qn7e2ciw3fz01onam35Z2Amna6ujI7Smk26vjJW05lu+I\n\t4H6urwPHsijlq09lMSEYMQBr9vwR5JnLBRarAjr4BvnnBtqf0L8tiQ7i4V1RBFpop6\n\tkWdAM8BlDMlGjdDTvZTU0F7auCXTV0XECd9T9iqU=","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 15 Jan 2019 18:08:21 +0200","Message-ID":"<7167521.uPQFF21isS@avalon>","Organization":"Ideas on Board Oy","In-Reply-To":"<20190115140749.8297-2-jacopo@jmondi.org>","References":"<20190115140749.8297-1-jacopo@jmondi.org>\n\t<20190115140749.8297-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Transfer-Encoding":"7Bit","Content-Type":"text/plain; charset=\"us-ascii\"","Subject":"Re: [libcamera-devel] [PATCH 1/4] 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":"Tue, 15 Jan 2019 16:07:05 -0000"}},{"id":332,"web_url":"https://patchwork.libcamera.org/comment/332/","msgid":"<20190115172908.dz5korqpzxzsty4v@uno.localdomain>","date":"2019-01-15T17:29:08","subject":"Re: [libcamera-devel] [PATCH 1/4] 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 Tue, Jan 15, 2019 at 06:08:21PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Tuesday, 15 January 2019 16:07:46 EET 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 | 52 +++++++++++++++++++++++++++++++++++++------\n> > 1 file changed, 45 insertions(+), 7 deletions(-)\n> >\n> > diff --git a/test/list-cameras.cpp b/test/list-cameras.cpp\n> > index e2026c9..b6b0a39 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,64 @@\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 un-noticed.\n> > + */\n> > class ListTest : public Test\n> > {\n> > protected:\n> > \tint init()\n> > \t{\n> > \t\tcm = CameraManager::instance();\n> > -\t\tcm->start();\n> > +\t\tif (!cm) {\n> > +\t\t\tcerr << \"Failed to get CameraManager instance\" << endl;\n>\n> This can't happen, CameraManager::instance() is guaranteed to succeed.\n>\n\nCorrect, I'll drop.\n\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> >\n> > -\t\treturn 0;\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 TestPass;\n>\n> Semantically speaking, this doesn't indicate that the test passed. I won't if\n> return 0 wouldn't be better.\n>\n\nOk...\n\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\"\n> > +\t\t\t << endl\n> > +\t\t\t << \"This might be expected if no pipeline handler\"\n> > +\t\t\t << \" supports the testing platform\"\n> > +\t\t\t << endl;\n>\n> Will we support multi-line messages when moving to TAP ?\n>\n\nMight there be additional messages printed out along with the\nTAP-compatible string? What I mean is, can I \"cerr << \" and then\ncreate a TestResult object with the TAP messages.\n\n> What exactly are we testing here ? If the goal is to test that libcamera\n> successfully enumerated cameras, we should instead check that at least one\n> supported media device is available in the init() function, and make this case\n> a failure.\n>\n\nWell, so, list-cameras test has an issue: we don't know if something\nwent wrong in the device enumeration, pipeline handler matching and\ncamera creation, or it is actually the case no pipeline handler is\nregistered for the current test platform.\n\nPreviously, the test failed if no pipeline handler got matched, even\nif there was actually no pipeline handler for the platform, or the\nVIMC driver was not loaded.\n\nMy decision here was to skip the test, but to point out that this\nmight hide some issues.\n\nIf we manually query the media devices in the system, how would you\ndecide that one media device has a valid pipeline manager associated?\nWe can do that if we restrict the test to a known platform, like I've\ndone for VIMC in the link handling test for media-device, but in the\ngeneral case, how could we do that?\n\nThe media device \"xyz\" might not be supported now, so no camera should be\nenumerated, but when an XYZPipelineHanlder is added, how should\nwe determine that \"xyz\" is now supposed to be picked? How many cameras\nwould we expect on \"xyz\" ?\n\nI had a thought on that, and the best option is to skip the test if no\ncameras gets created. I expect platform specific test to tell if\neverything is fine for the current platform or not.. We could have an\nIPU3-list-camera test soon, that verifies everything is fine when run\non IPU3 and skip otherwise, but for this simple 'bootstrap' test, I\nthink that's the best we could do now. At least it does not fail as it\nused to do if you don't have VIMC loaded.\n\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\n> > +\t\t\t\t << \"' 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 << \"Camera '\" << cam->name()\n> > +\t\t\t << \"' registered\" << endl;\n>\n> Maybe \"Found camera '...'\" ? The test doesn't register cameras.\n>\n\nOk...\n\nThanks\n j\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\n>\n>\n>","headers":{"Return-Path":"","Received":["from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E4E4C60C88\n\tfor ;\n\tTue, 15 Jan 2019 18:28:59 +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 relay10.mail.gandi.net (Postfix) with ESMTPSA id 7963F24000D;\n\tTue, 15 Jan 2019 17:28:59 +0000 (UTC)"],"Date":"Tue, 15 Jan 2019 18:29:08 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190115172908.dz5korqpzxzsty4v@uno.localdomain>","References":"<20190115140749.8297-1-jacopo@jmondi.org>\n\t<20190115140749.8297-2-jacopo@jmondi.org>\n\t<7167521.uPQFF21isS@avalon>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"zzo6t4tloca6lgjx\"","Content-Disposition":"inline","In-Reply-To":"<7167521.uPQFF21isS@avalon>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 1/4] 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":"Tue, 15 Jan 2019 17:29:00 -0000"}}]