[libcamera-devel] [PATCH v2 6/6] test: ipa: Add test for the IPA Interface
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Oct 4 23:25:46 CEST 2019
Hi Jacopo,
Thank you for the patch.
On Fri, Oct 04, 2019 at 06:37:34PM +0200, Jacopo Mondi wrote:
> Implementing a basic test for IPA Interface using the VIMC dummy IPA
> module. The test implements a communication channel between the test and
s/a communication channel/an out-of-band feedback communication channel/
?
> 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 at jmondi.org>
> ---
> src/ipa/meson.build | 2 +
> test/ipa/ipa_interface_test.cpp | 136 ++++++++++++++++++++++++++++++++
> test/ipa/meson.build | 10 ++-
> 3 files changed, 146 insertions(+), 2 deletions(-)
> create mode 100644 test/ipa/ipa_interface_test.cpp
>
> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> index 2827dc0303b2..3e11e166f283 100644
> --- a/src/ipa/meson.build
> +++ b/src/ipa/meson.build
> @@ -5,6 +5,8 @@ ipa_vimc_sources = [
>
> ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')
>
> +ipa_internal_includes = include_directories('.')
I think you won't need this if you move the vimc IPA header to
include/ipa/
> +
> ipa_includes = [
> libcamera_includes,
> libcamera_internal_includes,
> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
> new file mode 100644
> index 000000000000..b6d0a4e7de20
> --- /dev/null
> +++ b/test/ipa/ipa_interface_test.cpp
> @@ -0,0 +1,136 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipa_interface_test.cpp - Test the IPA interface
> + */
> +
> +#include <iostream>
> +
> +#include <fcntl.h>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
Here too the C headers should come before the C++ headers if you want to
separate them.
> +#include <libcamera/event_dispatcher.h>
> +#include <libcamera/event_notifier.h>
> +#include <libcamera/timer.h>
> +
> +#include "device_enumerator.h"
> +#include "ipa_manager.h"
> +#include "ipa_module.h"
> +#include "pipeline_handler.h"
> +#include "test.h"
> +#include "thread.h"
> +
> +#include "ipa_vimc.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);
Not something that you need to address now (but feel free to still do so
:-)), what will happen if this test is interrupted for any reason
(including a crash) ?
> + if (ret) {
> + cerr << "Failed to create IPA test fifo at: "
"... test FIFO at '" << path << "': " ...
> + << vimcFifoPath << ": " << strerror(errno) << endl;
You need to store -errno in ret as usual.
> + return TestFail;
> + }
> +
> + ret = open(vimcFifoPath, O_RDONLY | O_NONBLOCK);
> + if (ret < 0) {
> + cerr << "Failed to open IPA test fifo at: "
> + << vimcFifoPath << ": " << strerror(errno) << endl;
Same two comments.
> + 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) {
> + cerr << "Failed to read from IPA test fifo: "
> + << strerror(errno) << endl;
Same here.
> + trace_ = IPAOperationNone;
> + }
> + }
> +
> + std::shared_ptr<PipelineHandler> pipe_;
> + std::unique_ptr<IPAInterface> ipa_;
> + enum IPAOperationCodes trace_;
> + EventNotifier *notifier_;
> + int fd_;
> +};
> +
> +TEST_REGISTER(IPAInterfaceTest)
> diff --git a/test/ipa/meson.build b/test/ipa/meson.build
> index e174671d05e1..d19de18ab97e 100644
> --- a/test/ipa/meson.build
> +++ b/test/ipa/meson.build
> @@ -1,12 +1,18 @@
> ipa_test = [
> - ['ipa_module_test', 'ipa_module_test.cpp'],
> + ['ipa_module_test', 'ipa_module_test.cpp'],
> + ['ipa_interface_test', 'ipa_interface_test.cpp'],
> +]
> +
> +test_includes_ipa = [
> + test_includes_internal,
> + ipa_internal_includes,
> ]
>
> foreach t : ipa_test
> exe = executable(t[0], t[1],
> dependencies : libcamera_dep,
> link_with : test_libraries,
> - include_directories : test_includes_internal)
> + include_directories : test_includes_ipa)
>
> test(t[0], exe, suite : 'ipa')
> endforeach
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list