Data browser behavior may appear inconsistent

[georgweiss]

So I have this corner case (?) where a PV is currently:

CWM-CWS03::ExchangerLoopStop <undefined>              INVALID DRIVER UDF (0) 

At the same time there is an archived value some three weeks old. Adding the PV to the data browser it plots the archived value (1) plus the live value (0) when PV connects. However, if user chooses to refresh the plot from context menu the live value is not added to the samples array. Consequence is that the "virtual" sample now becomes a copy of the archived value, and so the plot will look different, see attached screenshot where right hand side is first case.

One solution to this is to update PVSamples, L212:

 org.phoebus.core.vtypes.VTypeHelper.getSeverity(sample.getVType()) == AlarmSeverity.UNDEFINED

to

 (org.phoebus.core.vtypes.VTypeHelper.getSeverity(sample.getVType()) == AlarmSeverity.UNDEFINED ||
                            org.phoebus.core.vtypes.VTypeHelper.getSeverity(sample.getVType()) == AlarmSeverity.INVALID))

@kasemir, would this be OK or are there any side effect or pitfalls?

[kasemir]

PVSamples L212 was originally a workaround for the PVManager which sent initial UNDEFINED/Disconnected samples until it had an actual value. In your case, it looks like you are connected to the PV, it's sending a value, but that value happens to have no time stamp and INVALID severity.

Since we do have a live data sample, we ought to show it...but it has no time stamp. That should be fixed on the IOC side (PINI, ...), but since it happens, we do patch such data via transformTimestampToNow in PVSamples L197. This is a workaround so that we can somehow show the current value of a PV in this situation, but it means we get a different "now" for the time stamp whenever you run this, so the result isn't 100% reproducible, which I think is what you see.

I don't think that simply ignoring any first sample with either UNDEFINED or INVALID is the solution. Better would be fixing the IOC so that it sends data with a proper time stamp in the first place.

[georgweiss]

Thanks @kasemir for your input.

Agree that IOC issues should not be worked around in this manner.

[georgweiss]

Tio follow up... My users are asking for some kind of indication in the plot for samples that are "invalid" (for whatever reason). I can understand this requirement as there is otherwise nothing in the plot indicating this, see screen shot where archived value is 1, while live (invalid) values is 0. Only way to establish this in the data browser is to inspect samples.

On the other hand, the data browser is not an IOC debugging tool...

@kasemir, what is your take on this?

[kasemir]

Annotations by default include the status/alarm info

Otherwise, the data browser is simply a tool that shows the value of a PV over time. It can't tell you why the value is what it is, how it was obtained etc.

[georgweiss]

Fine, but we could add severity to the annotation...

[kasemir]

Severity is in the annotation. "MAJOR" in the screenshot is the alarm severity. "INVALID" or "UNDEFINED" would show there.

[georgweiss]

Not as far as I can see...

[kasemir]

Hmm. MINOR/MAJOR sure enough show up, but INVALID for some reason doesn't. Can you check why it's not put into the {3} placeholder of the annotation?

[georgweiss]

Sure, n.p.

[georgweiss]

Seems the field info in AnnotationImpl is never set as in AnnotationImpl.setLocation the parameters position and value are always the same for each call.

[georgweiss]

@kasemir, would this be an acceptable solution:

boolean setLocation(final XTYPE position, final double value, final String info)
    {
        if (this.position != position  ||
            this.value != value)
        {
            this.position = position;
            this.value = value;
            return true;
        }
        if(info != null){
            this.info = info;
        }
        return false;
    }

[georgweiss]

@kasemir, the AnnotationImpl.setLocation method appears to me as incorrect. In fact, the info field is never set, so regardless of PV status the annotation will not show status. Moreover, the boolean return values is never used by calling code.

Your feedback would be appreciated.

[kasemir]

Looks like you found the issue. Yes, that update to setLocation might do it. Also correct that the return value isn't used, but let's leave that in because the result of the simpler setPosition is actually used, and we might eventually also optimize redraws based on the setLocation return value.