ControlSystemStudio / phoebus

A framework and set of tools to monitor and operate large scale control systems, such as the ones in the accelerator community.
http://phoebus.org/
Eclipse Public License 1.0
89 stars 87 forks source link

Nested pva structures not resolving correctly #1098

Closed jjaraalm closed 4 years ago

jjaraalm commented 4 years ago

If you have the following pvaccess variables for example:

pv1 structure 
    int value 1

pv2 structure 
    structure a
        int value 2

pv3 structure 
    structure b
        structure a
            int value 3

then in phoebus I try to access them and get the following:

pva://pv1      -> 1
pva://pv2/a    -> 2
pva://pv3/b/a  -> "structure     structure b        structure a            int value 3"

pv3/b/a also reports an UNDEFINED/Unknown type alarm. I think this might be because PVAStructure.get() returns the entire structure hierarchy, just with non-relevant branches pruned. This is also how the cli works:

$ pvget -M json -r b.a pv3
pv3 {"b": {"a": {"value": 3}}}

so there needs to be some additional logic added to PVAStructureHelper.getVType() to test against the actual field requested and not the pruned tree.

kasemir commented 4 years ago

I'll look into it. Do you have a simple PVA server that generates this example data?

kasemir commented 4 years ago

OK, can duplicate when using this as an example server:

from time import sleep
from pvaccess import PvObject, INT, PvaServer

inner = {'value': INT }
outer = {'a': inner }
pv = PvObject({'b': outer })
server = PvaServer('pv3', pv)

print("\nServes:")
print("pv3 structure")
print("    structure b")
print("        structure a")
print("            int value 3")

x = 1
while True:
    pv['b.a.value'] = x
    print(x)
    server.update(pv)
    sleep(1)
    x = x + 1

So far, our use cases are basically the qsrv record data and the area detector image. For custom data like this nested structure, note that there may be no optimization. For example, qsrv ignores the request, so when you ask for -r display to fetch only the display info you still get the complete record structure, for example NTScalar.

It's thus a bad idea to create a PV "TheAccelerator" with sub-elements "RF", "Vacuum", ... and expect that reading "pva://TheAccelerator/RF" will only fetch the RF info.

Still, on the client side I agree that it's reasonable to expect for pva://pv3/b/a to just wrap the int value into a VType, so you'll see "3" in for example Probe instead of "... structure b structure a int value 3". And if you fetched "pva://TheAccelerator/RF/Sector1/Amplitude/Setpoint" it would just show that number, but again note that the PVA server might still send the complete "TheAccelerator" structure, not just the one number that we requested.

jjaraalm commented 4 years ago

Thanks for looking into this so quickly. A couple of questions (might not be the right place):

  1. I thought pvAccess only transmitted deltas (i.e., changes in the pv value). So if phoebus updates using channel monitors shouldn't there automatically be performance gains even with nested structures?
  2. Regardless, I actually agree and would prefer to avoid nested structures. I do like the uri syntax, pva://path/to/pv, though. The / character is not forbidden in PV names, so would you all be open to implementing a name resolution ordering so that pva://path/to/pv would resolve as:
    1. PV with name path/to/pv if it exists
    2. Field pv in PV with name path/to if it exists
    3. Field to/pv in PV with name path if it exists
    4. Else, undefined.

Currently, it always goes to case 3. Alternative separators would also work and may end up being what I use.

kasemir commented 4 years ago

The protocol supports sending only partial structures, yes. But not partial arrays, so the one-PV-for-everything ala pva://TheAccelerator/RF/cavity[2]/... doesn't work. You will always get the complete array of cavities.

As for the 'URI syntax', it's not 100% described, but earlier attempts like http://epics-pvdata.sourceforge.net/alpha/EPICS_URI_proposal.html state that the first part without slashes, the "URI authority", is the PV name. Your suggestion would require N connection attempts: 1) Is "path/to/pv" a PV to which I can connect? No result? Wait 10 seconds, still no result? OK, move on to 2) Is just "path/to" a PV to which I can connect? ..

So as it stands now we don't support PVs with '/' in the name with this client. It's OK for the underlying CA or PVA library, but the UI doesn't support it.

jjaraalm commented 4 years ago

I see, that makes sense. Thanks for the information.

kasemir commented 4 years ago

What is your actual use case? Are you connecting to an actual PVA server used in production, or are you still trying to figure out how to package your data?

jjaraalm commented 4 years ago

Somewhere in the middle, I have systems working in a test setup and am moving to OPI development. We're moving to production in early summer. At this point, I am using nested structures on each PVA server (~40 in production), but switching to individual PVs would be easy. I don't want to change after creating the OPIs though.

kasemir commented 4 years ago

As a rough guideline, I would use the basic normative types like NTScalar whenever possible, so your custom PVA server PVs look just like PVs provided by an IOC. Those NT types can for the most part be used "as is" by just entering the PV name into a widget, no need to drill down to some path within the nested structure.

Custom data structures are useful for special cases, like at SNS we use them for neutron data where we need to ship a combination of time-of-flight and detector pixel IDs, http://accelconf.web.cern.ch/AccelConf/ICALEPCS2015/papers/wepgf105.pdf . But those custom data structures require both 1) A custom server 2) A custom client

With a generic client, you can peek into the data to for example debug it, like "pva://neutrons/proton_charge" or "pva://neutrons/time_of_flight", but that 'raw' data is not useful for direct display. It's instead read by custom middle-layer clients which accumulate charge, fill time-of-flight histograms or compute d-space histograms, and those are then again plain PVs which can directly be displayed.

jjaraalm commented 4 years ago

Closing since this is solved.

jjaraalm commented 4 years ago

Updated phoebus and this turns out to only be solved for reads, not writes. Using both pva://pv/x and pva://pva?request=field(x) both read correctly but give an error on write:

java.lang.Exception: Cannot locate 'value' to write in structure 
    structure x
        int value 0
    at org.epics.pva.client.PutRequest.encodeRequest(PutRequest.java:106)
    at org.epics.pva.common.TCPHandler.sender(TCPHandler.java:179)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at java.base/java.lang.Thread.run(Thread.java:830)
kasemir commented 4 years ago

How do you write it from 'pvput'?

jjaraalm commented 4 years ago

Unnested:

$ pvput -M raw unnested value=1
Old : epics:nt/NTScalar:1.0 
    int value 0
New : epics:nt/NTScalar:1.0 
    int value 1

Nested:

$ pvput -M raw nested x.value=1
Old : structure 
    epics:nt/NTScalar:1.0 x
        int value 0
New : structure 
    epics:nt/NTScalar:1.0 x
        int value 1
kasemir commented 4 years ago

OK, the update in PR 1123 works as in the nested example.

You can read pva://pv3/b/a, show that in for example the Phoebus 'probe', and also write by entering a new value in 'probe'.

It's sending a PV request for field(b.a) to read. Ideally, the PVA server only returns that skeleton data, but QSRV will always return the complete structure. In any case, we then look for a "b.a.value" to get the actual value to show in the generic UI client.

When writing, it will write with a request field(b.a.value), and now correctly set that ...value down in the nested structure, fixing the bug where it looked for value at the root. So again the generic UI will only support writing to custom structs that have a ...value at the end of the path.