Add PVA RPC Action

[Synthetica9]

WIP.

TODO:

• Handle arguments
  • Take arguments in interface
  • Send arguments in query
• Handle return value (unclear how this even should be done actually looks like loc:// is good for this)
• Make a decent icon

[kasemir]

Well, happy to see that you can implement that without any changes to the PVA lib, so we do have sufficient RCP support in there.
Unclear how much sense it makes to add this as an 'action' to widgets.
PVA RPC is meant to call remote procedures, passing some arguments in AND then getting a result.
Examples: Fetch beam parameters, apply corrections and then get the new orbit etc.
That type of RPC behavior where you wait for some result and then actually use the result doesn't fit the widget action idea, which are more send-and-forget, a plain write or 'put'.
In your implementation, you're in fact implementing what's basically a put:

PVAChannel channel = client.getChannel(pv_name);
channel.connect().get(10, TimeUnit.SECONDS);
channel.invoke(request).get(10, TimeUnit.SECONDS);
channel.close();

The result of the request is not used, even though we spend up to 20 seconds getting it?!
Even if you implemented the full RPC behavior, what should a widget do with the result?

[Synthetica9]

Unclear how much sense it makes to add this as an 'action' to widgets.

It seems like a good fit to me, but I must admit I'm quite new to Phoebus.

That type of RPC behavior where you wait for some result and then actually use the result doesn't fit the widget action idea, which are more send-and-forget, a plain write or 'put'.

I was thinking of adding a callback of some sort, but just a writeback seems good to me. I'm working on this feature on my local checkout at the moment. Do you think a callback would make more sense?

Even if you implemented the full RPC behavior, what should a widget do with the result?

I assume one would store it in a loc:// variable most of the time. (i.e. LED goes to green or red on success or failure, or similar)

In any case, this PR is still a draft but if it ends up not getting merged we will most likely still use it in our local fork.

[kasemir]

I see how you store the result in a (local) PV.
Are the arguments truly a HashMap<String, String> args or do they need to be ordered, some form of List?

In the commits, the ActionUtil and WidgetRuntime somehow look like a complete replacement, hiding the actual changes. Maybe line ending changes were accidentally submitted?

I'm still not convinced that this justifies a general widget action update, since it only applies to one of the supported PV types, and the use cases seem to specific.
For what it's worth, what is your use case?
I'm remotely aware of PVA RPC being used to fetch beam parameters or to invoke some beam physics code. The result is then typically a complex type. You might be able to temporarily store it in a local PV, but you'll need a script to decode it, there's no use in displaying the basic result in a text update widget.
If a script is needed anyway, we might as well have a script for a button widget call the RCP and then handle the result.

[Synthetica9]

In the commits, the ActionUtil and WidgetRuntime somehow look like a complete replacement, hiding the actual changes. Maybe line ending changes were accidentally submitted?

Ugh looks like about 10% of files have windows line endings checked in, I wasn't aware of that. Is there some policy on this? Should I continue to use windows line endings, should I add a single commit to change the line endings and a separate commit with changes, should I do a secret third thing?

[vincent-hve]

I'm still not convinced that this justifies a general widget action update, since it only applies to one of the supported PV types, and the use cases seem to specific. For what it's worth, what is your use case? I'm remotely aware of PVA RPC being used to fetch beam parameters or to invoke some beam physics code. The result is then typically a complex type. You might be able to temporarily store it in a local PV, but you'll need a script to decode it, there's no use in displaying the basic result in a text update widget. If a script is needed anyway, we might as well have a script for a button widget call the RCP and then handle the result.

Our current use-case is for rapid control of PLC code. Next to variables, we also export methods. Quickly being able to invoke these methods without much scripting, and optionally see what is returns. However, the most common case is invoking a single action on a PLC. Though this can be done by a script, this is simpler. We may extend it to invoke methods in our application SoftIoC in the future.

[kasemir]

the most common case is invoking a single action on a PLC.

Can't that be handled by simply writing to the PLC? So far, PLC integration tended to use channel access, where all you can do is read or write, so writing was used for this. Which PLC has PVA support at this time?

[vincent-hve]

the most common case is invoking a single action on a PLC.

Can't that be handled by simply writing to the PLC? So far, PLC integration tended to use channel access, where all you can do is read or write, so writing was used for this. Which PLC has PVA support at this time?

We have our own PVA RPC <-> ADS gateway. Next to that we configured coselab's adsDriver to support PVA for variables. Beckhoff PLCs support RPC calls.

[jembishop]

I would encourage more use of RPC, as I think it is a better way to control devices rather than the "fire-and-forget" method of modifying the setpoint. For example if the request to the device causes an internal error it can be reported back to the user.

[Synthetica9]

Is there a good way to do this? I wanted to add a constructor to (e.g.) PVAData but that would create a cross-cutting dependency, so I didn't want to just blindly go into that.

[kasemir]

I would encourage more use of RPC

#2913 enhance MQTT support

So far, no phoebus app directly talks to CA, PVA, MQTT, local, simulated, formula PVs.
We have a layer with core.pv.PV and VTypes to abstract the phoebus apps from the underlying network protocols, their data types, and also to pool connections.
It does result in certain restriction to the common-denominator of all supported PV types.
We have some meaningful defaults: The default PV.asyncRead/Write calls the basic read/write and assumes we completed right away. Protocols that actually support async completion (CA, PVA) then implement something that actually completes when the protocol tells us so.

I don't like how this PR circumvents that abstraction layer. The WidgetRuntime which usually deals with the PV/VType now directly creates, uses and then disposes a PVA PV. A "disconnected" state is only detected when you press the button, whereas we otherwise have all PVs connected and see what's disconnected before we push a button or enter a value. Plus you have the expected question of how to map arbitrary PVA data structures to VTypes.

To properly support RPC, that should be added to the PV layer.
For example, there could be this addition to the PV:

class PV
{
...
/** Perform an RPC call * /
public CompletableFuture<VType> call(VType... parameters) throws Exception
{
    // Default implementation falls back to asyncWrite with single parameter.
    if (parameters.length != 1)
        throw new Exception("PV " + getName() + ".call expects one parameter");
    return asyncWrite(parameters[0]);
}
...
}

For PVA and MQTT you could implement an actual RPC call.
The result would have to be placed into a VType. Ideally, just a number or array. Worst case, a VTable. No, it won't be able to handle ANY type of PVA structure. That goes back to the basic idea that common tools like phoebus handle what you get from an IOC. For completely custom PVA data, you need to write your own client.