[{"id":2217,"web_url":"https://patchwork.libcamera.org/comment/2217/","msgid":"<20190711055347.GM1557@wyvern>","date":"2019-07-11T05:53:47","subject":"Re: [libcamera-devel] [PATCH 6/6] test: Add test case for signal\n\tdelivery across threads","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your test.\n\nOn 2019-07-10 22:17:08 +0300, Laurent Pinchart wrote:\n> The test case creates a receiver inheriting from Object, connects a\n> signal to one of its slot, moves the receiver to a different thread,\n> emits the signal and verifies that it gets delivered in the correct\n> thread with the expected value.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> test/meson.build | 1 +\n> test/signal-threads.cpp | 125 ++++++++++++++++++++++++++++++++++++++++\n> 2 files changed, 126 insertions(+)\n> create mode 100644 test/signal-threads.cpp\n> \n> diff --git a/test/meson.build b/test/meson.build\n> index 1f87319aeb65..60ce9601cc55 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -22,6 +22,7 @@ public_tests = [\n> internal_tests = [\n> ['camera-sensor', 'camera-sensor.cpp'],\n> ['message', 'message.cpp'],\n> + ['signal-threads', 'signal-threads.cpp'],\n> ['threads', 'threads.cpp'],\n> ]\n> \n> diff --git a/test/signal-threads.cpp b/test/signal-threads.cpp\n> new file mode 100644\n> index 000000000000..c21f32ae0c20\n> --- /dev/null\n> +++ b/test/signal-threads.cpp\n> @@ -0,0 +1,125 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * signal-threads.cpp - Cross-thread signal delivery test\n> + */\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \"message.h\"\n> +#include \"thread.h\"\n> +#include \"test.h\"\n> +#include \"utils.h\"\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +class SignalReceiver : public Object\n> +{\n> +public:\n> +\tenum Status {\n> +\t\tNoSignal,\n> +\t\tInvalidThread,\n> +\t\tSignalReceived,\n> +\t};\n> +\n> +\tSignalReceiver()\n> +\t\t: status_(NoSignal)\n> +\t{\n> +\t}\n> +\n> +\tStatus status() const { return status_; }\n> +\tint value() const { return value_; }\n> +\tvoid reset()\n> +\t{\n> +\t\tstatus_ = NoSignal;\n> +\t\tvalue_ = 0;\n> +\t}\n> +\n> +\tvoid slot(int value)\n> +\t{\n> +\t\tif (Thread::current() != thread())\n> +\t\t\tstatus_ = InvalidThread;\n> +\t\telse\n> +\t\t\tstatus_ = SignalReceived;\n> +\n> +\t\tvalue_ = value;\n> +\t}\n> +\n> +private:\n> +\tStatus status_;\n> +\tint value_;\n> +};\n> +\n> +class SignalThreadsTest : public Test\n> +{\n> +protected:\n> +\tint run()\n> +\t{\n> +\t\tSignalReceiver receiver;\n> +\t\tsignal_.connect(&receiver, &SignalReceiver::slot);\n> +\n> +\t\t/* Test that a signal is received in the main thread. */\n> +\t\tsignal_.emit(42);\n> +\n> +\t\tswitch (receiver.status()) {\n> +\t\tcase SignalReceiver::NoSignal:\n> +\t\t\tcout << \"No signal received for direct connection\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\tcase SignalReceiver::InvalidThread:\n> +\t\t\tcout << \"Signal received in incorrect thread \"\n> +\t\t\t\t\"for direct connection\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\tdefault:\n> +\t\t\tbreak;\n> +\t\t}\n\nIt's verified in other Signal tests that the argument(s) are delivered \nbut for the sake of us with OCD you might consider adding a \nreceiver.value() != 42 check here ;-)\n\nWith this fixed,\n\nReviewed-by: Niklas Söderlund \n\n> +\n> +\t\t/*\n> +\t\t * Move the object to a thread and verify that the signal is\n> +\t\t * correctly delivered, with the correct data.\n> +\t\t */\n> +\t\treceiver.reset();\n> +\t\treceiver.moveToThread(&thread_);\n> +\n> +\t\tthread_.start();\n> +\n> +\t\tsignal_.emit(42);\n> +\n> +\t\tthis_thread::sleep_for(chrono::milliseconds(100));\n> +\n> +\t\tswitch (receiver.status()) {\n> +\t\tcase SignalReceiver::NoSignal:\n> +\t\t\tcout << \"No signal received for message connection\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\tcase SignalReceiver::InvalidThread:\n> +\t\t\tcout << \"Signal received in incorrect thread \"\n> +\t\t\t\t\"for message connection\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\tdefault:\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tif (receiver.value() != 42) {\n> +\t\t\tcout << \"Signal received with incorrect value\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tvoid cleanup()\n> +\t{\n> +\t\tthread_.exit(0);\n> +\t\tthread_.wait();\n> +\t}\n> +\n> +private:\n> +\tThread thread_;\n> +\n> +\tSignal signal_;\n> +};\n> +\n> +TEST_REGISTER(SignalThreadsTest)\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 mail-pf1-x444.google.com (mail-pf1-x444.google.com\n\t[IPv6:2607:f8b0:4864:20::444])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D9DD260C23\n\tfor ;\n\tThu, 11 Jul 2019 07:53:52 +0200 (CEST)","by mail-pf1-x444.google.com with SMTP id c73so2195527pfb.13\n\tfor ;\n\tWed, 10 Jul 2019 22:53:52 -0700 (PDT)","from localhost (softbank126163157105.bbtec.net. [126.163.157.105])\n\tby smtp.gmail.com with ESMTPSA id\n\ts22sm4176105pfh.107.2019.07.10.22.53.49\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tWed, 10 Jul 2019 22:53:50 -0700 (PDT)"],"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=RWVjAgb8tXHIeeUT0pms3n5vn52PXTwlgUajxGlNn0k=;\n\tb=JdMrw8eIRJCKl8/neYcsay5twM4uwH+xcvaQoB/fQuKrEB0tmpIlrC+Wl9+1QzYHKd\n\txI7GVuSh1csUWEWZuADEA1Nh9QDELoH5nnGKl4WE9j0OFcyzD4oEKK6doUmpUil5RSnK\n\tyjCjtOE+4/6eb9v6RtFVsMFf45Dm9FAsW0nVRylnxKAmxtdM5BQl6S3MVyDQYPzyyPze\n\thZ8BsWoHKZPhdtacjzGAApjxptCNDLmcHUQRxg9hpFLDBYESQrnfbFcV9wCeQaKmU3Pm\n\tRosq72FKkoSeSpAtVy1mrC3DPhRzY1beBFhi/vdmziNxteX5OmEHnPUYTtN1njVSQP3i\n\teUGw==","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=RWVjAgb8tXHIeeUT0pms3n5vn52PXTwlgUajxGlNn0k=;\n\tb=HfM6qcxedTQjDvySU4PW1GLyqTSa5U4LFDDv/XceuHSTVpfTO/1aQvREY5SIF7eSN3\n\tTf6sOn4xR9dECsz+yR8t57AlctcOVG0eo57D3yO7dRN3JYSzHUiq5e1LZ76QLtJvqAcf\n\t07et94J4G7l8qcZDYnmZnD2KoKNcdDLwQuiXXbqfbSMygNFUUXkIIKFOYUz42UkANUPL\n\t20alO7957SuQN7+ixMya60ph5Q5hxmzt/30hERu/FIJEyKkBQeEmzjrROMs1+r5GYulp\n\te/Sx8uWE7GwvJKlrkUpnsF8y+Zd16vVz8F44WtpUkJ8reBm1ub7pX7Ks75TAWPl4G8l8\n\t5xDg==","X-Gm-Message-State":"APjAAAWSpTbdAXEWqasFJZReeo/mOSwVq5VvhmfLOd2rNkNr2ILH396d\n\ta2DvrZl76P2TWgcFhX/wvu4=","X-Google-Smtp-Source":"APXvYqzLEC1LKa56d5vafLMtwJZ6y/8vTtaFR1elT/szfguGvS2M9f1vvW03/i079o9NoEcT1lkWVQ==","X-Received":"by 2002:a63:df46:: with SMTP id h6mr2427071pgj.181.1562824431270;\n\tWed, 10 Jul 2019 22:53:51 -0700 (PDT)","Date":"Thu, 11 Jul 2019 14:53:47 +0900","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190711055347.GM1557@wyvern>","References":"<20190710191708.13049-1-laurent.pinchart@ideasonboard.com>\n\t<20190710191708.13049-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190710191708.13049-6-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH 6/6] test: Add test case for signal\n\tdelivery across threads","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, 11 Jul 2019 05:53:53 -0000"}},{"id":2222,"web_url":"https://patchwork.libcamera.org/comment/2222/","msgid":"<20190711065347.GJ5247@pendragon.ideasonboard.com>","date":"2019-07-11T06:53:47","subject":"Re: [libcamera-devel] [PATCH 6/6] test: Add test case for signal\n\tdelivery across threads","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Thu, Jul 11, 2019 at 02:53:47PM +0900, Niklas Söderlund wrote:\n> On 2019-07-10 22:17:08 +0300, Laurent Pinchart wrote:\n> > The test case creates a receiver inheriting from Object, connects a\n> > signal to one of its slot, moves the receiver to a different thread,\n> > emits the signal and verifies that it gets delivered in the correct\n> > thread with the expected value.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > test/meson.build | 1 +\n> > test/signal-threads.cpp | 125 ++++++++++++++++++++++++++++++++++++++++\n> > 2 files changed, 126 insertions(+)\n> > create mode 100644 test/signal-threads.cpp\n> > \n> > diff --git a/test/meson.build b/test/meson.build\n> > index 1f87319aeb65..60ce9601cc55 100644\n> > --- a/test/meson.build\n> > +++ b/test/meson.build\n> > @@ -22,6 +22,7 @@ public_tests = [\n> > internal_tests = [\n> > ['camera-sensor', 'camera-sensor.cpp'],\n> > ['message', 'message.cpp'],\n> > + ['signal-threads', 'signal-threads.cpp'],\n> > ['threads', 'threads.cpp'],\n> > ]\n> > \n> > diff --git a/test/signal-threads.cpp b/test/signal-threads.cpp\n> > new file mode 100644\n> > index 000000000000..c21f32ae0c20\n> > --- /dev/null\n> > +++ b/test/signal-threads.cpp\n> > @@ -0,0 +1,125 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * signal-threads.cpp - Cross-thread signal delivery test\n> > + */\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \"message.h\"\n> > +#include \"thread.h\"\n> > +#include \"test.h\"\n> > +#include \"utils.h\"\n> > +\n> > +using namespace std;\n> > +using namespace libcamera;\n> > +\n> > +class SignalReceiver : public Object\n> > +{\n> > +public:\n> > +\tenum Status {\n> > +\t\tNoSignal,\n> > +\t\tInvalidThread,\n> > +\t\tSignalReceived,\n> > +\t};\n> > +\n> > +\tSignalReceiver()\n> > +\t\t: status_(NoSignal)\n> > +\t{\n> > +\t}\n> > +\n> > +\tStatus status() const { return status_; }\n> > +\tint value() const { return value_; }\n> > +\tvoid reset()\n> > +\t{\n> > +\t\tstatus_ = NoSignal;\n> > +\t\tvalue_ = 0;\n> > +\t}\n> > +\n> > +\tvoid slot(int value)\n> > +\t{\n> > +\t\tif (Thread::current() != thread())\n> > +\t\t\tstatus_ = InvalidThread;\n> > +\t\telse\n> > +\t\t\tstatus_ = SignalReceived;\n> > +\n> > +\t\tvalue_ = value;\n> > +\t}\n> > +\n> > +private:\n> > +\tStatus status_;\n> > +\tint value_;\n> > +};\n> > +\n> > +class SignalThreadsTest : public Test\n> > +{\n> > +protected:\n> > +\tint run()\n> > +\t{\n> > +\t\tSignalReceiver receiver;\n> > +\t\tsignal_.connect(&receiver, &SignalReceiver::slot);\n> > +\n> > +\t\t/* Test that a signal is received in the main thread. */\n> > +\t\tsignal_.emit(42);\n> > +\n> > +\t\tswitch (receiver.status()) {\n> > +\t\tcase SignalReceiver::NoSignal:\n> > +\t\t\tcout << \"No signal received for direct connection\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\tcase SignalReceiver::InvalidThread:\n> > +\t\t\tcout << \"Signal received in incorrect thread \"\n> > +\t\t\t\t\"for direct connection\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\tdefault:\n> > +\t\t\tbreak;\n> > +\t\t}\n> \n> It's verified in other Signal tests that the argument(s) are delivered \n> but for the sake of us with OCD you might consider adding a \n> receiver.value() != 42 check here ;-)\n\nThe purpose of this first test is to make sure that the signal gets\ndelivered directly when the receiver lives in the same thread as the\nsender. More extended value checks are done in the signal.cpp test. The\ntest below, however, tests that values can be carried across threads, so\nit's needed.\n\n> With this fixed,\n> \n> Reviewed-by: Niklas Söderlund \n> \n> > +\n> > +\t\t/*\n> > +\t\t * Move the object to a thread and verify that the signal is\n> > +\t\t * correctly delivered, with the correct data.\n> > +\t\t */\n> > +\t\treceiver.reset();\n> > +\t\treceiver.moveToThread(&thread_);\n> > +\n> > +\t\tthread_.start();\n> > +\n> > +\t\tsignal_.emit(42);\n> > +\n> > +\t\tthis_thread::sleep_for(chrono::milliseconds(100));\n> > +\n> > +\t\tswitch (receiver.status()) {\n> > +\t\tcase SignalReceiver::NoSignal:\n> > +\t\t\tcout << \"No signal received for message connection\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\tcase SignalReceiver::InvalidThread:\n> > +\t\t\tcout << \"Signal received in incorrect thread \"\n> > +\t\t\t\t\"for message connection\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\tdefault:\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\n> > +\t\tif (receiver.value() != 42) {\n> > +\t\t\tcout << \"Signal received with incorrect value\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tvoid cleanup()\n> > +\t{\n> > +\t\tthread_.exit(0);\n> > +\t\tthread_.wait();\n> > +\t}\n> > +\n> > +private:\n> > +\tThread thread_;\n> > +\n> > +\tSignal signal_;\n> > +};\n> > +\n> > +TEST_REGISTER(SignalThreadsTest)","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 72D6560C23\n\tfor ;\n\tThu, 11 Jul 2019 08:54:17 +0200 (CEST)","from pendragon.ideasonboard.com (softbank126163157105.bbtec.net\n\t[126.163.157.105])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 14FBC31C;\n\tThu, 11 Jul 2019 08:54:15 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1562828057;\n\tbh=jqO5QSkDy0AVak5hVLfzvOnINfpKXJKDZXR2druEe/w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LATgiTYc5R6E6tC+XzpdGZjoiiy4jBWP6QB26LXa0eCdoQlMkhhiu5P0ezKG9r0Xw\n\ty5d6UiYsuHa7+1tp1hT1p0l8FkAK3YEMpYGqYs0m2bEbkds1Tyfkru0fZbVVwUyh9+\n\tdWsdc0tVWkW7IsnyxNtYw9kbd1Jnk5WYt2uXmKaA=","Date":"Thu, 11 Jul 2019 09:53:47 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190711065347.GJ5247@pendragon.ideasonboard.com>","References":"<20190710191708.13049-1-laurent.pinchart@ideasonboard.com>\n\t<20190710191708.13049-6-laurent.pinchart@ideasonboard.com>\n\t<20190711055347.GM1557@wyvern>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190711055347.GM1557@wyvern>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 6/6] test: Add test case for signal\n\tdelivery across threads","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, 11 Jul 2019 06:54:17 -0000"}}]