pvtools rework #129

mdavidsaver · 2018-10-02T21:58:58Z

Companion to epics-base/pvDataCPP#58 with changes to pvget et al. This is almost a rewrite. Net delta is

11 files changed, 833 insertions(+), 3437 deletions(-)

An incomplete list of feature changes:

Output options determined by PVStructure::stream().
New executables: pvmonitor and pvcall
Many arguments now ignored. -f is an error (arguments from file no longer supported).
** pvget: -q, -t, -i, -n, -F
** pvput: -n, -s, -F, -t
eget has been removed to https://github.com/epics-base/eget
pvcall makes -s and -a optional.
default pvRequest is always "field()".

kasemir · 2018-10-03T03:11:34Z

This includes / obsoletes #125

kasemir · 2018-10-30T12:11:56Z

With the last commit, pvinfo works.
Having a pvmonitor to go with pvget also sounds good since it eases the transition from caget and camonitor.

Also good is that the request defaults to getting all, so just pvget neutrons for a custom structure again succeeds as it did in the older V4 code, instead of failing as in base 7.0.1.1.

So overall, if this was a democracy, I'd vote for merging this.

kasemir · 2018-10-30T12:18:48Z

Just comments on behaviour.. Given this data:

$ pvinfo training:ai1
training:ai1 epics:nt/NTScalar:1.0
    double value
    alarm_t alarm..
    time_t timeStamp..
    display_t display..
    control_t control..
    valueAlarm_t valueAlarm...

.. the old pvget allowed fetching selected pieces. Now I always get the complete structure:

$ pvget -r 'control' training:ai1
training:ai1 2018-10-30T08:14:43.558 1 MAJOR DEVICE LOLO 

$ pvget -r 'field(control)' training:ai1
training:ai1 2018-10-30T08:14:45.561 3 MINOR DEVICE LOW 

$ pvget -r 'field(control)' -M raw training:ai1
training:ai1 epics:nt/NTScalar:1.0 
    double value 8
    alarm_t alarm MAJOR DEVICE HIHI 
        int severity 2
..
    valueAlarm_t valueAlarm
..
       int highAlarmSeverity 0
        double hysteresis 0

Maybe that's not an issue with pvget but actually caused by qsrv, since it works as expected with a custom data source:

$ pvinfo neutrons
neutrons 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 [352030,352030,352030,352030,352030,352030,352030,352030,352030,352030]

$ pvget -r 'time_of_flight, pixel' -M raw neutrons
neutrons structure 
    epics:nt/NTScalarArray:1.0 time_of_flight
        uint[] value [36329,36329,36329,36329,36329,36329,36329,36329,36329,36329]
    epics:nt/NTScalarArray:1.0 pixel
        uint[] value [363290,363290,363290,363290,363290,363290,363290,363290,363290,363290]

anjohnson · 2018-10-30T18:31:16Z

I recommend reading this comment on the Github website to see the proper formatting.

Format comparisons

`caget`

tux$ ./bin/linux-x86_64/caget anj:BaseVersion
anj:BaseVersion                7.0.1.2-DEV
tux$ ./bin/linux-x86_64/caget -a anj:BaseVersion
anj:BaseVersion                2018-10-30 12:08:47.001625 7.0.1.2-DEV  
tux$ ./bin/linux-x86_64/caget -t anj:BaseVersion
7.0.1.2-DEV

When not in terse mode (-t) PV names are displayed with a fixed minimum width of ~30 characters, which makes it easier to read when fetching multiple PVs at once. The caget timestamp format uses a space instead of a T between the date and the time (for humans to read so not strictly ISO-8601) and it shows fractional seconds to 6 decimal places.

Original `pvget`

tux$ ./bin/linux-x86_64/pvget anj:BaseVersion
anj:BaseVersion                7.0.1.2-DEV
tux$ ./bin/linux-x86_64/pvget -t anj:BaseVersion
7.0.1.2-DEV NO_ALARM NO_STATUS NO_ALARM 2018-10-30T12:08:47.002 0 0 0 EPICS Base Version   0 0 0
tux$ ./bin/linux-x86_64/pvget -v anj:BaseVersion
anj:BaseVersion
epics:nt/NTScalar:1.0 
    string value 7.0.1.2-DEV
    alarm_t alarm NO_ALARM NO_STATUS NO_ALARM
    time_t timeStamp 2018-10-30T12:08:47.002 0
    display_t display
        double limitLow 0
        double limitHigh 0
        string description EPICS Base Version
        string format 
        string units 
    control_t control
        double limitLow 0
        double limitHigh 0
        double minStep 0

