Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mdavidsaver opened this issue Aug 29, 2018 · 5 comments
Assignees
Labels

Comments

@mdavidsaver
Copy link
Member

@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 mdavidsaver self-assigned this Aug 29, 2018
@mdavidsaver
Copy link
Member Author

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
Copy link
Member Author

-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
Copy link
Contributor

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
Copy link
Contributor

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

@mdavidsaver
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants