Add pva links to pvxs #57

simon-ess · 2023-09-12T14:09:26Z

Port PVA links from pva2pva.

TODO

Add atomic as link option.
Switch logger names to pvxs.*.
Test coverage for AfterPut
Test coverage for disconnected link
Fully resolve dbEvent: double cancel causes hang epics-base#423
Interoperability with QSRV1. Automatically disable QSRV2 during iocInit when QSRV1 also present.
Clean up pvxs/iochooks.h.

AppVeyorBot · 2023-09-12T19:55:02Z

❌ Build pvxs 1.0.942 failed (commit 7f9f4b70a8 by @simon-ess)

AppVeyorBot · 2023-09-13T23:13:02Z

❌ Build pvxs 1.0.946 failed (commit 939c51a120 by @simon-ess)

AppVeyorBot · 2023-09-14T22:30:09Z

✅ Build pvxs 1.0.947 completed (commit c0a961e4da by @simon-ess)

AppVeyorBot · 2023-09-15T23:56:04Z

❌ Build pvxs 1.0.949 failed (commit e565a11af6 by @simon-ess)

AppVeyorBot · 2023-09-19T18:36:35Z

❌ Build pvxs 1.0.951 failed (commit 90447de627 by @simon-ess)

mdavidsaver · 2023-10-02T23:59:14Z

I have pushed some additional changes. Including a possible workaround for epics-base/epics-base#423 with older Base (currently all released versions...). I have also reworked the test synchronization code. However, I am still seeing some intermittent test failures in GHA runs.

AppVeyorBot · 2023-10-03T01:18:28Z

❌ Build pvxs 1.0.957 failed (commit dd5698dd47 by @mdavidsaver)

mdavidsaver · 2023-10-11T03:21:45Z

Another update. At this point all tests are passing for me. Although, coverage.sh shows there are a couple of cases which I think need testing.

wrt. the db_cancel_event() issue, I think I understand now what was happening. The trigger was a self-cancel, which should not be possible. However, due to an issue with the lifetime, objects bound into lambda functions were no released immediately when the associated subscription was canceled. This should be fixed by 9b099be. I think this will act as a workaround for the Base issue.

The other lingering issue was an intermittent failure in testSevr() of testpvalink. I think I traced this to an issue with the pvaGlobal_t::channels cache. A cache hit which was processed before an additional data update did not call pvaLink::onTypeChange(), so it was using previous Value s.

AppVeyorBot · 2023-10-11T14:19:12Z

✅ Build pvxs 1.0.960 completed (commit be6492fe26 by @mdavidsaver)

mdavidsaver · 2023-10-31T18:58:52Z

@simon-ess I think I have this PR is a more or less final form. At this point, the blocker to merging is a resolution of epics-base/epics-base#423, which causes testpvalink to fail.

