[{"id":4841,"web_url":"https://patchwork.libcamera.org/comment/4841/","msgid":"<5883188c-b5fd-0f1d-2286-3476c8209d51@ideasonboard.com>","date":"2020-05-18T15:57:08","subject":"Re: [libcamera-devel] [PATCH v2 5/5] tests: Introduce hotplug\n\thot-unplug unit test","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Umang,\n\nI'm happy to see a hotplug test here, and I know this one is quite a\npain to write ;-)\n\nOn 13/05/2020 18:30, Umang Jain wrote:\n\nA bit of blurb about what happens in the test is always useful in the\ncommit message here.\n\nIt's also particularly worth mentioning that the test will skip if not\nrun as root due to the requirement to modify the bind state of the\ndevice, and of course that it requires a UVC device to be connected for\ntest.\n\n\n> Signed-off-by: Umang Jain \n> ---\n> test/hotplug-cameras.cpp | 152 +++++++++++++++++++++++++++++++++++++++\n> test/meson.build | 1 +\n> 2 files changed, 153 insertions(+)\n> create mode 100644 test/hotplug-cameras.cpp\n> \n> diff --git a/test/hotplug-cameras.cpp b/test/hotplug-cameras.cpp\n> new file mode 100644\n> index 0000000..cbf68af\n> --- /dev/null\n> +++ b/test/hotplug-cameras.cpp\n> @@ -0,0 +1,152 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Umang Jain \n> + *\n> + * hotplug-cameras.cpp - Emulate cameraAdded/cameraRemoved signals in CameraManager\n> + */\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \"file.h\"\n> +#include \"test.h\"\n> +#include \"thread.h\"\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +class HotplugTest : public Test\n> +{\n> +protected:\n> +\tvoid cameraAddedHandler(std::shared_ptr cam)\n> +\t{\n> +\t\tcameraAddedPass_ = true;\n> +\t}\n> +\n> +\tvoid cameraRemovedHandler(std::shared_ptr cam)\n> +\t{\n> +\t\tcameraRemovedPass_ = true;\n> +\t}\n> +\n> +\tint init()\n> +\t{\n> +\t\tif (!File::exists(\"/sys/module/uvcvideo\")) {\n> +\t\t\tstd::cout << \"uvcvideo driver is not loaded, skipping\" << std::endl;\n> +\t\t\treturn TestSkip;\n> +\t\t}\n> +\n> +\t\tif (geteuid() != 0) {\n> +\t\t\tstd::cout << \"This test requires root permissions, skipping\" << std::endl;\n> +\t\t\treturn TestSkip;\n> +\t\t}\n> +\n> +\t\tcm_ = new CameraManager();\n> +\t\tif (cm_->start()) {\n> +\t\t\tstd::cout << \"Failed to start camera manager\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tcameraAddedPass_ = false;\n> +\t\tcameraRemovedPass_ = false;\n> +\n> +\t\tcm_->cameraAdded.connect(this, &HotplugTest::cameraAddedHandler);\n> +\t\tcm_->cameraRemoved.connect(this, &HotplugTest::cameraRemovedHandler);\n> +\n> +\t\tuvc_toplevel_ = \"/sys/module/uvcvideo/drivers/usb:uvcvideo/\";\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tint run()\n> +\t{\n> +\t\tDIR *dir;\n> +\t\tstruct dirent *dirent, *dirent2;\n> +\t\tstd::string uvc_driver_dir;\n> +\t\tbool uvc_driver_found = false;\n> +\n> +\t\tdir = opendir(uvc_toplevel_.c_str());\n> +\t\t/* Find a UVC device driver symlink, which we can bind/unbind */\n> +\t\twhile ((dirent = readdir(dir)) != nullptr) {\n> +\t\t\tif (dirent->d_type != DT_LNK)\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tstd::string child_dir = uvc_toplevel_ + dirent->d_name;\n> +\t\t\tDIR *device_driver = opendir(child_dir.c_str());\n> +\t\t\twhile ((dirent2 = readdir(device_driver)) != nullptr) {\n> +\t\t\t\tif (strncmp(dirent2->d_name, \"video4linux\", 11) == 0) {\n> +\t\t\t\t\tuvc_driver_dir = dirent->d_name;\n> +\t\t\t\t\tuvc_driver_found = true;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t\tclosedir(device_driver);\n> +\n> +\t\t\tif (uvc_driver_found)\n> +\t\t\t\tbreak;\n> +\t\t}\n> +\t\tclosedir(dir);\n> +\n> +\t\t/* If no UVC driver found, skip */\n> +\t\tif (!uvc_driver_found)\n> +\t\t\treturn TestSkip;\n> +\n> +\t\t/* Unbind a camera, process events */\n> +\t\tint fd1 = open(\"/sys/module/uvcvideo/drivers/usb:uvcvideo/unbind\", O_WRONLY);\n> +\t\twrite(fd1, uvc_driver_dir.c_str(), uvc_driver_dir.size());\n> +\t\tclose(fd1);\n> +\t\tTimer timer;\n> +\t\ttimer.start(1000);\n> +\t\twhile (timer.isRunning())\n> +\t\t\tThread::current()->eventDispatcher()->processEvents();\n> +\n> +\t\t/* \\todo: Fix this workaround of stopping and starting the camera-manager.\n> +\t\t * We need to do this, so that cm_ release all references to the uvc media symlinks.\n\nEep, I can't get my head around this just yet. You mean having the\ncamera manager running, it has open handles to the media symlinks in\nsysfs perhaps?\n\nI guess it's implying that something in the cm_ is preventing the clean\nunbind of the camera - but I don't understand what that is yet to know\nif we must fix that, or if it's something unavoidable?\n\nBut we're not going to expect applications to stop and restart the\ncamera manager, so I certainly hope this is just because of the fact\nwe're using the unbind/bind mechanism?\n\nIf not - then we really need to fix it ;-)\n\n> +\t\t */\n> +\t\tcm_->stop();\n> +\t\tif (cm_->start()) {\n> +\t\t\tstd::cout << \"Failed to restart camera-manager\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Bind the camera again, process events */\n> +\t\tint fd2 = open(\"/sys/module/uvcvideo/drivers/usb:uvcvideo/bind\", O_WRONLY);\n> +\t\twrite(fd2, uvc_driver_dir.c_str(), uvc_driver_dir.size());\n> +\t\tclose(fd2);\n> +\n> +\t\ttimer.start(1000);\n> +\t\twhile (timer.isRunning())\n> +\t\t\tThread::current()->eventDispatcher()->processEvents();\n> +\n> +\t\tif (cameraAddedPass_ && cameraRemovedPass_)\n> +\t\t\treturn TestPass;\n> +\t\telse\n> +\t\t\treturn TestFail;\n> +\t}\n> +\n> +\tvoid cleanup()\n> +\t{\n> +\t\tcm_->stop();\n> +\t\tdelete cm_;\n> +\t}\n> +\n> +private:\n> +\tCameraManager *cm_;\n> +\tstd::string uvc_toplevel_;\n> +\tbool cameraRemovedPass_;\n> +\tbool cameraAddedPass_;\n> +};\n> +\n> +TEST_REGISTER(HotplugTest)\n> +\n> diff --git a/test/meson.build b/test/meson.build\n> index 5a45a85..383a7ea 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -29,6 +29,7 @@ internal_tests = [\n> ['file', 'file.cpp'],\n> ['file-descriptor', 'file-descriptor.cpp'],\n> ['message', 'message.cpp'],\n> + ['hotplug-cameras', 'hotplug-cameras.cpp'],\n> ['object', 'object.cpp'],\n> ['object-invoke', 'object-invoke.cpp'],\n> ['signal-threads', 'signal-threads.cpp'],\n>","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 07CB9603DA\n\tfor ;\n\tMon, 18 May 2020 17:57:13 +0200 (CEST)","from [192.168.0.20]\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 6955D258;\n\tMon, 18 May 2020 17:57:12 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"VkZjMnWM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1589817432;\n\tbh=H6s9Xn/Ej89xdmZUVaVIrrPyz8OGmBpJoKQa2CV8Szw=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=VkZjMnWMHIPae29TwG2D99+j8NvE4ihyYqftnN/y1SN6WrFdcyPc7bI9xwPGI4rsc\n\t0Nm2ZrFnl7lThYRRM1s/wLmWzZypQRQtinxfTE65j+2RrgtflToSQM+/SbchB8p35M\n\tVoP0/C3iqSqCFItWyoRh24OaivYiJSoE1o6ZP05I=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Umang Jain ,\n\tlibcamera-devel ","References":"<20200513172950.72685-1-email@uajain.com>\n\t<20200513172950.72685-6-email@uajain.com>","From":"Kieran Bingham ","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\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<5883188c-b5fd-0f1d-2286-3476c8209d51@ideasonboard.com>","Date":"Mon, 18 May 2020 16:57:08 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.7.0","MIME-Version":"1.0","In-Reply-To":"<20200513172950.72685-6-email@uajain.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 5/5] tests: Introduce hotplug\n\thot-unplug unit test","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","X-List-Received-Date":"Mon, 18 May 2020 15:57:13 -0000"}},{"id":4857,"web_url":"https://patchwork.libcamera.org/comment/4857/","msgid":"<45fe95ab51bece01a2ab0d5c7e8ed66da01acec8.camel@uajain.com>","date":"2020-05-19T14:15:26","subject":"Re: [libcamera-devel] [PATCH v2 5/5] tests: Introduce hotplug\n\thot-unplug unit test","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Kieran,\n\nOn Mon, 2020-05-18 at 16:57 +0100, Kieran Bingham wrote:\n> Hi Umang,\n> \n> I'm happy to see a hotplug test here, and I know this one is quite a\n> pain to write ;-)\n> \n> On 13/05/2020 18:30, Umang Jain wrote:\n> \n> A bit of blurb about what happens in the test is always useful in the\n> commit message here.\n> \n> It's also particularly worth mentioning that the test will skip if\n> not\n> run as root due to the requirement to modify the bind state of the\n> device, and of course that it requires a UVC device to be connected\n> for\n> test.\n> \n> \n> > Signed-off-by: Umang Jain \n> > ---\n> > test/hotplug-cameras.cpp | 152\n> > +++++++++++++++++++++++++++++++++++++++\n> > test/meson.build | 1 +\n> > 2 files changed, 153 insertions(+)\n> > create mode 100644 test/hotplug-cameras.cpp\n> > \n> > diff --git a/test/hotplug-cameras.cpp b/test/hotplug-cameras.cpp\n> > new file mode 100644\n> > index 0000000..cbf68af\n> > --- /dev/null\n> > +++ b/test/hotplug-cameras.cpp\n> > @@ -0,0 +1,152 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Umang Jain \n> > + *\n> > + * hotplug-cameras.cpp - Emulate cameraAdded/cameraRemoved signals\n> > in CameraManager\n> > + */\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \"file.h\"\n> > +#include \"test.h\"\n> > +#include \"thread.h\"\n> > +\n> > +using namespace std;\n> > +using namespace libcamera;\n> > +\n> > +class HotplugTest : public Test\n> > +{\n> > +protected:\n> > +\tvoid cameraAddedHandler(std::shared_ptr cam)\n> > +\t{\n> > +\t\tcameraAddedPass_ = true;\n> > +\t}\n> > +\n> > +\tvoid cameraRemovedHandler(std::shared_ptr cam)\n> > +\t{\n> > +\t\tcameraRemovedPass_ = true;\n> > +\t}\n> > +\n> > +\tint init()\n> > +\t{\n> > +\t\tif (!File::exists(\"/sys/module/uvcvideo\")) {\n> > +\t\t\tstd::cout << \"uvcvideo driver is not loaded,\n> > skipping\" << std::endl;\n> > +\t\t\treturn TestSkip;\n> > +\t\t}\n> > +\n> > +\t\tif (geteuid() != 0) {\n> > +\t\t\tstd::cout << \"This test requires root\n> > permissions, skipping\" << std::endl;\n> > +\t\t\treturn TestSkip;\n> > +\t\t}\n> > +\n> > +\t\tcm_ = new CameraManager();\n> > +\t\tif (cm_->start()) {\n> > +\t\t\tstd::cout << \"Failed to start camera manager\"\n> > << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tcameraAddedPass_ = false;\n> > +\t\tcameraRemovedPass_ = false;\n> > +\n> > +\t\tcm_->cameraAdded.connect(this,\n> > &HotplugTest::cameraAddedHandler);\n> > +\t\tcm_->cameraRemoved.connect(this,\n> > &HotplugTest::cameraRemovedHandler);\n> > +\n> > +\t\tuvc_toplevel_ =\n> > \"/sys/module/uvcvideo/drivers/usb:uvcvideo/\";\n> > +\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\tint run()\n> > +\t{\n> > +\t\tDIR *dir;\n> > +\t\tstruct dirent *dirent, *dirent2;\n> > +\t\tstd::string uvc_driver_dir;\n> > +\t\tbool uvc_driver_found = false;\n> > +\n> > +\t\tdir = opendir(uvc_toplevel_.c_str());\n> > +\t\t/* Find a UVC device driver symlink, which we can\n> > bind/unbind */\n> > +\t\twhile ((dirent = readdir(dir)) != nullptr) {\n> > +\t\t\tif (dirent->d_type != DT_LNK)\n> > +\t\t\t\tcontinue;\n> > +\n> > +\t\t\tstd::string child_dir = uvc_toplevel_ + dirent-\n> > >d_name;\n> > +\t\t\tDIR *device_driver =\n> > opendir(child_dir.c_str());\n> > +\t\t\twhile ((dirent2 = readdir(device_driver)) !=\n> > nullptr) {\n> > +\t\t\t\tif (strncmp(dirent2->d_name,\n> > \"video4linux\", 11) == 0) {\n> > +\t\t\t\t\tuvc_driver_dir = dirent-\n> > >d_name;\n> > +\t\t\t\t\tuvc_driver_found = true;\n> > +\t\t\t\t\tbreak;\n> > +\t\t\t\t}\n> > +\t\t\t}\n> > +\t\t\tclosedir(device_driver);\n> > +\n> > +\t\t\tif (uvc_driver_found)\n> > +\t\t\t\tbreak;\n> > +\t\t}\n> > +\t\tclosedir(dir);\n> > +\n> > +\t\t/* If no UVC driver found, skip */\n> > +\t\tif (!uvc_driver_found)\n> > +\t\t\treturn TestSkip;\n> > +\n> > +\t\t/* Unbind a camera, process events */\n> > +\t\tint fd1 =\n> > open(\"/sys/module/uvcvideo/drivers/usb:uvcvideo/unbind\", O_WRONLY);\n> > +\t\twrite(fd1, uvc_driver_dir.c_str(),\n> > uvc_driver_dir.size());\n> > +\t\tclose(fd1);\n> > +\t\tTimer timer;\n> > +\t\ttimer.start(1000);\n> > +\t\twhile (timer.isRunning())\n> > +\t\t\tThread::current()->eventDispatcher()-\n> > >processEvents();\n> > +\n> > +\t\t/* \\todo: Fix this workaround of stopping and starting\n> > the camera-manager.\n> > +\t\t * We need to do this, so that cm_ release all\n> > references to the uvc media symlinks.\n> \n> Eep, I can't get my head around this just yet. You mean having the\n> camera manager running, it has open handles to the media symlinks in\n> sysfs perhaps?\n> \n> I guess it's implying that something in the cm_ is preventing the\n> clean\n> unbind of the camera - but I don't understand what that is yet to\n> know\n> if we must fix that, or if it's something unavoidable?\n\nYes, let me explain with an observation from the 'master' branch of\nlibcamera(i.e. without these patches).\n\nI have a UVC camera and is denoted by \"1-1:1.0\" and \"1-1:1.1\" symlinks\nin '/sys/module/uvcvideo/drivers/usb:uvcvideo'. Now, with qcam\nstreaming from this device, try to unbind and re-bind the \"1-1:1.0\"\nsymlink (which is the primary video device node). Obviously, qcam\nwon't pick up the re-binding again but notice what happens when you\nexit the qcam and try to re-run qcam again; qcam won't notice the\ndevice when it's restarted.\n\nYet another observation while following the above mentioned process,\nwhen I have qcam running and I unbind \"1-1:1.0\", the symlink \"1-1:1.1\"\ndoesn't get dis-appeared(apparently it should). It only gets disappered\n(i.e. device has been cleanly unbind) only when qcam is closed at this\npoint; which explains CameraManager is holding some kind of ref on this\nsymlink.\n> \n> But we're not going to expect applications to stop and restart the\n> camera manager, so I certainly hope this is just because of the fact\n> we're using the unbind/bind mechanism?\n\nYes, that's my observation as well. This problem(and hence the\nworkaround in the test) is specific to bind/unbind mechanism. I tested\nwith a handy USB webcam around, and it doesn't leave any dangling\nsymlinks.\n\n> If not - then we really need to fix it ;-)\n> \nI am still looking into it, it might be a silly leak somewhere but\ngetting hard to find :)\n\n> > +\t\t */\n> > +\t\tcm_->stop();\n> > +\t\tif (cm_->start()) {\n> > +\t\t\tstd::cout << \"Failed to restart camera-manager\" \n> > << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Bind the camera again, process events */\n> > +\t\tint fd2 =\n> > open(\"/sys/module/uvcvideo/drivers/usb:uvcvideo/bind\", O_WRONLY);\n> > +\t\twrite(fd2, uvc_driver_dir.c_str(),\n> > uvc_driver_dir.size());\n> > +\t\tclose(fd2);\n> > +\n> > +\t\ttimer.start(1000);\n> > +\t\twhile (timer.isRunning())\n> > +\t\t\tThread::current()->eventDispatcher()-\n> > >processEvents();\n> > +\n> > +\t\tif (cameraAddedPass_ && cameraRemovedPass_)\n> > +\t\t\treturn TestPass;\n> > +\t\telse\n> > +\t\t\treturn TestFail;\n> > +\t}\n> > +\n> > +\tvoid cleanup()\n> > +\t{\n> > +\t\tcm_->stop();\n> > +\t\tdelete cm_;\n> > +\t}\n> > +\n> > +private:\n> > +\tCameraManager *cm_;\n> > +\tstd::string uvc_toplevel_;\n> > +\tbool cameraRemovedPass_;\n> > +\tbool cameraAddedPass_;\n> > +};\n> > +\n> > +TEST_REGISTER(HotplugTest)\n> > +\n> > diff --git a/test/meson.build b/test/meson.build\n> > index 5a45a85..383a7ea 100644\n> > --- a/test/meson.build\n> > +++ b/test/meson.build\n> > @@ -29,6 +29,7 @@ internal_tests = [\n> > ['file', 'file.cpp'],\n> > ['file-descriptor', 'file-descriptor.cpp'],\n> > ['message', 'message.cpp'],\n> > + ['hotplug-cameras', 'hotplug-cameras.cpp'],\n> > ['object', 'object.cpp'],\n> > ['object-invoke', 'object-invoke.cpp'],\n> > ['signal-threads', 'signal-threads.cpp'],\n> >","headers":{"Return-Path":"","Received":["from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B4CEC603D9\n\tfor ;\n\tTue, 19 May 2020 16:15:27 +0200 (CEST)","by filterdrecv-p3iad2-8ddf98858-c27gg with SMTP id\n\tfilterdrecv-p3iad2-8ddf98858-c27gg-18-5EC3E9FD-133\n\t2020-05-19 14:15:26.161350373 +0000 UTC m=+331245.724463725","from mail.uajain.com (unknown)\n\tby ismtpd0007p1maa1.sendgrid.net (SG) with ESMTP\n\tid i5u6i0bdQrSvWFOkWvJgiQ Tue, 19 May 2020 14:15:25.655 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=uajain.com\n\theader.i=@uajain.com header.b=\"ZQXuSDv7\"; \n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com;\n\th=subject:from:in-reply-to:references:content-type:mime-version:to:\n\tcontent-transfer-encoding;\n\ts=s1; bh=Kq7xLPjU53qkDVHCAHu9sKWqNdXqA0dIa9OtX5ZYvtQ=;\n\tb=ZQXuSDv7zBX8Ojf3IUW8LTErviLaml1f10DedR7eR3UHX958VAgLGCHuREao2ctozO/q\n\tDL8awomonagvowYkDUOGdzGFbUZG8+0s6lekYuFp0zQqQZAMLu3Dgzrf79XclAjSUjbon/\n\tRBQnxiGCe46l7hwsSd2HUhHcBHRE/Bimc=","Message-ID":"<45fe95ab51bece01a2ab0d5c7e8ed66da01acec8.camel@uajain.com>","From":"Umang Jain ","Date":"Tue, 19 May 2020 14:15:26 +0000 (UTC)","In-Reply-To":"<5883188c-b5fd-0f1d-2286-3476c8209d51@ideasonboard.com>","References":"<20200513172950.72685-1-email@uajain.com>\n\t<20200513172950.72685-6-email@uajain.com>\n\t<5883188c-b5fd-0f1d-2286-3476c8209d51@ideasonboard.com>","Content-Type":"text/plain; charset=us-ascii","Mime-Version":"1.0","X-SG-EID":"1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPceG7ZR2xwscG2UrwfeUfpmpsJVh+MKukQBycQM4GdF5DYDqM+fbIekv7rrou6Cdl+U4ElCiqihnsxKdPF/WVlP8zMJLMkCIzbqV8rHEzRzUk5z34cG8h9its9ZU0gmcahplymjjOj4wh2i8UzDOQS00d1Ndo+HGMfbvwUwnRGH5zu2/2IyE5VpcSgWH4E+HXSV6ErfS7QPmfarO0vDkGitQ==","To":"kieran.bingham@ideasonboard.com, libcamera-devel\n\t","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 5/5] tests: Introduce hotplug\n\thot-unplug unit test","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","X-List-Received-Date":"Tue, 19 May 2020 14:15:28 -0000"}},{"id":5112,"web_url":"https://patchwork.libcamera.org/comment/5112/","msgid":"<20200607172123.GA25279@pendragon.ideasonboard.com>","date":"2020-06-07T17:21:23","subject":"Re: [libcamera-devel] [PATCH v2 5/5] tests: Introduce hotplug\n\thot-unplug unit test","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Tue, May 19, 2020 at 02:15:26PM +0000, Umang Jain wrote:\n> On Mon, 2020-05-18 at 16:57 +0100, Kieran Bingham wrote:\n> > Hi Umang,\n> > \n> > I'm happy to see a hotplug test here, and I know this one is quite a\n> > pain to write ;-)\n> > \n> > On 13/05/2020 18:30, Umang Jain wrote:\n> > \n> > A bit of blurb about what happens in the test is always useful in the\n> > commit message here.\n> > \n> > It's also particularly worth mentioning that the test will skip if not\n> > run as root due to the requirement to modify the bind state of the\n> > device, and of course that it requires a UVC device to be connected for\n> > test.\n> > \n> > > Signed-off-by: Umang Jain \n> > > ---\n> > > test/hotplug-cameras.cpp | 152 +++++++++++++++++++++++++++++++++++++++\n> > > test/meson.build | 1 +\n> > > 2 files changed, 153 insertions(+)\n> > > create mode 100644 test/hotplug-cameras.cpp\n> > > \n> > > diff --git a/test/hotplug-cameras.cpp b/test/hotplug-cameras.cpp\n> > > new file mode 100644\n> > > index 0000000..cbf68af\n> > > --- /dev/null\n> > > +++ b/test/hotplug-cameras.cpp\n> > > @@ -0,0 +1,152 @@\n> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > > +/*\n> > > + * Copyright (C) 2020, Umang Jain \n> > > + *\n> > > + * hotplug-cameras.cpp - Emulate cameraAdded/cameraRemoved signals\n> > > in CameraManager\n> > > + */\n> > > +\n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \"file.h\"\n> > > +#include \"test.h\"\n> > > +#include \"thread.h\"\n> > > +\n> > > +using namespace std;\n> > > +using namespace libcamera;\n> > > +\n> > > +class HotplugTest : public Test\n> > > +{\n> > > +protected:\n> > > +\tvoid cameraAddedHandler(std::shared_ptr cam)\n> > > +\t{\n> > > +\t\tcameraAddedPass_ = true;\n> > > +\t}\n> > > +\n> > > +\tvoid cameraRemovedHandler(std::shared_ptr cam)\n> > > +\t{\n> > > +\t\tcameraRemovedPass_ = true;\n> > > +\t}\n> > > +\n> > > +\tint init()\n> > > +\t{\n> > > +\t\tif (!File::exists(\"/sys/module/uvcvideo\")) {\n> > > +\t\t\tstd::cout << \"uvcvideo driver is not loaded,\n> > > skipping\" << std::endl;\n> > > +\t\t\treturn TestSkip;\n> > > +\t\t}\n> > > +\n> > > +\t\tif (geteuid() != 0) {\n> > > +\t\t\tstd::cout << \"This test requires root permissions, skipping\" << std::endl;\n> > > +\t\t\treturn TestSkip;\n> > > +\t\t}\n> > > +\n> > > +\t\tcm_ = new CameraManager();\n> > > +\t\tif (cm_->start()) {\n> > > +\t\t\tstd::cout << \"Failed to start camera manager\" << std::endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > +\t\tcameraAddedPass_ = false;\n> > > +\t\tcameraRemovedPass_ = false;\n> > > +\n> > > +\t\tcm_->cameraAdded.connect(this, &HotplugTest::cameraAddedHandler);\n> > > +\t\tcm_->cameraRemoved.connect(this, &HotplugTest::cameraRemovedHandler);\n> > > +\n> > > +\t\tuvc_toplevel_ = \"/sys/module/uvcvideo/drivers/usb:uvcvideo/\";\n> > > +\n> > > +\t\treturn 0;\n> > > +\t}\n> > > +\n> > > +\tint run()\n> > > +\t{\n> > > +\t\tDIR *dir;\n> > > +\t\tstruct dirent *dirent, *dirent2;\n> > > +\t\tstd::string uvc_driver_dir;\n> > > +\t\tbool uvc_driver_found = false;\n> > > +\n> > > +\t\tdir = opendir(uvc_toplevel_.c_str());\n> > > +\t\t/* Find a UVC device driver symlink, which we can bind/unbind */\n> > > +\t\twhile ((dirent = readdir(dir)) != nullptr) {\n> > > +\t\t\tif (dirent->d_type != DT_LNK)\n> > > +\t\t\t\tcontinue;\n> > > +\n> > > +\t\t\tstd::string child_dir = uvc_toplevel_ + dirent- >d_name;\n> > > +\t\t\tDIR *device_driver = opendir(child_dir.c_str());\n> > > +\t\t\twhile ((dirent2 = readdir(device_driver)) != nullptr) {\n> > > +\t\t\t\tif (strncmp(dirent2->d_name, \"video4linux\", 11) == 0) {\n> > > +\t\t\t\t\tuvc_driver_dir = dirent- >d_name;\n> > > +\t\t\t\t\tuvc_driver_found = true;\n> > > +\t\t\t\t\tbreak;\n> > > +\t\t\t\t}\n> > > +\t\t\t}\n> > > +\t\t\tclosedir(device_driver);\n> > > +\n> > > +\t\t\tif (uvc_driver_found)\n> > > +\t\t\t\tbreak;\n> > > +\t\t}\n> > > +\t\tclosedir(dir);\n> > > +\n> > > +\t\t/* If no UVC driver found, skip */\n> > > +\t\tif (!uvc_driver_found)\n> > > +\t\t\treturn TestSkip;\n> > > +\n> > > +\t\t/* Unbind a camera, process events */\n> > > +\t\tint fd1 = open(\"/sys/module/uvcvideo/drivers/usb:uvcvideo/unbind\", O_WRONLY);\n> > > +\t\twrite(fd1, uvc_driver_dir.c_str(), uvc_driver_dir.size());\n> > > +\t\tclose(fd1);\n> > > +\t\tTimer timer;\n> > > +\t\ttimer.start(1000);\n> > > +\t\twhile (timer.isRunning())\n> > > +\t\t\tThread::current()->eventDispatcher()- >processEvents();\n> > > +\n> > > +\t\t/* \\todo: Fix this workaround of stopping and starting the camera-manager.\n> > > +\t\t * We need to do this, so that cm_ release all references to the uvc media symlinks.\n> > \n> > Eep, I can't get my head around this just yet. You mean having the\n> > camera manager running, it has open handles to the media symlinks in\n> > sysfs perhaps?\n> > \n> > I guess it's implying that something in the cm_ is preventing the clean\n> > unbind of the camera - but I don't understand what that is yet to know\n> > if we must fix that, or if it's something unavoidable?\n> \n> Yes, let me explain with an observation from the 'master' branch of\n> libcamera(i.e. without these patches).\n> \n> I have a UVC camera and is denoted by \"1-1:1.0\" and \"1-1:1.1\" symlinks\n> in '/sys/module/uvcvideo/drivers/usb:uvcvideo'. Now, with qcam\n> streaming from this device, try to unbind and re-bind the \"1-1:1.0\"\n> symlink (which is the primary video device node). Obviously, qcam\n> won't pick up the re-binding again but notice what happens when you\n> exit the qcam and try to re-run qcam again; qcam won't notice the\n> device when it's restarted.\n> \n> Yet another observation while following the above mentioned process,\n> when I have qcam running and I unbind \"1-1:1.0\", the symlink \"1-1:1.1\"\n> doesn't get dis-appeared(apparently it should). It only gets disappered\n> (i.e. device has been cleanly unbind) only when qcam is closed at this\n> point; which explains CameraManager is holding some kind of ref on this\n> symlink.\n>\n> > But we're not going to expect applications to stop and restart the\n> > camera manager, so I certainly hope this is just because of the fact\n> > we're using the unbind/bind mechanism?\n> \n> Yes, that's my observation as well. This problem(and hence the\n> workaround in the test) is specific to bind/unbind mechanism. I tested\n> with a handy USB webcam around, and it doesn't leave any dangling\n> symlinks.\n> \n> > If not - then we really need to fix it ;-)\n>\n> I am still looking into it, it might be a silly leak somewhere but\n> getting hard to find :)\n\nThanks for the analysis. Have you managed to figure out why this\nhappens, or is it still an open question ?\n\n> > > +\t\t */\n> > > +\t\tcm_->stop();\n> > > +\t\tif (cm_->start()) {\n> > > +\t\t\tstd::cout << \"Failed to restart camera-manager\" << std::endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > +\t\t/* Bind the camera again, process events */\n> > > +\t\tint fd2 = open(\"/sys/module/uvcvideo/drivers/usb:uvcvideo/bind\", O_WRONLY);\n> > > +\t\twrite(fd2, uvc_driver_dir.c_str(), uvc_driver_dir.size());\n> > > +\t\tclose(fd2);\n> > > +\n> > > +\t\ttimer.start(1000);\n> > > +\t\twhile (timer.isRunning())\n> > > +\t\t\tThread::current()->eventDispatcher()- >processEvents();\n> > > +\n> > > +\t\tif (cameraAddedPass_ && cameraRemovedPass_)\n> > > +\t\t\treturn TestPass;\n> > > +\t\telse\n> > > +\t\t\treturn TestFail;\n> > > +\t}\n> > > +\n> > > +\tvoid cleanup()\n> > > +\t{\n> > > +\t\tcm_->stop();\n> > > +\t\tdelete cm_;\n> > > +\t}\n> > > +\n> > > +private:\n> > > +\tCameraManager *cm_;\n> > > +\tstd::string uvc_toplevel_;\n> > > +\tbool cameraRemovedPass_;\n> > > +\tbool cameraAddedPass_;\n> > > +};\n> > > +\n> > > +TEST_REGISTER(HotplugTest)\n> > > +\n> > > diff --git a/test/meson.build b/test/meson.build\n> > > index 5a45a85..383a7ea 100644\n> > > --- a/test/meson.build\n> > > +++ b/test/meson.build\n> > > @@ -29,6 +29,7 @@ internal_tests = [\n> > > ['file', 'file.cpp'],\n> > > ['file-descriptor', 'file-descriptor.cpp'],\n> > > ['message', 'message.cpp'],\n> > > + ['hotplug-cameras', 'hotplug-cameras.cpp'],\n> > > ['object', 'object.cpp'],\n> > > ['object-invoke', 'object-invoke.cpp'],\n> > > ['signal-threads', 'signal-threads.cpp'],","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 19167600F7\n\tfor ;\n\tSun, 7 Jun 2020 19:21:43 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 835542C9;\n\tSun, 7 Jun 2020 19:21:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"EbiSV3yY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591550502;\n\tbh=rxPUT6VcNG35mw2NoHzqOoosC2FD4WG9MJlFobjNh0U=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EbiSV3yYH9+ZrV4rKd9eMbnyisADiXSfoIr/6tK/6ynsQKA25dyWy0bt3q9HVW0SF\n\teH5cEZAtvbl+H+iBGDHH2gPq6daZquuxK4jZPFKZ3xPVV9EeohVljXnWFGxI+BKR7p\n\tWR0PZlbySkgfaBR3pfBBG7bvhFE6xt/jg1yOTLBc=","Date":"Sun, 7 Jun 2020 20:21:23 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Cc":"kieran.bingham@ideasonboard.com,\n\tlibcamera-devel ","Message-ID":"<20200607172123.GA25279@pendragon.ideasonboard.com>","References":"<20200513172950.72685-1-email@uajain.com>\n\t<20200513172950.72685-6-email@uajain.com>\n\t<5883188c-b5fd-0f1d-2286-3476c8209d51@ideasonboard.com>\n\t<45fe95ab51bece01a2ab0d5c7e8ed66da01acec8.camel@uajain.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<45fe95ab51bece01a2ab0d5c7e8ed66da01acec8.camel@uajain.com>","Subject":"Re: [libcamera-devel] [PATCH v2 5/5] tests: Introduce hotplug\n\thot-unplug unit test","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","X-List-Received-Date":"Sun, 07 Jun 2020 17:21:43 -0000"}}]