[libcamera-devel] [PATCH v1 4/6] libcamera: base: signal: Support connecting signals to functors

Umang Jain umang.jain at ideasonboard.com
Tue Aug 31 08:02:41 CEST 2021


Hi Laurent,

Thanks for the patch. I have a few questions regarding the documentation

On 8/27/21 8:08 AM, Laurent Pinchart wrote:
> It can be useful to connect a signal to a functor, and in particular a
> lambda function, while still operating in the context of a receiver
> object (to support both object-based disconnection and queued
> connections to Object instances).
>
> Add a BoundMethodFunctor class to bind a functor, and a corresponding
> Signal::connect() function. There is no corresponding disconnect()
> function, as a lambda passed to connect() can't be later passed to
> disconnect(). Disconnection typically uses disconnect(T *object), which
> will cover the vast majority of use cases.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>   Documentation/Doxyfile.in             |  1 +
>   include/libcamera/base/bound_method.h | 31 +++++++++++++++++++++
>   include/libcamera/base/signal.h       | 19 +++++++++++++
>   src/libcamera/base/signal.cpp         | 24 +++++++++++++++++
>   test/signal.cpp                       | 39 +++++++++++++++++++++++++++
>   5 files changed, 114 insertions(+)
>
> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> index dc03cbea4b02..d562510e902e 100644
> --- a/Documentation/Doxyfile.in
> +++ b/Documentation/Doxyfile.in
> @@ -878,6 +878,7 @@ EXCLUDE_PATTERNS       = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \
>   
>   EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
>                            libcamera::BoundMethodBase \
> +                         libcamera::BoundMethodFunctor \
>                            libcamera::BoundMethodMember \
>                            libcamera::BoundMethodPack \
>                            libcamera::BoundMethodPackBase \
> diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h
> index 76ce8017e721..ebd297ab8209 100644
> --- a/include/libcamera/base/bound_method.h
> +++ b/include/libcamera/base/bound_method.h
> @@ -128,6 +128,37 @@ public:
>   	virtual R invoke(Args... args) = 0;
>   };
>   
> +template<typename T, typename R, typename Func, typename... Args>
> +class BoundMethodFunctor : public BoundMethodArgs<R, Args...>
> +{
> +public:
> +	using PackType = typename BoundMethodArgs<R, Args...>::PackType;
> +
> +	BoundMethodFunctor(T *obj, Object *object, Func func,
> +			   ConnectionType type = ConnectionTypeAuto)
> +		: BoundMethodArgs<R, Args...>(obj, object, type), func_(func)
> +	{
> +	}
> +
> +	R activate(Args... args, bool deleteMethod = false) override
> +	{
> +		if (!this->object_)
> +			return func_(args...);
> +
> +		auto pack = std::make_shared<PackType>(args...);
> +		bool sync = BoundMethodBase::activatePack(pack, deleteMethod);
> +		return sync ? pack->returnValue() : R();
> +	}
> +
> +	R invoke(Args... args) override
> +	{
> +		return func_(args...);
> +	}
> +
> +private:
> +	Func func_;
> +};
> +
>   template<typename T, typename R, typename... Args>
>   class BoundMethodMember : public BoundMethodArgs<R, Args...>
>   {
> diff --git a/include/libcamera/base/signal.h b/include/libcamera/base/signal.h
> index c2521769a843..8d9f82f62d0d 100644
> --- a/include/libcamera/base/signal.h
> +++ b/include/libcamera/base/signal.h
> @@ -61,6 +61,25 @@ public:
>   		SignalBase::connect(new BoundMethodMember<T, R, Args...>(obj, nullptr, func));
>   	}
>   
> +#ifndef __DOXYGEN__
> +	template<typename T, typename Func,
> +		 typename std::enable_if_t<std::is_base_of<Object, T>::value> * = nullptr>
> +	void connect(T *obj, Func func, ConnectionType type = ConnectionTypeAuto)
> +	{
> +		Object *object = static_cast<Object *>(obj);
> +		SignalBase::connect(new BoundMethodFunctor<T, void, Func, Args...>(obj, object, func, type));
> +	}
> +
> +	template<typename T, typename Func,
> +		 typename std::enable_if_t<!std::is_base_of<Object, T>::value> * = nullptr>
> +#else
> +	template<typename T, typename Func>
> +#endif
> +	void connect(T *obj, Func func)
> +	{
> +		SignalBase::connect(new BoundMethodFunctor<T, void, Func, Args...>(obj, nullptr, func));
> +	}
> +
>   	template<typename R>
>   	void connect(R (*func)(Args...))
>   	{
> diff --git a/src/libcamera/base/signal.cpp b/src/libcamera/base/signal.cpp
> index adcfa796870e..9c2319c59106 100644
> --- a/src/libcamera/base/signal.cpp
> +++ b/src/libcamera/base/signal.cpp
> @@ -121,6 +121,30 @@ SignalBase::SlotList SignalBase::slots()
>    * \context This function is \threadsafe.
>    */
>   
> +/**
> + * \fn Signal::connect(T *object, Func func)
> + * \brief Connect the signal to a function object slot
> + * \param[in] object The slot object pointer
> + * \param[in] func The function object
> + *
> + * If the typename T inherits from Object, the signal will be automatically
> + * disconnected from the \a func slot of \a object when \a object is destroyed.
> + * Otherwise the caller shall disconnect signals manually before destroying \a
> + * object.


So, if I have a instance _not_ inherited from Object, how would I 
manually disconnect the signal ?


> + *
> + * The function object is typically a lambda function, but may be any object
> + * that satisfies the FunctionObject named requirements. The types of the
> + * function object arguments shall match the types of the signal arguments.
> + *
> + * No matching disconnect() function exist, as it wouldn't be possible to pass
> + * to a disconnect() function the same lambda that was passed to connect(). The
> + * connection created by this function can not be removed selectively if the
> + * signal is connected to multiple slots of the same receiver, but may be
> + * otherwise be removed using the disconnect(T *object) function.

I am not sure if I am understanding this correctly, but do you mean that 
the functor version of Signal, should essentially be used within a class 
inherited from Object /only/?

I ask this because, i the previous paragraph there is:

	+ * Otherwise the caller shall disconnect signals manually before destroying \a
	+ * object.

The 'Otherwise' seems to address the classes, that are not inherit from 
Object. It's stated that signal within those needs manual disconnection. 
But 'How?' isn't clear to me.

The documentation doesn't seem to clarify  to me that how a class _not_ 
inheriting from Object handle disconnection of the signal? Can you 
please clarify?

> + *
> + * \context This function is \threadsafe.
> + */
> +
>   /**
>    * \fn Signal::connect(R (*func)(Args...))
>    * \brief Connect the signal to a static function slot
> diff --git a/test/signal.cpp b/test/signal.cpp
> index 595782a2cd6e..fcf2def18df4 100644
> --- a/test/signal.cpp
> +++ b/test/signal.cpp
> @@ -191,6 +191,24 @@ protected:
>   		signalVoid_.connect(slotStaticReturn);
>   		signalVoid_.connect(this, &SignalTest::slotReturn);
>   
> +		/* Test signal connection to a lambda. */
> +		int value = 0;
> +		signalInt_.connect(this, [&](int v) { value = v; });
> +		signalInt_.emit(42);
> +
> +		if (value != 42) {
> +			cout << "Signal connection to lambda failed" << endl;
> +			return TestFail;
> +		}
> +
> +		signalInt_.disconnect(this);
> +		signalInt_.emit(0);
> +
> +		if (value != 42) {
> +			cout << "Signal disconnection from lambda failed" << endl;
> +			return TestFail;
> +		}
> +
>   		/* ----------------- Signal -> Object tests ----------------- */
>   
>   		/*
> @@ -256,6 +274,27 @@ protected:
>   
>   		delete slotObject;
>   
> +		/* Test signal connection to a lambda. */
> +		slotObject = new SlotObject();
> +		value = 0;
> +		signalInt_.connect(slotObject, [&](int v) { value = v; });
> +		signalInt_.emit(42);
> +
> +		if (value != 42) {
> +			cout << "Signal connection to Object lambda failed" << endl;
> +			return TestFail;
> +		}
> +
> +		signalInt_.disconnect(slotObject);
> +		signalInt_.emit(0);
> +
> +		if (value != 42) {
> +			cout << "Signal disconnection from Object lambda failed" << endl;
> +			return TestFail;
> +		}
> +
> +		delete slotObject;
> +
>   		/* --------- Signal -> Object (multiple inheritance) -------- */
>   
>   		/*


More information about the libcamera-devel mailing list