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
90 stars 90 forks source link

Binary values cannot be saved as a snapshot in Save-and-Restore #2755

Open Monarda opened 1 year ago

Monarda commented 1 year ago

It appears that these boolean PVs are not yet supported when taking snapshots in Save-And-Restore:

image

For some context, at the ISIS accelerators we're currently working with our accelerator physicists to deploy and test Phoebus Save-And-Restore functionality. Because we are using a bridge to our existing Vsystem control system while we migrate to EPICS many of our PVs are PVAccess NTScalars of boolean type.

kathryn-baker commented 1 year ago

For context, the binary PVs are created using pvapy with an NtScalar type. The pvinfo for one of the affected PVs is shown below:

IRFQ::DIPOLE_1_X:SET_POL
Type:
    epics:nt/NTScalar:1.0
        alarm_t alarm
            int severity
            int status
            string message
        string channelname
        control_t control
            double limitLow
            double limitHigh
            double minStep
        string descriptor
        display_t display
            double limitLow
            double limitHigh
            string description
            string format
            string units
        time_t timeStamp
            long secondsPastEpoch
            int nanoseconds
            int userTag
        boolean value
        valueAlarm_t valueAlarm
            boolean active
            float lowAlarmLimit
            float lowWarningLimit
            float highWarningLimit
            float highAlarmLimit
            int lowAlarmSeverity
            int lowWarningSeverity
            int highWarningSeverity
            int highAlarmSeverity
            byte hysteresis 

As we are operating in a hybrid mode while we transition to EPICS from Vsystem, the additional channelname field stores the associated Vsystem channel name but as far as I'm aware this is the only deviation from the standard NtScalar Structure for these PVs.

kathryn-baker commented 1 year ago

Here's a minimal reproducible example for how to create the PV using pvapy=5.2.0:

import time

from pvaccess import (
    BOOLEAN,
    FLOAT,
    STRING,
    PvAlarm,
    PvaServer,
    PvControl,
    PvDisplay,
    PvObject,
    PvTimeStamp,
    PvValueAlarm,
)

alarm = PvAlarm()
display = PvDisplay()
value_alarm = PvValueAlarm(FLOAT)
control = PvControl()

timestamp = time.time()
seconds = int(timestamp // 1)
nanoseconds = int((timestamp % 1) * 1e9)

pv_name = "IRFQ::DIPOLE_1_X:SET_POL"

pv_types = {
    "alarm": alarm,
    "channelname": STRING,
    "control": control,
    "descriptor": STRING,
    "display": display,
    "timeStamp": PvTimeStamp(),
    "value": BOOLEAN,
    "valueAlarm": value_alarm,
}

pv_values = {
    "alarm": alarm.toDict(),
    "channelname": pv_name.lower(),
    "control": control.toDict(),
    "descriptor": "",
    "display": display.toDict(),
    "timeStamp": {"nanoseconds": nanoseconds, "secondsPastEpoch": seconds},
    "value": 1,
    "valueAlarm": value_alarm.toDict(),
}

pv = PvObject(pv_types, pv_values, "epics:nt/NTScalar:1.0")

server = PvaServer()
server.addRecord(
    pv_name,
    pv.copy(),
)

try:
    while True:
        time.sleep(1)
except KeyboardInterrupt:
    server.stop()
finally:
    server.stop()
kasemir commented 1 year ago

binary PVs .. created using pvapy with an NtScalar type

For what it's worth, binary PVs are supposed to use an enumerated type NtEnum. Run softIocPVA with a binary record, then check what it looks like with pvinfo.

When you send an NtScalar, the client sees a number, so it doesn't know that the numbers are limited to zero and one, and that there are corresponding enumeration strings for the two states.

May be unrelated to this specific save/restore error, but for complete integration of binary values it's probably best to map them to NtEnum.

kasemir commented 1 year ago

The specific error is likely the boolean value. EPICS IOCs tend to not use boolean. The NtScalar types are all numbers (double, int, ...). The binary records as well as the multi-state "mbbi" and "mbbo" create NtEnum, again no boolean

kasemir commented 1 year ago

While PVAccess can create arbitrary data types, generic clients cannot handle any random data type. What's supported is mostly what we see coming out of IOCs, which is the NtScalar with numbers, NtEnum for binaries or multi-states, and NTNDArray for AreaDetector images.

kathryn-baker commented 1 year ago

That's very useful to know, thanks! I'm not sure whether that would cause the save-and-restore service to fail or not but it sounds very likely... We can look into changing our binary PVs to NTEnum types

kasemir commented 1 year ago

I don't know about the saverestore internals, but the "mapping exception" mentioning "value" sounds like "boolean value" is the issue. That's not been a problem so far because tests used IOCs, and IOCs don't generate "boolean" data.

Mind you, we have been adding some support for boolean (#2457, #1121, #1118, #1112, ...) because it seems like many people who create "custom" data start by using "boolean" inside structures and simply expect that to be handled, even though an IOC won't do that. Saverestore can likely be extended to handle boolean as well, but in the larger scheme it'll be an uphill climb to handle all sorts of custom structures, so for full compatibility it's best to either stick to the types generated by IOCs, or accept that adding your custom data on the server means you also need to write your own client; generic clients will never handle all data types.

georgweiss commented 1 year ago

Thanks @kasemir for your input.

@kathryn-baker, do you need me to dig deeper based on your script, or would you first wish to try to convert to NTEnum as proposed by @kasemir ?

kathryn-baker commented 1 year ago

@kasemir you make an excellent point and one that we want to follow! In general we want to keep our PVs relatively standard so that all the tools work out of the box. The NtEnum for binary PVs was something that we missed while writing the hybrid support code so we will make those changes now.

Hopefully that should resolve this issue so @georgweiss, don't worry about investigating further for now. If the problem persists after the change then we can re-open the issue