[libcamera-devel] [PATCH 2/4] ipa: vimc: Rename IPA trace enums

[Umang Jain]

Hi Paul,

On 8/10/21 3:25 PM, paul.elder at 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 at 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
>>