[libcamera-devel] [PATCH 13/18] test: Add EventNotifier thread move test

[Jacopo Mondi]

Hi Laurent,

On Thu, Aug 15, 2019 at 12:58:48PM +0300, Laurent Pinchart wrote:
> On Thu, Aug 15, 2019 at 12:56:02PM +0300, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > On Thu, Aug 15, 2019 at 11:50:15AM +0200, Jacopo Mondi wrote:
> > > On Mon, Aug 12, 2019 at 03:46:37PM +0300, Laurent Pinchart wrote:
> > > > The test verifies correct behaviour of an enabled event notifier moved
> > > > to a different thread.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > > ---
> > > >  src/libcamera/event_notifier.cpp |   2 +-
> > > >  test/event-thread.cpp            | 112 +++++++++++++++++++++++++++++++
> > > >  test/meson.build                 |   1 +
> > > >  3 files changed, 114 insertions(+), 1 deletion(-)
> > > >  create mode 100644 test/event-thread.cpp
> > > >
> > > > diff --git a/src/libcamera/event_notifier.cpp b/src/libcamera/event_notifier.cpp
> > > > index 515e6d1770a1..96be27601982 100644
> > > > --- a/src/libcamera/event_notifier.cpp
> > > > +++ b/src/libcamera/event_notifier.cpp
> > > > @@ -127,7 +127,7 @@ void EventNotifier::message(Message *msg)
> > > >  	if (msg->type() == Message::ThreadMoveMessage) {
> > > >  		if (enabled_) {
> > > >  			setEnabled(false);
> > > > -			invokeMethod(this, &EventNotifier::setEnabled, true);
> > > > +			invokeMethod(&EventNotifier::setEnabled, true);
> > >
> > > I've missed the reason for this change. Also, isnt't this the syntax
> > > to invoce static bound methods?
> >
> > This should be part of "libcamera: object: Add an asynchronous method
> > invocation method", I'll move it there. invokeMethod() used to be a
> > static method, hence the first argument. I've changed it to a member
> > method and this is a leftover.
>
> "libcamera: object: Add an asynchronous method invocation method" isn't
> the right patch. Looks like I've already reorganised this in my private
> branch anyway, so it will be moved to the right place in the next
> version.
>

I see. Please be aware that a similar hunk (but for timers) is present
in the following patch (14/18)

> > > >  		}
> > > >  	}
> > > >
> > > > diff --git a/test/event-thread.cpp b/test/event-thread.cpp
> > > > new file mode 100644
> > > > index 000000000000..4a82d49b94f1
> > > > --- /dev/null
> > > > +++ b/test/event-thread.cpp
> > > > @@ -0,0 +1,112 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > > +/*
> > > > + * Copyright (C) 2019, Google Inc.
> > > > + *
> > > > + * event-thread.cpp - Threaded event test
> > > > + */
> > > > +
> > > > +#include <chrono>
> > > > +#include <iostream>
> > > > +#include <string.h>
> > > > +#include <unistd.h>
> > > > +
> > > > +#include <libcamera/event_notifier.h>
> > > > +#include <libcamera/timer.h>
> > > > +
> > > > +#include "test.h"
> > > > +#include "thread.h"
> > > > +
> > > > +using namespace std;
> > > > +using namespace libcamera;
> > > > +
> > > > +class EventHandler : public Object
> > > > +{
> > > > +public:
> > > > +	EventHandler()
> > > > +	{
> > > > +		pipe(pipefd_);
> > > > +
> > > > +		notifier_ = new EventNotifier(pipefd_[0], EventNotifier::Read);
> > > > +		notifier_->activated.connect(this, &EventHandler::readReady);
> > > > +	}
> > > > +
> > > > +	~EventHandler()
> > > > +	{
> > > > +		delete notifier_;
> > > > +
> > > > +		close(pipefd_[0]);
> > > > +		close(pipefd_[1]);
> > > > +	}
> > > > +
> > > > +	int notify()
> > > > +	{
> > > > +		std::string data("H2G2");
> > > > +		ssize_t ret;
> > > > +
> > > > +		memset(data_, 0, sizeof(data_));
> > > > +		size_ = 0;
> > > > +
> > > > +		ret = write(pipefd_[1], data.data(), data.size());
> > > > +		if (ret < 0) {
> > > > +			cout << "Pipe write failed" << endl;
> > > > +			return TestFail;
> > > > +		}
> > > > +
> > > > +		return TestPass;
> > > > +	}
> > > > +
> > > > +	bool notified() const
> > > > +	{
> > > > +		return notified_;
> > > > +	}
> > > > +
> > > > +	void moveToThread(Thread *thread)
> > > > +	{
> > > > +		Object::moveToThread(thread);
> > > > +		notifier_->moveToThread(thread);
> > > > +	}
> > > > +
> > > > +private:
> > > > +	void readReady(EventNotifier *notifier)
> > > > +	{
> > > > +		size_ = read(notifier->fd(), data_, sizeof(data_));
> > > > +		notified_ = true;
> > > > +	}
> > > > +
> > > > +	EventNotifier *notifier_;
> > > > +
> > > > +	int pipefd_[2];
> > > > +
> > > > +	bool notified_;
> > > > +	char data_[16];
> > > > +	ssize_t size_;
> > > > +};
> > > > +
> > > > +class EventThreadTest : public Test
> > > > +{
> > > > +protected:
> > > > +	int run()
> > > > +	{
> > > > +		Thread thread;
> > > > +		thread.start();
> > > > +
> > > > +		EventHandler handler;
> > > > +		handler.notify();
> > > > +		handler.moveToThread(&thread);
> > > > +
> > > > +		this_thread::sleep_for(chrono::milliseconds(100));
> > > > +
> > > > +		/* Must stop thread before destroying the handler. */
> > > > +		thread.exit(0);
> > > > +		thread.wait();
> > > > +
> > > > +		if (!handler.notified()) {
> > > > +			cout << "Thread event handling test failed" << endl;
> > > > +			return TestFail;
> > > > +		}
> > > > +
> > > > +		return TestPass;
> > > > +	}
> > > > +};
> > > > +
> > > > +TEST_REGISTER(EventThreadTest)
> > > > diff --git a/test/meson.build b/test/meson.build
> > > > index c6601813db78..f695ffd7be44 100644
> > > > --- a/test/meson.build
> > > > +++ b/test/meson.build
> > > > @@ -23,6 +23,7 @@ public_tests = [
> > > >
> > > >  internal_tests = [
> > > >      ['camera-sensor',                   'camera-sensor.cpp'],
> > > > +    ['event-thread',                    'event-thread.cpp'],
> > > >      ['message',                         'message.cpp'],
> > > >      ['object',                          'object.cpp'],
> > > >      ['object-invoke',                   'object-invoke.cpp'],
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
>
> --
> Regards,
>
> Laurent Pinchart
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190815/7df1bb1c/attachment.sig>