[libcamera-devel] [PATCH v2 6/7] [TEST] ipa: vimc: Add IPAOperationCode to init() parameter list

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Thu, Aug 18, 2022 at 03:49:21PM +0900, Paul Elder via libcamera-devel wrote:
> For the purpose of testing serializing/deserializing enums in function
> parameters, add IPAOperationCode to the parameter list of init().
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> No changes in v2
> ---
>  include/libcamera/ipa/vimc.mojom     | 2 +-
>  src/ipa/vimc/vimc.cpp                | 6 ++++--
>  src/libcamera/pipeline/vimc/vimc.cpp | 2 +-
>  test/ipa/ipa_interface_test.cpp      | 2 +-
>  4 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
> index 718b9674..16149787 100644
> --- a/include/libcamera/ipa/vimc.mojom
> +++ b/include/libcamera/ipa/vimc.mojom
> @@ -18,7 +18,7 @@ enum IPAOperationCode {
>  };
>  
>  interface IPAVimcInterface {
> -	init(libcamera.IPASettings settings) => (int32 ret);
> +	init(libcamera.IPASettings settings, IPAOperationCode code) => (int32 ret);
>  
>  	configure(libcamera.IPACameraSensorInfo sensorInfo,
>  		  map<uint32, libcamera.IPAStream> streamConfig,
> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
> index 85afb279..6bf39a1c 100644
> --- a/src/ipa/vimc/vimc.cpp
> +++ b/src/ipa/vimc/vimc.cpp
> @@ -31,7 +31,7 @@ public:
>  	IPAVimc();
>  	~IPAVimc();
>  
> -	int init(const IPASettings &settings) override;
> +	int init(const IPASettings &settings, const ipa::vimc::IPAOperationCode &code) override;

Should we do for enums the same thing we do for integers, and pass by
value instead of const reference ?

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

>  
>  	int start() override;
>  	void stop() override;
> @@ -66,7 +66,7 @@ IPAVimc::~IPAVimc()
>  		::close(fd_);
>  }
>  
> -int IPAVimc::init(const IPASettings &settings)
> +int IPAVimc::init(const IPASettings &settings, const ipa::vimc::IPAOperationCode &code)
>  {
>  	trace(ipa::vimc::IPAOperationInit);
>  
> @@ -74,6 +74,8 @@ int IPAVimc::init(const IPASettings &settings)
>  		<< "initializing vimc IPA with configuration file "
>  		<< settings.configurationFile;
>  
> +	LOG(IPAVimc, Debug) << "Got opcode " << code;
> +
>  	File conf(settings.configurationFile);
>  	if (!conf.open(File::OpenModeFlag::ReadOnly)) {
>  		LOG(IPAVimc, Error) << "Failed to open configuration file";
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 153cf849..983bd514 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -471,7 +471,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>  	data->ipa_->paramsBufferReady.connect(data.get(), &VimcCameraData::paramsBufferReady);
>  
>  	std::string conf = data->ipa_->configurationFile("vimc.conf");
> -	data->ipa_->init(IPASettings{ conf, data->sensor_->model() });
> +	data->ipa_->init(IPASettings{ conf, data->sensor_->model() }, ipa::vimc::IPAOperationInit);
>  
>  	/* Create and register the camera. */
>  	std::set<Stream *> streams{ &data->stream_ };
> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
> index 3c0df843..b9fa15cd 100644
> --- a/test/ipa/ipa_interface_test.cpp
> +++ b/test/ipa/ipa_interface_test.cpp
> @@ -106,7 +106,7 @@ protected:
>  
>  		/* Test initialization of IPA module. */
>  		std::string conf = ipa_->configurationFile("vimc.conf");
> -		int ret = ipa_->init(IPASettings{ conf, "vimc" });
> +		int ret = ipa_->init(IPASettings{ conf, "vimc" }, ipa::vimc::IPAOperationInit);
>  		if (ret < 0) {
>  			cerr << "IPA interface init() failed" << endl;
>  			return TestFail;

-- 
Regards,

Laurent Pinchart