Merge ~bhill/epics-base:pvAccess-envDefs.h into ~epics-core/epics-base/+git/epics-base:7.0

[Bruce Hill]

Adds pvAccess env variables to envDefs.h

[Andrew Johnson (anj)]

Will you be making the necessary pvAccessCPP changes to have it use the envDefs.h envConfigParam APIs to match? Any appropriate default values should also be set in the configure/CONFIG_ENV and/or configure/CONFIG_SITE_ENV files.

6e96884... by Bruce Hill on 2018-11-02

Added pvAccess environment variable defaults to CONFIG_ENV.

[Bruce Hill (bhill)]

Sorry, didn't notice the env defaults in CONFIG_ENV.
I got the defaults from pvAccessConfig.xlsx

[mdavidsaver (mdavidsaver)]

> I got the defaults from pvAccessConfig.xlsx

Where did you find this file? I don't think I've come across it before.

I like the idea of allowing users to check the PVA settings in the same way as the CA settings. The unfortunate reality is that these two are managed quite differently. cf. pv/configuration.h

I don't think it would be a good idea to merge this without corresponding changes to pvAccessCPP. To do so would potentially confuse users by showing incorrect values when defaults are being override in code, of when environment variables are ignored altogether.

review: Needs Fixing

[Bruce Hill (bhill)]

There are 2 copies online of pvAccessConfig.xlsx I know of:
https://github.com/epics-base/pvDataWWW/blob/master/mainPage/pvAccessConfig.xlsx
https://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=2&cad=rja&uact=8&ved=2ahUKEwiMqY7h677eAhWHLnwKHSkmA2sQFjABegQIABAC&url=http%3A%2F%2Fepics-pvdata.sourceforge.net%2FpvAccessConfig.xlsx&usg=AOvVaw0zhw0cg-Urqyso6CLM_0Gf

Both appear to be compatible.

I agree w/ your concern re the settings matching w/ pvAccessCPP.

I noticed these env variables were missing when I was asked to update our iocAdmin (iocStats)
with the PVA env variables. We use it to create PV's that display how each
env variable is set, but I don't think it will reflect how one env variable can get
it's default from another env variable.

There doesn't appear to be any support in the CA libs either re staying in sync
with the env variables in envDefs.h.

[Andrew Johnson (anj)]

Hi Bruce,

I don't understand your point about env variables getting defaults from other env variables, we don't do that.

The CA code uses the routines in envDefs.h to access environment variables, but because of our default mechanism (and to avoid confusion) the code calls them environment parameters. There is a script libCom/src/env/bldEnvData.pl that reads values from Base's CONFIG_ENV, CONFIG_SITE_ENV and any os/CONFIG_SITE_ENV.$(T_A) files and the envDefs.h header and generates an envDefs.c file containing those values, which the API then provides as the default value for a parameter if no environment variable by that name exists at run-time.

The PVA code does not currently use the routines in envDefs.h to access environment variables, so it can't rely on the default value mechanism of environment parameters. This was probably because it's not possible to add extra environment parameters in a sub-module or support module, they currently have to be listed in the envDefs.h file in libCom/env.

I do want to change the way that this code works so modules can define their own parameters and this is probably the right time to do that (after the 7.0.2 release), but exactly how that should work isn't completely clear without doing some more development.

