epics-base / pvAccessCPP

pvAccessCPP is an EPICS V4 C++ module
https://epics-base.github.io/pvAccessCPP/
Other
9 stars 22 forks source link

"Invalid request" for simply "pvget ThePV" when all other requests seem fine #124

Open mdavidsaver opened 6 years ago

mdavidsaver commented 6 years ago

@kasemir

On 08/28/2018 04:06 PM, Kasemir, Kay wrote:

Hi:

I'm updating some example PVA server code, https://github.com/kasemir/EPICSV4Sandbox, that was originally developed for base-3.15.4-pre1 & EPICS-CPP-4.5.0.2, to compile and run with base-7.0.1.1.

It does for the most part, but there is one field request related problem. Before updating to EPICS 7, a plain "pvget" would function as if you had added "-r field()". Now, I can perform all types of "pvget -r XXXX ThePV" but the simplest "pvget ThePV" fails. What changed, what's missing in my PVA server code to again handle the simplest request?

As described in the README.md, the 'neutronsDemoServer' builds both into a standalone executable, or a PVA server that runs together with qsrv and CA in an IOC. For the following, there's no difference, I get the same response to pvlist, pvinfo, pvget, so I use the standalone executable.

$ ./neutronServerMain ... neutronServer running

I can successfully find and access that "neutrons" channel in several ways:

$ pvlist GUID 0x2654855B00000000EF85FA22, version 1: tcp@[10.0.2.15:5075]

$ pvlist 0x2654855B00000000EF85FA22 neutrons traceRecordPGRPC

$ pvinfo neutrons CHANNEL : neutrons STATE : CONNECTED ADDRESS : 10.0.2.15:5075 structure time_t timeStamp long secondsPastEpoch int nanoseconds int userTag epics:nt/NTScalar:1.0 proton_charge double value epics:nt/NTScalarArray:1.0 time_of_flight uint[] value epics:nt/NTScalarArray:1.0 pixel uint[] value

$ pvget -r pixel neutrons neutrons structure epics:nt/NTScalarArray:1.0 pixel uint[] value [32860,32860,32860,32860,32860,32860,32860,32860,32860,32860]

$ pvget -r "proton_charge, pixel" neutrons neutrons structure epics:nt/NTScalar:1.0 proton_charge double value 6e+08 epics:nt/NTScalarArray:1.0 pixel uint[] value [51950,51950,51950,51950,51950,51950,51950,51950,51950,51950]

$ pvget -r "field(proton_charge, pixel)" neutrons neutrons structure epics:nt/NTScalar:1.0 proton_charge double value 9e+08 epics:nt/NTScalarArray:1.0 pixel uint[] value [58180,58180,58180,58180,58180,58180,58180,58180,58180,58180]

$ pvget -r "field()" neutrons neutrons structure time_t timeStamp 2018-08-28T09:55:52.542 6162 epics:nt/NTScalar:1.0 proton_charge double value 3e+08 epics:nt/NTScalarArray:1.0 time_of_flight uint[] value [6162,6162,6162,6162,6162,6162,6162,6162,6162,6162] epics:nt/NTScalarArray:1.0 pixel uint[] value [61620,61620,61620,61620,61620,61620,61620,61620,61620,61620]

But here's the thing: If I try the simplest incantation of pvget, it fails:

$ pvget neutrons [neutrons] failed to create channel get: Status [type=ERROR, message=invalid pvRequest]

Thanks, Kay

mdavidsaver commented 6 years ago

On 08/28/2018 08:09 PM, Kasemir, Kay wrote:

Hi Andrew:

.. "field(value)" seems to have always been the default... What does your older pvget show for its help text?

Same as the new one:

-r : Request, specifies what fields to return and options, default is 'field(value)'

this change in behavior looks like a regression now that you point it out.

Yes, it is a change. In the old code, I find this in EPICS-CPP-4.6.0/pvAccessCPP/pvtoolsSrc/pvget.cpp:

           // probe for value field

... if (structure.get() == 0 || structure->getField("value").get() == 0) { // fallback to structure mode = StructureMode; pvRequest = CreateRequest::create()->createRequest("field()"); }

That must be what made it tolerant to a missing "value" field, falling back to getting everything.

The new pvget code doesn't update the pvRequest. In the printout section, it's prepared to say "no value field" if there is none, but then it's already too late, because it forced the "field(value)" request and failed, instead of falling back to "field()".

If this was a democracy, I'd vote for putting the more tolerant code back in.

Thanks, -Kay

mdavidsaver commented 6 years ago

-r : Request, specifies what fields to return and options, default is 'field(value)' ... if (structure.get() == 0 || structure->getField("value").get() == 0) { // fallback to structure mode = StructureMode; pvRequest = CreateRequest::create()->createRequest("field()");

So the issue is that the default pvRequest wasn't always the default, and now is. My inclination is to change the default to field() (get everything). This along with removing a "no 'value' field" warning should restore the previous behavior for Kay. Given that this is likely to be seen as contentious by some, I won't take any action until the face to face meeting next week.

ralphlange commented 6 years ago

Yes, should be discussed.

If I remember correctly, pvget/pvput were designed to be drop-in replacements for caget/caput. For that, a plain pvget would have to print the value of the 'value' field, not the whole structure, in the same way a plain caget only prints the value, not the union of all DBR_ structures.

ralphlange commented 6 years ago

Too bad that we're so horrible at proper development procedures, else one could just look up the specification...

mdavidsaver commented 5 years ago

I think this issue would be resolved by #129 which changes the default pvRequest to "field()" (among a great many other changes).