EPICS Base

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

  1. Git
  2. lp:~anj/epics-base/+git/base-7.0
  3. fix-1824277
  4. Merge into 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.
~anj/epics-base/+git/base-7.0:fix-1824277 updated
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)?

Reply
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?

Reply
~anj/epics-base/+git/base-7.0:fix-1824277 updated
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.

Reply
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.

Reply
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.

Reply
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.

Reply
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.

Reply
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...

Reply
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...

Reply
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.

Reply
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
Reply
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.

Reply
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
Reply
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.

Reply
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?

Reply
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?

Reply
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
Reply
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.

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

The rebased version is here: https://github.com/dirk-zimoch/epics-base/tree/fix-1824277

Reply
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.

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

Needs a PR on GitHub when ready for further review.

Reply

Unmerged commits

a50a0b2... by Andrew Johnson

Add tests confirming that input value fields still writable

Having a constant INP link doesn't break that.

27ba6a0... by Andrew Johnson

Release Notes entry with link to bug.

eb9baf7... by Andrew Johnson

Fixes for Async Soft Channel input device support

The add_record() routine wasn't properly checking for unsupported
link types, resulting in an assertion failure on IOC exit if the
record was left in a "bad" state.

e2ad9e5... by Andrew Johnson

Add regression tests for Async Soft Channel input links

Caused by the previous fix, but this is repairable.

5cd35ed... by Andrew Johnson

Fix for regression in link initialization

Fixes lp: #1824277

Shows a problem with Async Soft Channel input support though.

2d0bb39... by Andrew Johnson

