epics-base / pvDataCPP

pvDataCPP is an EPICS V4 C++ module
https://epics-base.github.io/pvDataCPP/
Other
6 stars 16 forks source link

Add PVStructure::stream() #58

Closed mdavidsaver closed 6 years ago

mdavidsaver commented 6 years ago

A work in progress which I don't have a hard timeline for merging. At this point I'm hoping to identify a minimal set of formating options.

This is a followup of my stated intent to formalize, and simplify, the output modes of the pvtools to get them into a state which I understand and am able to support.

My idea is that all options for controlling output format will flow through struct PVStructure::Formatter, making clear the possible combinations for the purposes of testing. This would include things like integer formatting (absent thus far). I don't expect that all combinations will be tested. This would be impractical. I would like to test the important combinations (aka. defaults and things people complain about).

What this code does have is the idea of output Format and Mode. Formats being: Raw, NT, and JSON. Modes being: Plain, ANSI (as in terminal escapes), and Auto (detects whether the output stream is a TTY).

My main motivations is to have the idea of showing and/or highlighting some fields in Raw and JSON formats. Highlighting so far is minimal (bold "\x1b[1m") to be used eg. with fields which change in a subscription update (helpful for visualizing which fields are actually sent).

The NT format handles the basic NTScalar/Array and NTEnum, as well as NTTable a la. 'eget'.

The JSON output format is a bit of an experiment, and a replacement for terse mode.

Use in eg. pvget will look like the following. The entry point is PVStructure::stream(), which is part of the reason this code appears in pvDataCPP atm.

virtual void getDone(const pvac::GetEvent& event) {
            ...
            pvd::PVStructure::Formatter fmt(event.value->stream()
                                            .format(outmode));

            if(verbosity>=2)
                fmt.highlight(*event.valid); // show all, highlight valid
            else
                fmt.show(*event.valid); // only show valid, highlight none

            std::cout<<fmt;
ralphlange commented 6 years ago

+1 for separating out the formatting part and make it common for all tools.

I think we would also need a common well-specified way of setting format and mode on the command line.

mdavidsaver commented 6 years ago

I was intending to leave a discussion of argument names for later, though if you are curious a preliminary version of the pvtools changes (a ruthless culling) is sitting in https://github.com/mdavidsaver/pvAccessCPP on either the master or inprog branches (it's getting rebased frequently, so I can't give a stable link)

$ git diff --stat HEAD^
 pvtoolsSrc/Makefile      |   12 +
 pvtoolsSrc/eget.cpp      | 1924 +++---------------------------------------------------------------------------------------------------------------------------
 pvtoolsSrc/pvcall.cpp    |  250 +++++++++++++++++
 pvtoolsSrc/pvget.cpp     |  728 +++++++++++++++++-------------------------------
 pvtoolsSrc/pvinfo.cpp    |  182 ++++--------
 pvtoolsSrc/pvlist.cpp    |    4 +
 pvtoolsSrc/pvmonitor.cpp |    3 +
 pvtoolsSrc/pvput.cpp     |  355 ++++++------------------
 pvtoolsSrc/pvutils.cpp   |  501 +++++----------------------------
 pvtoolsSrc/pvutils.h     |  286 +++----------------
 10 files changed, 819 insertions(+), 3426 deletions(-)
mdavidsaver commented 6 years ago

Items from this morning's discussion.

mdavidsaver commented 6 years ago

I'm thinking about removing the JSON mode as I haven't been able to come up with a real use case for it. As @anjohnson pointed out, because the PV name is printed as well, the output isn't properly formated JSON anyway.

I might replace it with a stripped down terse mode which would only print .value, and reverts to Raw mode if it is not present. This would satisfy the *nix piping use case.

ralphlange commented 6 years ago

+1 My only ideas for use cases were scripts that could easier digest the returned data if it were JSON. But - obviously - Python scripts would directly use the Python binding, and other scripting languages .... I don't know if that is a valid use case.

anjohnson commented 6 years ago

Nooo... I like (the idea of) JSON mode, it provides a quick way to get machine data into a format that can easily be post-processed by non-EPICS tools, for one-off manual operations. I'm not against putting back terse mode, but we shouldn't throw out the ability to turn our structured data into a generic structured text format. Capturing data and then post-processing it is much quicker and easier to do than writing scripts that have to use the bindings.

I'm Okay with the output not being 100% pure JSON, I'd rather have what you provide now than no JSON at all since I can hand-edit the text before processing if necessary. If the issue with it is adding string escaping, they would be handled automatically if the pvData JSON output routines were changed to call the yajl_gen.h API that is in libCom instead of rolling their own.

gregoryraymondwhite commented 6 years ago

First, I haven’t seen JSON mode and didn’t know the pvtools could do that! I’m still using 4.6, was that added in 7?

"I might replace it with a stripped down terse mode which would only print .value, …”

That sounds good for -t as long as the .value so printed in contexts where printing as normative type, also prints as normative type.

That is, eg eget -t for: NTTable prints the non-label rows NTMatrix prints unchanged from w/o -t NTHistogram prints space (or tab) separated bin counts.

Greg

On Oct 4, 2018, at 9:42 PM, mdavidsaver notifications@github.com wrote:

I'm thinking about removing the JSON mode as I haven't been able to come up with a real use case for it. As @anjohnson pointed out, because the PV name is printed as well, the output isn't properly formated JSON anyway.

I might replace it with a stripped down terse mode which would only print .value, and reverts to Raw mode if it is not present. This would satisfy the *nix piping use case.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

mdavidsaver commented 6 years ago

First, I haven’t seen JSON mode and didn’t know the pvtools could do that! I’m still using 4.6, was that added in 7?

JSON mode is added by this PR.

NTTable

The NT mode knows about this one, though I'm thinking to use CSV formatting for easy of parsing, w/ aligned columns for readability.

NTMatrix ... NTHistogram

At present these would no longer be treated specially.

mdavidsaver commented 6 years ago

An NTTable is now printed as eg.

2018-10-08T09:44:39.437   
"Column A", "Column B"
         1,          4
         2,          5
         3,          6

A string value is quoted if it contains commas, spaces, quotes, or escaped characters. Double quotes are escaped as "" per RFC4180. Not strict CSV (imo. an oxymoron) as the timestamp/alarm are printed if present.

mdavidsaver commented 6 years ago

@anjohnson says

The values don't line up when an timestamp is included.

This "<undefined>" is coming (ultimately) from epicsTime::strftime() in Base.

Something like this could be even easier on the eyes

I'm fine with changes to whitespace, but I'm not going to treat this as a blocker for merging.

The other thing that I would still like to see is some kind of terse mode

I'm not keen on this, but I won't close the door on adding additional modes (that's the point of -M <blah>). However, I don't have time to think about adding this today.

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

Sounds like there will be work to do at APS one way or the other ;)