[{"id":19204,"web_url":"https://patchwork.libcamera.org/comment/19204/","msgid":"<40c21b56-6eba-c01b-67b7-2ce24d203fc5@ideasonboard.com>","date":"2021-08-31T06:02:41","subject":"Re: [libcamera-devel] [PATCH v1 4/6] libcamera: base: signal:\n\tSupport connecting signals to functors","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nThanks for the patch. I have a few questions regarding the documentation\n\nOn 8/27/21 8:08 AM, Laurent Pinchart wrote:\n> It can be useful to connect a signal to a functor, and in particular a\n> lambda function, while still operating in the context of a receiver\n> object (to support both object-based disconnection and queued\n> connections to Object instances).\n>\n> Add a BoundMethodFunctor class to bind a functor, and a corresponding\n> Signal::connect() function. There is no corresponding disconnect()\n> function, as a lambda passed to connect() can't be later passed to\n> disconnect(). Disconnection typically uses disconnect(T *object), which\n> will cover the vast majority of use cases.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> Documentation/Doxyfile.in | 1 +\n> include/libcamera/base/bound_method.h | 31 +++++++++++++++++++++\n> include/libcamera/base/signal.h | 19 +++++++++++++\n> src/libcamera/base/signal.cpp | 24 +++++++++++++++++\n> test/signal.cpp | 39 +++++++++++++++++++++++++++\n> 5 files changed, 114 insertions(+)\n>\n> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> index dc03cbea4b02..d562510e902e 100644\n> --- a/Documentation/Doxyfile.in\n> +++ b/Documentation/Doxyfile.in\n> @@ -878,6 +878,7 @@ EXCLUDE_PATTERNS = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \\\n> \n> EXCLUDE_SYMBOLS = libcamera::BoundMethodArgs \\\n> libcamera::BoundMethodBase \\\n> + libcamera::BoundMethodFunctor \\\n> libcamera::BoundMethodMember \\\n> libcamera::BoundMethodPack \\\n> libcamera::BoundMethodPackBase \\\n> diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h\n> index 76ce8017e721..ebd297ab8209 100644\n> --- a/include/libcamera/base/bound_method.h\n> +++ b/include/libcamera/base/bound_method.h\n> @@ -128,6 +128,37 @@ public:\n> \tvirtual R invoke(Args... args) = 0;\n> };\n> \n> +template\n> +class BoundMethodFunctor : public BoundMethodArgs\n> +{\n> +public:\n> +\tusing PackType = typename BoundMethodArgs::PackType;\n> +\n> +\tBoundMethodFunctor(T *obj, Object *object, Func func,\n> +\t\t\t ConnectionType type = ConnectionTypeAuto)\n> +\t\t: BoundMethodArgs(obj, object, type), func_(func)\n> +\t{\n> +\t}\n> +\n> +\tR activate(Args... args, bool deleteMethod = false) override\n> +\t{\n> +\t\tif (!this->object_)\n> +\t\t\treturn func_(args...);\n> +\n> +\t\tauto pack = std::make_shared(args...);\n> +\t\tbool sync = BoundMethodBase::activatePack(pack, deleteMethod);\n> +\t\treturn sync ? pack->returnValue() : R();\n> +\t}\n> +\n> +\tR invoke(Args... args) override\n> +\t{\n> +\t\treturn func_(args...);\n> +\t}\n> +\n> +private:\n> +\tFunc func_;\n> +};\n> +\n> template\n> class BoundMethodMember : public BoundMethodArgs\n> {\n> diff --git a/include/libcamera/base/signal.h b/include/libcamera/base/signal.h\n> index c2521769a843..8d9f82f62d0d 100644\n> --- a/include/libcamera/base/signal.h\n> +++ b/include/libcamera/base/signal.h\n> @@ -61,6 +61,25 @@ public:\n> \t\tSignalBase::connect(new BoundMethodMember(obj, nullptr, func));\n> \t}\n> \n> +#ifndef __DOXYGEN__\n> +\ttemplate +\t\t typename std::enable_if_t::value> * = nullptr>\n> +\tvoid connect(T *obj, Func func, ConnectionType type = ConnectionTypeAuto)\n> +\t{\n> +\t\tObject *object = static_cast(obj);\n> +\t\tSignalBase::connect(new BoundMethodFunctor(obj, object, func, type));\n> +\t}\n> +\n> +\ttemplate +\t\t typename std::enable_if_t::value> * = nullptr>\n> +#else\n> +\ttemplate\n> +#endif\n> +\tvoid connect(T *obj, Func func)\n> +\t{\n> +\t\tSignalBase::connect(new BoundMethodFunctor(obj, nullptr, func));\n> +\t}\n> +\n> \ttemplate\n> \tvoid connect(R (*func)(Args...))\n> \t{\n> diff --git a/src/libcamera/base/signal.cpp b/src/libcamera/base/signal.cpp\n> index adcfa796870e..9c2319c59106 100644\n> --- a/src/libcamera/base/signal.cpp\n> +++ b/src/libcamera/base/signal.cpp\n> @@ -121,6 +121,30 @@ SignalBase::SlotList SignalBase::slots()\n> * \\context This function is \\threadsafe.\n> */\n> \n> +/**\n> + * \\fn Signal::connect(T *object, Func func)\n> + * \\brief Connect the signal to a function object slot\n> + * \\param[in] object The slot object pointer\n> + * \\param[in] func The function object\n> + *\n> + * If the typename T inherits from Object, the signal will be automatically\n> + * disconnected from the \\a func slot of \\a object when \\a object is destroyed.\n> + * Otherwise the caller shall disconnect signals manually before destroying \\a\n> + * object.\n\n\nSo, if I have a instance _not_ inherited from Object, how would I \nmanually disconnect the signal ?\n\n\n> + *\n> + * The function object is typically a lambda function, but may be any object\n> + * that satisfies the FunctionObject named requirements. The types of the\n> + * function object arguments shall match the types of the signal arguments.\n> + *\n> + * No matching disconnect() function exist, as it wouldn't be possible to pass\n> + * to a disconnect() function the same lambda that was passed to connect(). The\n> + * connection created by this function can not be removed selectively if the\n> + * signal is connected to multiple slots of the same receiver, but may be\n> + * otherwise be removed using the disconnect(T *object) function.\n\nI am not sure if I am understanding this correctly, but do you mean that \nthe functor version of Signal, should essentially be used within a class \ninherited from Object /only/?\n\nI ask this because, i the previous paragraph there is:\n\n\t+ * Otherwise the caller shall disconnect signals manually before destroying \\a\n\t+ * object.\n\nThe 'Otherwise' seems to address the classes, that are not inherit from \nObject. It's stated that signal within those needs manual disconnection. \nBut 'How?' isn't clear to me.\n\nThe documentation doesn't seem to clarify  to me that how a class _not_ \ninheriting from Object handle disconnection of the signal? Can you \nplease clarify?\n\n> + *\n> + * \\context This function is \\threadsafe.\n> + */\n> +\n> /**\n> * \\fn Signal::connect(R (*func)(Args...))\n> * \\brief Connect the signal to a static function slot\n> diff --git a/test/signal.cpp b/test/signal.cpp\n> index 595782a2cd6e..fcf2def18df4 100644\n> --- a/test/signal.cpp\n> +++ b/test/signal.cpp\n> @@ -191,6 +191,24 @@ protected:\n> \t\tsignalVoid_.connect(slotStaticReturn);\n> \t\tsignalVoid_.connect(this, &SignalTest::slotReturn);\n> \n> +\t\t/* Test signal connection to a lambda. */\n> +\t\tint value = 0;\n> +\t\tsignalInt_.connect(this, [&](int v) { value = v; });\n> +\t\tsignalInt_.emit(42);\n> +\n> +\t\tif (value != 42) {\n> +\t\t\tcout << \"Signal connection to lambda failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tsignalInt_.disconnect(this);\n> +\t\tsignalInt_.emit(0);\n> +\n> +\t\tif (value != 42) {\n> +\t\t\tcout << \"Signal disconnection from lambda failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> \t\t/* ----------------- Signal -> Object tests ----------------- */\n> \n> \t\t/*\n> @@ -256,6 +274,27 @@ protected:\n> \n> \t\tdelete slotObject;\n> \n> +\t\t/* Test signal connection to a lambda. */\n> +\t\tslotObject = new SlotObject();\n> +\t\tvalue = 0;\n> +\t\tsignalInt_.connect(slotObject, [&](int v) { value = v; });\n> +\t\tsignalInt_.emit(42);\n> +\n> +\t\tif (value != 42) {\n> +\t\t\tcout << \"Signal connection to Object lambda failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tsignalInt_.disconnect(slotObject);\n> +\t\tsignalInt_.emit(0);\n> +\n> +\t\tif (value != 42) {\n> +\t\t\tcout << \"Signal disconnection from Object lambda failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tdelete slotObject;\n> +\n> \t\t/* --------- Signal -> Object (multiple inheritance) -------- */\n> \n> \t\t/*","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EB5D8BDC71\n\tfor ;\n\tTue, 31 Aug 2021 06:02:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 453B56916A;\n\tTue, 31 Aug 2021 08:02:47 +0200 (CEST)","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 B52C260137\n\tfor ;\n\tTue, 31 Aug 2021 08:02:45 +0200 (CEST)","from [192.168.1.104] (unknown [103.238.109.29])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D0170323;\n\tTue, 31 Aug 2021 08:02:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Tkr5gGeh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630389765;\n\tbh=iH2lgTx2ZeBngAUiBrMZcIovNxmswDSmrGexR7/VzwA=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=Tkr5gGehL2xb3Cyn4WNPE75M+KoVkjQkHrsMBuJHECLUeCJ9MDG1ZEoU9bQiUkvTw\n\t23tr+RErp17ATBk5xkAz9H2dICu43Pd5Ik+sb7QovkiJF7sNnCo6/e5wCWjNZuWWUm\n\t5bXZAAleZhVO9vDnQZfCebEUVlWBtKDOJkQZV/SY=","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210827023829.5871-1-laurent.pinchart@ideasonboard.com>\n\t<20210827023829.5871-5-laurent.pinchart@ideasonboard.com>","From":"Umang Jain ","Message-ID":"<40c21b56-6eba-c01b-67b7-2ce24d203fc5@ideasonboard.com>","Date":"Tue, 31 Aug 2021 11:32:41 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20210827023829.5871-5-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v1 4/6] libcamera: base: signal:\n\tSupport connecting signals to functors","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19208,"web_url":"https://patchwork.libcamera.org/comment/19208/","msgid":"","date":"2021-08-31T09:32:13","subject":"Re: [libcamera-devel] [PATCH v1 4/6] libcamera: base: signal:\n\tSupport connecting signals to functors","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, Aug 31, 2021 at 11:32:41AM +0530, Umang Jain wrote:\n> Hi Laurent,\n> \n> Thanks for the patch. I have a few questions regarding the documentation\n> \n> On 8/27/21 8:08 AM, Laurent Pinchart wrote:\n> > It can be useful to connect a signal to a functor, and in particular a\n> > lambda function, while still operating in the context of a receiver\n> > object (to support both object-based disconnection and queued\n> > connections to Object instances).\n> >\n> > Add a BoundMethodFunctor class to bind a functor, and a corresponding\n> > Signal::connect() function. There is no corresponding disconnect()\n> > function, as a lambda passed to connect() can't be later passed to\n> > disconnect(). Disconnection typically uses disconnect(T *object), which\n> > will cover the vast majority of use cases.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Documentation/Doxyfile.in | 1 +\n> > include/libcamera/base/bound_method.h | 31 +++++++++++++++++++++\n> > include/libcamera/base/signal.h | 19 +++++++++++++\n> > src/libcamera/base/signal.cpp | 24 +++++++++++++++++\n> > test/signal.cpp | 39 +++++++++++++++++++++++++++\n> > 5 files changed, 114 insertions(+)\n> >\n> > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> > index dc03cbea4b02..d562510e902e 100644\n> > --- a/Documentation/Doxyfile.in\n> > +++ b/Documentation/Doxyfile.in\n> > @@ -878,6 +878,7 @@ EXCLUDE_PATTERNS = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \\\n> > \n> > EXCLUDE_SYMBOLS = libcamera::BoundMethodArgs \\\n> > libcamera::BoundMethodBase \\\n> > + libcamera::BoundMethodFunctor \\\n> > libcamera::BoundMethodMember \\\n> > libcamera::BoundMethodPack \\\n> > libcamera::BoundMethodPackBase \\\n> > diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h\n> > index 76ce8017e721..ebd297ab8209 100644\n> > --- a/include/libcamera/base/bound_method.h\n> > +++ b/include/libcamera/base/bound_method.h\n> > @@ -128,6 +128,37 @@ public:\n> > \tvirtual R invoke(Args... args) = 0;\n> > };\n> > \n> > +template\n> > +class BoundMethodFunctor : public BoundMethodArgs\n> > +{\n> > +public:\n> > +\tusing PackType = typename BoundMethodArgs::PackType;\n> > +\n> > +\tBoundMethodFunctor(T *obj, Object *object, Func func,\n> > +\t\t\t ConnectionType type = ConnectionTypeAuto)\n> > +\t\t: BoundMethodArgs(obj, object, type), func_(func)\n> > +\t{\n> > +\t}\n> > +\n> > +\tR activate(Args... args, bool deleteMethod = false) override\n> > +\t{\n> > +\t\tif (!this->object_)\n> > +\t\t\treturn func_(args...);\n> > +\n> > +\t\tauto pack = std::make_shared(args...);\n> > +\t\tbool sync = BoundMethodBase::activatePack(pack, deleteMethod);\n> > +\t\treturn sync ? pack->returnValue() : R();\n> > +\t}\n> > +\n> > +\tR invoke(Args... args) override\n> > +\t{\n> > +\t\treturn func_(args...);\n> > +\t}\n> > +\n> > +private:\n> > +\tFunc func_;\n> > +};\n> > +\n> > template\n> > class BoundMethodMember : public BoundMethodArgs\n> > {\n> > diff --git a/include/libcamera/base/signal.h b/include/libcamera/base/signal.h\n> > index c2521769a843..8d9f82f62d0d 100644\n> > --- a/include/libcamera/base/signal.h\n> > +++ b/include/libcamera/base/signal.h\n> > @@ -61,6 +61,25 @@ public:\n> > \t\tSignalBase::connect(new BoundMethodMember(obj, nullptr, func));\n> > \t}\n> > \n> > +#ifndef __DOXYGEN__\n> > +\ttemplate > +\t\t typename std::enable_if_t::value> * = nullptr>\n> > +\tvoid connect(T *obj, Func func, ConnectionType type = ConnectionTypeAuto)\n> > +\t{\n> > +\t\tObject *object = static_cast(obj);\n> > +\t\tSignalBase::connect(new BoundMethodFunctor(obj, object, func, type));\n> > +\t}\n> > +\n> > +\ttemplate > +\t\t typename std::enable_if_t::value> * = nullptr>\n> > +#else\n> > +\ttemplate\n> > +#endif\n> > +\tvoid connect(T *obj, Func func)\n> > +\t{\n> > +\t\tSignalBase::connect(new BoundMethodFunctor(obj, nullptr, func));\n> > +\t}\n> > +\n> > \ttemplate\n> > \tvoid connect(R (*func)(Args...))\n> > \t{\n> > diff --git a/src/libcamera/base/signal.cpp b/src/libcamera/base/signal.cpp\n> > index adcfa796870e..9c2319c59106 100644\n> > --- a/src/libcamera/base/signal.cpp\n> > +++ b/src/libcamera/base/signal.cpp\n> > @@ -121,6 +121,30 @@ SignalBase::SlotList SignalBase::slots()\n> > * \\context This function is \\threadsafe.\n> > */\n> > \n> > +/**\n> > + * \\fn Signal::connect(T *object, Func func)\n> > + * \\brief Connect the signal to a function object slot\n> > + * \\param[in] object The slot object pointer\n> > + * \\param[in] func The function object\n> > + *\n> > + * If the typename T inherits from Object, the signal will be automatically\n> > + * disconnected from the \\a func slot of \\a object when \\a object is destroyed.\n> > + * Otherwise the caller shall disconnect signals manually before destroying \\a\n> > + * object.\n> \n> So, if I have a instance _not_ inherited from Object, how would I \n> manually disconnect the signal ?\n\nWith one of the disconnect() functions of the signal.\n\n> > + *\n> > + * The function object is typically a lambda function, but may be any object\n> > + * that satisfies the FunctionObject named requirements. The types of the\n> > + * function object arguments shall match the types of the signal arguments.\n> > + *\n> > + * No matching disconnect() function exist, as it wouldn't be possible to pass\n> > + * to a disconnect() function the same lambda that was passed to connect(). The\n> > + * connection created by this function can not be removed selectively if the\n> > + * signal is connected to multiple slots of the same receiver, but may be\n> > + * otherwise be removed using the disconnect(T *object) function.\n> \n> I am not sure if I am understanding this correctly, but do you mean that \n> the functor version of Signal, should essentially be used within a class \n> inherited from Object /only/?\n\nNo, what it means is that\n\n\tSignal::connect(T *object, R T::*func)(Args...))\n\nhas a matching\n\n\tSignal::disconnect(T *object, R (T::*func)(Args...))\n\nfunction, but\n\n\tSignal::connect(T *object, Func func)\n\ndoesn't have\n\n\tSignal::disconnect(T *object, Func func)\n\nso the\n\n\tSignal::disconnect(T *object)\n\nfunction should be used instead.\n\n> I ask this because, i the previous paragraph there is:\n> \n> \t+ * Otherwise the caller shall disconnect signals manually before destroying \\a\n> \t+ * object.\n> \n> The 'Otherwise' seems to address the classes, that are not inherit from \n> Object. It's stated that signal within those needs manual disconnection. \n> But 'How?' isn't clear to me.\n> \n> The documentation doesn't seem to clarify  to me that how a class _not_ \n> inheriting from Object handle disconnection of the signal? Can you \n> please clarify?\n> \n> > + *\n> > + * \\context This function is \\threadsafe.\n> > + */\n> > +\n> > /**\n> > * \\fn Signal::connect(R (*func)(Args...))\n> > * \\brief Connect the signal to a static function slot\n> > diff --git a/test/signal.cpp b/test/signal.cpp\n> > index 595782a2cd6e..fcf2def18df4 100644\n> > --- a/test/signal.cpp\n> > +++ b/test/signal.cpp\n> > @@ -191,6 +191,24 @@ protected:\n> > \t\tsignalVoid_.connect(slotStaticReturn);\n> > \t\tsignalVoid_.connect(this, &SignalTest::slotReturn);\n> > \n> > +\t\t/* Test signal connection to a lambda. */\n> > +\t\tint value = 0;\n> > +\t\tsignalInt_.connect(this, [&](int v) { value = v; });\n> > +\t\tsignalInt_.emit(42);\n> > +\n> > +\t\tif (value != 42) {\n> > +\t\t\tcout << \"Signal connection to lambda failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tsignalInt_.disconnect(this);\n> > +\t\tsignalInt_.emit(0);\n> > +\n> > +\t\tif (value != 42) {\n> > +\t\t\tcout << \"Signal disconnection from lambda failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > \t\t/* ----------------- Signal -> Object tests ----------------- */\n> > \n> > \t\t/*\n> > @@ -256,6 +274,27 @@ protected:\n> > \n> > \t\tdelete slotObject;\n> > \n> > +\t\t/* Test signal connection to a lambda. */\n> > +\t\tslotObject = new SlotObject();\n> > +\t\tvalue = 0;\n> > +\t\tsignalInt_.connect(slotObject, [&](int v) { value = v; });\n> > +\t\tsignalInt_.emit(42);\n> > +\n> > +\t\tif (value != 42) {\n> > +\t\t\tcout << \"Signal connection to Object lambda failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tsignalInt_.disconnect(slotObject);\n> > +\t\tsignalInt_.emit(0);\n> > +\n> > +\t\tif (value != 42) {\n> > +\t\t\tcout << \"Signal disconnection from Object lambda failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tdelete slotObject;\n> > +\n> > \t\t/* --------- Signal -> Object (multiple inheritance) -------- */\n> > \n> > \t\t/*","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CCC63BD87D\n\tfor ;\n\tTue, 31 Aug 2021 09:32:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1C98C6916A;\n\tTue, 31 Aug 2021 11:32:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EEA8B60288\n\tfor ;\n\tTue, 31 Aug 2021 11:32:27 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6E4733D7;\n\tTue, 31 Aug 2021 11:32:27 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"sgmgxnJ0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630402347;\n\tbh=vXqZ3B/4NX+Hk/b/YBz6KnD/LseLA9OMnjmROn5w+Ws=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sgmgxnJ0eOrmag+/uumj+D5P/Ne8C/tpbE9blslMqX4NSvTO7FFDsHGPg6+zrLFwk\n\t0BOrKU8PILgapAtE5pY7z+lLXgkh6cwKY/VyiTyXWeKMvoajXUKwha2SwS+MV7L/uf\n\tsXUsIfLLkyN3J1oI2anWMzm/1HAmfl/LH+AtaQH0=","Date":"Tue, 31 Aug 2021 12:32:13 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20210827023829.5871-1-laurent.pinchart@ideasonboard.com>\n\t<20210827023829.5871-5-laurent.pinchart@ideasonboard.com>\n\t<40c21b56-6eba-c01b-67b7-2ce24d203fc5@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<40c21b56-6eba-c01b-67b7-2ce24d203fc5@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 4/6] libcamera: base: signal:\n\tSupport connecting signals to functors","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19216,"web_url":"https://patchwork.libcamera.org/comment/19216/","msgid":"<2285b735-f082-95ca-9b6d-6fb3a265394c@ideasonboard.com>","date":"2021-08-31T10:41:52","subject":"Re: [libcamera-devel] [PATCH v1 4/6] libcamera: base: signal:\n\tSupport connecting signals to functors","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 8/31/21 3:02 PM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> On Tue, Aug 31, 2021 at 11:32:41AM +0530, Umang Jain wrote:\n>> Hi Laurent,\n>>\n>> Thanks for the patch. I have a few questions regarding the documentation\n>>\n>> On 8/27/21 8:08 AM, Laurent Pinchart wrote:\n>>> It can be useful to connect a signal to a functor, and in particular a\n>>> lambda function, while still operating in the context of a receiver\n>>> object (to support both object-based disconnection and queued\n>>> connections to Object instances).\n>>>\n>>> Add a BoundMethodFunctor class to bind a functor, and a corresponding\n>>> Signal::connect() function. There is no corresponding disconnect()\n>>> function, as a lambda passed to connect() can't be later passed to\n>>> disconnect(). Disconnection typically uses disconnect(T *object), which\n>>> will cover the vast majority of use cases.\n>>>\n>>> Signed-off-by: Laurent Pinchart \n>>> ---\n>>> Documentation/Doxyfile.in | 1 +\n>>> include/libcamera/base/bound_method.h | 31 +++++++++++++++++++++\n>>> include/libcamera/base/signal.h | 19 +++++++++++++\n>>> src/libcamera/base/signal.cpp | 24 +++++++++++++++++\n>>> test/signal.cpp | 39 +++++++++++++++++++++++++++\n>>> 5 files changed, 114 insertions(+)\n>>>\n>>> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n>>> index dc03cbea4b02..d562510e902e 100644\n>>> --- a/Documentation/Doxyfile.in\n>>> +++ b/Documentation/Doxyfile.in\n>>> @@ -878,6 +878,7 @@ EXCLUDE_PATTERNS = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \\\n>>> \n>>> EXCLUDE_SYMBOLS = libcamera::BoundMethodArgs \\\n>>> libcamera::BoundMethodBase \\\n>>> + libcamera::BoundMethodFunctor \\\n>>> libcamera::BoundMethodMember \\\n>>> libcamera::BoundMethodPack \\\n>>> libcamera::BoundMethodPackBase \\\n>>> diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h\n>>> index 76ce8017e721..ebd297ab8209 100644\n>>> --- a/include/libcamera/base/bound_method.h\n>>> +++ b/include/libcamera/base/bound_method.h\n>>> @@ -128,6 +128,37 @@ public:\n>>> \tvirtual R invoke(Args... args) = 0;\n>>> };\n>>> \n>>> +template\n>>> +class BoundMethodFunctor : public BoundMethodArgs\n>>> +{\n>>> +public:\n>>> +\tusing PackType = typename BoundMethodArgs::PackType;\n>>> +\n>>> +\tBoundMethodFunctor(T *obj, Object *object, Func func,\n>>> +\t\t\t ConnectionType type = ConnectionTypeAuto)\n>>> +\t\t: BoundMethodArgs(obj, object, type), func_(func)\n>>> +\t{\n>>> +\t}\n>>> +\n>>> +\tR activate(Args... args, bool deleteMethod = false) override\n>>> +\t{\n>>> +\t\tif (!this->object_)\n>>> +\t\t\treturn func_(args...);\n>>> +\n>>> +\t\tauto pack = std::make_shared(args...);\n>>> +\t\tbool sync = BoundMethodBase::activatePack(pack, deleteMethod);\n>>> +\t\treturn sync ? pack->returnValue() : R();\n>>> +\t}\n>>> +\n>>> +\tR invoke(Args... args) override\n>>> +\t{\n>>> +\t\treturn func_(args...);\n>>> +\t}\n>>> +\n>>> +private:\n>>> +\tFunc func_;\n>>> +};\n>>> +\n>>> template\n>>> class BoundMethodMember : public BoundMethodArgs\n>>> {\n>>> diff --git a/include/libcamera/base/signal.h b/include/libcamera/base/signal.h\n>>> index c2521769a843..8d9f82f62d0d 100644\n>>> --- a/include/libcamera/base/signal.h\n>>> +++ b/include/libcamera/base/signal.h\n>>> @@ -61,6 +61,25 @@ public:\n>>> \t\tSignalBase::connect(new BoundMethodMember(obj, nullptr, func));\n>>> \t}\n>>> \n>>> +#ifndef __DOXYGEN__\n>>> +\ttemplate>> +\t\t typename std::enable_if_t::value> * = nullptr>\n>>> +\tvoid connect(T *obj, Func func, ConnectionType type = ConnectionTypeAuto)\n>>> +\t{\n>>> +\t\tObject *object = static_cast(obj);\n>>> +\t\tSignalBase::connect(new BoundMethodFunctor(obj, object, func, type));\n>>> +\t}\n>>> +\n>>> +\ttemplate>> +\t\t typename std::enable_if_t::value> * = nullptr>\n>>> +#else\n>>> +\ttemplate\n>>> +#endif\n>>> +\tvoid connect(T *obj, Func func)\n>>> +\t{\n>>> +\t\tSignalBase::connect(new BoundMethodFunctor(obj, nullptr, func));\n>>> +\t}\n>>> +\n>>> \ttemplate\n>>> \tvoid connect(R (*func)(Args...))\n>>> \t{\n>>> diff --git a/src/libcamera/base/signal.cpp b/src/libcamera/base/signal.cpp\n>>> index adcfa796870e..9c2319c59106 100644\n>>> --- a/src/libcamera/base/signal.cpp\n>>> +++ b/src/libcamera/base/signal.cpp\n>>> @@ -121,6 +121,30 @@ SignalBase::SlotList SignalBase::slots()\n>>> * \\context This function is \\threadsafe.\n>>> */\n>>> \n>>> +/**\n>>> + * \\fn Signal::connect(T *object, Func func)\n>>> + * \\brief Connect the signal to a function object slot\n>>> + * \\param[in] object The slot object pointer\n>>> + * \\param[in] func The function object\n>>> + *\n>>> + * If the typename T inherits from Object, the signal will be automatically\n>>> + * disconnected from the \\a func slot of \\a object when \\a object is destroyed.\n>>> + * Otherwise the caller shall disconnect signals manually before destroying \\a\n>>> + * object.\n>> So, if I have a instance _not_ inherited from Object, how would I\n>> manually disconnect the signal ?\n> With one of the disconnect() functions of the signal.\n>\n>>> + *\n>>> + * The function object is typically a lambda function, but may be any object\n>>> + * that satisfies the FunctionObject named requirements. The types of the\n>>> + * function object arguments shall match the types of the signal arguments.\n>>> + *\n>>> + * No matching disconnect() function exist, as it wouldn't be possible to pass\n>>> + * to a disconnect() function the same lambda that was passed to connect(). The\n>>> + * connection created by this function can not be removed selectively if the\n>>> + * signal is connected to multiple slots of the same receiver, but may be\n>>> + * otherwise be removed using the disconnect(T *object) function.\n>> I am not sure if I am understanding this correctly, but do you mean that\n>> the functor version of Signal, should essentially be used within a class\n>> inherited from Object /only/?\n> No, what it means is that\n>\n> \tSignal::connect(T *object, R T::*func)(Args...))\n>\n> has a matching\n>\n> \tSignal::disconnect(T *object, R (T::*func)(Args...))\n\n\nAh yeah, duh me!\n\n>\n> function, but\n>\n> \tSignal::connect(T *object, Func func)\n>\n> doesn't have\n>\n> \tSignal::disconnect(T *object, Func func)\n>\n> so the\n>\n> \tSignal::disconnect(T *object)\n>\n> function should be used instead.\n\nGot it.\n\nReviewed-by: Umang Jain \n\nthanks for clarifications :D\n\n>\n>> I ask this because, i the previous paragraph there is:\n>>\n>> \t+ * Otherwise the caller shall disconnect signals manually before destroying \\a\n>> \t+ * object.\n>>\n>> The 'Otherwise' seems to address the classes, that are not inherit from\n>> Object. It's stated that signal within those needs manual disconnection.\n>> But 'How?' isn't clear to me.\n>>\n>> The documentation doesn't seem to clarify  to me that how a class _not_\n>> inheriting from Object handle disconnection of the signal? Can you\n>> please clarify?\n>>\n>>> + *\n>>> + * \\context This function is \\threadsafe.\n>>> + */\n>>> +\n>>> /**\n>>> * \\fn Signal::connect(R (*func)(Args...))\n>>> * \\brief Connect the signal to a static function slot\n>>> diff --git a/test/signal.cpp b/test/signal.cpp\n>>> index 595782a2cd6e..fcf2def18df4 100644\n>>> --- a/test/signal.cpp\n>>> +++ b/test/signal.cpp\n>>> @@ -191,6 +191,24 @@ protected:\n>>> \t\tsignalVoid_.connect(slotStaticReturn);\n>>> \t\tsignalVoid_.connect(this, &SignalTest::slotReturn);\n>>> \n>>> +\t\t/* Test signal connection to a lambda. */\n>>> +\t\tint value = 0;\n>>> +\t\tsignalInt_.connect(this, [&](int v) { value = v; });\n>>> +\t\tsignalInt_.emit(42);\n>>> +\n>>> +\t\tif (value != 42) {\n>>> +\t\t\tcout << \"Signal connection to lambda failed\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\tsignalInt_.disconnect(this);\n>>> +\t\tsignalInt_.emit(0);\n>>> +\n>>> +\t\tif (value != 42) {\n>>> +\t\t\tcout << \"Signal disconnection from lambda failed\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> \t\t/* ----------------- Signal -> Object tests ----------------- */\n>>> \n>>> \t\t/*\n>>> @@ -256,6 +274,27 @@ protected:\n>>> \n>>> \t\tdelete slotObject;\n>>> \n>>> +\t\t/* Test signal connection to a lambda. */\n>>> +\t\tslotObject = new SlotObject();\n>>> +\t\tvalue = 0;\n>>> +\t\tsignalInt_.connect(slotObject, [&](int v) { value = v; });\n>>> +\t\tsignalInt_.emit(42);\n>>> +\n>>> +\t\tif (value != 42) {\n>>> +\t\t\tcout << \"Signal connection to Object lambda failed\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\tsignalInt_.disconnect(slotObject);\n>>> +\t\tsignalInt_.emit(0);\n>>> +\n>>> +\t\tif (value != 42) {\n>>> +\t\t\tcout << \"Signal disconnection from Object lambda failed\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\tdelete slotObject;\n>>> +\n>>> \t\t/* --------- Signal -> Object (multiple inheritance) -------- */\n>>> \n>>> \t\t/*","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D9DE5BDC71\n\tfor ;\n\tTue, 31 Aug 2021 10:42:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 37FB46916C;\n\tTue, 31 Aug 2021 12:42:00 +0200 (CEST)","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 4604768890\n\tfor ;\n\tTue, 31 Aug 2021 12:41:58 +0200 (CEST)","from [192.168.1.105] (unknown [103.251.226.196])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4049F3D7;\n\tTue, 31 Aug 2021 12:41:57 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"m6UQdp1X\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630406517;\n\tbh=WBWjo60vo6Xytfdfd+aep99KjotBckujPE5HMoleRSk=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=m6UQdp1XQvjyAdYaNLBvf+oi2Xlx1xyF/cIotYZha2zSNOcxZxLAw6gpEt7QcoDUf\n\t46PgXXiUj3FD98Prs52T7ociYS1IsDoshobo0bdjjdmAMU5myyIguni6GGlGn4ZjjK\n\ty4EF8/JmVxjmQ5QJ1iB7kaVkTN/RE2+xHMcg21mE=","To":"Laurent Pinchart ","References":"<20210827023829.5871-1-laurent.pinchart@ideasonboard.com>\n\t<20210827023829.5871-5-laurent.pinchart@ideasonboard.com>\n\t<40c21b56-6eba-c01b-67b7-2ce24d203fc5@ideasonboard.com>\n\t","From":"Umang Jain ","Message-ID":"<2285b735-f082-95ca-9b6d-6fb3a265394c@ideasonboard.com>","Date":"Tue, 31 Aug 2021 16:11:52 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v1 4/6] libcamera: base: signal:\n\tSupport connecting signals to functors","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]