The ENV_PARAM objects with their default values should be compiled into the library built by the module, so the bldEnvData.pl script will need to be modified to load a module-specific header file. How and where we specify default values (particularly for parameters that we expect sites to want to set themselves — they shouldn't have to edit CONFIG_SITE* files inside sub-modules) still needs some thought and discussion; it would be best to have that discussion before writing significant amounts of code to implement something.

- Andrew

review: Needs Fixing

f6b12f8... by Bruce Hill on 2018-11-07

Replaced defaults w/ empty for EPICS_PVAS_* env vars that default to their EPICS_PVA_* equivalent.

Allows code such as the following from rsrc/caservertask.c to work for EPICS_PVA_* environment parameters.
    if ( envGetConfigParamPtr ( &EPICS_CAS_SERVER_PORT ) ) {
        ca_server_port = envGetInetPortConfigParam ( &EPICS_CAS_SERVER_PORT, (unsigned short) CA_SERVER_PORT );
    }
    else {
        ca_server_port = envGetInetPortConfigParam ( &EPICS_CA_SERVER_PORT, (unsigned short) CA_SERVER_PORT );
    }

[Bruce Hill (bhill)]

So looking more in to how defaults are handled in CA vs PVA code, I needed to change
CONFIG_ENV to set most of the EPICS_PVAS_* defaults to empty or empty string.
The empty setting allows code such as this from rsrv/caservertask.c to use
EPICS_CA_SERVER_PORT as the default for EPICS_CAS_SERVER_PORT, even if neither are
actually defined as process environment variables.

if ( envGetConfigParamPtr ( &EPICS_CAS_SERVER_PORT ) ) {
    ca_server_port = envGetInetPortConfigParam ( &EPICS_CAS_SERVER_PORT,
                                 (unsigned short) CA_SERVER_PORT );
}
else {
    ca_server_port = envGetInetPortConfigParam ( &EPICS_CA_SERVER_PORT,
                                 (unsigned short) CA_SERVER_PORT );
}

My point from this is that the envGet*ConfigParam*() routines don't
provide any insight to client code such as in iocStats() re this defaulting
of EPICS_CAS_SERVER_PORT to EPICS_CA_SERVER_PORT, which I think refutes
part of Michael's objection.

As I see it, the make variables defined in CONFIG_ENV allow user overrides
via CONFIG_SITE_ENV to set site defaults for CA environment parameters.

The only change needed to make pvAccess compatible is to use those make
variables to override the default values in pvAccess/src/pva/pv/pvaConstants.h

pvAccess wouldn't need to use the envGet*ConfigParam*() calls as it's current
code in src/server/serverContext.cpp that calls getPropertyAs*() routines
can stay as is.

[Bruce Hill (bhill)]

I was reviewing this merge request to see what remaining work I needed to do and am not convinced any more changes are needed other than perhaps modifying pvAccessCPP to allow EPICS_PVA* defaults to be configured via configure/CONFIG_ENV.

As I see it, there are two issues here.
1. Allowing configuration of CA and PVA defaults via configure/CONFIG_ENV. This isn't something that we do at SLAC, but should perhaps be supported for PVA for sites which use CONFIG_ENV. Since most sites likely follow our practice and configure via scripts that set the actual environment variables, the values in CONFIG_ENV are only knowable if you examine the file or startup a virgin softIOC and use envGet*() calls to see the default values.

2. Allowing runtime examination of CA and PVA settings via the various envGet*() functions and modules such as iocStats that expose them as PVs.

I think the current state of the patch supports the 2nd use case, and a separate patch would be needed for pvAccessCPP to support build time configuration of PVA defaults.

[Andrew Johnson (anj)]

Core Mtg: AI on MAD tyo trim the list of var's and Bruce to adjust this. Approved after that.

[mdavidsaver (mdavidsaver)]

Some of these ENVs need to be removed. I need to go through the list again.

review: Needs Fixing

688a339... by Bruce Hill on 2019-04-03

Added envFindConfigParam() function so clients can find ENV_PARAM instances.

[Bruce Hill (bhill)]

Should I remove EPICS_PVAS_MAX_ARRAY_BYTES from envDefs.h?
Any others?

[Bruce Hill (bhill)]

Also EPICS_PVA_MAX_ARRAY_BYTES?

[mdavidsaver (mdavidsaver)]

pvAccessCPP will now use these parameters.

However, I'm having second (third?) thoughts about the idea of having a second or third set of default values for these parameters, which are sometimes are used, and sometimes ignored.

Probably the most confusing is EPICS_PVAS_PROVIDER_NAMES. A default value of "local" will actually prevent QSRV from starting! ("local" is the name used by pvDatabaseCPP) The actual default is "<all>", which selects all registered server-type ChannelProviders.

I'm tempted to say that it would be better to defer to the "pvasr" iocsh function, which can print the configuration actually being used by the running server (in IOCs).

That said, the set of parameters which I think should be advertised are:

EPICS_PVA_ADDR_LIST
EPICS_PVA_AUTO_ADDR_LIST
EPICS_PVA_BROADCAST_PORT
EPICS_PVA_SERVER_PORT
EPICS_PVAS_BEACON_ADDR_LIST
EPICS_PVAS_SERVER_PORT
EPICS_PVAS_INTF_ADDR_LIST
EPICS_PVAS_BROADCAST_PORT

Unmerged commits

688a339... by Bruce Hill on 2019-04-03

Added envFindConfigParam() function so clients can find ENV_PARAM instances.

f6b12f8... by Bruce Hill on 2018-11-07

Replaced defaults w/ empty for EPICS_PVAS_* env vars that default to their EPICS_PVA_* equivalent.

Allows code such as the following from rsrc/caservertask.c to work for EPICS_PVA_* environment parameters.
    if ( envGetConfigParamPtr ( &EPICS_CAS_SERVER_PORT ) ) {
        ca_server_port = envGetInetPortConfigParam ( &EPICS_CAS_SERVER_PORT, (unsigned short) CA_SERVER_PORT );
    }
    else {
        ca_server_port = envGetInetPortConfigParam ( &EPICS_CA_SERVER_PORT, (unsigned short) CA_SERVER_PORT );
    }

6e96884... by Bruce Hill on 2018-11-02

Added pvAccess environment variable defaults to CONFIG_ENV.

78addcc... by Bruce Hill on 2018-10-30

Adding pvAccess env variables to envDefs.h

diff --git a/configure/CONFIG_ENV b/configure/CONFIG_ENV
index 59e72d8..826be9f 100644
--- a/configure/CONFIG_ENV
+++ b/configure/CONFIG_ENV
@@ -47,6 +47,24 @@ EPICS_CAS_SERVER_PORT=
 EPICS_CAS_INTF_ADDR_LIST=""
 EPICS_CAS_IGNORE_ADDR_LIST=""
 
+EPICS_PVA_ADDR_LIST=""
+EPICS_PVA_AUTO_ADDR_LIST=YES
+EPICS_PVA_BEACON_PERIOD=15.0
+EPICS_PVA_BROADCAST_PORT=5076
+EPICS_PVA_CONN_TMO=30.0
+EPICS_PVA_DEBUG=0
+EPICS_PVA_MAX_ARRAY_BYTES=16384
+EPICS_PVA_PROVIDER_NAMES="local"
+EPICS_PVA_SERVER_PORT=5075
+
+EPICS_PVAS_AUTO_BEACON_ADDR_LIST=""
+EPICS_PVAS_BEACON_ADDR_LIST=""
+EPICS_PVAS_BEACON_PERIOD=
+EPICS_PVAS_BROADCAST_PORT=
+EPICS_PVAS_MAX_ARRAY_BYTES=
+EPICS_PVAS_PROVIDER_NAMES=""
+EPICS_PVAS_SERVER_PORT=
+
 # Servers to disable
 EPICS_IOC_IGNORE_SERVERS=""
 
diff --git a/modules/libcom/src/env/envDefs.h b/modules/libcom/src/env/envDefs.h
index 8be00a9..915e822 100644
--- a/modules/libcom/src/env/envDefs.h
+++ b/modules/libcom/src/env/envDefs.h
@@ -60,6 +60,25 @@ epicsShareExtern const ENV_PARAM EPICS_CAS_SERVER_PORT;
 epicsShareExtern const ENV_PARAM EPICS_CA_BEACON_PERIOD; /* deprecated */
 epicsShareExtern const ENV_PARAM EPICS_CAS_BEACON_PERIOD;
 epicsShareExtern const ENV_PARAM EPICS_CAS_BEACON_PORT;
+
+epicsShareExtern const ENV_PARAM EPICS_PVA_ADDR_LIST;
+epicsShareExtern const ENV_PARAM EPICS_PVA_AUTO_ADDR_LIST;
+epicsShareExtern const ENV_PARAM EPICS_PVA_BEACON_PERIOD;
+epicsShareExtern const ENV_PARAM EPICS_PVA_BROADCAST_PORT;
+epicsShareExtern const ENV_PARAM EPICS_PVA_CONN_TMO;
+epicsShareExtern const ENV_PARAM EPICS_PVA_DEBUG;
+epicsShareExtern const ENV_PARAM EPICS_PVA_MAX_ARRAY_BYTES;
+epicsShareExtern const ENV_PARAM EPICS_PVA_PROVIDER_NAMES;
+epicsShareExtern const ENV_PARAM EPICS_PVA_SERVER_PORT;
+
+epicsShareExtern const ENV_PARAM EPICS_PVAS_AUTO_BEACON_ADDR_LIST;
+epicsShareExtern const ENV_PARAM EPICS_PVAS_BEACON_ADDR_LIST;
+epicsShareExtern const ENV_PARAM EPICS_PVAS_BEACON_PERIOD;
+epicsShareExtern const ENV_PARAM EPICS_PVAS_BROADCAST_PORT;
+epicsShareExtern const ENV_PARAM EPICS_PVAS_MAX_ARRAY_BYTES;
+epicsShareExtern const ENV_PARAM EPICS_PVAS_PROVIDER_NAMES;
+epicsShareExtern const ENV_PARAM EPICS_PVAS_SERVER_PORT;
+
 epicsShareExtern const ENV_PARAM EPICS_BUILD_COMPILER_CLASS;
 epicsShareExtern const ENV_PARAM EPICS_BUILD_OS_CLASS;
 epicsShareExtern const ENV_PARAM EPICS_BUILD_TARGET_ARCH;
@@ -87,6 +106,7 @@ epicsShareFunc const char * epicsShareAPI
 	envGetConfigParamPtr(const ENV_PARAM *pParam);
 epicsShareFunc long epicsShareAPI 
 	envPrtConfigParam(const ENV_PARAM *pParam);
+epicsShareFunc const ENV_PARAM * envFindConfigParam( const char * envVarName );
 epicsShareFunc long epicsShareAPI 
 	envGetInetAddrConfigParam(const ENV_PARAM *pParam, struct in_addr *pAddr);
 epicsShareFunc long epicsShareAPI 
diff --git a/modules/libcom/src/env/envSubr.c b/modules/libcom/src/env/envSubr.c
index 283b132..1e0f9ba 100644
--- a/modules/libcom/src/env/envSubr.c
+++ b/modules/libcom/src/env/envSubr.c
@@ -24,6 +24,7 @@
 * QUICK REFERENCE
 * #include "envDefs.h"
 * ENV_PARAM	param;
+*  ENV_PARAM * envFindConfigParam(   envVarName           )
 *  char *envGetConfigParamPtr(       pParam			      )
 *  char *envGetConfigParam(          pParam,    bufDim,    pBuf       )
 *  long  envGetLongConfigParam(      pParam,    pLong                 )
@@ -52,6 +53,47 @@
 
 

 /*+/subr**********************************************************************
+* NAME  envFindConfigParam - Find an ENV_PARAM for the specified environment variable.
+*
+* DESCRIPTION
+*	Returns a pointer to an environment configuration parameter. 
+*	If the configuration parameter isn't found in the environment, 
+*	then a NULL pointer is returned.
+*
+* RETURNS
+*	pointer to the environment variable, or
+*	NULL if no environment configuration parameter was found
+*
+* EXAMPLES
+* 1.	Get a pointer to the ENV_PARAM for the EPICS-defined
+*   environment parameter EPICS_TS_NTP_INET.
+*
+*	#include "envDefs.h"
+*	const ENV_PARAM * pParam;
+*
+*	pParam = envFindConfigParam( "EPICS_TS_NTP_INET" );
+*	if (!pParam) {
+*		printf("No ENV_PARAM for EPICS_TS_NTP_INET\n" );
+*	}
+*
+*-*/
+const ENV_PARAM * envFindConfigParam( const char * envVarName )
+{
+	const ENV_PARAM **ppParam = env_param_list;
+
+	/* Find a match with one of the EPICS env vars */
+	while (*ppParam) {
+		if ( strcmp( envVarName, (*ppParam)->name ) == 0 ) {
+			return *ppParam;
+		}
+	ppParam++;
+	}
+
+	return 0;
+}
+
+

+/*+/subr**********************************************************************
 * NAME	envGetConfigParamPtr - returns a pointer to the configuration 
 * 		parameter value string
 *