[{"id":2409,"web_url":"https://patchwork.libcamera.org/comment/2409/","msgid":"<20190815095014.kz2vvkn3ogad57jb@uno.localdomain>","date":"2019-08-15T09:50:15","subject":"Re: [libcamera-devel] [PATCH 13/18] test: Add EventNotifier thread\n\tmove test","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"HI Laurent,\n\nOn Mon, Aug 12, 2019 at 03:46:37PM +0300, Laurent Pinchart wrote:\n> The test verifies correct behaviour of an enabled event notifier moved\n> to a different thread.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/event_notifier.cpp | 2 +-\n> test/event-thread.cpp | 112 +++++++++++++++++++++++++++++++\n> test/meson.build | 1 +\n> 3 files changed, 114 insertions(+), 1 deletion(-)\n> create mode 100644 test/event-thread.cpp\n>\n> diff --git a/src/libcamera/event_notifier.cpp b/src/libcamera/event_notifier.cpp\n> index 515e6d1770a1..96be27601982 100644\n> --- a/src/libcamera/event_notifier.cpp\n> +++ b/src/libcamera/event_notifier.cpp\n> @@ -127,7 +127,7 @@ void EventNotifier::message(Message *msg)\n> \tif (msg->type() == Message::ThreadMoveMessage) {\n> \t\tif (enabled_) {\n> \t\t\tsetEnabled(false);\n> -\t\t\tinvokeMethod(this, &EventNotifier::setEnabled, true);\n> +\t\t\tinvokeMethod(&EventNotifier::setEnabled, true);\n\nI've missed the reason for this change. Also, isnt't this the syntax\nto invoce static bound methods?\n\n> \t\t}\n> \t}\n>\n> diff --git a/test/event-thread.cpp b/test/event-thread.cpp\n> new file mode 100644\n> index 000000000000..4a82d49b94f1\n> --- /dev/null\n> +++ b/test/event-thread.cpp\n> @@ -0,0 +1,112 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * event-thread.cpp - Threaded event test\n> + */\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"test.h\"\n> +#include \"thread.h\"\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +class EventHandler : public Object\n> +{\n> +public:\n> +\tEventHandler()\n> +\t{\n> +\t\tpipe(pipefd_);\n> +\n> +\t\tnotifier_ = new EventNotifier(pipefd_[0], EventNotifier::Read);\n> +\t\tnotifier_->activated.connect(this, &EventHandler::readReady);\n> +\t}\n> +\n> +\t~EventHandler()\n> +\t{\n> +\t\tdelete notifier_;\n> +\n> +\t\tclose(pipefd_[0]);\n> +\t\tclose(pipefd_[1]);\n> +\t}\n> +\n> +\tint notify()\n> +\t{\n> +\t\tstd::string data(\"H2G2\");\n> +\t\tssize_t ret;\n> +\n> +\t\tmemset(data_, 0, sizeof(data_));\n> +\t\tsize_ = 0;\n> +\n> +\t\tret = write(pipefd_[1], data.data(), data.size());\n> +\t\tif (ret < 0) {\n> +\t\t\tcout << \"Pipe write failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tbool notified() const\n> +\t{\n> +\t\treturn notified_;\n> +\t}\n> +\n> +\tvoid moveToThread(Thread *thread)\n> +\t{\n> +\t\tObject::moveToThread(thread);\n> +\t\tnotifier_->moveToThread(thread);\n> +\t}\n> +\n> +private:\n> +\tvoid readReady(EventNotifier *notifier)\n> +\t{\n> +\t\tsize_ = read(notifier->fd(), data_, sizeof(data_));\n> +\t\tnotified_ = true;\n> +\t}\n> +\n> +\tEventNotifier *notifier_;\n> +\n> +\tint pipefd_[2];\n> +\n> +\tbool notified_;\n> +\tchar data_[16];\n> +\tssize_t size_;\n> +};\n> +\n> +class EventThreadTest : public Test\n> +{\n> +protected:\n> +\tint run()\n> +\t{\n> +\t\tThread thread;\n> +\t\tthread.start();\n> +\n> +\t\tEventHandler handler;\n> +\t\thandler.notify();\n> +\t\thandler.moveToThread(&thread);\n> +\n> +\t\tthis_thread::sleep_for(chrono::milliseconds(100));\n> +\n> +\t\t/* Must stop thread before destroying the handler. */\n> +\t\tthread.exit(0);\n> +\t\tthread.wait();\n> +\n> +\t\tif (!handler.notified()) {\n> +\t\t\tcout << \"Thread event handling test failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +};\n> +\n> +TEST_REGISTER(EventThreadTest)\n> diff --git a/test/meson.build b/test/meson.build\n> index c6601813db78..f695ffd7be44 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -23,6 +23,7 @@ public_tests = [\n>\n> internal_tests = [\n> ['camera-sensor', 'camera-sensor.cpp'],\n> + ['event-thread', 'event-thread.cpp'],\n> ['message', 'message.cpp'],\n> ['object', 'object.cpp'],\n> ['object-invoke', 'object-invoke.cpp'],\n> --\n> Regards,\n>\n> Laurent Pinchart\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 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 1B4AD60E2C\n\tfor ;\n\tThu, 15 Aug 2019 11:48:49 +0200 (CEST)","from uno.localdomain\n\t(host64-130-dynamic.5-87-r.retail.telecomitalia.it [87.5.130.64])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 36B3A1BF209;\n\tThu, 15 Aug 2019 09:48:47 +0000 (UTC)"],"X-Originating-IP":"87.5.130.64","Date":"Thu, 15 Aug 2019 11:50:15 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190815095014.kz2vvkn3ogad57jb@uno.localdomain>","References":"<20190812124642.24287-1-laurent.pinchart@ideasonboard.com>\n\t<20190812124642.24287-14-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"jbchtaxswsyryocz\"","Content-Disposition":"inline","In-Reply-To":"<20190812124642.24287-14-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 13/18] test: Add EventNotifier thread\n\tmove test","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":"Thu, 15 Aug 2019 09:48:49 -0000"}},{"id":2413,"web_url":"https://patchwork.libcamera.org/comment/2413/","msgid":"<20190815095602.GD5011@pendragon.ideasonboard.com>","date":"2019-08-15T09:56:02","subject":"Re: [libcamera-devel] [PATCH 13/18] test: Add EventNotifier thread\n\tmove test","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Aug 15, 2019 at 11:50:15AM +0200, Jacopo Mondi wrote:\n> On Mon, Aug 12, 2019 at 03:46:37PM +0300, Laurent Pinchart wrote:\n> > The test verifies correct behaviour of an enabled event notifier moved\n> > to a different thread.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/event_notifier.cpp | 2 +-\n> > test/event-thread.cpp | 112 +++++++++++++++++++++++++++++++\n> > test/meson.build | 1 +\n> > 3 files changed, 114 insertions(+), 1 deletion(-)\n> > create mode 100644 test/event-thread.cpp\n> >\n> > diff --git a/src/libcamera/event_notifier.cpp b/src/libcamera/event_notifier.cpp\n> > index 515e6d1770a1..96be27601982 100644\n> > --- a/src/libcamera/event_notifier.cpp\n> > +++ b/src/libcamera/event_notifier.cpp\n> > @@ -127,7 +127,7 @@ void EventNotifier::message(Message *msg)\n> > \tif (msg->type() == Message::ThreadMoveMessage) {\n> > \t\tif (enabled_) {\n> > \t\t\tsetEnabled(false);\n> > -\t\t\tinvokeMethod(this, &EventNotifier::setEnabled, true);\n> > +\t\t\tinvokeMethod(&EventNotifier::setEnabled, true);\n> \n> I've missed the reason for this change. Also, isnt't this the syntax\n> to invoce static bound methods?\n\nThis should be part of \"libcamera: object: Add an asynchronous method\ninvocation method\", I'll move it there. invokeMethod() used to be a\nstatic method, hence the first argument. I've changed it to a member\nmethod and this is a leftover.\n\n> > \t\t}\n> > \t}\n> >\n> > diff --git a/test/event-thread.cpp b/test/event-thread.cpp\n> > new file mode 100644\n> > index 000000000000..4a82d49b94f1\n> > --- /dev/null\n> > +++ b/test/event-thread.cpp\n> > @@ -0,0 +1,112 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * event-thread.cpp - Threaded event test\n> > + */\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \"test.h\"\n> > +#include \"thread.h\"\n> > +\n> > +using namespace std;\n> > +using namespace libcamera;\n> > +\n> > +class EventHandler : public Object\n> > +{\n> > +public:\n> > +\tEventHandler()\n> > +\t{\n> > +\t\tpipe(pipefd_);\n> > +\n> > +\t\tnotifier_ = new EventNotifier(pipefd_[0], EventNotifier::Read);\n> > +\t\tnotifier_->activated.connect(this, &EventHandler::readReady);\n> > +\t}\n> > +\n> > +\t~EventHandler()\n> > +\t{\n> > +\t\tdelete notifier_;\n> > +\n> > +\t\tclose(pipefd_[0]);\n> > +\t\tclose(pipefd_[1]);\n> > +\t}\n> > +\n> > +\tint notify()\n> > +\t{\n> > +\t\tstd::string data(\"H2G2\");\n> > +\t\tssize_t ret;\n> > +\n> > +\t\tmemset(data_, 0, sizeof(data_));\n> > +\t\tsize_ = 0;\n> > +\n> > +\t\tret = write(pipefd_[1], data.data(), data.size());\n> > +\t\tif (ret < 0) {\n> > +\t\t\tcout << \"Pipe write failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tbool notified() const\n> > +\t{\n> > +\t\treturn notified_;\n> > +\t}\n> > +\n> > +\tvoid moveToThread(Thread *thread)\n> > +\t{\n> > +\t\tObject::moveToThread(thread);\n> > +\t\tnotifier_->moveToThread(thread);\n> > +\t}\n> > +\n> > +private:\n> > +\tvoid readReady(EventNotifier *notifier)\n> > +\t{\n> > +\t\tsize_ = read(notifier->fd(), data_, sizeof(data_));\n> > +\t\tnotified_ = true;\n> > +\t}\n> > +\n> > +\tEventNotifier *notifier_;\n> > +\n> > +\tint pipefd_[2];\n> > +\n> > +\tbool notified_;\n> > +\tchar data_[16];\n> > +\tssize_t size_;\n> > +};\n> > +\n> > +class EventThreadTest : public Test\n> > +{\n> > +protected:\n> > +\tint run()\n> > +\t{\n> > +\t\tThread thread;\n> > +\t\tthread.start();\n> > +\n> > +\t\tEventHandler handler;\n> > +\t\thandler.notify();\n> > +\t\thandler.moveToThread(&thread);\n> > +\n> > +\t\tthis_thread::sleep_for(chrono::milliseconds(100));\n> > +\n> > +\t\t/* Must stop thread before destroying the handler. */\n> > +\t\tthread.exit(0);\n> > +\t\tthread.wait();\n> > +\n> > +\t\tif (!handler.notified()) {\n> > +\t\t\tcout << \"Thread event handling test failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +};\n> > +\n> > +TEST_REGISTER(EventThreadTest)\n> > diff --git a/test/meson.build b/test/meson.build\n> > index c6601813db78..f695ffd7be44 100644\n> > --- a/test/meson.build\n> > +++ b/test/meson.build\n> > @@ -23,6 +23,7 @@ public_tests = [\n> >\n> > internal_tests = [\n> > ['camera-sensor', 'camera-sensor.cpp'],\n> > + ['event-thread', 'event-thread.cpp'],\n> > ['message', 'message.cpp'],\n> > ['object', 'object.cpp'],\n> > ['object-invoke', 'object-invoke.cpp'],","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 7E2D860E2C\n\tfor ;\n\tThu, 15 Aug 2019 11:56:10 +0200 (CEST)","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 152BD2B2;\n\tThu, 15 Aug 2019 11:56:10 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1565862970;\n\tbh=1aCe7tmAdRgGJn0o+0Eq/LRX3P4BiiCBZZPvO3QJPR8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oxY+911NdFDDcrfTsa/dkp6TWMq+EoyGvZC7X/kHh0/iLmFy+5hqGfXYslAPHVeNQ\n\ts1C0YdYJEqGH8F5LdUFTO1Rt9gEADkF6LEULJUqcs8i4O9RUCoaCxkQhizLiL5n1MM\n\tUYeVYeadJD2SoOt/EpBW1PMBypGxcajrEOOzXO/c=","Date":"Thu, 15 Aug 2019 12:56:02 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190815095602.GD5011@pendragon.ideasonboard.com>","References":"<20190812124642.24287-1-laurent.pinchart@ideasonboard.com>\n\t<20190812124642.24287-14-laurent.pinchart@ideasonboard.com>\n\t<20190815095014.kz2vvkn3ogad57jb@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190815095014.kz2vvkn3ogad57jb@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 13/18] test: Add EventNotifier thread\n\tmove test","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":"Thu, 15 Aug 2019 09:56:10 -0000"}},{"id":2415,"web_url":"https://patchwork.libcamera.org/comment/2415/","msgid":"<20190815095848.GE5011@pendragon.ideasonboard.com>","date":"2019-08-15T09:58:48","subject":"Re: [libcamera-devel] [PATCH 13/18] test: Add EventNotifier thread\n\tmove test","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Aug 15, 2019 at 12:56:02PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n> \n> On Thu, Aug 15, 2019 at 11:50:15AM +0200, Jacopo Mondi wrote:\n> > On Mon, Aug 12, 2019 at 03:46:37PM +0300, Laurent Pinchart wrote:\n> > > The test verifies correct behaviour of an enabled event notifier moved\n> > > to a different thread.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/libcamera/event_notifier.cpp | 2 +-\n> > > test/event-thread.cpp | 112 +++++++++++++++++++++++++++++++\n> > > test/meson.build | 1 +\n> > > 3 files changed, 114 insertions(+), 1 deletion(-)\n> > > create mode 100644 test/event-thread.cpp\n> > >\n> > > diff --git a/src/libcamera/event_notifier.cpp b/src/libcamera/event_notifier.cpp\n> > > index 515e6d1770a1..96be27601982 100644\n> > > --- a/src/libcamera/event_notifier.cpp\n> > > +++ b/src/libcamera/event_notifier.cpp\n> > > @@ -127,7 +127,7 @@ void EventNotifier::message(Message *msg)\n> > > \tif (msg->type() == Message::ThreadMoveMessage) {\n> > > \t\tif (enabled_) {\n> > > \t\t\tsetEnabled(false);\n> > > -\t\t\tinvokeMethod(this, &EventNotifier::setEnabled, true);\n> > > +\t\t\tinvokeMethod(&EventNotifier::setEnabled, true);\n> > \n> > I've missed the reason for this change. Also, isnt't this the syntax\n> > to invoce static bound methods?\n> \n> This should be part of \"libcamera: object: Add an asynchronous method\n> invocation method\", I'll move it there. invokeMethod() used to be a\n> static method, hence the first argument. I've changed it to a member\n> method and this is a leftover.\n\n\"libcamera: object: Add an asynchronous method invocation method\" isn't\nthe right patch. Looks like I've already reorganised this in my private\nbranch anyway, so it will be moved to the right place in the next\nversion.\n\n> > > \t\t}\n> > > \t}\n> > >\n> > > diff --git a/test/event-thread.cpp b/test/event-thread.cpp\n> > > new file mode 100644\n> > > index 000000000000..4a82d49b94f1\n> > > --- /dev/null\n> > > +++ b/test/event-thread.cpp\n> > > @@ -0,0 +1,112 @@\n> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > > +/*\n> > > + * Copyright (C) 2019, Google Inc.\n> > > + *\n> > > + * event-thread.cpp - Threaded event test\n> > > + */\n> > > +\n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \"test.h\"\n> > > +#include \"thread.h\"\n> > > +\n> > > +using namespace std;\n> > > +using namespace libcamera;\n> > > +\n> > > +class EventHandler : public Object\n> > > +{\n> > > +public:\n> > > +\tEventHandler()\n> > > +\t{\n> > > +\t\tpipe(pipefd_);\n> > > +\n> > > +\t\tnotifier_ = new EventNotifier(pipefd_[0], EventNotifier::Read);\n> > > +\t\tnotifier_->activated.connect(this, &EventHandler::readReady);\n> > > +\t}\n> > > +\n> > > +\t~EventHandler()\n> > > +\t{\n> > > +\t\tdelete notifier_;\n> > > +\n> > > +\t\tclose(pipefd_[0]);\n> > > +\t\tclose(pipefd_[1]);\n> > > +\t}\n> > > +\n> > > +\tint notify()\n> > > +\t{\n> > > +\t\tstd::string data(\"H2G2\");\n> > > +\t\tssize_t ret;\n> > > +\n> > > +\t\tmemset(data_, 0, sizeof(data_));\n> > > +\t\tsize_ = 0;\n> > > +\n> > > +\t\tret = write(pipefd_[1], data.data(), data.size());\n> > > +\t\tif (ret < 0) {\n> > > +\t\t\tcout << \"Pipe write failed\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > +\t\treturn TestPass;\n> > > +\t}\n> > > +\n> > > +\tbool notified() const\n> > > +\t{\n> > > +\t\treturn notified_;\n> > > +\t}\n> > > +\n> > > +\tvoid moveToThread(Thread *thread)\n> > > +\t{\n> > > +\t\tObject::moveToThread(thread);\n> > > +\t\tnotifier_->moveToThread(thread);\n> > > +\t}\n> > > +\n> > > +private:\n> > > +\tvoid readReady(EventNotifier *notifier)\n> > > +\t{\n> > > +\t\tsize_ = read(notifier->fd(), data_, sizeof(data_));\n> > > +\t\tnotified_ = true;\n> > > +\t}\n> > > +\n> > > +\tEventNotifier *notifier_;\n> > > +\n> > > +\tint pipefd_[2];\n> > > +\n> > > +\tbool notified_;\n> > > +\tchar data_[16];\n> > > +\tssize_t size_;\n> > > +};\n> > > +\n> > > +class EventThreadTest : public Test\n> > > +{\n> > > +protected:\n> > > +\tint run()\n> > > +\t{\n> > > +\t\tThread thread;\n> > > +\t\tthread.start();\n> > > +\n> > > +\t\tEventHandler handler;\n> > > +\t\thandler.notify();\n> > > +\t\thandler.moveToThread(&thread);\n> > > +\n> > > +\t\tthis_thread::sleep_for(chrono::milliseconds(100));\n> > > +\n> > > +\t\t/* Must stop thread before destroying the handler. */\n> > > +\t\tthread.exit(0);\n> > > +\t\tthread.wait();\n> > > +\n> > > +\t\tif (!handler.notified()) {\n> > > +\t\t\tcout << \"Thread event handling test failed\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > +\t\treturn TestPass;\n> > > +\t}\n> > > +};\n> > > +\n> > > +TEST_REGISTER(EventThreadTest)\n> > > diff --git a/test/meson.build b/test/meson.build\n> > > index c6601813db78..f695ffd7be44 100644\n> > > --- a/test/meson.build\n> > > +++ b/test/meson.build\n> > > @@ -23,6 +23,7 @@ public_tests = [\n> > >\n> > > internal_tests = [\n> > > ['camera-sensor', 'camera-sensor.cpp'],\n> > > + ['event-thread', 'event-thread.cpp'],\n> > > ['message', 'message.cpp'],\n> > > ['object', 'object.cpp'],\n> > > ['object-invoke', 'object-invoke.cpp'],\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 300CC60E2C\n\tfor ;\n\tThu, 15 Aug 2019 11:58:52 +0200 (CEST)","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 93D5C2B2;\n\tThu, 15 Aug 2019 11:58:51 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1565863131;\n\tbh=DaCh4q19uiJDjxi9W2GAayYBArIzheZ7dLagGlxFdBk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GT0hnKTCvV90Rbr8hcMGn8qDY/vpZLUBXX97+06BB5Fo2k3WX/7Ucd+MaUJZHB7y3\n\tujoigXy3Mim+BG58jzHXC0QFN94zGHCQWJb18932rnuK+U/FNSk3UdcUHr6uDrvk3M\n\trqZonC8tCgnsYOPZ3iAyjLfgZcQbDMWwQ1Q6jO2c=","Date":"Thu, 15 Aug 2019 12:58:48 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190815095848.GE5011@pendragon.ideasonboard.com>","References":"<20190812124642.24287-1-laurent.pinchart@ideasonboard.com>\n\t<20190812124642.24287-14-laurent.pinchart@ideasonboard.com>\n\t<20190815095014.kz2vvkn3ogad57jb@uno.localdomain>\n\t<20190815095602.GD5011@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190815095602.GD5011@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 13/18] test: Add EventNotifier thread\n\tmove test","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":"Thu, 15 Aug 2019 09:58:52 -0000"}},{"id":2417,"web_url":"https://patchwork.libcamera.org/comment/2417/","msgid":"<20190815101216.vvqipwqqkdaskn5p@uno.localdomain>","date":"2019-08-15T10:12:16","subject":"Re: [libcamera-devel] [PATCH 13/18] test: Add EventNotifier thread\n\tmove test","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Thu, Aug 15, 2019 at 12:56:02PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Thu, Aug 15, 2019 at 11:50:15AM +0200, Jacopo Mondi wrote:\n> > On Mon, Aug 12, 2019 at 03:46:37PM +0300, Laurent Pinchart wrote:\n> > > The test verifies correct behaviour of an enabled event notifier moved\n> > > to a different thread.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/libcamera/event_notifier.cpp | 2 +-\n> > > test/event-thread.cpp | 112 +++++++++++++++++++++++++++++++\n> > > test/meson.build | 1 +\n> > > 3 files changed, 114 insertions(+), 1 deletion(-)\n> > > create mode 100644 test/event-thread.cpp\n> > >\n> > > diff --git a/src/libcamera/event_notifier.cpp b/src/libcamera/event_notifier.cpp\n> > > index 515e6d1770a1..96be27601982 100644\n> > > --- a/src/libcamera/event_notifier.cpp\n> > > +++ b/src/libcamera/event_notifier.cpp\n> > > @@ -127,7 +127,7 @@ void EventNotifier::message(Message *msg)\n> > > \tif (msg->type() == Message::ThreadMoveMessage) {\n> > > \t\tif (enabled_) {\n> > > \t\t\tsetEnabled(false);\n> > > -\t\t\tinvokeMethod(this, &EventNotifier::setEnabled, true);\n> > > +\t\t\tinvokeMethod(&EventNotifier::setEnabled, true);\n> >\n> > I've missed the reason for this change. Also, isnt't this the syntax\n> > to invoce static bound methods?\n>\n> This should be part of \"libcamera: object: Add an asynchronous method\n> invocation method\", I'll move it there. invokeMethod() used to be a\n> static method, hence the first argument. I've changed it to a member\n> method and this is a leftover.\n>\n\nNot sure I follow here. The line of code you have here changed has\nbeen introduced by 11/18 but you say this change should be part of\n6/18 ?\n\nIn general I thought the first argument of invokeMethod should be a\npointer to instance the method should be invoked on to provide a\ntypename to 'obj'\n\nI'm most probably not reading this right (from 6/18)\n\n + template::value>::type * = nullptr>\n + void invokeMethod(void (T::*func)(Args...), Args... args)\n + {\n + T *obj = static_cast(this);\n + BoundMethodBase *method = new BoundMemberMethod(obj, this, func);\n + void *pack = new typename BoundMemberMethod::PackType{ args... };\n +\n + invokeMethod(method, pack);\n + }\n\n\n> > > \t\t}\n> > > \t}\n> > >\n> > > diff --git a/test/event-thread.cpp b/test/event-thread.cpp\n> > > new file mode 100644\n> > > index 000000000000..4a82d49b94f1\n> > > --- /dev/null\n> > > +++ b/test/event-thread.cpp\n> > > @@ -0,0 +1,112 @@\n> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > > +/*\n> > > + * Copyright (C) 2019, Google Inc.\n> > > + *\n> > > + * event-thread.cpp - Threaded event test\n> > > + */\n> > > +\n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \"test.h\"\n> > > +#include \"thread.h\"\n> > > +\n> > > +using namespace std;\n> > > +using namespace libcamera;\n> > > +\n> > > +class EventHandler : public Object\n> > > +{\n> > > +public:\n> > > +\tEventHandler()\n> > > +\t{\n> > > +\t\tpipe(pipefd_);\n> > > +\n> > > +\t\tnotifier_ = new EventNotifier(pipefd_[0], EventNotifier::Read);\n> > > +\t\tnotifier_->activated.connect(this, &EventHandler::readReady);\n> > > +\t}\n> > > +\n> > > +\t~EventHandler()\n> > > +\t{\n> > > +\t\tdelete notifier_;\n> > > +\n> > > +\t\tclose(pipefd_[0]);\n> > > +\t\tclose(pipefd_[1]);\n> > > +\t}\n> > > +\n> > > +\tint notify()\n> > > +\t{\n> > > +\t\tstd::string data(\"H2G2\");\n> > > +\t\tssize_t ret;\n> > > +\n> > > +\t\tmemset(data_, 0, sizeof(data_));\n> > > +\t\tsize_ = 0;\n> > > +\n> > > +\t\tret = write(pipefd_[1], data.data(), data.size());\n> > > +\t\tif (ret < 0) {\n> > > +\t\t\tcout << \"Pipe write failed\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > +\t\treturn TestPass;\n> > > +\t}\n> > > +\n> > > +\tbool notified() const\n> > > +\t{\n> > > +\t\treturn notified_;\n> > > +\t}\n> > > +\n> > > +\tvoid moveToThread(Thread *thread)\n> > > +\t{\n> > > +\t\tObject::moveToThread(thread);\n> > > +\t\tnotifier_->moveToThread(thread);\n> > > +\t}\n> > > +\n> > > +private:\n> > > +\tvoid readReady(EventNotifier *notifier)\n> > > +\t{\n> > > +\t\tsize_ = read(notifier->fd(), data_, sizeof(data_));\n> > > +\t\tnotified_ = true;\n> > > +\t}\n> > > +\n> > > +\tEventNotifier *notifier_;\n> > > +\n> > > +\tint pipefd_[2];\n> > > +\n> > > +\tbool notified_;\n> > > +\tchar data_[16];\n> > > +\tssize_t size_;\n> > > +};\n> > > +\n> > > +class EventThreadTest : public Test\n> > > +{\n> > > +protected:\n> > > +\tint run()\n> > > +\t{\n> > > +\t\tThread thread;\n> > > +\t\tthread.start();\n> > > +\n> > > +\t\tEventHandler handler;\n> > > +\t\thandler.notify();\n> > > +\t\thandler.moveToThread(&thread);\n> > > +\n> > > +\t\tthis_thread::sleep_for(chrono::milliseconds(100));\n> > > +\n> > > +\t\t/* Must stop thread before destroying the handler. */\n> > > +\t\tthread.exit(0);\n> > > +\t\tthread.wait();\n> > > +\n> > > +\t\tif (!handler.notified()) {\n> > > +\t\t\tcout << \"Thread event handling test failed\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > +\t\treturn TestPass;\n> > > +\t}\n> > > +};\n> > > +\n> > > +TEST_REGISTER(EventThreadTest)\n> > > diff --git a/test/meson.build b/test/meson.build\n> > > index c6601813db78..f695ffd7be44 100644\n> > > --- a/test/meson.build\n> > > +++ b/test/meson.build\n> > > @@ -23,6 +23,7 @@ public_tests = [\n> > >\n> > > internal_tests = [\n> > > ['camera-sensor', 'camera-sensor.cpp'],\n> > > + ['event-thread', 'event-thread.cpp'],\n> > > ['message', 'message.cpp'],\n> > > ['object', 'object.cpp'],\n> > > ['object-invoke', 'object-invoke.cpp'],\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6CDF960E2D\n\tfor ;\n\tThu, 15 Aug 2019 12:10:51 +0200 (CEST)","from uno.localdomain\n\t(host64-130-dynamic.5-87-r.retail.telecomitalia.it [87.5.130.64])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id BB8A140003;\n\tThu, 15 Aug 2019 10:10:49 +0000 (UTC)"],"X-Originating-IP":"87.5.130.64","Date":"Thu, 15 Aug 2019 12:12:16 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190815101216.vvqipwqqkdaskn5p@uno.localdomain>","References":"<20190812124642.24287-1-laurent.pinchart@ideasonboard.com>\n\t<20190812124642.24287-14-laurent.pinchart@ideasonboard.com>\n\t<20190815095014.kz2vvkn3ogad57jb@uno.localdomain>\n\t<20190815095602.GD5011@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"62cex6nwfek6n2lg\"","Content-Disposition":"inline","In-Reply-To":"<20190815095602.GD5011@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 13/18] test: Add EventNotifier thread\n\tmove test","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":"Thu, 15 Aug 2019 10:10:51 -0000"}},{"id":2418,"web_url":"https://patchwork.libcamera.org/comment/2418/","msgid":"<20190815101348.3n5jkawmtit6awgy@uno.localdomain>","date":"2019-08-15T10:13:48","subject":"Re: [libcamera-devel] [PATCH 13/18] test: Add EventNotifier thread\n\tmove test","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Thu, Aug 15, 2019 at 12:58:48PM +0300, Laurent Pinchart wrote:\n> On Thu, Aug 15, 2019 at 12:56:02PM +0300, Laurent Pinchart wrote:\n> > Hi Jacopo,\n> >\n> > On Thu, Aug 15, 2019 at 11:50:15AM +0200, Jacopo Mondi wrote:\n> > > On Mon, Aug 12, 2019 at 03:46:37PM +0300, Laurent Pinchart wrote:\n> > > > The test verifies correct behaviour of an enabled event notifier moved\n> > > > to a different thread.\n> > > >\n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > src/libcamera/event_notifier.cpp | 2 +-\n> > > > test/event-thread.cpp | 112 +++++++++++++++++++++++++++++++\n> > > > test/meson.build | 1 +\n> > > > 3 files changed, 114 insertions(+), 1 deletion(-)\n> > > > create mode 100644 test/event-thread.cpp\n> > > >\n> > > > diff --git a/src/libcamera/event_notifier.cpp b/src/libcamera/event_notifier.cpp\n> > > > index 515e6d1770a1..96be27601982 100644\n> > > > --- a/src/libcamera/event_notifier.cpp\n> > > > +++ b/src/libcamera/event_notifier.cpp\n> > > > @@ -127,7 +127,7 @@ void EventNotifier::message(Message *msg)\n> > > > \tif (msg->type() == Message::ThreadMoveMessage) {\n> > > > \t\tif (enabled_) {\n> > > > \t\t\tsetEnabled(false);\n> > > > -\t\t\tinvokeMethod(this, &EventNotifier::setEnabled, true);\n> > > > +\t\t\tinvokeMethod(&EventNotifier::setEnabled, true);\n> > >\n> > > I've missed the reason for this change. Also, isnt't this the syntax\n> > > to invoce static bound methods?\n> >\n> > This should be part of \"libcamera: object: Add an asynchronous method\n> > invocation method\", I'll move it there. invokeMethod() used to be a\n> > static method, hence the first argument. I've changed it to a member\n> > method and this is a leftover.\n>\n> \"libcamera: object: Add an asynchronous method invocation method\" isn't\n> the right patch. Looks like I've already reorganised this in my private\n> branch anyway, so it will be moved to the right place in the next\n> version.\n>\n\nI see. Please be aware that a similar hunk (but for timers) is present\nin the following patch (14/18)\n\n> > > > \t\t}\n> > > > \t}\n> > > >\n> > > > diff --git a/test/event-thread.cpp b/test/event-thread.cpp\n> > > > new file mode 100644\n> > > > index 000000000000..4a82d49b94f1\n> > > > --- /dev/null\n> > > > +++ b/test/event-thread.cpp\n> > > > @@ -0,0 +1,112 @@\n> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > > > +/*\n> > > > + * Copyright (C) 2019, Google Inc.\n> > > > + *\n> > > > + * event-thread.cpp - Threaded event test\n> > > > + */\n> > > > +\n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +\n> > > > +#include \n> > > > +#include \n> > > > +\n> > > > +#include \"test.h\"\n> > > > +#include \"thread.h\"\n> > > > +\n> > > > +using namespace std;\n> > > > +using namespace libcamera;\n> > > > +\n> > > > +class EventHandler : public Object\n> > > > +{\n> > > > +public:\n> > > > +\tEventHandler()\n> > > > +\t{\n> > > > +\t\tpipe(pipefd_);\n> > > > +\n> > > > +\t\tnotifier_ = new EventNotifier(pipefd_[0], EventNotifier::Read);\n> > > > +\t\tnotifier_->activated.connect(this, &EventHandler::readReady);\n> > > > +\t}\n> > > > +\n> > > > +\t~EventHandler()\n> > > > +\t{\n> > > > +\t\tdelete notifier_;\n> > > > +\n> > > > +\t\tclose(pipefd_[0]);\n> > > > +\t\tclose(pipefd_[1]);\n> > > > +\t}\n> > > > +\n> > > > +\tint notify()\n> > > > +\t{\n> > > > +\t\tstd::string data(\"H2G2\");\n> > > > +\t\tssize_t ret;\n> > > > +\n> > > > +\t\tmemset(data_, 0, sizeof(data_));\n> > > > +\t\tsize_ = 0;\n> > > > +\n> > > > +\t\tret = write(pipefd_[1], data.data(), data.size());\n> > > > +\t\tif (ret < 0) {\n> > > > +\t\t\tcout << \"Pipe write failed\" << endl;\n> > > > +\t\t\treturn TestFail;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\treturn TestPass;\n> > > > +\t}\n> > > > +\n> > > > +\tbool notified() const\n> > > > +\t{\n> > > > +\t\treturn notified_;\n> > > > +\t}\n> > > > +\n> > > > +\tvoid moveToThread(Thread *thread)\n> > > > +\t{\n> > > > +\t\tObject::moveToThread(thread);\n> > > > +\t\tnotifier_->moveToThread(thread);\n> > > > +\t}\n> > > > +\n> > > > +private:\n> > > > +\tvoid readReady(EventNotifier *notifier)\n> > > > +\t{\n> > > > +\t\tsize_ = read(notifier->fd(), data_, sizeof(data_));\n> > > > +\t\tnotified_ = true;\n> > > > +\t}\n> > > > +\n> > > > +\tEventNotifier *notifier_;\n> > > > +\n> > > > +\tint pipefd_[2];\n> > > > +\n> > > > +\tbool notified_;\n> > > > +\tchar data_[16];\n> > > > +\tssize_t size_;\n> > > > +};\n> > > > +\n> > > > +class EventThreadTest : public Test\n> > > > +{\n> > > > +protected:\n> > > > +\tint run()\n> > > > +\t{\n> > > > +\t\tThread thread;\n> > > > +\t\tthread.start();\n> > > > +\n> > > > +\t\tEventHandler handler;\n> > > > +\t\thandler.notify();\n> > > > +\t\thandler.moveToThread(&thread);\n> > > > +\n> > > > +\t\tthis_thread::sleep_for(chrono::milliseconds(100));\n> > > > +\n> > > > +\t\t/* Must stop thread before destroying the handler. */\n> > > > +\t\tthread.exit(0);\n> > > > +\t\tthread.wait();\n> > > > +\n> > > > +\t\tif (!handler.notified()) {\n> > > > +\t\t\tcout << \"Thread event handling test failed\" << endl;\n> > > > +\t\t\treturn TestFail;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\treturn TestPass;\n> > > > +\t}\n> > > > +};\n> > > > +\n> > > > +TEST_REGISTER(EventThreadTest)\n> > > > diff --git a/test/meson.build b/test/meson.build\n> > > > index c6601813db78..f695ffd7be44 100644\n> > > > --- a/test/meson.build\n> > > > +++ b/test/meson.build\n> > > > @@ -23,6 +23,7 @@ public_tests = [\n> > > >\n> > > > internal_tests = [\n> > > > ['camera-sensor', 'camera-sensor.cpp'],\n> > > > + ['event-thread', 'event-thread.cpp'],\n> > > > ['message', 'message.cpp'],\n> > > > ['object', 'object.cpp'],\n> > > > ['object-invoke', 'object-invoke.cpp'],\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AF9D760E2D\n\tfor ;\n\tThu, 15 Aug 2019 12:12:22 +0200 (CEST)","from uno.localdomain\n\t(host64-130-dynamic.5-87-r.retail.telecomitalia.it [87.5.130.64])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id C2AB9FF805;\n\tThu, 15 Aug 2019 10:12:21 +0000 (UTC)"],"X-Originating-IP":"87.5.130.64","Date":"Thu, 15 Aug 2019 12:13:48 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190815101348.3n5jkawmtit6awgy@uno.localdomain>","References":"<20190812124642.24287-1-laurent.pinchart@ideasonboard.com>\n\t<20190812124642.24287-14-laurent.pinchart@ideasonboard.com>\n\t<20190815095014.kz2vvkn3ogad57jb@uno.localdomain>\n\t<20190815095602.GD5011@pendragon.ideasonboard.com>\n\t<20190815095848.GE5011@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"xpmmwwvlcciqtpq2\"","Content-Disposition":"inline","In-Reply-To":"<20190815095848.GE5011@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 13/18] test: Add EventNotifier thread\n\tmove test","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":"Thu, 15 Aug 2019 10:12:22 -0000"}}]