With no formatting options PV names are displayed with a fixed minimum width of ~30 characters, which makes it easier to read when fetching multiple PVs at once. The pvget timestamp format is ISO-8601 and shows fractional seconds to 3 decimal places.

Original `pvget -p ca`

tux$ ./bin/linux-x86_64/pvget -p ca anj:BaseVersion
anj:BaseVersion                7.0.1.2-DEV
tux$ ./bin/linux-x86_64/pvget -p ca -t anj:BaseVersion
7.0.1.2-DEV NO_ALARM NO_STATUS <no message> 2018-10-30T12:08:47.002 0
tux$ ./bin/linux-x86_64/pvget -p ca -v anj:BaseVersion
anj:BaseVersion
structure 
    string value 7.0.1.2-DEV
    alarm_t alarm NO_ALARM NO_STATUS <no message>
    time_t timeStamp 2018-10-30T12:08:47.002 0

A no-alarm message is different between the ca and pv providers, <no message> vs. NO_ALARM. This may be an issue with caProvider.

New `pvget`

tux$ ./bin/linux-x86_64/pvget anj:BaseVersion
anj:BaseVersion 2018-10-30T12:08:47.002 7.0.1.2-DEV 
tux$ ./bin/linux-x86_64/pvget -M json anj:BaseVersion
anj:BaseVersion {"value": "7.0.1.2-DEV","alarm": {"severity": 0,"status": 0,"message": "NO_ALARM"},"timeStamp": {"secondsPastEpoch": 1540919327,"nanoseconds": 1624623,"userTag": 0},"display": {"limitLow": 0,"limitHigh": 0,"description": "EPICS Base Version","format": "","units": ""},"control": {"limitLow": 0,"limitHigh": 0,"minStep": 0}}
tux$ ./bin/linux-x86_64/pvget -M raw anj:BaseVersion
anj:BaseVersion epics:nt/NTScalar:1.0 
    string value 7.0.1.2-DEV
    alarm_t alarm 
        int severity 0
        int status 0
        string message NO_ALARM
    time_t timeStamp 2018-10-30T12:08:47.002 
        long secondsPastEpoch 1540919327
        int nanoseconds 1624623
        int userTag 0
    display_t display
        double limitLow 0
        double limitHigh 0
        string description EPICS Base Version
        string format 
        string units 
    control_t control
        double limitLow 0
        double limitHigh 0
        double minStep 0

With no formatting options only one space separates the PV names from their values, so it's a bit harder to identify where the value starts when fetching many PVs at once. We always get a timestamp that wasn't present with the older tools (not a problem, just pointing out the difference). I like the new -M json option. The blank lines at the end of the 'raw' output format have been removed (good).

New `pvget -p ca`

tux$ ./bin/linux-x86_64/pvget -p ca anj:BaseVersion
Segmentation fault

Oops! I had built pvAccessCPP here by merging your branch into the latest 7.0. The seg-fault is in Marty's update to the caProvider so I will take a look at it.

mdavidsaver · 2018-10-30T19:02:41Z

With no formatting options only one space separates the PV names from their values

