[libcamera-devel,v3,5/5] test: ipa: Add test for the IPA Interface

2121 diff mbox series

Implementing a basic test for IPA Interface using the VIMC dummy IPA
module. The test implements an out-of-band feedback channel between the
test and the dummy IPA module to verify the success of the IPA
interactions.

Test the only available operation defined by the IPA interface by
receiving a confirmation code on the fifo communication channel.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 test/ipa/ipa_interface_test.cpp | 138 ++++++++++++++++++++++++++++++++
 test/ipa/meson.build            |   3 +-
 2 files changed, 140 insertions(+), 1 deletion(-)
 create mode 100644 test/ipa/ipa_interface_test.cpp

Hi Jacopo,

Thank you for the patch.

On Sun, Oct 06, 2019 at 10:08:52PM +0200, Jacopo Mondi wrote:
> Implementing a basic test for IPA Interface using the VIMC dummy IPA

s/Implementing/Implement/

> module. The test implements an out-of-band feedback channel between the
> test and the dummy IPA module to verify the success of the IPA
> interactions.
> 
> Test the only available operation defined by the IPA interface by
> receiving a confirmation code on the fifo communication channel.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  test/ipa/ipa_interface_test.cpp | 138 ++++++++++++++++++++++++++++++++
>  test/ipa/meson.build            |   3 +-
>  2 files changed, 140 insertions(+), 1 deletion(-)
>  create mode 100644 test/ipa/ipa_interface_test.cpp
> 
> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
> new file mode 100644
> index 000000000000..71bdcc651392
> --- /dev/null
> +++ b/test/ipa/ipa_interface_test.cpp
> @@ -0,0 +1,138 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipa_interface_test.cpp - Test the IPA interface
> + */
> +
> +#include <fcntl.h>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include <iostream>
> +
> +#include <libcamera/event_dispatcher.h>
> +#include <libcamera/event_notifier.h>
> +#include <libcamera/timer.h>

I'd add a blank line here, or move it before the libcamera headers to
sort them alphabetically (blank line is my preference).

> +#include <ipa/ipa_vimc.h>
> +
> +#include "device_enumerator.h"
> +#include "ipa_manager.h"
> +#include "ipa_module.h"
> +#include "pipeline_handler.h"
> +#include "test.h"
> +#include "thread.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +
> +class IPAInterfaceTest : public Test, public Object
> +{
> +public:
> +	IPAInterfaceTest()
> +		: trace_(IPAOperationNone), notifier_(nullptr), fd_(-1)
> +	{
> +	}
> +
> +	~IPAInterfaceTest()
> +	{
> +		delete notifier_;
> +	}
> +
> +protected:
> +	int init() override
> +	{
> +		/* Create a pipeline handler for vimc. */
> +		std::vector<PipelineHandlerFactory *> &factories =
> +			PipelineHandlerFactory::factories();
> +		for (PipelineHandlerFactory *factory : factories) {
> +			if (factory->name() == "PipelineHandlerVimc") {
> +				pipe_ = factory->create(nullptr);
> +				break;
> +			}
> +		}
> +
> +		if (!pipe_) {
> +			cerr << "Vimc pipeline not found" << endl;
> +			return TestPass;
> +		}
> +
> +		/* Create and open the communication FIFO. */
> +		int ret = mkfifo(vimcFifoPath, S_IRUSR | S_IWUSR);
> +		if (ret) {
> +			ret = -errno;
> +			cerr << "Failed to create IPA test FIFO at: "
> +			     << vimcFifoPath << ": " << strerror(-ret) << endl;

Could you please re-read my comments on the previous version regarding
this message and the next one ? After addressing them,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +			return TestFail;
> +		}
> +
> +		ret = open(vimcFifoPath, O_RDONLY | O_NONBLOCK);
> +		if (ret < 0) {
> +			ret = -errno;
> +			cerr << "Failed to open IPA test FIFO at: "
> +			     << vimcFifoPath << ": " << strerror(-ret) << endl;
> +			unlink(vimcFifoPath);
> +			return TestFail;
> +		}
> +		fd_ = ret;
> +
> +		notifier_ = new EventNotifier(fd_, EventNotifier::Read, this);
> +		notifier_->activated.connect(this, &IPAInterfaceTest::readTrace);
> +
> +		return TestPass;
> +	}
> +
> +	int run() override
> +	{
> +		EventDispatcher *dispatcher = thread()->eventDispatcher();
> +		Timer timer;
> +
> +		ipa_ = IPAManager::instance()->createIPA(pipe_.get(), 0, 0);
> +		if (!ipa_) {
> +			cerr << "Failed to create VIMC IPA interface" << endl;
> +			return TestFail;
> +		}
> +
> +		/* Test initialization of IPA module. */
> +		ipa_->init();
> +		timer.start(1000);
> +		while (timer.isRunning() && trace_ != IPAOperationInit)
> +			dispatcher->processEvents();
> +
> +		if (trace_ != IPAOperationInit) {
> +			cerr << "Failed to test IPA initialization sequence"
> +			     << endl;
> +			return TestFail;
> +		}
> +
> +		return TestPass;
> +	}
> +
> +	void cleanup() override
> +	{
> +		close(fd_);
> +		unlink(vimcFifoPath);
> +	}
> +
> +private:
> +	void readTrace(EventNotifier *notifier)
> +	{
> +		ssize_t s = read(notifier->fd(), &trace_, sizeof(trace_));
> +		if (s < 0) {
> +			int ret = -errno;
> +			cerr << "Failed to read from IPA test FIFO: "
> +			     << strerror(-ret) << endl;
> +			trace_ = IPAOperationNone;
> +		}
> +	}
> +
> +	std::shared_ptr<PipelineHandler> pipe_;
> +	std::unique_ptr<IPAInterface> ipa_;
> +	enum IPAOperationCode trace_;
> +	EventNotifier *notifier_;
> +	int fd_;
> +};
> +
> +TEST_REGISTER(IPAInterfaceTest)
> diff --git a/test/ipa/meson.build b/test/ipa/meson.build
> index e174671d05e1..c501fcf99aed 100644
> --- a/test/ipa/meson.build
> +++ b/test/ipa/meson.build
> @@ -1,5 +1,6 @@
>  ipa_test = [
> -    ['ipa_module_test', 'ipa_module_test.cpp'],
> +    ['ipa_module_test',     'ipa_module_test.cpp'],
> +    ['ipa_interface_test',  'ipa_interface_test.cpp'],
>  ]
>  
>  foreach t : ipa_test