Experimental Physics and
Industrial Control System

Andrew Johnson <[email protected]> · Fri, 15 Sep 2017 16:27:08 -0500

Hi Ralph

On 09/15/2017 03:03 PM, Ralph Lange wrote:
>     Did you look at making the two info items into fields instead? I don't
>     like all the extra complexity involved in using a combination of info
>     items and members of simpvt for storing state that you are doing.
> 
> 
> The info fields carry only configuration. All state in is the simpvt
> structure.
> I will probably still need the simpvt structure to store state.
> Or are you suggesting to completely remove the private structure and
> store everything (configuration and state) in fields?

Currently struct xsimm is defined as

typedef struct xsimm {
    epicsEnum16  scan;
    epicsEnum16  oldsimm;
    double       delay;
    CALLBACK     cb;
} xsimm;

If scan and delay become record fields you'd still be left with the
CALLBACK which is a reasonable size (3 pointers + int):

typedef struct callbackPvt {
	void (*callback)(struct callbackPvt*);
	int		priority;
	void		*user; /*for use by callback user*/
        void            *timer; /*for use by callback itself*/
} CALLBACK;

Thus I think it makes sense to keep the simpvt structure. The oldsimm
could turn into a field, but I don't think that matters much either way.

>     I would like to see a comparison of the two approaches, as I'm not
>     convinced that the extra code complexity is worth the memory saved by
>     not using regular fields.
> 
> I am not against adding fields. All my users said they don't need the
> settings to be run-time configurable, but it does not hurt if they are.
> I will try to avoid making them special(), as that would add more code.

The original idea behind info items was that they would be for device
support and/or external software to attach data to record instances, not
that they be additional design-time-only. We have been moving away from
having fields that can't be changed at runtime for some time now, and
I'd rather that we not reverse that without good reason.

> If a new simmSCAN field is of type menuScan, to what do I have to set
> the default value (in the dbd) to detect the field not having been set?
> I need this to not swap the SCAN periods if simmSCAN has not been set.

The mbbi record uses 65535 when it can't match RVAL against one of the
xxVL fields, while dbGetMenuIndexFromString() returns -1 (which should
truncate to the same thing). If this doesn't work as a default field
value I think it would be reasonable to allow it and make the necessary
changes to dbStaticLib and/or the Perl DBD parsing code.

>     At least one change in recGbl.c seems to be undoing a link support
>     change (explicitly testing a plink->type value should be avoided in new
>     code to be compatible with new link types):
> 
>     > -    if (!dbLinkIsConstant(plink)) {
>     > +    if (plink->type != CONSTANT) {
>     >          struct pv_link *ppv_link = &plink->value.pv_link;
> 
>     However it looks like the line immediately following the changed one
>     hasn't been properly converted for link support anyway (it could crash
>     if someone makes TSEL a JSON link type). There are other similar tests
>     of plink->type fields which should be using the dbLink*() equivalent
>     routine in recGblGetTimeStampSimm() and recGblInitSimm().
> 
> 
> Sorry. I didn't check the documentation and thought I was using the new
> style constructs. Will fix this.

I will take a closer look at the incorrect recGbl.c code and try to work
out how to handle the pvlOptTSELisTime flag properly, I could see it
becoming DBLINK_FLAG_TSELisTIME.

> Thanks a lot for the review. I will wait for your comment on the field
> vs. simpvt issue and do the changes.
> These will not affect the simulation mode run time behavior.

Great, thanks!

- Andrew

-- 
Arguing for surveillance because you have nothing to hide is no
different than making the claim, "I don't care about freedom of
speech because I have nothing to say." -- Edward Snowdon

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System