Merge ~anj/epics-base/+git/base-7.0:fix-1824277 into ~epics-core/epics-base/+git/epics-base:7.0

Proposed by Andrew Johnson
Status: Needs review
Proposed branch: ~anj/epics-base/+git/base-7.0:fix-1824277
Merge into: ~epics-core/epics-base/+git/epics-base:7.0
Diff against target: 317 lines (+133/-20)
12 files modified
documentation/RELEASE_NOTES.html (+12/-0)
modules/database/src/ioc/db/dbAccess.c (+36/-10)
modules/database/src/std/dev/devAiSoftCallback.c (+1/-1)
modules/database/src/std/dev/devBiSoftCallback.c (+1/-1)
modules/database/src/std/dev/devI64inSoftCallback.c (+1/-1)
modules/database/src/std/dev/devLiSoftCallback.c (+1/-1)
modules/database/src/std/dev/devMbbiDirectSoftCallback.c (+1/-1)
modules/database/src/std/dev/devMbbiSoftCallback.c (+1/-1)
modules/database/src/std/dev/devSiSoftCallback.c (+1/-1)
modules/database/test/std/rec/Makefile (+2/-2)
modules/database/test/std/rec/regressCalcout.db (+27/-0)
modules/database/test/std/rec/regressTest.c (+49/-1)
Reviewer Review Type Date Requested Status
Andrew Johnson Needs Fixing
mdavidsaver Needs Fixing
Review via email: mp+366247@code.launchpad.net

Commit message

Fix for bug lp: #1824277 and related issues.

This could go into 7.0.2.2 if nobody objects. The bug has existed since 3.16.1.

To post a comment you must log in.
27ba6a0... by Andrew Johnson

Release Notes entry with link to bug.

Revision history for this message
Andrew Johnson (anj) wrote :

Looking for any final requests or veto's to this PR before tomorrow when I'm planning to merge it and work on the 7.0.2.2 release. Do you guys all understand it enough now (after the discussion that took place last week on the linked bug report)?

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

I'd like to see some test coverage for the more common case of writing to eg. '.A', both when INPA="" and when INPA="5" (or other non-empty).

Does this change effect all link fields now? (eg. INP for aiRecord)

Also, have you ever seen external record support special() looking for PV_LINK? It looks to me like such cases would instead see DB_LINK or CA_LINK.

My only objection is to rushing it out. lp:1824277 does not seem to effect many users. Let's not break something which does. Maybe we merge this just after 7.0.2.2?

a50a0b2... by Andrew Johnson

Add tests confirming that input value fields still writable

Having a constant INP link doesn't break that.

Revision history for this message
Andrew Johnson (anj) wrote :

There should be no change to the behavior of writing to the related value field, which for the calcout isn't marked as special although it is pp so doing so does trigger record processing. Nothing in that chain of processing has been touched though, it's only dbPutFieldLink() that I've modified, the routine that handles puts to a link field. However I just added a couple of testdbPutFieldOk() calls, writing to the two input-value fields that have const INP links.

The fix may slightly alter the code path taken anytime something writes to a link field at runtime, ai.INP fields included, but in most cases there should be no change to the result. It is unusual for a link field to be marked special, and in Base the calcout is the only type which has any special links.

There are a few references to PV_LINK in the epics-modules repo's, but I think you're right that most code won't normally see plink->type == PV_LINK; the only time that should happen with this fix is in a dsxt::add_record() routine where the link type in the device() entry in the DBD file is given as CONSTANT (which means "or DB/CA/PV/JSON LINK").

The bug hasn't affected many users because there hasn't been a release of synApps for EPICS 7 yet, and it's the synApps users who will see and complain about it. Keenan Lang is working to release synApps 6.1 by the end of May (see his question at https://github.com/epics-modules/iocStats/issues/33#issuecomment-484995327), but I don't know if that is intended to support EPICS 7 — synApps 6.0 was aimed at Base-3.15.5. If we're planning to merge other major changes after 7.0.2.2 that could preclude us putting out a patch release for this bug-fix unless we start a release branch.

I understand this seems like a rush, but a synApps release strikes me as a good reason to try and get this fix included.

Revision history for this message
rivers (rivers) wrote :

> The bug hasn't affected many users because there hasn't been a release of synApps for EPICS 7 yet, and it's the synApps users who will see and complain about it.

