Set AlarmSeverity back to None when we reconnect the widget.

[hhslepicka]

If a widget was not at an alarm state and we loose the connection it goes into DISCONNECTED. If the connection is recovered we don't send the new AlarmSeverity as NONE.

[teddyrendahl]

👍

[mattgibbs]

Is this a problem with the PyDMWidget implementation, or a problem with the EPICS data plugins? I have a hunch we need to fix something at the data plugin side.

I don't think we should just set the alarm severity back to 'NONE' on reconnect. Lets say I am connected to a PV, and it has no alarm. Then, I disconnect my ethernet cable. I see the PV go to 'disconnected' state, but the IOC is still up and running in the field. Something changes, and the IOC sets the alarm severity to 'MAJOR'. Then, I plug my ethernet cable back in. Widgets need to tell me the PV now has a 'MAJOR' alarm, not 'NONE'.

We need to investigate exactly whats happening at the plugin level. I think that after a PV reconnects, we need to make sure that we always send an up-to-date value and ctrl vars too, just in case the client missed some changes that happened on the server.

[hhslepicka]

@mattgibbs the case you described works.... mainly because at reconnection we send the alarm state in case we are at an alarm... The bug I believe is inside PyDMWidget on the base as when we change the connection we don't go back to check the alarm... I will work more on that and steps to reproduce.

[teddyrendahl]

@mattgibbs I ran into this when implementing a new Plugin The issue was the source (a non-EPICS source) had no concept of an "alarm state", so I only had it send connected and access. The BaseWidget still drew a Disconnected border unless I sent a the alarm states as NO_ALARM specifically. Not a huge deal, just a slight pain.

I agree the state-machine is tricky. We should just make sure that at the plugin level on every "re-connection" event we also dump any information that may have been updated when we weren't listening.

[mattgibbs]

Ahhhhh, after that explanation of how this bug came up, the proposed solution is more reasonable. I just want to be sure we don't end up unintentionally hiding alarm state.

If I were starting over, I'd probably change how we implement and show disconnected status, and not tack it on as a pseudo-severity level, which would help strange cases like this, but I think its probably too late to change that.

[hhslepicka]

It is never too late if we give notice about it! ;) For the styling implementation I will propose a change on alarms so let's get together and discuss this new idea before moving forward with the fix! ;)

[teddyrendahl]

On kind of a side note, it might be nice to put some of the state machine logic on the BasePlugin itself. Reduces the overhead for new data sources. i.e when ever you want to run connection_state_changed we send the updated values of alarm / value for you.