Add regression test for lp: #1824277

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/documentation/RELEASE_NOTES.html b/documentation/RELEASE_NOTES.html
index 62a70a6..f8e5499 100644
--- a/documentation/RELEASE_NOTES.html
+++ b/documentation/RELEASE_NOTES.html
@@ -30,6 +30,18 @@ release.</p>
3030
31-->31-->
3232
33<h3>Fix for input links marked "special"</h3>
34
35<p>The calcout record (and a number of synApps record types) marks its input
36link fields with the attribute <tt>special(SPC_MOD)</tt> and provides code in
37the record's <tt>special()</tt> routine to reinitialize the related value field
38whenever the input link field is set to a numeric constant. Unfortunately the
39changes to the link handling code broke this behaviour (reported as Launchpad
40<a href="https://bugs.launchpad.net/epics-base/+bug/1824277">bug #1824277</a>)
41back in the Base 3.16.1 release. This issue has been fixed in Base, although
42external record types may require some fixing to ensure they are correctly
43checking for and initializing the link in their <tt>special()</tt> routine.</p>
44
33<h3>Build System changes</h3>45<h3>Build System changes</h3>
3446
35<ul>47<ul>
diff --git a/modules/database/src/ioc/db/dbAccess.c b/modules/database/src/ioc/db/dbAccess.c
index be81d22..35f19e2 100644
--- a/modules/database/src/ioc/db/dbAccess.c
+++ b/modules/database/src/ioc/db/dbAccess.c
@@ -1140,8 +1140,6 @@ static long dbPutFieldLink(DBADDR *paddr,
11401140
1141 if (!status) status = dbSetLink(plink, &link_info, new_devsup);1141 if (!status) status = dbSetLink(plink, &link_info, new_devsup);
11421142
1143 if (!status && special) status = dbPutSpecial(paddr, 1);
1144
1145 if (status) {1143 if (status) {
1146 if (isDevLink) {1144 if (isDevLink) {
1147 precord->dset = NULL;1145 precord->dset = NULL;
@@ -1150,28 +1148,57 @@ static long dbPutFieldLink(DBADDR *paddr,
1150 goto postScanEvent;1148 goto postScanEvent;
1151 }1149 }
11521150
1153 if (isDevLink) {1151 /* We need to initialize any links with a link support layer, i.e.
1152 * any CONSTANT, JSON_LINK, or PV_LINK types. However for a PV_LINK
1153 * when isDevLink is set (i.e. this is the record's INP or OUT link)
1154 * we must wait until after calling dsxt->add_record(). This allows
1155 * the Async Soft Channel input supports to change it to a PN_LINK.
1156 * For other cases we initialize the link before the second call to
1157 * dbPutSpecial() because some record types such as calcout need to
1158 * be able to call link support methods from prset->special().
1159 */
1160
1161 switch (plink->type) { /* New type */
1162 case PV_LINK:
1163 if (isDevLink)
1164 break;
1165 /* else fall through */
1166 case CONSTANT:
1167 case JSON_LINK:
1168 dbAddLink(&locker, plink, pfldDes->field_type, pdbaddr);
1169 }
1170
1171 if (special) status = dbPutSpecial(paddr, 1);
1172
1173 if (!status && isDevLink) {
1154 precord->dpvt = NULL;1174 precord->dpvt = NULL;
1155 precord->dset = new_dset;1175 precord->dset = new_dset;
1156 precord->pact = FALSE;1176 precord->pact = FALSE;
11571177
1158 status = new_dsxt->add_record(precord);1178 status = new_dsxt->add_record(precord);
1159 if (status) {1179 }
1180
1181 if (status) {
1182 if (isDevLink) {
1160 precord->dset = NULL;1183 precord->dset = NULL;
1161 precord->pact = TRUE;1184 precord->pact = TRUE;
1162 goto postScanEvent;
1163 }1185 }
1186 goto postScanEvent;
1164 }1187 }
11651188
1166 switch (plink->type) { /* New link type */1189 switch (plink->type) { /* New link type */
1167 case PV_LINK:
1168 case CONSTANT:1190 case CONSTANT:
1191 case CA_LINK:
1192 case DB_LINK:
1193 case PN_LINK:
1169 case JSON_LINK:1194 case JSON_LINK:
1170 dbAddLink(&locker, plink, pfldDes->field_type, pdbaddr);
1171 break;1195 break;
11721196
1173 case DB_LINK:1197 case PV_LINK:
1174 case CA_LINK:1198 if (isDevLink)
1199 dbAddLink(&locker, plink, pfldDes->field_type, pdbaddr);
1200 break;
1201
1175 case MACRO_LINK:1202 case MACRO_LINK:
1176 break; /* should never get here */1203 break; /* should never get here */
11771204
@@ -1180,7 +1207,6 @@ static long dbPutFieldLink(DBADDR *paddr,
1180 status = S_db_badHWaddr;1207 status = S_db_badHWaddr;
1181 goto postScanEvent;1208 goto postScanEvent;
1182 }1209 }
1183 break;
1184 }1210 }
1185 db_post_events(precord, plink, DBE_VALUE | DBE_LOG);1211 db_post_events(precord, plink, DBE_VALUE | DBE_LOG);
11861212
diff --git a/modules/database/src/std/dev/devAiSoftCallback.c b/modules/database/src/std/dev/devAiSoftCallback.c
index cf38c87..872684c 100644
--- a/modules/database/src/std/dev/devAiSoftCallback.c
+++ b/modules/database/src/std/dev/devAiSoftCallback.c
@@ -80,7 +80,7 @@ static long add_record(dbCommon *pcommon)
80 devPvt *pdevPvt;80 devPvt *pdevPvt;
81 processNotify *ppn;81 processNotify *ppn;
8282
83 if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))83 if (plink->type == CONSTANT)
84 return 0;84 return 0;
8585
86 if (plink->type != PV_LINK) {86 if (plink->type != PV_LINK) {
diff --git a/modules/database/src/std/dev/devBiSoftCallback.c b/modules/database/src/std/dev/devBiSoftCallback.c
index 3144600..88b96f8 100644
--- a/modules/database/src/std/dev/devBiSoftCallback.c
+++ b/modules/database/src/std/dev/devBiSoftCallback.c
@@ -78,7 +78,7 @@ static long add_record(dbCommon *pcommon)
78 devPvt *pdevPvt;78 devPvt *pdevPvt;
79 processNotify *ppn;79 processNotify *ppn;
8080
81 if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))81 if (plink->type == CONSTANT)
82 return 0;82 return 0;
8383
84 if (plink->type != PV_LINK) {84 if (plink->type != PV_LINK) {
diff --git a/modules/database/src/std/dev/devI64inSoftCallback.c b/modules/database/src/std/dev/devI64inSoftCallback.c
index 9eb5656..43c48f6 100644
--- a/modules/database/src/std/dev/devI64inSoftCallback.c
+++ b/modules/database/src/std/dev/devI64inSoftCallback.c
@@ -78,7 +78,7 @@ static long add_record(dbCommon *pcommon)
78 devPvt *pdevPvt;78 devPvt *pdevPvt;
79 processNotify *ppn;79 processNotify *ppn;
8080
81 if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))81 if (plink->type == CONSTANT)
82 return 0;82 return 0;
8383
84 if (plink->type != PV_LINK) {84 if (plink->type != PV_LINK) {
diff --git a/modules/database/src/std/dev/devLiSoftCallback.c b/modules/database/src/std/dev/devLiSoftCallback.c
index caab523..a95f989 100644
--- a/modules/database/src/std/dev/devLiSoftCallback.c
+++ b/modules/database/src/std/dev/devLiSoftCallback.c
@@ -78,7 +78,7 @@ static long add_record(dbCommon *pcommon)
78 devPvt *pdevPvt;78 devPvt *pdevPvt;
79 processNotify *ppn;79 processNotify *ppn;
8080
81 if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))81 if (plink->type == CONSTANT)
82 return 0;82 return 0;
8383
84 if (plink->type != PV_LINK) {84 if (plink->type != PV_LINK) {
diff --git a/modules/database/src/std/dev/devMbbiDirectSoftCallback.c b/modules/database/src/std/dev/devMbbiDirectSoftCallback.c
index d785f73..947b9c0 100644
--- a/modules/database/src/std/dev/devMbbiDirectSoftCallback.c
+++ b/modules/database/src/std/dev/devMbbiDirectSoftCallback.c
@@ -78,7 +78,7 @@ static long add_record(dbCommon *pcommon)
78 devPvt *pdevPvt;78 devPvt *pdevPvt;
79 processNotify *ppn;79 processNotify *ppn;
8080
81 if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))81 if (plink->type == CONSTANT)
82 return 0;82 return 0;
8383
84 if (plink->type != PV_LINK) {84 if (plink->type != PV_LINK) {
diff --git a/modules/database/src/std/dev/devMbbiSoftCallback.c b/modules/database/src/std/dev/devMbbiSoftCallback.c
index 3796bce..5b3370f 100644
--- a/modules/database/src/std/dev/devMbbiSoftCallback.c
+++ b/modules/database/src/std/dev/devMbbiSoftCallback.c
@@ -78,7 +78,7 @@ static long add_record(dbCommon *pcommon)
78 devPvt *pdevPvt;78 devPvt *pdevPvt;
79 processNotify *ppn;79 processNotify *ppn;
8080
81 if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))81 if (plink->type == CONSTANT)
82 return 0;82 return 0;
8383
84 if (plink->type != PV_LINK) {84 if (plink->type != PV_LINK) {
diff --git a/modules/database/src/std/dev/devSiSoftCallback.c b/modules/database/src/std/dev/devSiSoftCallback.c
index 8f67988..2b0922c 100644
--- a/modules/database/src/std/dev/devSiSoftCallback.c
+++ b/modules/database/src/std/dev/devSiSoftCallback.c
@@ -80,7 +80,7 @@ static long add_record(dbCommon *pcommon)
80 devPvt *pdevPvt;80 devPvt *pdevPvt;
81 processNotify *ppn;81 processNotify *ppn;
8282
83 if (dbLinkIsDefined(plink) && dbLinkIsConstant(plink))83 if (plink->type == CONSTANT)
84 return 0;84 return 0;
8585
86 if (plink->type != PV_LINK) {86 if (plink->type != PV_LINK) {
diff --git a/modules/database/test/std/rec/Makefile b/modules/database/test/std/rec/Makefile
index 185c2c0..cd7316a 100644
--- a/modules/database/test/std/rec/Makefile
+++ b/modules/database/test/std/rec/Makefile
@@ -113,8 +113,8 @@ regressTest_DBD += base.dbd
113TESTPROD_HOST += regressTest113TESTPROD_HOST += regressTest
114regressTest_SRCS += regressTest.c114regressTest_SRCS += regressTest.c
115regressTest_SRCS += regressTest_registerRecordDeviceDriver.cpp115regressTest_SRCS += regressTest_registerRecordDeviceDriver.cpp
116TESTFILES += $(COMMON_DIR)/regressTest.dbd ../regressArray1.db ../regressHex.db ../regressLinkMS.db116TESTFILES += $(COMMON_DIR)/regressTest.dbd ../regressArray1.db ../regressHex.db
117TESTFILES += ../badCaLink.db117TESTFILES += ../regressLinkMS.db ../badCaLink.db ../regressCalcout.db
118TESTS += regressTest118TESTS += regressTest
119119
120TARGETS += $(COMMON_DIR)/simmTest.dbd120TARGETS += $(COMMON_DIR)/simmTest.dbd
diff --git a/modules/database/test/std/rec/regressCalcout.db b/modules/database/test/std/rec/regressCalcout.db
121new file mode 100644121new file mode 100644
index 0000000..8896131
--- /dev/null
+++ b/modules/database/test/std/rec/regressCalcout.db
@@ -0,0 +1,27 @@
1record(calcout, "cout") {
2 field(INPA, "99")
3 field(INPB, "99")
4 field(INPC, "99")
5 field(INPD, "99")
6}
7record(ai, "ain") {
8 field(DTYP, "Async Soft Channel")
9}
10record(bi, "bin") {
11 field(DTYP, "Async Soft Channel")
12}
13record(int64in, "iin") {
14 field(DTYP, "Async Soft Channel")
15}
16record(longin, "lin") {
17 field(DTYP, "Async Soft Channel")
18}
19record(mbbi, "min") {
20 field(DTYP, "Async Soft Channel")
21}
22record(mbbiDirect, "din") {
23 field(DTYP, "Async Soft Channel")
24}
25record(stringin, "sin") {
26 field(DTYP, "Async Soft Channel")
27}
diff --git a/modules/database/test/std/rec/regressTest.c b/modules/database/test/std/rec/regressTest.c
index 6614639..6809ccb 100644
--- a/modules/database/test/std/rec/regressTest.c
+++ b/modules/database/test/std/rec/regressTest.c
@@ -135,15 +135,63 @@ void testCADisconn(void)
135 testdbPutFieldOk("ai:disconn.PROC", DBF_LONG, 1);135 testdbPutFieldOk("ai:disconn.PROC", DBF_LONG, 1);
136 testdbGetFieldEqual("ai:disconn.SEVR", DBF_LONG, INVALID_ALARM);136 testdbGetFieldEqual("ai:disconn.SEVR", DBF_LONG, INVALID_ALARM);
137 testdbGetFieldEqual("ai:disconn.STAT", DBF_LONG, LINK_ALARM);137 testdbGetFieldEqual("ai:disconn.STAT", DBF_LONG, LINK_ALARM);
138
139 testIocShutdownOk();
140 testdbCleanup();
141}
142
143/* lp:1824277 Regression in calcout, setting links at runtime */
144static void
145testSpecialLinks(void)
146{
147 testDiag("In testSpecialLinks()");
148
149 startRegressTestIoc("regressCalcout.db");
150
151 testdbPutFieldOk("cout.INPA", DBF_STRING, "10");
152 testdbGetFieldEqual("cout.A", DBF_LONG, 10);
153 testdbGetFieldEqual("cout.INAV", DBF_LONG, calcoutINAV_CON);
154 testdbPutFieldOk("cout.A", DBF_LONG, 15);
155 testdbPutFieldOk("cout.INPB", DBF_STRING, "{\"const\":20}");
156 testdbGetFieldEqual("cout.B", DBF_LONG, 20);
157 testdbGetFieldEqual("cout.INBV", DBF_LONG, calcoutINAV_CON);
158 testdbPutFieldOk("cout.B", DBF_LONG, 25);
159 testdbPutFieldOk("cout.INPC", DBF_STRING, "cout.A");
160 testdbGetFieldEqual("cout.C", DBF_LONG, 99);
161 testdbGetFieldEqual("cout.INCV", DBF_LONG, calcoutINAV_LOC);
162 testdbPutFieldOk("cout.INPD", DBF_STRING, "no-such-pv");
163 testdbGetFieldEqual("cout.D", DBF_LONG, 99);
164 testdbGetFieldEqual("cout.INDV", DBF_LONG, calcoutINAV_EXT_NC);
165
166 eltc(0);
167 testdbPutFieldOk("ain.INP", DBF_STRING, "cout");
168 testdbPutFieldFail(S_db_badField, "ain.INP", DBF_STRING, "{\"const\":1}");
169 testdbPutFieldOk("bin.INP", DBF_STRING, "cout");
170 testdbPutFieldFail(S_db_badField, "bin.INP", DBF_STRING, "{\"const\":1}");
171 testdbPutFieldOk("iin.INP", DBF_STRING, "cout");
172 testdbPutFieldFail(S_db_badField, "iin.INP", DBF_STRING, "{\"const\":1}");
173 testdbPutFieldOk("lin.INP", DBF_STRING, "cout");
174 testdbPutFieldFail(S_db_badField, "lin.INP", DBF_STRING, "{\"const\":1}");
175 testdbPutFieldOk("min.INP", DBF_STRING, "cout");
176 testdbPutFieldFail(S_db_badField, "min.INP", DBF_STRING, "{\"const\":1}");
177 testdbPutFieldOk("din.INP", DBF_STRING, "cout");
178 testdbPutFieldFail(S_db_badField, "din.INP", DBF_STRING, "{\"const\":1}");
179 testdbPutFieldOk("sin.INP", DBF_STRING, "cout");
180 testdbPutFieldFail(S_db_badField, "sin.INP", DBF_STRING, "{\"const\":1}");
181 eltc(1);
182
183 testIocShutdownOk();
184 testdbCleanup();
138}185}
139186
140187
141MAIN(regressTest)188MAIN(regressTest)
142{189{
143 testPlan(34);190 testPlan(62);
144 testArrayLength1();191 testArrayLength1();
145 testHexConstantLinks();192 testHexConstantLinks();
146 testLinkMS();193 testLinkMS();
147 testCADisconn();194 testCADisconn();
195 testSpecialLinks();
148 return testDone();196 return testDone();
149}197}

Subscribers

People subscribed via source and target branches