As a remaining task, I am working on some logic to detect when qsrv.dbd from pva2pva has also been included, and will use this to automatically disable QSRV2 during startup. Of course, then absent QSRV2 would then default to automatically starting in full. (like #58)

AppVeyorBot · 2023-10-31T22:57:43Z

❌ Build pvxs 1.0.964 failed (commit 5ab45fc0df by @mdavidsaver)

AppVeyorBot · 2023-11-05T02:45:50Z

✅ Build pvxs 1.0.965 completed (commit ffe5261180 by @mdavidsaver)

simon-ess

Also, one other oddity I noticed: if you have the following as a db file:

record(ai, "FOO") {
    field(INP, {pva: {pv: "BAR", proc: "CPP"}})
}

record(aai, "BAR") {
    field(NELM, 10)
}

then running pvput BAR "[1,2,3]" with an IOC running softIocPVA the linked ai record fetches the first value. However, with softIocPVX it does not.

ioc/pvalink_link.cpp

simon-ess · 2023-11-06T15:56:12Z

ioc/pvalink_channel.cpp

+            atomic_lock = ioc::DBManyLock(atomicrecs);
+            atomic_records = std::move(atomic);
+            nonatomic_records = std::move(nonatomic);
+
+            links_changed = false;
+        }
+
+        update_seq++;
+        update_evt.signal();
+        log_debug_printf(_logger, "%s Sequence point %u\n", key.first.c_str(), update_seq);
+    }
+    // unlock link
+
+    if(!atomic_records.empty()) {
+        ioc::DBManyLocker L(atomic_lock);


This is perhaps an odd pedantic point, and probably out of scope... but aren't these names semantically backwards? It seems to me that DBManyLock should be doing the action, while DBManyLocker is the thing that does the action.

Not that this affects the code, but it gives me a bit of a hiccup on reading, as it looks like the lock is taken on line 416 (and not obviously ever let go), but it is in fact taken on line 430 (and let go shortly thereafter).

Looking at the Git Blame, it seems that we can blame @george-mcintyre...

george-mcintyre · 2023-11-06T17:27:51Z

Comments interspersed below.

On 6 Nov 2023, at 17:09, Simon Rose ***@***.***> wrote: @simon-ess commented on this pull request. Also, one other oddity I noticed: if you have the following as a db file: record(ai, "FOO") { field(INP, {pva: {pv: "BAR", proc: "CPP"}}) } record(aai, "BAR") { field(NELM, 10) } then running pvput BAR "[1,2,3]" with an IOC running softIocPVA the linked ai record fetches the first value. However, with softIocPVX it does not. In ioc/pvalink_link.cpp <#57 (comment)>: > + log_debug_printf(_logger, "%s type change V=%c S=%c N=%c S=%c M=%c\n", + plink->precord->name, + fld_value ? 'Y' : 'N', + fld_seconds ? 'Y' : 'N', + fld_nanoseconds ? 'Y' : 'N', + fld_severity ? 'Y' : 'N', + fld_meta ? 'Y' : 'N'); I'm a bit confused by this debug statement: isn't root["value"] the actual value? This debug message seems to be trying to say that the value/timestamp/etc. are being modified, but that's not how I read the code... In ioc/pvalink_channel.cpp <#57 (comment)>: > + atomic_lock = ioc::DBManyLock(atomicrecs); + atomic_records = std::move(atomic); + nonatomic_records = std::move(nonatomic); + + links_changed = false; + } + + update_seq++; + update_evt.signal(); + log_debug_printf(_logger, "%s Sequence point %u\n", key.first.c_str(), update_seq); + } + // unlock link + + if(!atomic_records.empty()) { + ioc::DBManyLocker L(atomic_lock); This is perhaps an odd pedantic point, and probably out of scope... but aren't these names semantically backwards? It seems to me that DBManyLock should be doing the action, while DBManyLocker is the thing that does the action. Not that this affects the code, but it gives me a bit of a hiccup on reading, as it looks like the lock is taken on line 416 (and not obviously ever let go), but it is in fact taken on line 430 (and let go shortly thereafter). Looking at the Git Blame, it seems that we can blame @george-mcintyre <https://github.com/george-mcintyre>...

Yeah, like employee and employer, DBManyLock is the actual subject/object - the actual lock object, and DBManyLocker is the think that does the locking. Though I can see how it looks confusing.

…

— Reply to this email directly, view it on GitHub <#57 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJQ2YXNBLL2AFW6FKIDQTDYDEDTXAVCNFSM6AAAAAA4U5MWA6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMJVGI2DGNZZGQ>. You are receiving this because you were mentioned.

mdavidsaver · 2023-11-06T18:23:54Z

Also, one other oddity I noticed ...

You may have meant to set FTVL on the aai record, which despite the name defaults to STRING (imo. a better name would be arrin). So this case is trying to read string[] into DBR_DOUBLE, which is a TODO. Currently string[] can only be read into DBR_STRING.

mdavidsaver · 2023-11-06T22:03:37Z

Another update. Adds string[] conversions on GET. Also a significant reorganization of the initHooks and atexit sequencing. Now the four sequences (server, single pv, group pv, and pvalink) are combined together into one. I have also extended the dbUnitTest.h helper APIs in pvxs/iochooks.h to (hopefully) allow for external testing w/ QSRV.

At this point, I have no more major TODOs.

mdavidsaver · 2023-11-08T02:01:44Z

I have found an issue where input links with CPP don't get an initial scan if the associated Channel is already connected and idle. This is annoyingly similar to a previous issue with onTypeChange() (#57 (comment)).

AppVeyorBot · 2023-11-08T03:33:50Z

❌ Build pvxs 1.0.967 failed (commit bf8f7fd85f by @mdavidsaver)

simon-ess · 2023-11-08T14:05:42Z

@george-mcintyre I think that part of my confusion comes from the code in PVXS using the Guard object, but now that I think of it more in that case, Guard is both noun and verb, so maybe I've just been reading it backwards from everyone else the whole time... 🙃

simon-ess · 2023-11-08T14:06:39Z

Also, one other oddity I noticed ...

You may have meant to set FTVL on the aai record, which despite the name defaults to STRING (imo. a better name would be arrin). So this case is trying to read string[] into DBR_DOUBLE, which is a TODO. Currently string[] can only be read into DBR_STRING.

I think my question is more that I would have expected softIovPVA and softIocPVX to behave the same with the same .db file, but they do not.

mdavidsaver · 2023-11-08T15:35:31Z

... I would have expected softIovPVA and softIocPVX to behave the same ...

This is the expectation I would like you to have. I was only making a point about why the were different. (this difference should be fixed) Maybe I am the only one who keeps forgetting that aai defaults to STRING...

mdavidsaver · 2023-11-08T15:37:14Z

... part of my confusion comes from ...

... the established and oft demonstrated fact the I am not as attuned to the subtitles and nuisances of naming ;)

