Merge pull request #2309 from ControlSystemStudio/alarm_disable_fix

kasemir · web-flow · commit 5dface58467e · 2022-06-24T13:40:42.000-04:00
Fix disabled/delayed/filtered/shelved alarms
diff --git a/app/alarm/Readme.md b/app/alarm/Readme.md
@@ -24,10 +24,10 @@ kafka in `/opt/kafka`.
     # that can be used with a kafka server in the same directory
     cd examples
 
-    # Use wget, 'curl -O', or web browser
-    wget http://ftp.wayne.edu/apache/kafka/2.3.0/kafka_2.12-2.3.0.tgz
-    tar -vzxf kafka_2.12-2.3.0.tgz
-    ln -s kafka_2.12-2.3.0 kafka
+    # Use wget, 'curl -O', or web browser to fetch a recent version of kafka
+    wget https://dlcdn.apache.org/kafka/3.2.0/kafka_2.13-3.2.0.tgz
+    tar -vzxf kafka_2.13-3.2.0.tgz
+    ln -s kafka_2.13-3.2.0 kafka
 
 Check `config/zookeeper.properties` and `config/server.properties`.
 By default these contain settings for keeping data in `/tmp/`, which works for initial tests,
@@ -45,11 +45,11 @@ Similarly, change the directory setting in `server.properties`
     log.dirs=/tmp/kafka-logs
 
 
-Kafka depends on Zookeeper. By default, Kafka will quit if it cannot connect to Zookeeper within 6 seconds.
-When the Linux host boots up, this may not be long enough to allow Zookeeper to start.
+Kafka depends on Zookeeper. Kafka will quit if it cannot connect to Zookeeper within some timeout.
+When the Linux host boots up, the default timeout may not be long enough to allow Zookeeper to start.
 
-    # Timeout in ms for connecting to zookeeper defaults to 6000ms.
-    # Suggest a much longer time (5 minutes)
+    # Timeout in ms for connecting to zookeeper
+    # Suggest about 5 minutes
     zookeeper.connection.timeout.ms=300000
 
 By default, Kafka will automatically create topics.
@@ -66,6 +66,7 @@ If the following "First steps" generate errors of the type
 
     WARN Error while fetching metadata with correlation id 39 : .. LEADER_NOT_AVAILABLE
 or
+
     ERROR ..TimeoutException: Timed out waiting for a node assignment
     
 then define the host name in  `config/server.properties`.
diff --git a/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/AlarmTreeView.java b/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/AlarmTreeView.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2018-2020 Oak Ridge National Laboratory.
+ * Copyright (c) 2018-2022 Oak Ridge National Laboratory.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -22,7 +22,6 @@
 import java.util.logging.Level;
 import java.util.stream.Collectors;
 
-import javafx.scene.image.ImageView;
 import org.phoebus.applications.alarm.client.AlarmClient;
 import org.phoebus.applications.alarm.client.AlarmClientLeaf;
 import org.phoebus.applications.alarm.client.AlarmClientListener;
@@ -62,6 +61,7 @@
 import javafx.scene.control.Tooltip;
 import javafx.scene.control.TreeItem;
 import javafx.scene.control.TreeView;
+import javafx.scene.image.ImageView;
 import javafx.scene.input.ClipboardContent;
 import javafx.scene.input.Dragboard;
 import javafx.scene.input.TransferMode;
@@ -493,7 +493,7 @@ private void addClickSupport()
 
             final AlarmTreeItem<?> item = tree_view.getSelectionModel().getSelectedItems().get(0).getValue();
             final ItemConfigDialog dialog = new ItemConfigDialog(model, item);
-            DialogHelper.positionDialog(dialog, tree_view, -250, -400);
+            DialogHelper.positionDialog(dialog, tree_view, -150, -300);
             // Show dialog, not waiting for it to close with OK or Cancel
             dialog.show();
         });