I have been running 7.0.2.2 with synApps for 2 APS run cycles now. What is the bug?

Mark

________________________________
From: <email address hidden> <email address hidden> on behalf of Andrew Johnson via Core-talk <email address hidden>
Sent: Monday, April 22, 2019 11:40 PM
To: Andrew Johnson
Subject: Re: [Merge] ~anj/epics-base/+git/base-7.0:fix-1824277 into epics-base:7.0

There should be no change to the behavior of writing to the related value field, which for the calcout isn't marked as special although it is pp so doing so does trigger record processing. Nothing in that chain of processing has been touched though, it's only dbPutFieldLink() that I've modified, the routine that handles puts to a link field. However I just added a couple of testdbPutFieldOk() calls, writing to the two input-value fields that have const INP links.

The fix may slightly alter the code path taken anytime something writes to a link field at runtime, ai.INP fields included, but in most cases there should be no change to the result. It is unusual for a link field to be marked special, and in Base the calcout is the only type which has any special links.

There are a few references to PV_LINK in the epics-modules repo's, but I think you're right that most code won't normally see plink->type == PV_LINK; the only time that should happen with this fix is in a dsxt::add_record() routine where the link type in the device() entry in the DBD file is given as CONSTANT (which means "or DB/CA/PV/JSON LINK").

The bug hasn't affected many users because there hasn't been a release of synApps for EPICS 7 yet, and it's the synApps users who will see and complain about it. Keenan Lang is working to release synApps 6.1 by the end of May (see his question at https://github.com/epics-modules/iocStats/issues/33#issuecomment-484995327), but I don't know if that is intended to support EPICS 7 - synApps 6.0 was aimed at Base-3.15.5. If we're planning to merge other major changes after 7.0.2.2 that could preclude us putting out a patch release for this bug-fix unless we start a release branch.

I understand this seems like a rush, but a synApps release strikes me as a good reason to try and get this fix included.
--
https://code.launchpad.net/~anj/epics-base/+git/base-7.0/+merge/366247
Your team EPICS Core Developers is requested to review the proposed merge of ~anj/epics-base/+git/base-7.0:fix-1824277 into epics-base:7.0.

Revision history for this message
Keenan Lang (klang) wrote :