mdavidsaver · 2023-11-08T15:38:29Z

subtitles and nuisances of naming

Ah, of course I meant nuances...

AppVeyorBot · 2023-11-09T16:05:32Z

❌ Build pvxs 1.0.972 failed (commit 5548c3f716 by @mdavidsaver)

AppVeyorBot · 2023-11-14T02:56:35Z

❌ Build pvxs 1.0.976 failed (commit a35b8cf25e by @mdavidsaver)

mdavidsaver · 2023-11-14T06:25:02Z

For some reason testqsingle is now failing on all appveyor jobs, but not on the GHA windows jobs.

testqsingle.tap .. 
not ok 89 - Instance leak ServerPvt : 1
not ok 90 - Instance leak ServerSource : 1
not ok 91 - Instance leak StructTop : 1
not ok 92 - Instance leak UDPListener : 1
not ok 93 - Instance leak evbase : 2
not ok 94 - Instance leak evbaseRunning : 2
All 88 subtests passed

These counts point to a Server being orphaned somehow. The trigger is likely my changing testqsingle to stop and restart the IOC. Why this only manifests on appveyor is at this point a mystery to me.

AppVeyorBot · 2023-11-20T07:20:24Z

✅ Build pvxs 1.0.977 completed (commit a1d0d2ee59 by @mdavidsaver)

mdavidsaver · 2023-11-20T08:10:08Z

The (re)initialization issue which was causing leaks and (sometimes) test failures should now be fixed.

The default seems to copy the shared_ptr member?

from pva2pva f1a3db44158a239a44d14b99b7823f340e95d7e0

Fix pvaGetValue for string scalars Remove pvaLink* variables Move close() call to pvaGlobal_t from worker queue Removed latch state Update .gitignore to ignore VS code configuration Add lset(pva) support for base 7.x Remove pvalink support for base 3.x Update cached value object in pvaLinkChannel::run Removing queued state from pvaLikeChannel Add debug functionality Rename internal fields to match json spec prepare for puts Fix array response size Add tests for pvalink properties

add pvalink json schema avoid JSON5 in testpvalink for portability. fixup build with pvalink trap bad_weak_ptr open during dtor Not sure why this is happening, but need not be CRIT. c++11, cleanup, and notes fix pvalink test sync fix test cleanup on exit pvalink disconnected link is always INVALID pvalink logging pvalink capture Disconnect time pvalink eliminate providerName restrict local to dbChannelTest() aka. no qsrv groups pvalink onTypeChange when attaching link to existing channel pvalink eliminate unused Connecting state pvalink add InstCounter pvalink AfterPut can be const pvalink add atomic jlif flag include epicsStdio.h later avoid #define printf troubles assert cleanup state on exit pvalink add newer lset functions test link disconnect testpvalink redo testPutAsync() pvalink fill out meta-data fetch pvalink fix FLNK pvalink cache putReq pvalink test atomic monitor pvalink test enum handling pvalink handle scalar read of empty array make it well defined anyway... pvalink test array of strings handle db_add_event() failure handle record._options.DBE

also consolidates initHook and epicsAtExit() hooks into a single sequence.

mdavidsaver · 2023-11-21T00:00:30Z

Ok. I think this is good enough. Thanks @simon-ess .

simon-ess force-pushed the pvalink branch from a2f4e78 to 4b089f9 Compare September 12, 2023 14:13

simon-ess force-pushed the pvalink branch 2 times, most recently from f913c73 to 8c60a84 Compare September 13, 2023 14:49

simon-ess force-pushed the pvalink branch from 8c60a84 to 7d80472 Compare September 14, 2023 14:17

simon-ess force-pushed the pvalink branch from 1d44d8d to 3bae573 Compare September 15, 2023 11:21

simon-ess force-pushed the pvalink branch from 3bae573 to fc57cfe Compare September 19, 2023 08:16

mdavidsaver self-assigned this Oct 3, 2023

mdavidsaver added the enhancement New feature or request label Oct 3, 2023

mdavidsaver force-pushed the pvalink branch 2 times, most recently from b5a2fa3 to 75df3a5 Compare October 11, 2023 03:15

mdavidsaver force-pushed the pvalink branch from 75df3a5 to b75f79e Compare October 31, 2023 18:52

mdavidsaver changed the title ~~Add pvaget links to pvxs~~ Add pva links to pvxs Oct 31, 2023

mdavidsaver force-pushed the pvalink branch from b75f79e to 1975a36 Compare November 4, 2023 21:20

simon-ess commented Nov 6, 2023

View reviewed changes

