[libcamera-devel,2/4] ipa: vimc: Rename IPA trace enums
diff mbox series

Message ID 20210806101409.324645-3-umang.jain@ideasonboard.com
State Superseded
Delegated to: Umang Jain
Headers show
Series
  • Pass buffers to VIMC IPA
Related show

Commit Message

Umang Jain Aug. 6, 2021, 10:14 a.m. UTC 
To avoid the conflict of naming with IPAOperations in future,
rename trace enum to IPATrace*.

This commit does not introduce any functional changes.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 include/libcamera/ipa/vimc.mojom | 10 +++++-----
 src/ipa/vimc/vimc.cpp            | 10 +++++-----
 test/ipa/ipa_interface_test.cpp  | 18 +++++++++---------
 3 files changed, 19 insertions(+), 19 deletions(-)

Comments

Paul Elder Aug. 10, 2021, 9:55 a.m. UTC | #1 
Hi Umang,

On Fri, Aug 06, 2021 at 03:44:07PM +0530, Umang Jain wrote:
> To avoid the conflict of naming with IPAOperations in future,
> rename trace enum to IPATrace*.

What kind of conflicts might we have? I'm not seeing the future that
you're seeing :S


Paul

> 
> This commit does not introduce any functional changes.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  include/libcamera/ipa/vimc.mojom | 10 +++++-----
>  src/ipa/vimc/vimc.cpp            | 10 +++++-----
>  test/ipa/ipa_interface_test.cpp  | 18 +++++++++---------
>  3 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
> index ee66353d..99b6412b 100644
> --- a/include/libcamera/ipa/vimc.mojom
> +++ b/include/libcamera/ipa/vimc.mojom
> @@ -10,11 +10,11 @@ import "include/libcamera/ipa/core.mojom";
>  
>  const string VimcIPAFIFOPath = "/tmp/libcamera_ipa_vimc_fifo";
>  
> -enum IPAOperationCode {
> -	IPAOperationNone,
> -	IPAOperationInit,
> -	IPAOperationStart,
> -	IPAOperationStop,
> +enum IPATraceCode {
> +	IPATraceNone,
> +	IPATraceInit,
> +	IPATraceStart,
> +	IPATraceStop,
>  };
>  
>  interface IPAVimcInterface {
> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
> index fb134084..54d9086a 100644
> --- a/src/ipa/vimc/vimc.cpp
> +++ b/src/ipa/vimc/vimc.cpp
> @@ -40,7 +40,7 @@ public:
>  
>  private:
>  	void initTrace();
> -	void trace(enum ipa::vimc::IPAOperationCode operation);
> +	void trace(enum ipa::vimc::IPATraceCode operation);
>  
>  	int fd_;
>  };
> @@ -59,7 +59,7 @@ IPAVimc::~IPAVimc()
>  
>  int IPAVimc::init(const IPASettings &settings)
>  {
> -	trace(ipa::vimc::IPAOperationInit);
> +	trace(ipa::vimc::IPATraceInit);
>  
>  	LOG(IPAVimc, Debug)
>  		<< "initializing vimc IPA with configuration file "
> @@ -76,7 +76,7 @@ int IPAVimc::init(const IPASettings &settings)
>  
>  int IPAVimc::start()
>  {
> -	trace(ipa::vimc::IPAOperationStart);
> +	trace(ipa::vimc::IPATraceStart);
>  
>  	LOG(IPAVimc, Debug) << "start vimc IPA!";
>  
> @@ -85,7 +85,7 @@ int IPAVimc::start()
>  
>  void IPAVimc::stop()
>  {
> -	trace(ipa::vimc::IPAOperationStop);
> +	trace(ipa::vimc::IPATraceStop);
>  
>  	LOG(IPAVimc, Debug) << "stop vimc IPA!";
>  }
> @@ -117,7 +117,7 @@ void IPAVimc::initTrace()
>  	fd_ = ret;
>  }
>  
> -void IPAVimc::trace(enum ipa::vimc::IPAOperationCode operation)
> +void IPAVimc::trace(enum ipa::vimc::IPATraceCode operation)
>  {
>  	if (fd_ < 0)
>  		return;
> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
> index ee9f2651..2f30b26a 100644
> --- a/test/ipa/ipa_interface_test.cpp
> +++ b/test/ipa/ipa_interface_test.cpp
> @@ -34,7 +34,7 @@ class IPAInterfaceTest : public Test, public Object
>  {
>  public:
>  	IPAInterfaceTest()
> -		: trace_(ipa::vimc::IPAOperationNone), notifier_(nullptr), fd_(-1)
> +		: trace_(ipa::vimc::IPATraceNone), notifier_(nullptr), fd_(-1)
>  	{
>  	}
>  
> @@ -112,10 +112,10 @@ protected:
>  		}
>  
>  		timer.start(1000);
> -		while (timer.isRunning() && trace_ != ipa::vimc::IPAOperationInit)
> +		while (timer.isRunning() && trace_ != ipa::vimc::IPATraceInit)
>  			dispatcher->processEvents();
>  
> -		if (trace_ != ipa::vimc::IPAOperationInit) {
> +		if (trace_ != ipa::vimc::IPATraceInit) {
>  			cerr << "Failed to test IPA initialization sequence"
>  			     << endl;
>  			return TestFail;
> @@ -124,10 +124,10 @@ protected:
>  		/* Test start of IPA module. */
>  		ipa_->start();
>  		timer.start(1000);
> -		while (timer.isRunning() && trace_ != ipa::vimc::IPAOperationStart)
> +		while (timer.isRunning() && trace_ != ipa::vimc::IPATraceStart)
>  			dispatcher->processEvents();
>  
> -		if (trace_ != ipa::vimc::IPAOperationStart) {
> +		if (trace_ != ipa::vimc::IPATraceStart) {
>  			cerr << "Failed to test IPA start sequence" << endl;
>  			return TestFail;
>  		}
> @@ -135,10 +135,10 @@ protected:
>  		/* Test stop of IPA module. */
>  		ipa_->stop();
>  		timer.start(1000);
> -		while (timer.isRunning() && trace_ != ipa::vimc::IPAOperationStop)
> +		while (timer.isRunning() && trace_ != ipa::vimc::IPATraceStop)
>  			dispatcher->processEvents();
>  
> -		if (trace_ != ipa::vimc::IPAOperationStop) {
> +		if (trace_ != ipa::vimc::IPATraceStop) {
>  			cerr << "Failed to test IPA stop sequence" << endl;
>  			return TestFail;
>  		}
> @@ -161,7 +161,7 @@ private:
>  			cerr << "Failed to read from IPA test FIFO at '"
>  			     << ipa::vimc::VimcIPAFIFOPath << "': " << strerror(ret)
>  			     << endl;
> -			trace_ = ipa::vimc::IPAOperationNone;
> +			trace_ = ipa::vimc::IPATraceNone;
>  		}
>  	}
>  
> @@ -170,7 +170,7 @@ private:
>  	std::shared_ptr<PipelineHandler> pipe_;
>  	std::unique_ptr<ipa::vimc::IPAProxyVimc> ipa_;
>  	std::unique_ptr<IPAManager> ipaManager_;
> -	enum ipa::vimc::IPAOperationCode trace_;
> +	enum ipa::vimc::IPATraceCode trace_;
>  	EventNotifier *notifier_;
>  	int fd_;
>  };
> -- 
> 2.31.1
>
Umang Jain Aug. 10, 2021, 10:16 a.m. UTC | #2 
Hi Paul,

On 8/10/21 3:25 PM, paul.elder@ideasonboard.com wrote:
> Hi Umang,
>
> On Fri, Aug 06, 2021 at 03:44:07PM +0530, Umang Jain wrote:
>> To avoid the conflict of naming with IPAOperations in future,
>> rename trace enum to IPATrace*.
> What kind of conflicts might we have? I'm not seeing the future that
> you're seeing :S
Patch 4/4 might put things in context, but probably will be dropped as 
part of re-work.

Anyway I think IPATraceCode is a more suitable naming for tracing codes.


>
>
> Paul
>
>> This commit does not introduce any functional changes.
>>
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> ---
>>   include/libcamera/ipa/vimc.mojom | 10 +++++-----
>>   src/ipa/vimc/vimc.cpp            | 10 +++++-----
>>   test/ipa/ipa_interface_test.cpp  | 18 +++++++++---------
>>   3 files changed, 19 insertions(+), 19 deletions(-)
>>
>> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
>> index ee66353d..99b6412b 100644
>> --- a/include/libcamera/ipa/vimc.mojom
>> +++ b/include/libcamera/ipa/vimc.mojom
>> @@ -10,11 +10,11 @@ import "include/libcamera/ipa/core.mojom";
>>   
>>   const string VimcIPAFIFOPath = "/tmp/libcamera_ipa_vimc_fifo";
>>   
>> -enum IPAOperationCode {
>> -	IPAOperationNone,
>> -	IPAOperationInit,
>> -	IPAOperationStart,
>> -	IPAOperationStop,
>> +enum IPATraceCode {
>> +	IPATraceNone,
>> +	IPATraceInit,
>> +	IPATraceStart,
>> +	IPATraceStop,
>>   };
>>   
>>   interface IPAVimcInterface {
>> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
>> index fb134084..54d9086a 100644
>> --- a/src/ipa/vimc/vimc.cpp
>> +++ b/src/ipa/vimc/vimc.cpp
>> @@ -40,7 +40,7 @@ public:
>>   
>>   private:
>>   	void initTrace();
>> -	void trace(enum ipa::vimc::IPAOperationCode operation);
>> +	void trace(enum ipa::vimc::IPATraceCode operation);
>>   
>>   	int fd_;
>>   };
>> @@ -59,7 +59,7 @@ IPAVimc::~IPAVimc()
>>   
>>   int IPAVimc::init(const IPASettings &settings)
>>   {
>> -	trace(ipa::vimc::IPAOperationInit);
>> +	trace(ipa::vimc::IPATraceInit);
>>   
>>   	LOG(IPAVimc, Debug)
>>   		<< "initializing vimc IPA with configuration file "
>> @@ -76,7 +76,7 @@ int IPAVimc::init(const IPASettings &settings)
>>   
>>   int IPAVimc::start()
>>   {
>> -	trace(ipa::vimc::IPAOperationStart);
>> +	trace(ipa::vimc::IPATraceStart);
>>   
>>   	LOG(IPAVimc, Debug) << "start vimc IPA!";
>>   
>> @@ -85,7 +85,7 @@ int IPAVimc::start()
>>   
>>   void IPAVimc::stop()
>>   {
>> -	trace(ipa::vimc::IPAOperationStop);
>> +	trace(ipa::vimc::IPATraceStop);
>>   
>>   	LOG(IPAVimc, Debug) << "stop vimc IPA!";
>>   }
>> @@ -117,7 +117,7 @@ void IPAVimc::initTrace()
>>   	fd_ = ret;
>>   }
>>   
>> -void IPAVimc::trace(enum ipa::vimc::IPAOperationCode operation)
>> +void IPAVimc::trace(enum ipa::vimc::IPATraceCode operation)
>>   {
>>   	if (fd_ < 0)
>>   		return;
>> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
>> index ee9f2651..2f30b26a 100644
>> --- a/test/ipa/ipa_interface_test.cpp
>> +++ b/test/ipa/ipa_interface_test.cpp
>> @@ -34,7 +34,7 @@ class IPAInterfaceTest : public Test, public Object
>>   {
>>   public:
>>   	IPAInterfaceTest()
>> -		: trace_(ipa::vimc::IPAOperationNone), notifier_(nullptr), fd_(-1)
>> +		: trace_(ipa::vimc::IPATraceNone), notifier_(nullptr), fd_(-1)
>>   	{
>>   	}
>>   
>> @@ -112,10 +112,10 @@ protected:
>>   		}
>>   
>>   		timer.start(1000);
>> -		while (timer.isRunning() && trace_ != ipa::vimc::IPAOperationInit)
>> +		while (timer.isRunning() && trace_ != ipa::vimc::IPATraceInit)
>>   			dispatcher->processEvents();
>>   
>> -		if (trace_ != ipa::vimc::IPAOperationInit) {
>> +		if (trace_ != ipa::vimc::IPATraceInit) {
>>   			cerr << "Failed to test IPA initialization sequence"
>>   			     << endl;
>>   			return TestFail;
>> @@ -124,10 +124,10 @@ protected:
>>   		/* Test start of IPA module. */
>>   		ipa_->start();
>>   		timer.start(1000);
>> -		while (timer.isRunning() && trace_ != ipa::vimc::IPAOperationStart)
>> +		while (timer.isRunning() && trace_ != ipa::vimc::IPATraceStart)
>>   			dispatcher->processEvents();
>>   
>> -		if (trace_ != ipa::vimc::IPAOperationStart) {
>> +		if (trace_ != ipa::vimc::IPATraceStart) {
>>   			cerr << "Failed to test IPA start sequence" << endl;
>>   			return TestFail;
>>   		}
>> @@ -135,10 +135,10 @@ protected:
>>   		/* Test stop of IPA module. */
>>   		ipa_->stop();
>>   		timer.start(1000);
>> -		while (timer.isRunning() && trace_ != ipa::vimc::IPAOperationStop)
>> +		while (timer.isRunning() && trace_ != ipa::vimc::IPATraceStop)
>>   			dispatcher->processEvents();
>>   
>> -		if (trace_ != ipa::vimc::IPAOperationStop) {
>> +		if (trace_ != ipa::vimc::IPATraceStop) {
>>   			cerr << "Failed to test IPA stop sequence" << endl;
>>   			return TestFail;
>>   		}
>> @@ -161,7 +161,7 @@ private:
>>   			cerr << "Failed to read from IPA test FIFO at '"
>>   			     << ipa::vimc::VimcIPAFIFOPath << "': " << strerror(ret)
>>   			     << endl;
>> -			trace_ = ipa::vimc::IPAOperationNone;
>> +			trace_ = ipa::vimc::IPATraceNone;
>>   		}
>>   	}
>>   
>> @@ -170,7 +170,7 @@ private:
>>   	std::shared_ptr<PipelineHandler> pipe_;
>>   	std::unique_ptr<ipa::vimc::IPAProxyVimc> ipa_;
>>   	std::unique_ptr<IPAManager> ipaManager_;
>> -	enum ipa::vimc::IPAOperationCode trace_;
>> +	enum ipa::vimc::IPATraceCode trace_;
>>   	EventNotifier *notifier_;
>>   	int fd_;
>>   };
>> -- 
>> 2.31.1
>>

Patch
diff mbox series 

diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
index ee66353d..99b6412b 100644
--- a/include/libcamera/ipa/vimc.mojom
+++ b/include/libcamera/ipa/vimc.mojom
@@ -10,11 +10,11 @@  import "include/libcamera/ipa/core.mojom";
 
 const string VimcIPAFIFOPath = "/tmp/libcamera_ipa_vimc_fifo";
 
-enum IPAOperationCode {
-	IPAOperationNone,
-	IPAOperationInit,
-	IPAOperationStart,
-	IPAOperationStop,
+enum IPATraceCode {
+	IPATraceNone,
+	IPATraceInit,
+	IPATraceStart,
+	IPATraceStop,
 };
 
 interface IPAVimcInterface {
diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
index fb134084..54d9086a 100644
--- a/src/ipa/vimc/vimc.cpp
+++ b/src/ipa/vimc/vimc.cpp
@@ -40,7 +40,7 @@  public:
 
 private:
 	void initTrace();
-	void trace(enum ipa::vimc::IPAOperationCode operation);
+	void trace(enum ipa::vimc::IPATraceCode operation);
 
 	int fd_;
 };
@@ -59,7 +59,7 @@  IPAVimc::~IPAVimc()
 
 int IPAVimc::init(const IPASettings &settings)
 {
-	trace(ipa::vimc::IPAOperationInit);
+	trace(ipa::vimc::IPATraceInit);
 
 	LOG(IPAVimc, Debug)
 		<< "initializing vimc IPA with configuration file "
@@ -76,7 +76,7 @@  int IPAVimc::init(const IPASettings &settings)
 
 int IPAVimc::start()
 {
-	trace(ipa::vimc::IPAOperationStart);
+	trace(ipa::vimc::IPATraceStart);
 
 	LOG(IPAVimc, Debug) << "start vimc IPA!";
 
@@ -85,7 +85,7 @@  int IPAVimc::start()
 
 void IPAVimc::stop()
 {
-	trace(ipa::vimc::IPAOperationStop);
+	trace(ipa::vimc::IPATraceStop);
 
 	LOG(IPAVimc, Debug) << "stop vimc IPA!";
 }
@@ -117,7 +117,7 @@  void IPAVimc::initTrace()
 	fd_ = ret;
 }
 
-void IPAVimc::trace(enum ipa::vimc::IPAOperationCode operation)
+void IPAVimc::trace(enum ipa::vimc::IPATraceCode operation)
 {
 	if (fd_ < 0)
 		return;
diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
index ee9f2651..2f30b26a 100644
--- a/test/ipa/ipa_interface_test.cpp
+++ b/test/ipa/ipa_interface_test.cpp
@@ -34,7 +34,7 @@  class IPAInterfaceTest : public Test, public Object
 {
 public:
 	IPAInterfaceTest()
-		: trace_(ipa::vimc::IPAOperationNone), notifier_(nullptr), fd_(-1)
+		: trace_(ipa::vimc::IPATraceNone), notifier_(nullptr), fd_(-1)
 	{
 	}
 
@@ -112,10 +112,10 @@  protected:
 		}
 
 		timer.start(1000);
-		while (timer.isRunning() && trace_ != ipa::vimc::IPAOperationInit)
+		while (timer.isRunning() && trace_ != ipa::vimc::IPATraceInit)
 			dispatcher->processEvents();
 
-		if (trace_ != ipa::vimc::IPAOperationInit) {
+		if (trace_ != ipa::vimc::IPATraceInit) {
 			cerr << "Failed to test IPA initialization sequence"
 			     << endl;
 			return TestFail;
@@ -124,10 +124,10 @@  protected:
 		/* Test start of IPA module. */
 		ipa_->start();
 		timer.start(1000);
-		while (timer.isRunning() && trace_ != ipa::vimc::IPAOperationStart)
+		while (timer.isRunning() && trace_ != ipa::vimc::IPATraceStart)
 			dispatcher->processEvents();
 
-		if (trace_ != ipa::vimc::IPAOperationStart) {
+		if (trace_ != ipa::vimc::IPATraceStart) {
 			cerr << "Failed to test IPA start sequence" << endl;
 			return TestFail;
 		}
@@ -135,10 +135,10 @@  protected:
 		/* Test stop of IPA module. */
 		ipa_->stop();
 		timer.start(1000);
-		while (timer.isRunning() && trace_ != ipa::vimc::IPAOperationStop)
+		while (timer.isRunning() && trace_ != ipa::vimc::IPATraceStop)
 			dispatcher->processEvents();
 
-		if (trace_ != ipa::vimc::IPAOperationStop) {
+		if (trace_ != ipa::vimc::IPATraceStop) {
 			cerr << "Failed to test IPA stop sequence" << endl;
 			return TestFail;
 		}
@@ -161,7 +161,7 @@  private:
 			cerr << "Failed to read from IPA test FIFO at '"
 			     << ipa::vimc::VimcIPAFIFOPath << "': " << strerror(ret)
 			     << endl;
-			trace_ = ipa::vimc::IPAOperationNone;
+			trace_ = ipa::vimc::IPATraceNone;
 		}
 	}
 
@@ -170,7 +170,7 @@  private:
 	std::shared_ptr<PipelineHandler> pipe_;
 	std::unique_ptr<ipa::vimc::IPAProxyVimc> ipa_;
 	std::unique_ptr<IPAManager> ipaManager_;
-	enum ipa::vimc::IPAOperationCode trace_;
+	enum ipa::vimc::IPATraceCode trace_;
 	EventNotifier *notifier_;
 	int fd_;
 };