>Keenan Lang is working to release synApps 6.1 by the end of May (see his question at https://github.com/epics-modules/iocStats/issues/33#issuecomment-484995327), but I don't know if that is intended to support EPICS 7

I don't think this release will officially support EPICS 7, but I am definitely going to try to start getting all of our code compliant with newer versions. That's what ended up finding this issue as I was updating some of my own code for the changes to links.

>I understand this seems like a rush, but a synApps release strikes me as a good reason to try and get this fix included.

Since the whole of synApps isn't likely to use EPICSv7 at least for the next year or more, there's not really a rush. The only IOC's going in that use it will be for specific areaDetector cameras and they don't really use the record types affected.

>What is the bug?

Record types that are based off of calcout can no longer have a constant value written to their input fields and have the value of the correct data field change. For example, writing '5' to a calcout's INPA field no longer changes the A field to 5. This affects the calcout, aCalcout, sCalcout, transform, and luascript records.

Revision history for this message
rivers (rivers) wrote :

> Since the whole of synApps isn't likely to use EPICSv7 at least for the next year or more, there's not really a rush. The only IOC's going in that use it will be for specific areaDetector cameras and they don't really use the record types affected.

synApps modules are in production use on APS sector 13, and I believe also sector 16, with base 7.0.2.2 right now

Mark

________________________________
From: <email address hidden> <email address hidden> on behalf of Keenan Lang via Core-talk <email address hidden>
Sent: Tuesday, April 23, 2019 9:43 AM
To: Andrew Johnson
Subject: Re: [Merge] ~anj/epics-base/+git/base-7.0:fix-1824277 into epics-base:7.0

>Keenan Lang is working to release synApps 6.1 by the end of May (see his question at https://github.com/epics-modules/iocStats/issues/33#issuecomment-484995327), but I don't know if that is intended to support EPICS 7

I don't think this release will officially support EPICS 7, but I am definitely going to try to start getting all of our code compliant with newer versions. That's what ended up finding this issue as I was updating some of my own code for the changes to links.

>I understand this seems like a rush, but a synApps release strikes me as a good reason to try and get this fix included.

Since the whole of synApps isn't likely to use EPICSv7 at least for the next year or more, there's not really a rush. The only IOC's going in that use it will be for specific areaDetector cameras and they don't really use the record types affected.

>What is the bug?

Record types that are based off of calcout can no longer have a constant value written to their input fields and have the value of the correct data field change. For example, writing '5' to a calcout's INPA field no longer changes the A field to 5. This affects the calcout, aCalcout, sCalcout, transform, and luascript records.
--
https://code.launchpad.net/~anj/epics-base/+git/base-7.0/+merge/366247
Your team EPICS Core Developers is requested to review the proposed merge of ~anj/epics-base/+git/base-7.0:fix-1824277 into epics-base:7.0.

Revision history for this message
Keenan Lang (klang) wrote :

Then I guess it's up to you guys how important it is to have this fix. Do you think you are likely to switch between links and constant values in user calculations? If you aren't switching between the two, you can just change the A-L fields directly.

Revision history for this message
Andrew Johnson (anj) wrote :
Download full text (3.4 KiB)

Hi Keenan,

Does synApps have GUI screens that have a text-entry widget for the INP fields but only a text-display widget for the corresponding value field on these record types? That's the other case I'm most concerned about.

- Andrew

On 4/23/19 10:41 AM, Lang, Keenan C. wrote:

From a scan of our IOC's, it crops up in user calcs, pid calculations and limits, slit transforms, bpm calculations, cmdReply formatting, shutter arming, and a database for fastshutter limits.

________________________________
From: Mooney, Tim M.
Sent: Tuesday, April 23, 2019 10:11:53 AM
To: Johnson, Andrew N.; <email address hidden><mailto:<email address hidden>>; Lang, Keenan C.; <email address hidden><mailto:<email address hidden>>
Subject: Re: [Merge] ~anj/epics-base/+git/base-7.0:fix-1824277 into epics-base:7.0

>>What is the bug?

>Record types that are based off of calcout can no longer have a constant value written to their input fields and have the >value of the correct data field change. For example, writing '5' to a calcout's INPA field no longer changes the A field to >5. This affects the calcout, aCalcout, sCalcout, transform, and luascript records.

I'm sure I've never written a database that relies on this behavior.

Tim Mooney (<email address hidden><mailto:<email address hidden>>) (630)252-5417
Beamline Controls Group (www.aps.anl.gov<http://www.aps.anl.gov>)
Advanced Photon Source, Argonne National Lab

________________________________
From: <email address hidden><mailto:<email address hidden>> <email address hidden><mailto:<email address hidden>> on behalf of Keenan Lang via Core-talk <email address hidden><mailto:<email address hidden>>
Sent: Tuesday, April 23, 2019 9:43 AM
To: Johnson, Andrew N.
Subject: Re: [Merge] ~anj/epics-base/+git/base-7.0:fix-1824277 into epics-base:7.0

>Keenan Lang is working to release synApps 6.1 by the end of May (see his question at https://github.com/epics-modules/iocStats/issues/33#issuecomment-484995327), but I don't know if that is intended to support EPICS 7

I don't think this release will officially support EPICS 7, but I am definitely going to try to start getting all of our code compliant with newer versions. That's what ended up finding this issue as I was updating some of my own code for the changes to links.

>I understand this seems like a rush, but a synApps release strikes me as a good reason to try and get this fix included.

Since the whole of synApps isn't likely to use EPICSv7 at least for the next year or more, there's not really a rush. The only IOC's going in that use it will be for specific areaDetector cameras and they don't really use the record types affected.

>What is the bug?

Record types that are based off of calcout can no longer have a constant value written to their input fields and have the value of the correct data field change. For example, writing '5' to a calcout's INPA field no longer changes the A field to 5. This affects the calcout, aCalcout, sCalcout, transform, and luascript records.
--
https://code.launchpad.net/~anj/epics-base/+git/base-7.0/+merge/366247
Your team EPICS Core Developers is requested...

Read more...

Revision history for this message
rivers (rivers) wrote :
Download full text (4.4 KiB)

I believe all of the synApps screens have text-entry widgets for both the INPx and x value fields.

I checked the auto settings .req files and they save both the INPx and x value fields in autosave.

Changing the behavior so constant link fields cannot be changed at run-time should thus not break existing databases, since the x value fields will be autorestored. It would require training users to enter new constants into the value fields, not INP fields.

Mark

-----Original Message-----
From: <email address hidden> <email address hidden> On Behalf Of Andrew Johnson via Core-talk
Sent: Tuesday, April 23, 2019 10:57 AM
To: Andrew Johnson <email address hidden>
Subject: Re: [Merge] ~anj/epics-base/+git/base-7.0:fix-1824277 into epics-base:7.0

Hi Keenan,

Does synApps have GUI screens that have a text-entry widget for the INP fields but only a text-display widget for the corresponding value field on these record types? That's the other case I'm most concerned about.

- Andrew

On 4/23/19 10:41 AM, Lang, Keenan C. wrote:

>From a scan of our IOC's, it crops up in user calcs, pid calculations and limits, slit transforms, bpm calculations, cmdReply formatting, shutter arming, and a database for fastshutter limits.

________________________________
From: Mooney, Tim M.
Sent: Tuesday, April 23, 2019 10:11:53 AM
To: Johnson, Andrew N.; <email address hidden><mailto:<email address hidden>>; Lang, Keenan C.; <email address hidden><mailto:<email address hidden>>
Subject: Re: [Merge] ~anj/epics-base/+git/base-7.0:fix-1824277 into epics-base:7.0

>>What is the bug?

>Record types that are based off of calcout can no longer have a constant value written to their input fields and have the >value of the correct data field change. For example, writing '5' to a calcout's INPA field no longer changes the A field to >5. This affects the calcout, aCalcout, sCalcout, transform, and luascript records.

I'm sure I've never written a database that relies on this behavior.

Tim Mooney (<email address hidden><mailto:<email address hidden>>) (630)252-5417 Beamline Controls Group (www.aps.anl.gov<http://www.aps.anl.gov>)
Advanced Photon Source, Argonne National Lab

________________________________
From: <email address hidden><mailto:<email address hidden>> <email address hidden><mailto:<email address hidden>> on behalf of Keenan Lang via Core-talk <email address hidden><mailto:<email address hidden>>
Sent: Tuesday, April 23, 2019 9:43 AM
To: Johnson, Andrew N.
Subject: Re: [Merge] ~anj/epics-base/+git/base-7.0:fix-1824277 into epics-base:7.0

>Keenan Lang is working to release synApps 6.1 by the end of May (see
>his question at
>https://github.com/epics-modules/iocStats/issues/33#issuecomment-484995
>327), but I don't know if that is intended to support EPICS 7

I don't think this release will officially support EPICS 7, but I am definitely going to try to start getting all of our code compliant with newer versions. That's what ended up finding this issue as I was updating some of my own code for the changes to links.

>I understand this seems like a rush, but a synApps release str...

Read more...

Revision history for this message
Keenan Lang (klang) wrote :

>Changing the behavior so constant link fields cannot be changed at run-time

That's already how it is working in 3.16 onward. This change would be to restore the previous operation.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

Looking through the new code for dbPutFieldLink(), I'm suspicious that the allocated 'pdbaddr' would be leaked in some (probably error) cases. This may already be possible. Good practice might be to set 'pdbaddr=NULL' when it is consumed by dbAddLink(), then add a 'free(pdbaddr)' to the cleanup step. Also need to make dbAddLink() free it on error.

review: Needs Fixing
Revision history for this message
mdavidsaver (mdavidsaver) wrote :

Looking at this again. What concerns me is that dbPutFieldLink() sits at the conjunction of three external interfaces (rset, dset, and lset). It isn't clear to me what the expectations and allowed actions are. Particularly of link support. If this is clear to anyone (Andrew) I would find it quite helpful to see some comments added to describe the state(s) that the target link may be in at, and after, the external interface calls.

As an example. I've just noticed that dbDbRemoveLink() always free()s the memory pointed to be 'plink->value.pv_link.pvt', but doesn't always NULL the pointer.

Revision history for this message
Andrew Johnson (anj) wrote :

Group 10/4: Needs more thorough testing, extra cases. AJ to re-evaluate the fix.

review: Needs Fixing
Revision history for this message
Dirk Zimoch (dirk.zimoch) wrote :

Any progress on this issue in the last two years? I had users this Monday complaining that upgrading their IOCs from 3.14.12 to EPICS 7.0.5 broke their application.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

@Dirk, This MR is waiting on review, expanded test coverage, and test results. Can you help with these? Does this MR fix the specific "breakage" seen by your users?

Revision history for this message
Dirk Zimoch (dirk.zimoch) wrote :

The specific breakage was: Changing calcout.INPx to a differenct constant does not work any longer.
The application is a user configureable calc where the users enter CALC and set INPx fields to other records or to constants on the fly. The very use case this fix has been made for. Thus I guess it will work, but before trying it, I wanted to know why it is on hold (priority "high", "In Progress", "needs fixing") for so long. Any severe doubts that it will work?

Revision history for this message
Andrew Johnson (anj) wrote :

@Dirk That does sound like exactly the issue that I was working on fixing. My proposal for the fix was questioned by the other Core developers with the concerns described above, but due to the demands of my APS Upgrade development work I just haven't had time to address them yet. If you have time to pick this up I'd be very happy for you to do that.

There is one merge/rebase conflict in dbAccess.c which needs an understanding of both this branch and the other changes that have happened for it to be fixed (replacing pdbaddr with chan in the call to dbAddLink()), that will probably require some additional changes outside of the conflicting area. I suggest you first rebase this branch onto 7.0 and resolve that issue, then push it to your GitHub repo. Once you've started a PR there instead I will close this MR – the ability to review code and make comments works much better there.

review: Needs Fixing
Revision history for this message
Dirk Zimoch (dirk.zimoch) wrote :

I had re-based the change onto the latest 7.0 yesterday, including resolving the conflict in dbAccess.c which came from the change to use dbChannel.I have addressed the issue Michael mentioned:
"Good practice might be to set 'pdbaddr=NULL' when it is consumed by dbAddLink(), then add a 'free(pdbaddr)' to the cleanup step. Also need to make dbAddLink() free it on error."
I already did exactly that when changing dbAccess to used dbChannel and resolved that now in the merge.

It builds and fixes the problem with calcout. Of course it does not magically provide the same feature to all other records, e.g. calc. I still have to understand why the changes in *SoftCallback supports are needed.

Revision history for this message
Dirk Zimoch (dirk.zimoch) wrote :
Revision history for this message
Andrew Johnson (anj) wrote :

Rebased again (because I'd forgotten that Dirk already did it once, and the number of tests in regressTest.c has gone up since) to https://github.com/anjohnson/epics-base/tree/fix-1824277
I will use that branch to address Michael's requests for additional comments.

Revision history for this message
Andrew Johnson (anj) wrote :

Needs a PR on GitHub when ready for further review.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/documentation/RELEASE_NOTES.html b/documentation/RELEASE_NOTES.html
2index 62a70a6..f8e5499 100644
3--- a/documentation/RELEASE_NOTES.html
4+++ b/documentation/RELEASE_NOTES.html
5@@ -30,6 +30,18 @@ release.</p>
6
7 -->
8
9+<h3>Fix for input links marked "special"</h3>
10+
11+<p>The calcout record (and a number of synApps record types) marks its input
12+link fields with the attribute <tt>special(SPC_MOD)</tt> and provides code in
13+the record's <tt>special()</tt> routine to reinitialize the related value field
14+whenever the input link field is set to a numeric constant. Unfortunately the
15+changes to the link handling code broke this behaviour (reported as Launchpad
16+<a href="https://bugs.launchpad.net/epics-base/+bug/1824277">bug #1824277</a>)
17+back in the Base 3.16.1 release. This issue has been fixed in Base, although
18+external record types may require some fixing to ensure they are correctly
19+checking for and initializing the link in their <tt>special()</tt> routine.</p>
20+
21 <h3>Build System changes</h3>
22
23 <ul>
24diff --git a/modules/database/src/ioc/db/dbAccess.c b/modules/database/src/ioc/db/dbAccess.c
25index be81d22..35f19e2 100644
26--- a/modules/database/src/ioc/db/dbAccess.c
27+++ b/modules/database/src/ioc/db/dbAccess.c
28@@ -1140,8 +1140,6 @@ static long dbPutFieldLink(DBADDR *paddr,
29
30 if (!status) status = dbSetLink(plink, &link_info, new_devsup);
31
32- if (!status && special) status = dbPutSpecial(paddr, 1);
33-
34 if (status) {
35 if (isDevLink) {
36 precord->dset = NULL;
37@@ -1150,28 +1148,57 @@ static long dbPutFieldLink(DBADDR *paddr,
38 goto postScanEvent;
39 }
40
41- if (isDevLink) {
42+ /* We need to initialize any links with a link support layer, i.e.
43+ * any CONSTANT, JSON_LINK, or PV_LINK types. However for a PV_LINK
44+ * when isDevLink is set (i.e. this is the record's INP or OUT link)
45+ * we must wait until after calling dsxt->add_record(). This allows
46+ * the Async Soft Channel input supports to change it to a PN_LINK.
47+ * For other cases we initialize the link before the second call to
48+ * dbPutSpecial() because some record types such as calcout need to
49+ * be able to call link support methods from prset->special().
50+ */
51+
52+ switch (plink->type) { /* New type */
53+ case PV_LINK:
54+ if (isDevLink)
55+ break;
56+ /* else fall through */
57+ case CONSTANT:
58+ case JSON_LINK:
59+ dbAddLink(&locker, plink, pfldDes->field_type, pdbaddr);
60+ }
61+
62+ if (special) status = dbPutSpecial(paddr, 1);
63+
64+ if (!status && isDevLink) {
65 precord->dpvt = NULL;
66 precord->dset = new_dset;
67 precord->pact = FALSE;
68
69 status = new_dsxt->add_record(precord);
70- if (status) {
71+ }
72+
73+ if (status) {
74+ if (isDevLink) {
75 precord->dset = NULL;
76 precord->pact = TRUE;
77- goto postScanEvent;
78 }
79+ goto postScanEvent;
80 }
81
82 switch (plink->type) { /* New link type */
83- case PV_LINK:
84 case CONSTANT:
85+ case CA_LINK:
86+ case DB_LINK:
87+ case PN_LINK:
88 case JSON_LINK:
89- dbAddLink(&locker, plink, pfldDes->field_type, pdbaddr);
90 break;
91
92- case DB_LINK:
93- case CA_LINK:
94+ case PV_LINK:
95+ if (isDevLink)
96+ dbAddLink(&locker, plink, pfldDes->field_type, pdbaddr);
97+ break;
98+
99 case MACRO_LINK:
100 break; /* should never get here */
101
102@@ -1180,7 +1207,6 @@ static long dbPutFieldLink(DBADDR *paddr,
103 status = S_db_badHWaddr;
104 goto postScanEvent;
105 }
106- break;
107 }
108 db_post_events(precord, plink, DBE_VALUE | DBE_LOG);
109
110diff --git a/modules/database/src/std/dev/devAiSoftCallback.c b/modules/database/src/std/dev/devAiSoftCallback.c
111index cf38c87..872684c 100644
112--- a/modules/database/src/std/dev/devAiSoftCallback.c
113+++ b/modules/database/src/std/dev/devAiSoftCallback.c
114@@ -80,7 +80,7 @@ static long add_record(dbCommon *pcommon)
115 devPvt *pdevPvt;
116 processNotify *ppn;
117
118- if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))
119+ if (plink->type == CONSTANT)
120 return 0;
121
122 if (plink->type != PV_LINK) {
123diff --git a/modules/database/src/std/dev/devBiSoftCallback.c b/modules/database/src/std/dev/devBiSoftCallback.c
124index 3144600..88b96f8 100644
125--- a/modules/database/src/std/dev/devBiSoftCallback.c
126+++ b/modules/database/src/std/dev/devBiSoftCallback.c
127@@ -78,7 +78,7 @@ static long add_record(dbCommon *pcommon)
128 devPvt *pdevPvt;
129 processNotify *ppn;
130
131- if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))
132+ if (plink->type == CONSTANT)
133 return 0;
134
135 if (plink->type != PV_LINK) {
136diff --git a/modules/database/src/std/dev/devI64inSoftCallback.c b/modules/database/src/std/dev/devI64inSoftCallback.c
137index 9eb5656..43c48f6 100644
138--- a/modules/database/src/std/dev/devI64inSoftCallback.c
139+++ b/modules/database/src/std/dev/devI64inSoftCallback.c
140@@ -78,7 +78,7 @@ static long add_record(dbCommon *pcommon)
141 devPvt *pdevPvt;
142 processNotify *ppn;
143
144- if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))
145+ if (plink->type == CONSTANT)
146 return 0;
147
148 if (plink->type != PV_LINK) {
149diff --git a/modules/database/src/std/dev/devLiSoftCallback.c b/modules/database/src/std/dev/devLiSoftCallback.c
150index caab523..a95f989 100644
151--- a/modules/database/src/std/dev/devLiSoftCallback.c
152+++ b/modules/database/src/std/dev/devLiSoftCallback.c
153@@ -78,7 +78,7 @@ static long add_record(dbCommon *pcommon)
154 devPvt *pdevPvt;
155 processNotify *ppn;
156
157- if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))
158+ if (plink->type == CONSTANT)
159 return 0;
160
161 if (plink->type != PV_LINK) {
162diff --git a/modules/database/src/std/dev/devMbbiDirectSoftCallback.c b/modules/database/src/std/dev/devMbbiDirectSoftCallback.c
163index d785f73..947b9c0 100644
164--- a/modules/database/src/std/dev/devMbbiDirectSoftCallback.c
165+++ b/modules/database/src/std/dev/devMbbiDirectSoftCallback.c
166@@ -78,7 +78,7 @@ static long add_record(dbCommon *pcommon)
167 devPvt *pdevPvt;
168 processNotify *ppn;
169
170- if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))
171+ if (plink->type == CONSTANT)
172 return 0;
173
174 if (plink->type != PV_LINK) {
175diff --git a/modules/database/src/std/dev/devMbbiSoftCallback.c b/modules/database/src/std/dev/devMbbiSoftCallback.c
176index 3796bce..5b3370f 100644
177--- a/modules/database/src/std/dev/devMbbiSoftCallback.c
178+++ b/modules/database/src/std/dev/devMbbiSoftCallback.c
179@@ -78,7 +78,7 @@ static long add_record(dbCommon *pcommon)
180 devPvt *pdevPvt;
181 processNotify *ppn;
182
183- if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))
184+ if (plink->type == CONSTANT)
185 return 0;
186
187 if (plink->type != PV_LINK) {
188diff --git a/modules/database/src/std/dev/devSiSoftCallback.c b/modules/database/src/std/dev/devSiSoftCallback.c
189index 8f67988..2b0922c 100644
190--- a/modules/database/src/std/dev/devSiSoftCallback.c
191+++ b/modules/database/src/std/dev/devSiSoftCallback.c
192@@ -80,7 +80,7 @@ static long add_record(dbCommon *pcommon)
193 devPvt *pdevPvt;
194 processNotify *ppn;
195
196- if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))
197+ if (plink->type == CONSTANT)
198 return 0;
199
200 if (plink->type != PV_LINK) {
201diff --git a/modules/database/test/std/rec/Makefile b/modules/database/test/std/rec/Makefile
202index 185c2c0..cd7316a 100644
203--- a/modules/database/test/std/rec/Makefile
204+++ b/modules/database/test/std/rec/Makefile
205@@ -113,8 +113,8 @@ regressTest_DBD += base.dbd
206 TESTPROD_HOST += regressTest
207 regressTest_SRCS += regressTest.c
208 regressTest_SRCS += regressTest_registerRecordDeviceDriver.cpp
209-TESTFILES += $(COMMON_DIR)/regressTest.dbd ../regressArray1.db ../regressHex.db ../regressLinkMS.db
210-TESTFILES += ../badCaLink.db
211+TESTFILES += $(COMMON_DIR)/regressTest.dbd ../regressArray1.db ../regressHex.db
212+TESTFILES += ../regressLinkMS.db ../badCaLink.db ../regressCalcout.db
213 TESTS += regressTest
214
215 TARGETS += $(COMMON_DIR)/simmTest.dbd
216diff --git a/modules/database/test/std/rec/regressCalcout.db b/modules/database/test/std/rec/regressCalcout.db
217new file mode 100644
218index 0000000..8896131
219--- /dev/null
220+++ b/modules/database/test/std/rec/regressCalcout.db
221@@ -0,0 +1,27 @@
222+record(calcout, "cout") {
223+ field(INPA, "99")
224+ field(INPB, "99")
225+ field(INPC, "99")
226+ field(INPD, "99")
227+}
228+record(ai, "ain") {
229+ field(DTYP, "Async Soft Channel")
230+}
231+record(bi, "bin") {
232+ field(DTYP, "Async Soft Channel")
233+}
234+record(int64in, "iin") {
235+ field(DTYP, "Async Soft Channel")
236+}
237+record(longin, "lin") {
238+ field(DTYP, "Async Soft Channel")
239+}
240+record(mbbi, "min") {
241+ field(DTYP, "Async Soft Channel")
242+}
243+record(mbbiDirect, "din") {
244+ field(DTYP, "Async Soft Channel")
245+}
246+record(stringin, "sin") {
247+ field(DTYP, "Async Soft Channel")
248+}
249diff --git a/modules/database/test/std/rec/regressTest.c b/modules/database/test/std/rec/regressTest.c
250index 6614639..6809ccb 100644
251--- a/modules/database/test/std/rec/regressTest.c
252+++ b/modules/database/test/std/rec/regressTest.c
253@@ -135,15 +135,63 @@ void testCADisconn(void)
254 testdbPutFieldOk("ai:disconn.PROC", DBF_LONG, 1);
255 testdbGetFieldEqual("ai:disconn.SEVR", DBF_LONG, INVALID_ALARM);
256 testdbGetFieldEqual("ai:disconn.STAT", DBF_LONG, LINK_ALARM);
257+
258+ testIocShutdownOk();
259+ testdbCleanup();
260+}
261+
262+/* lp:1824277 Regression in calcout, setting links at runtime */
263+static void
264+testSpecialLinks(void)
265+{
266+ testDiag("In testSpecialLinks()");
267+
268+ startRegressTestIoc("regressCalcout.db");
269+
270+ testdbPutFieldOk("cout.INPA", DBF_STRING, "10");
271+ testdbGetFieldEqual("cout.A", DBF_LONG, 10);
272+ testdbGetFieldEqual("cout.INAV", DBF_LONG, calcoutINAV_CON);
273+ testdbPutFieldOk("cout.A", DBF_LONG, 15);
274+ testdbPutFieldOk("cout.INPB", DBF_STRING, "{\"const\":20}");
275+ testdbGetFieldEqual("cout.B", DBF_LONG, 20);
276+ testdbGetFieldEqual("cout.INBV", DBF_LONG, calcoutINAV_CON);
277+ testdbPutFieldOk("cout.B", DBF_LONG, 25);
278+ testdbPutFieldOk("cout.INPC", DBF_STRING, "cout.A");
279+ testdbGetFieldEqual("cout.C", DBF_LONG, 99);
280+ testdbGetFieldEqual("cout.INCV", DBF_LONG, calcoutINAV_LOC);
281+ testdbPutFieldOk("cout.INPD", DBF_STRING, "no-such-pv");
282+ testdbGetFieldEqual("cout.D", DBF_LONG, 99);
283+ testdbGetFieldEqual("cout.INDV", DBF_LONG, calcoutINAV_EXT_NC);
284+
285+ eltc(0);
286+ testdbPutFieldOk("ain.INP", DBF_STRING, "cout");
287+ testdbPutFieldFail(S_db_badField, "ain.INP", DBF_STRING, "{\"const\":1}");
288+ testdbPutFieldOk("bin.INP", DBF_STRING, "cout");
289+ testdbPutFieldFail(S_db_badField, "bin.INP", DBF_STRING, "{\"const\":1}");
290+ testdbPutFieldOk("iin.INP", DBF_STRING, "cout");
291+ testdbPutFieldFail(S_db_badField, "iin.INP", DBF_STRING, "{\"const\":1}");
292+ testdbPutFieldOk("lin.INP", DBF_STRING, "cout");
293+ testdbPutFieldFail(S_db_badField, "lin.INP", DBF_STRING, "{\"const\":1}");
294+ testdbPutFieldOk("min.INP", DBF_STRING, "cout");
295+ testdbPutFieldFail(S_db_badField, "min.INP", DBF_STRING, "{\"const\":1}");
296+ testdbPutFieldOk("din.INP", DBF_STRING, "cout");
297+ testdbPutFieldFail(S_db_badField, "din.INP", DBF_STRING, "{\"const\":1}");
298+ testdbPutFieldOk("sin.INP", DBF_STRING, "cout");
299+ testdbPutFieldFail(S_db_badField, "sin.INP", DBF_STRING, "{\"const\":1}");
300+ eltc(1);
301+
302+ testIocShutdownOk();
303+ testdbCleanup();
304 }
305
306
307 MAIN(regressTest)
308 {
309- testPlan(34);
310+ testPlan(62);
311 testArrayLength1();
312 testHexConstantLinks();
313 testLinkMS();
314 testCADisconn();
315+ testSpecialLinks();
316 return testDone();
317 }

Subscribers

People subscribed via source and target branches