ArEventLog utility functions in C++

[kenneth.tham]

Hi, I am developing a utility file for ArEventLog in C++ for use in my PLC code. The code is largely built upon another member contribution from my earlier queries. I was wondering if there were any suggestions of possible improvements. Thanks.

1. I am not sure if I need the CopyPLCString function. Is there a better way to convert/copy the strings?
2. I am storing some messages for log later in case the logger is not at the correct step.
3. I have hardcoded the message severities as when I used the enums to pass into the function to get the severity EventID, it returns 0.

I call LogInit in the Cyclic function. And use LogMsg to log a message.

Regards,
Kenneth

.h file:

#pragma once

#include
#include

#include <globalVAR.h>//custom file
#ifdef _DEFAULT_INCLUDES
#endif
#include <ArEventLog.h>

namespace ArEventLogUtils
{

enum
{
	LOG_SEVERITY_SUCCESS=536870912,
	LOG_SEVERITY_INFO=1610612736,
	LOG_SEVERITY_WARNING=-1610612736,
	LOG_SEVERITY_ERROR=-536870912
};

void CopyPLCString(STRING* strPLC, std::string& str);

void LogMsg(const char* strMsg, DINT severity=ArEventLogUtils::LOG_SEVERITY_INFO);

void ProcessMsgs();

void LogInit();

}

cpp file:

#include “ArEventLogUtils.hpp”

namespace ArEventLogUtils
{
std::vectorstd::string store;
std::vector severityValues;

void CopyPLCString(STRING* strPLC, const char* str)
{
	for (int i = 0; ; i++)
	{
		if (str[i]=='\0'||i>=30)
		{
			break;	
		}
		strPLC[i] = str[i];
	}
}

void LogMsg(const char* strMsg, DINT severity)
{
	if (Step==40)
	{
		ArEventLogWrite_0.Ident				= Ident; 			
		ArEventLogWrite_0.OriginRecordID	= 0; 
		CopyPLCString(ArEventLogWrite_0.ObjectID, "PLC");
		ArEventLogWrite_0.TimeStamp			= 0; 		
		ArEventLogWrite_0.EventID = severity;
		ArEventLogWrite_0.AddData = (unsigned int)strMsg;
		ArEventLogWrite_0.AddDataSize = strlen(strMsg)+1;
		ArEventLogWrite_0.AddDataFormat = arEVENTLOG_ADDFORMAT_TEXT;
		
		ArEventLogWrite_0.Execute			        = true; 
		ArEventLogWrite(&ArEventLogWrite_0);	// synchron fub call 
	
		ArEventLogWrite_0.Execute			       = false; 
		ArEventLogWrite(&ArEventLogWrite_0);	// synchron fub call 		
	}
	else
	{
		store.push_back(strMsg);	
		severityValues.push_back(severity);
	}
}

void ProcessMsgs()
{
	for (int i = 0;  i < store.size(); i++)
	{
		LogMsg(store[i].c_str(), severityValues[i]);	
	}
	store.clear();
	severityValues.clear();
}

void LogInit()
{
	switch (Step)
	{
		case 0: 
			// Add Start Condition if nessesary 
			Step = 5;
	
			break;
	
		case 5:
			CopyPLCString(ArEventLogGetIdent_0.Name, "Sys");
			ArEventLogGetIdent_0.Execute = false; 

			ArEventLogGetIdent(&ArEventLogGetIdent_0);
	
			Step = 10; 
	
			break;
	
		case 10:

			ArEventLogGetIdent_0.Execute = true; 
			ArEventLogGetIdent(&ArEventLogGetIdent_0);
		
			if (ArEventLogGetIdent_0.Done) 
			{
				Ident = ArEventLogGetIdent_0.Ident; 
				ArEventLogGetIdent_0.Execute = false; 
				Step = 30;		// Write 
			}			
			else if (ArEventLogGetIdent_0.Error) 
			{
				if (ArEventLogGetIdent_0.StatusID == arEVENTLOG_ERR_LOGBOOK_NOT_FOUND)
				{
					Step = 20; 	// create
					CopyPLCString(ArEventLogGetIdent_0.Name, "Sys");
					ArEventLogCreate_0.Size 		= 65535;
					ArEventLogCreate_0.Persistence 	= arEVENTLOG_PERSISTENCE_PERSIST;
					ArEventLogCreate_0.Info 		= 0;
					ArEventLogCreate_0.Execute 		= false;
					ArEventLogCreate(&ArEventLogCreate_0);
				
				}
				else
				{
					Step = 255; // error 	
				}
			}
		
			break;
	
		case 20:
		
		
			ArEventLogCreate_0.Execute = true; 
			ArEventLogCreate(&ArEventLogCreate_0);

			if (ArEventLogCreate_0.Done) 
			{
				Ident = ArEventLogCreate_0.Ident;
				ArEventLogCreate_0.Execute = false; 
				Step = 30;		// Write 
			}
			else if (ArEventLogCreate_0.Error) 
			{
				Step = 255; // error 	
			}
		
			break;
		
		case 30:				
			Step = 40;
	
			break;
	
		case 40:
			ProcessMsgs();
			break;
	
	}
}

}

[michael_w]

Hi,

my suggestions on a quick glance:

• The EventID is supposed to be an ID which contains more than just a severity - see B&R Online Help
• On the same page you can take a look at the ArEventLogMakeEventID() function which I recommend using, it also takes existing severity constants (like arEVENTLOG_SEVERITY_SUCCESS etc) so you can get rid of your own definitions
• The CopyPLCString seems unneccessary: The STRING type looks like this (taken from bur/plctypes.h):

typedef char           plcstring;
typedef plcstring      STRING;

So it’s

strcpy(ArEventLogWrite_0.ObjectID, "PLC")

• FUB names: the AS default of giving you names with _0 is not really well chosen, I’d recommend a prefix of “FB_” → “FB_EventLogGetIdent” etc.
• switch-case: “magic numbers”, I’d recommend naming the several steps using an enum, that way it’d be smth like

case sWAIT:
    // start on condition
    Step = sGET_IDENT;
    break;

case sGET_IDENT:
    // ...

Easier readable, less possibilities to jump to the wrong step, better maintainability

Maybe there is more but that’s the things I noticed immediately
Hope It’s of use to you

Best regards
Michael

[kenneth.tham]

Thanks for the inputs.

Previously I did try to use ArEventLogMakeEventID function, but always got zero for the event id. I was wondering if anyone encountered this issue?

[michael.bertsch]

Hello,
i think the ArEventLogMakeEventID is working as intended. I made a short test but found no issues. I documented my Test below, i hope it helps you to find the issue.

void _CYCLIC ProgramCyclic(void)
{
		EventID = ArEventLogMakeEventID(Severity,Facility,ErrorID);
}

Greetings
Michael