mdavidsaver force-pushed the pvalink branch from 1975a36 to a9938c6 Compare November 6, 2023 21:40

mdavidsaver marked this pull request as ready for review November 6, 2023 22:03

mdavidsaver force-pushed the pvalink branch from a9938c6 to 8cc9eaa Compare November 9, 2023 00:19

mdavidsaver force-pushed the pvalink branch from 8cc9eaa to 8ee3146 Compare November 13, 2023 20:59

mdavidsaver force-pushed the pvalink branch from 8ee3146 to 9fc1ab4 Compare November 19, 2023 21:07

mdavidsaver and others added 6 commits November 20, 2023 10:59

server: explicit move ctor

be7fab5

The default seems to copy the shared_ptr member?

pvalink: import existing

1dcdd8e

from pva2pva f1a3db44158a239a44d14b99b7823f340e95d7e0

pvalink: porting part 1

c009386

ioc: combine registrars and detect QSRV1

eddc687

also consolidates initHook and epicsAtExit() hooks into a single sequence.

mdavidsaver force-pushed the pvalink branch from 9fc1ab4 to eddc687 Compare November 20, 2023 23:58

mdavidsaver merged commit eddc687 into epics-base:master Nov 20, 2023
46 of 47 checks passed

mdavidsaver mentioned this pull request Dec 6, 2023

ioc: add PVA link support #43

Closed

mdavidsaver mentioned this pull request Dec 14, 2023

Incorrect .dbd included in python builds #65

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pva links to pvxs #57

Add pva links to pvxs #57

simon-ess commented Sep 12, 2023 •

edited by mdavidsaver

Loading

AppVeyorBot commented Sep 12, 2023

AppVeyorBot commented Sep 13, 2023

AppVeyorBot commented Sep 14, 2023

AppVeyorBot commented Sep 15, 2023

AppVeyorBot commented Sep 19, 2023

mdavidsaver commented Oct 2, 2023

AppVeyorBot commented Oct 3, 2023

mdavidsaver commented Oct 11, 2023

AppVeyorBot commented Oct 11, 2023

mdavidsaver commented Oct 31, 2023

AppVeyorBot commented Oct 31, 2023

AppVeyorBot commented Nov 5, 2023

simon-ess left a comment

simon-ess Nov 6, 2023

george-mcintyre commented Nov 6, 2023 via email

mdavidsaver commented Nov 6, 2023

mdavidsaver commented Nov 6, 2023

mdavidsaver commented Nov 8, 2023

AppVeyorBot commented Nov 8, 2023

simon-ess commented Nov 8, 2023

simon-ess commented Nov 8, 2023

mdavidsaver commented Nov 8, 2023

mdavidsaver commented Nov 8, 2023

mdavidsaver commented Nov 8, 2023

AppVeyorBot commented Nov 9, 2023

AppVeyorBot commented Nov 14, 2023

mdavidsaver commented Nov 14, 2023

AppVeyorBot commented Nov 20, 2023

mdavidsaver commented Nov 20, 2023

mdavidsaver commented Nov 21, 2023

Add pva links to pvxs #57

Add pva links to pvxs #57

Conversation

simon-ess commented Sep 12, 2023 • edited by mdavidsaver Loading

AppVeyorBot commented Sep 12, 2023

AppVeyorBot commented Sep 13, 2023

AppVeyorBot commented Sep 14, 2023

AppVeyorBot commented Sep 15, 2023

AppVeyorBot commented Sep 19, 2023

mdavidsaver commented Oct 2, 2023

AppVeyorBot commented Oct 3, 2023

mdavidsaver commented Oct 11, 2023

AppVeyorBot commented Oct 11, 2023

mdavidsaver commented Oct 31, 2023

AppVeyorBot commented Oct 31, 2023

AppVeyorBot commented Nov 5, 2023

simon-ess left a comment

Choose a reason for hiding this comment

simon-ess Nov 6, 2023

Choose a reason for hiding this comment

george-mcintyre commented Nov 6, 2023 via email

mdavidsaver commented Nov 6, 2023

mdavidsaver commented Nov 6, 2023

mdavidsaver commented Nov 8, 2023

AppVeyorBot commented Nov 8, 2023

simon-ess commented Nov 8, 2023

simon-ess commented Nov 8, 2023

mdavidsaver commented Nov 8, 2023

mdavidsaver commented Nov 8, 2023

mdavidsaver commented Nov 8, 2023

AppVeyorBot commented Nov 9, 2023

AppVeyorBot commented Nov 14, 2023

mdavidsaver commented Nov 14, 2023

AppVeyorBot commented Nov 20, 2023

mdavidsaver commented Nov 20, 2023

mdavidsaver commented Nov 21, 2023

simon-ess commented Sep 12, 2023 •

edited by mdavidsaver

Loading