Try with multiple PV names. Values are indented based on the longest PV name, rather than a fixed 30 characters. (I've had to deal with too many really long PV names)

so it's a bit harder to identify where the value starts

What would you have? 2 spaces? 4?

The caget timestamp format uses a space instead of a T between the date and the time

Doesn't bother me one way or the other. If you care enough, you can change this after epics-base/pvDataCPP#58 is merged (in printer.cpp).

anjohnson · 2018-10-30T19:44:50Z

Try with multiple PV names.

My bad, yours is obviously an improvement, I should have actually tried it instead of assuming:

tux$ ./bin/linux-x86_64/pvget anj:BaseVersion anj:exit
anj:BaseVersion 2018-10-30T12:08:47.002 7.0.1.2-DEV 
anj:exit        <undefined> 0 INVALID DRIVER UDF

How about using a fixed width for the timestamp field though? The values don't line up when an <undefined> timestamp is included. Something like this could be even easier on the eyes:

anj:BaseVersion 2018-10-30T12:08:47.002 7.0.1.2-DEV 
anj:exit                    <undefined> 0 INVALID DRIVER UDF

The other thing that I would still like to see is some kind of terse mode, which for caget prints just the addressed field value with no decorations or metadata. If we don't provide something close to that our users are going to complain about having to strip off all this other stuff that they don't want (PV name, timestamp, alarm status etc.). At least the old pvget's terse mode outputs the value first so it's easily extracted.

There's only so much pushing that we can do to get users to write clients in higher level languages, and in the APS case our physicists will probably be writing tcl/tk for some time to come and thus calling out to someone's pvget client (I'd much rather they use ours than write their own).

gregoryraymondwhite · 2018-10-30T19:46:55Z

On Oct 30, 2018, at 10:57 AM, Michael Davidsaver ***@***.***> wrote: Hi Greg, Not sure how you will like this. ... 'eget' in its present form will be moved to a separate repository, which will not be included in 7.0.2.

Ok, but let me ask you what this really means. I don’t need eget explicitly of course. Like all users who now use the control system framework for scientific data, or do ad-hoc investigations of a machine, or rapid prototype by seeing what is available and then coding to that, I just want easy access to the facilities EPICS 7 brings: In particular, what about "-a arg argvalue”? Would -a and RPC be integrated into pvget pvput? What about direct addressing? Is that staying in the API? If so, would that be preserved in pvget/put? How about URL form? What about your pvget? - Does it preserve -f, including -f from stdio “-f -“. That’s critical. - Does it preserve direct addressing - I suspect that will be critical for a number of labs in future. At SLAC the controls department tells us 60% of our net traffic is ca search broadcasts. Now we have a great directory service we have the basis to fix all of that. In summary, we just need: 1. Command line support for talking to an rpc server. pvget/pvput are taught -a. 2. Direct addressing. pva://server:port 3. -f 4. -T (Transpose) for by-eye investigations and pipelining 5. -t (terse) for pipelining. Maybe if pvRequest was usable wouldn’t need this 6. Drop -x. A matrix with 9 rows and 4 columns is everywhere a matrix with 9 rows and 4 columns regardless of the language that formed it or the client that displays it - and the NTMatrix contains its dimensions, so -x is just misguided. 7. Fix pvlist. Make it so qSrv can list the pvs of an IOC trivially 8. Fix pvinfo. Fix its output format to make it easier to extract the direct address! 9. Fix pvRequest!!!! Ideally replace it with a known semi-structured query syntax 10. Fix the help for these tools in general. It’s objectively awful, and has been for years. The -h help being hard coded in the source is bizarre. And no man pages!! That would have been fine in 1990. In 2018 it's weird. Later in the financial year I can help fund this I think. But need to see how other projects actually spend. Greg

…

On Oct 30, 2018, at 10:57 AM, Michael Davidsaver ***@***.***> wrote: Hi Greg, Not sure how you will like this. Faced with the competing factors of your attachment to eget's many features, the recent discovery the 'pvlist' and 'pvinfo' were broken for some months, and the impending 7.0.2 release, have lead to a change in plans. I will be merging my rewrite of pvget/put for inclusion in 7.0.2. 'eget' in its present form will be moved to a separate repository, which will not be included in 7.0.2. You will have ownership of this repo. I could also move over pvget and pvput as well (with different names) if you wish. I'll set up travis-ci configuration and make sure it builds again 7.0.2. From that point though, it will be up to you/SLAC. Michael On Oct 30, 2018, at 12:10 PM, mdavidsaver ***@***.***> wrote: With no formatting options only one space separates the PV names from their values Try with multiple PV names. Values are indented based on the longest PV name, rather than a fixed 30 characters. (I've had to deal with too many really long PV names) so it's a bit harder to identify where the value starts What would you have? 2 spaces? 4? The caget timestamp format uses a space instead of a T between the date and the time Doesn't bother me one way or the other. If you care enough, you can change this after epics-base/pvDataCPP#58 <epics-base/pvDataCPP#58> is merged (in printer.cpp). — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#129 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMV0AaZQu2dM9KcdEuxS7YfMbS-0ALvpks5uqKQagaJpZM4XE8Ve>.

mdavidsaver · 2018-10-30T19:54:11Z

How about using a fixed width for the timestamp field though?

Can we move this thread over to epics-base/pvDataCPP#58 ? This is where the relevant code is anyway.

anjohnson · 2018-10-30T19:54:58Z

@gregoryraymondwhite Re your point 9, would JSON be an acceptable syntax? Take a look at the Suggestions section towards the bottom of this document to see what it might look like. That we can do relatively easily, but making big changes to how pvRequest works would be a major undertaking that could require rewriting existing servers.

mdavidsaver · 2018-10-30T20:07:57Z

In particular, what about "-a arg argvalue”? Would -a and RPC be integrated into pvget pvput?

Along with pvmonitor there is a new executable pvcall which takes over the core role of eget -s. Though both the -s and -a are now optional.

$ ./bin/linux-x86_64/pvcall -h

Usage: pvcall [options] <PV name> [<arg1>=<value>]...


options:
  -h: Help: Print this message
  -V: Print version and exit
  -s <service name>:   legacy form of PV name
  -a <service arg>:    legacy form of argument
  -r <pv request>:   Request, specifies what fields to return and options, default is ''
  -w <sec>:          Wait time, specifies timeout, default is 5.000000 seconds for get, inf. for monitor
  -p <provider>:     Set default provider name, default is 'pva'
  -M <raw|nt|json>:  Output mode.  default is 'nt'
  -v:                Show entire structure (implies Raw mode)
  -d:                Enable debug output
 deprecated options:
  -q, -t, -i, -n, -F: ignored
  -f <input file>:   errors

example: pvcall pv:name:add lhs=1 rhs=2

mdavidsaver · 2018-10-30T20:23:26Z

In summary, we just need:

"just"?

What about direct addressing? Is that staying in the API?

No. Remember that I didn't even know this worked until Wednesday. These sort of features may be re-added in future along with test coverage!

Fix pvlist. Make it so qSrv can list the pvs of an IOC trivially

This at least is already done.

Later in the financial year I can help fund this I think

I look forward to this. Right now though, I have to proceed on the basis of available funding ($0). With this level of support, I can only work on pvget et. al incidentally. This limits their scope to the low level troubleshooting tools I need them to be.

mdavidsaver · 2018-10-30T21:17:46Z

@gregoryraymondwhite

I should also point out that, as with pvput, it will be possible to send arrays and structures as RPC arguments. Quoting will naturally be a pain in non-trivial situations, but hey there is only so much I can do.

pvcall pv:blah 'myarr=[1,2,3]' 'mystr={"sub":4}'

mdavidsaver · 2018-10-31T01:55:02Z

I've merged epics-base/pvDataCPP#52 and created a new home for eget at https://github.com/epics-base/eget

I'm going to wait until tomorrow morning before a merging this PR.

mdavidsaver added the enhancement label Oct 2, 2018

mdavidsaver self-assigned this Oct 2, 2018

mdavidsaver mentioned this pull request Oct 2, 2018

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

Open

mdavidsaver mentioned this pull request Oct 3, 2018

Changing default request string to 'field()' #125

Merged

mdavidsaver force-pushed the pvtoolsrework branch from 886ea08 to 1e6178b Compare October 8, 2018 16:59

mdavidsaver force-pushed the pvtoolsrework branch 2 times, most recently from 8ab2ec3 to 1500a17 Compare October 29, 2018 23:36

pvutils rework

6dea9d9

mdavidsaver force-pushed the pvtoolsrework branch from 1500a17 to d832280 Compare October 30, 2018 23:33

mdavidsaver added 4 commits October 30, 2018 17:20

fix pvinfo

c2f3864

pvtools: update options message

a4e7891

drop eget

728822b

update release notes

f87e305

mdavidsaver force-pushed the pvtoolsrework branch from d832280 to f87e305 Compare October 31, 2018 01:52

mdavidsaver merged commit f87e305 into epics-base:master Oct 31, 2018

mdavidsaver mentioned this pull request Jan 12, 2019

Use pvRequest to mask monitor updates epics-base/pva2pva#11

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pvtools rework #129

pvtools rework #129

mdavidsaver commented Oct 2, 2018 •

edited

kasemir commented Oct 3, 2018

kasemir commented Oct 30, 2018

kasemir commented Oct 30, 2018

anjohnson commented Oct 30, 2018

mdavidsaver commented Oct 30, 2018

anjohnson commented Oct 30, 2018

gregoryraymondwhite commented Oct 30, 2018 via email

mdavidsaver commented Oct 30, 2018

anjohnson commented Oct 30, 2018

mdavidsaver commented Oct 30, 2018

mdavidsaver commented Oct 30, 2018

mdavidsaver commented Oct 30, 2018

mdavidsaver commented Oct 31, 2018

pvtools rework #129

pvtools rework #129

Conversation

mdavidsaver commented Oct 2, 2018 • edited

kasemir commented Oct 3, 2018

kasemir commented Oct 30, 2018

kasemir commented Oct 30, 2018

anjohnson commented Oct 30, 2018

Format comparisons

caget

Original pvget

Original pvget -p ca

New pvget

New pvget -p ca

mdavidsaver commented Oct 30, 2018

anjohnson commented Oct 30, 2018

gregoryraymondwhite commented Oct 30, 2018 via email

mdavidsaver commented Oct 30, 2018

anjohnson commented Oct 30, 2018

mdavidsaver commented Oct 30, 2018

mdavidsaver commented Oct 30, 2018

mdavidsaver commented Oct 30, 2018

mdavidsaver commented Oct 31, 2018

mdavidsaver commented Oct 2, 2018 •

edited

`caget`

Original `pvget`

Original `pvget -p ca`

New `pvget`

New `pvget -p ca`