diff --git a/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/ConfigureComponentAction.java b/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/ConfigureComponentAction.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2018 Oak Ridge National Laboratory.
+ * Copyright (c) 2018-2022 Oak Ridge National Laboratory.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -33,7 +33,7 @@ public ConfigureComponentAction(final Node node, final AlarmClient model, final
         setOnAction(event -> Platform.runLater(() ->
         {
             final ItemConfigDialog dialog = new ItemConfigDialog(model, item);
-            DialogHelper.positionDialog(dialog, node, -250, -400);
+            DialogHelper.positionDialog(dialog, node, -150, -300);
             // Show dialog, not waiting for it to close with OK or Cancel
             dialog.show();
         }));
diff --git a/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/ItemConfigDialog.java b/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/ItemConfigDialog.java
@@ -24,6 +24,7 @@
 import javafx.beans.value.ChangeListener;
 import javafx.event.ActionEvent;
 import javafx.event.EventHandler;
+import javafx.geometry.Pos;
 import javafx.scene.control.Button;
 import javafx.scene.control.ButtonType;
 import javafx.scene.control.CheckBox;
@@ -34,6 +35,7 @@
 import javafx.scene.control.Spinner;
 import javafx.scene.control.TextField;
 import javafx.scene.control.Tooltip;
+import javafx.scene.layout.ColumnConstraints;
 import javafx.scene.layout.GridPane;
 import javafx.scene.layout.HBox;
 import javafx.scene.layout.Priority;
@@ -69,6 +71,13 @@ public ItemConfigDialog(final AlarmClient model, final AlarmTreeItem<?> item)
         layout.setHgap(5);
         layout.setVgap(5);
 
+        // First fixed-size column for labels
+        // Second column grows
+        final ColumnConstraints col1 = new ColumnConstraints(190);
+        final ColumnConstraints col2 = new ColumnConstraints();
+        col2.setHgrow(Priority.ALWAYS);
+        layout.getColumnConstraints().setAll(col1, col2);
+
         int row = 0;
 
         // Show item path, allow copying it out.
@@ -90,9 +99,10 @@ public ItemConfigDialog(final AlarmClient model, final AlarmTreeItem<?> item)
 
             layout.add(new Label("Behavior:"), 0, row);
             enabled = new CheckBox("Enabled");
-            enabled.setTooltip(new Tooltip("Enable alarms? See also filter expression"));
+            enabled.setTooltip(new Tooltip("Enable alarms? See also 'Enabling Filter'"));
             enabled.setSelected(leaf.isEnabled());
-            enabled.setOnAction((event) -> {
+            enabled.setOnAction((event) ->
+            {
                 relative_date.getSelectionModel().clearSelection();
                 relative_date.setValue(null);
                 enabled_date_picker.getEditor().clear();
@@ -112,34 +122,41 @@ public ItemConfigDialog(final AlarmClient model, final AlarmTreeItem<?> item)
 
             layout.add(new Label("Disable until:"), 0, row);
             enabled_date_picker = new DateTimePicker();
+            enabled_date_picker.setTooltip(new Tooltip("Select a date until which the alarm should be disabled"));
             enabled_date_picker.setDateTimeValue(leaf.getEnabledDate());
+            enabled_date_picker.setPrefSize(280, 25);
+
             relative_date = new ComboBox<String>();
+            relative_date.setTooltip(new Tooltip("Select a predefined duration for disabling the alarm"));
             relative_date.getItems().addAll(AlarmSystem.shelving_options);
+            relative_date.setPrefSize(200, 25);
 
-            final EventHandler<ActionEvent> relative_event_handler = new EventHandler<>() {
-                @Override public void handle(ActionEvent e) {
-                    enabled.setSelected(false);
-                    enabled_date_picker.getEditor().clear();
-                }
+            final EventHandler<ActionEvent> relative_event_handler = (ActionEvent e) ->
+            {
+                enabled.setSelected(false);
+                enabled_date_picker.getEditor().clear();
             };
 
             relative_date.setOnAction(relative_event_handler);
 
             // setOnAction for relative date must be set to null as to not trigger event when setting value
-            enabled_date_picker.setOnAction(new EventHandler<ActionEvent>() {
-                    @Override public void handle(ActionEvent e) {
-                        if (enabled_date_picker.getDateTimeValue() != null) {
-                            relative_date.setOnAction(null);
-                            enabled.setSelected(false);
-                            enabled_date_picker.getEditor().commitValue();
-                            relative_date.getSelectionModel().clearSelection();
-                            relative_date.setValue(null);
-                            relative_date.setOnAction(relative_event_handler);
-                        };
-                    }
+            enabled_date_picker.setOnAction((ActionEvent e) ->
+            {
+                if (enabled_date_picker.getDateTimeValue() != null)
+                {
+                    relative_date.setOnAction(null);
+                    enabled.setSelected(false);
+                    enabled_date_picker.getEditor().commitValue();
+                    relative_date.getSelectionModel().clearSelection();
+                    relative_date.setValue(null);
+                    relative_date.setOnAction(relative_event_handler);
+                };
             });
 
-            layout.add(new HBox(10, enabled_date_picker, relative_date), 1, row++);
+            final HBox until_box = new HBox(10, enabled_date_picker, relative_date);
+            until_box.setAlignment(Pos.CENTER);
+            HBox.setHgrow(relative_date, Priority.ALWAYS);
+            layout.add(until_box, 1, row++);
 
             layout.add(new Label("Alarm Delay [seconds]:"), 0, row);
             delay = new Spinner<>(0, Integer.MAX_VALUE, leaf.getDelay());
@@ -170,7 +187,7 @@ public ItemConfigDialog(final AlarmClient model, final AlarmTreeItem<?> item)
             count.setPrefWidth(80);
             layout.add(count, 1, row++);
 
-            layout.add(new Label("Enabling Filter"), 0, row);
+            layout.add(new Label("Enabling Filter:"), 0, row);
             filter = new TextField(leaf.getFilter());
             filter.setTooltip(new Tooltip("Optional expression for enabling the alarm"));
             layout.add(filter, 1, row++);
@@ -201,28 +218,25 @@ public ItemConfigDialog(final AlarmClient model, final AlarmTreeItem<?> item)
         // 'dummy' is used for that.
 
         // Guidance:
-        layout.add(new Label("Guidance:"), 0, row);
-        final Label dummy = new Label("");
-        GridPane.setHgrow(dummy, Priority.ALWAYS);
-        layout.add(dummy, 1, row++);
+        layout.add(new Label("Guidance:"), 0, row++, 2, 1);
         guidance = new TitleDetailTable(item.getGuidance());
         guidance.setPrefHeight(100);
         layout.add(guidance, 0, row++, 2, 1);
 
         // Displays:
-        layout.add(new Label("Displays:"), 0, row++);
+        layout.add(new Label("Displays:"), 0, row++, 2, 1);
         displays = new TitleDetailTable(item.getDisplays());
         displays.setPrefHeight(100);
         layout.add(displays, 0, row++, 2, 1);
 
         // Commands:
-        layout.add(new Label("Commands:"), 0, row++);
+        layout.add(new Label("Commands:"), 0, row++, 2, 1);
         commands = new TitleDetailTable(item.getCommands());
         commands.setPrefHeight(100);
         layout.add(commands, 0, row++, 2, 1);
 
         // Automated Actions:
-        layout.add(new Label("Automated Actions:"), 0, row++);
+        layout.add(new Label("Automated Actions:"), 0, row++, 2, 1);
         actions = new TitleDetailDelayTable(item.getActions());
         actions.setPrefHeight(100);
         layout.add(actions, 0, row++, 2, 1);
@@ -231,9 +245,9 @@ public ItemConfigDialog(final AlarmClient model, final AlarmTreeItem<?> item)
         final ScrollPane scroll = new ScrollPane(layout);
 
         // Scroll pane stops the content from resizing,
-        // so tell content to use the widths of the scroll pane,
-        // minus some border, and suggest minimum width
-        scroll.widthProperty().addListener((p, old, width) -> layout.setPrefWidth(Math.max(width.doubleValue()-25, 450)));
+        // so tell content to use the widths of the scroll pane
+        // minus 40 to provide space for the scroll bar, and suggest minimum width
+        scroll.widthProperty().addListener((p, old, width) -> layout.setPrefWidth(Math.max(width.doubleValue()-40, 450)));
 
         getDialogPane().setContent(scroll);
         setResizable(true);
diff --git a/services/alarm-server/src/main/java/org/phoebus/applications/alarm/server/AlarmLogic.java b/services/alarm-server/src/main/java/org/phoebus/applications/alarm/server/AlarmLogic.java
@@ -61,7 +61,7 @@ public class AlarmLogic // implements GlobalAlarmListener
     /** 'Current' state that was received while disabled.
      *  Is cached in case we get re-enabled, whereupon it is used
      */
-    private volatile AlarmState disabled_state = null;
+    private final AtomicReference<AlarmState> disabled_state = new AtomicReference<>();
 
     /** Latch the highest received alarm severity/status?
      *  When <code>false</code>, the latched alarm state actually
@@ -189,7 +189,7 @@ public boolean setEnabled(final boolean enable)
         if (!enable)
         {   // Disabled
             // Remember current PV state in case we're re-enabled
-            disabled_state = current_state;
+            disabled_state.set(current_state);
             // Otherwise pretend all is OK, using special message
 
             final AlarmState current = AlarmState.createClearState(current_state.value);
@@ -205,11 +205,9 @@ public boolean setEnabled(final boolean enable)
                 alarm_state = new AlarmState(SeverityLevel.OK,
                                              SeverityLevel.OK.name(), "", Instant.now());
             // (Re-)enabled
-            if (disabled_state != null)
-            {
-                computeNewState(disabled_state);
-                disabled_state = null;
-            }
+            final AlarmState saved_state = disabled_state.getAndSet(null);
+            if (saved_state != null)
+                computeNewState(saved_state);
         }
         return true;
     }
@@ -321,7 +319,7 @@ public void computeNewState(final AlarmState received_state)
             // When disabled, ignore...
             if (!enabled.get())
             {
-                disabled_state = received_state;
+                disabled_state.set(received_state);
                 return;
             }
             // Remember what used to be the 'current' severity
@@ -590,6 +588,10 @@ public void acknowledge(boolean acknowledge)
      */
     public void dispose()
     {
+        // Indicate that PV has been disposed, but call it OK.
+        // This way a re-started PV which then receives its first update will take
+        // any non-OK state as a change and react accordingly
+        current_state = alarm_state = new AlarmState(SeverityLevel.OK, "Disposed", "Disposed", Instant.now());
         delayed_check.cancel();
     }
 
diff --git a/services/alarm-server/src/main/java/org/phoebus/applications/alarm/server/AlarmServerPV.java b/services/alarm-server/src/main/java/org/phoebus/applications/alarm/server/AlarmServerPV.java
@@ -25,9 +25,9 @@
 import org.phoebus.applications.alarm.Messages;
 import org.phoebus.applications.alarm.client.ClientState;
 import org.phoebus.applications.alarm.model.AlarmState;
-import org.phoebus.applications.alarm.model.EnabledState;
 import org.phoebus.applications.alarm.model.AlarmTreeItem;
 import org.phoebus.applications.alarm.model.AlarmTreeLeaf;
+import org.phoebus.applications.alarm.model.EnabledState;
 import org.phoebus.applications.alarm.model.SeverityLevel;
 import org.phoebus.applications.alarm.model.TitleDetailDelay;
 import org.phoebus.applications.alarm.server.actions.AutomatedActions;
@@ -59,6 +59,8 @@ public class AlarmServerPV extends AlarmTreeItem<AlarmState> implements AlarmTre
         return thread;
     });
 
+    private final ServerModel model;
+
     private volatile String description = "";
 
     private final AlarmLogic logic;
@@ -84,6 +86,7 @@ public class AlarmServerPV extends AlarmTreeItem<AlarmState> implements AlarmTre
     public AlarmServerPV(final ServerModel model, final String parent_path, final String name, final ClientState initial)
     {
         super(parent_path, name, Collections.emptyList());
+        this.model = model;
         description = name;
 
         final AlarmState current_state;
@@ -123,12 +126,6 @@ public void annunciateAlarm(final SeverityLevel severity)
             {
                 model.sendAnnunciatorMessage(getPathName(), severity, description);
             }
-            @Override
-            public void alarmEnablementChanged(boolean enable) {
-                model.sendConfigUpdate(getPathName(), AlarmServerPV.this);
-            }
-
-
         };
         logic = new AlarmLogic(listener, true, true, 0, 0, current_state, alarm_state, 0);
     }
@@ -416,8 +413,12 @@ private void filterChanged(final double value)
     private void enabledDateTimeFilterChanged(final boolean enabled)
     {
         setEnabled(enabled);
-    }
 
+        // Alarm server should _listen_ to config changes, not send them, with one exception:
+        // If a 'shelved' alarm gets re-enabled by the alarm server, it sends that out
+        if (enabled)
+            model.sendConfigUpdate(getPathName(), this);
+    }
 
     public void stop()
     {
diff --git a/services/alarm-server/src/main/java/org/phoebus/applications/alarm/server/EnabledDateTimeFilter.java b/services/alarm-server/src/main/java/org/phoebus/applications/alarm/server/EnabledDateTimeFilter.java
@@ -7,18 +7,14 @@
  ******************************************************************************/
 package org.phoebus.applications.alarm.server;
 
-import static org.phoebus.applications.alarm.AlarmSystem.logger;
-
-import org.phoebus.applications.alarm.client.AlarmClient;
-
+import java.time.Duration;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
-import java.time.LocalDateTime;
-import java.time.Duration;
-import java.time.format.DateTimeFormatter;
 
 import org.phoebus.framework.jobs.NamedThreadFactory;
 
