diff --git a/app/save-and-restore/app/doc/images/filter-pv-items.png b/app/save-and-restore/app/doc/images/filter-pv-items.png new file mode 100644 index 0000000000..6adec2d567 Binary files /dev/null and b/app/save-and-restore/app/doc/images/filter-pv-items.png differ diff --git a/app/save-and-restore/app/doc/index.rst b/app/save-and-restore/app/doc/index.rst index eab505e600..eebaa67192 100644 --- a/app/save-and-restore/app/doc/index.rst +++ b/app/save-and-restore/app/doc/index.rst @@ -324,12 +324,25 @@ Prior to restore user has the option to: Restoring from a composite snapshot works in the same manner as the restore operation from a single-snapshot. +Filter PV items in list +^^^^^^^^^^^^^^^^^^^^^^^ +The list of items in the snapshot view can be filtered based on the PV name. See screenshot for highlighted UI element +where user may specify a string pattern to match PV names. Non-matching items will be hidden from the list view and +**also excluded from a restore operation**. + +To filter the view without excluding PV items from a restore operation, user needs to tick the “Preserve selection…” checkbox. + +.. image:: images/filter-pv-items.png + :width: 80% + + Restore from context menu ^^^^^^^^^^^^^^^^^^^^^^^^^ User may invoke a restore operation (from client or from service) from context menu items in the tree -view or in the search-and-filer view. In this case user will not have the possibility to unselect specific PVs. -However, PV items configured as read-only will not be restored. +view or in the search-and-filter view. In this case user will not have the possibility to deselect specific PVs. +Filtering/exclusion based on PV name will not be possible either. +However, PV items configured as read-only will always be excluded from a restore operation. Restore result ^^^^^^^^^^^^^^ diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/Messages.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/Messages.java index e5ec5bf940..db0047186a 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/Messages.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/Messages.java @@ -34,7 +34,10 @@ public class Messages { public static String cannotCompareTitle; public static String closeConfigurationWarning; public static String closeCompositeSnapshotWarning; - public static String closeTabPrompt; + public static String closeSnapshotWarning; + public static String closeCompositeSnapshotTabPrompt; + public static String closeConfigurationTabPrompt; + public static String closeSnapshotTabPrompt; public static String compositeSnapshotConsistencyCheckFailed; public static String contextMenuAddTag; @Deprecated @@ -120,7 +123,6 @@ public class Messages { public static String overwrite; public static String paste; - public static String promptCloseSnapshotTabContent; public static String promptDeleteSelectedTitle; public static String promptDeleteSelectedHeader; public static String promptDeleteSelectedContent; diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/SaveAndRestoreInstance.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/SaveAndRestoreInstance.java index e763e3e75b..c566fdafe0 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/SaveAndRestoreInstance.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/SaveAndRestoreInstance.java @@ -18,6 +18,7 @@ package org.phoebus.applications.saveandrestore; +import javafx.application.Platform; import javafx.fxml.FXMLLoader; import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreController; import org.phoebus.framework.nls.NLS; @@ -25,7 +26,7 @@ import org.phoebus.framework.spi.AppDescriptor; import org.phoebus.framework.spi.AppInstance; import org.phoebus.security.tokens.ScopedAuthenticationToken; -import org.phoebus.ui.docking.DockItem; +import org.phoebus.ui.docking.DockItemWithInput; import org.phoebus.ui.docking.DockPane; import java.net.URI; @@ -37,9 +38,10 @@ public class SaveAndRestoreInstance implements AppInstance { + private final AppDescriptor appDescriptor; private final SaveAndRestoreController saveAndRestoreController; - private DockItem dockItem; + private DockItemWithInput dockItem; public static SaveAndRestoreInstance INSTANCE; @@ -53,21 +55,65 @@ public SaveAndRestoreInstance(AppDescriptor appDescriptor) { ResourceBundle resourceBundle = NLS.getMessages(Messages.class); loader.setResources(resourceBundle); loader.setLocation(SaveAndRestoreApplication.class.getResource("ui/SaveAndRestoreUI.fxml")); - dockItem = new DockItem(this, loader.load()); + dockItem = new DockItemWithInput(this, loader.load(), null, null, null); } catch (Exception e) { Logger.getLogger(SaveAndRestoreInstance.class.getName()).log(Level.SEVERE, "Failed loading fxml", e); } saveAndRestoreController = loader.getController(); + + // When user closes Phoebus... dockItem.addCloseCheck(() -> { - saveAndRestoreController.handleTabClosed(); - INSTANCE = null; - return CompletableFuture.completedFuture(true); + boolean mayClose = mayClose(); + if (mayClose) { + saveAndRestoreController.handleTabClosed(); + INSTANCE = null; + } + return CompletableFuture.completedFuture(mayClose); + }); + + // When user closes save&restore app... + dockItem.setOnCloseRequest(event -> { + if (mayClose()) { + saveAndRestoreController.handleTabClosed(); + INSTANCE = null; + } else { + event.consume(); + } }); DockPane.getActiveDockPane().addTab(dockItem); } + /** + * Checks with each tab if it may be closed. + * + * @return false if any of the tabs contains unsaved data, otherwise true. + */ + private boolean mayClose() { + if (Platform.isFxApplicationThread()) { + return saveAndRestoreController.doCloseCheck(); + } else { + CompletableFuture completableFuture = new CompletableFuture<>(); + Platform.runLater(() -> { + try { + boolean okToClose = saveAndRestoreController.doCloseCheck(); + completableFuture.complete(okToClose); + } catch (Exception e) { + Logger.getLogger(SaveAndRestoreInstance.class.getName()).log(Level.SEVERE, + "Got exception when checking if OK to close", e); + completableFuture.complete(true); + } + }); + try { + return completableFuture.get(); + } catch (Exception e) { + Logger.getLogger(SaveAndRestoreInstance.class.getName()).log(Level.WARNING, "Got exception when waiting for close check evaluation", e); + return true; + } + } + } + @Override public AppDescriptor getAppDescriptor() { return appDescriptor; diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreBaseController.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreBaseController.java index fc30652bcf..9b992cf579 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreBaseController.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreBaseController.java @@ -76,8 +76,14 @@ public SimpleStringProperty getUserIdentity() { protected void handleWebSocketMessage(SaveAndRestoreWebSocketMessage webSocketMessage){ } + /** + * Performs suitable cleanup, e.g. close web socket and PVs (where applicable). + */ + public abstract void handleTabClosed(); - protected boolean handleTabClosed(){ - return true; - } + /** + * Checks if the tab may be closed, e.g. if data managed in the UI has been saved. + * @return false if tab contains unsaved data, otherwise true + */ + public abstract boolean doCloseCheck(); } diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java index ce510528f7..db4b76af71 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java @@ -937,11 +937,11 @@ public void saveLocalState() { } @Override - public boolean handleTabClosed() { + public void handleTabClosed() { + tabPane.getTabs().forEach(t -> ((SaveAndRestoreTab)t).handleTabClosed()); saveLocalState(); webSocketClientService.closeWebSocket(); filterActivators.forEach(FilterActivator::stop); - return true; } /** @@ -1436,7 +1436,7 @@ private void addOptionalLoggingMenuItem() { } @Override - public void handleWebSocketMessage(SaveAndRestoreWebSocketMessage saveAndRestoreWebSocketMessage) { + public void handleWebSocketMessage(SaveAndRestoreWebSocketMessage saveAndRestoreWebSocketMessage) { switch (saveAndRestoreWebSocketMessage.messageType()) { case NODE_ADDED -> nodeAdded((String) saveAndRestoreWebSocketMessage.payload()); case NODE_REMOVED -> nodeRemoved((String) saveAndRestoreWebSocketMessage.payload()); @@ -1496,4 +1496,14 @@ private void handleWebSocketDisconnected() { serviceConnected.setValue(false); saveLocalState(); } + + @Override + public boolean doCloseCheck(){ + for(Tab tab : tabPane.getTabs()){ + if(!((SaveAndRestoreTab)tab).doCloseCheck()){ + return false; + } + } + return true; + } } diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreTab.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreTab.java index 195532a589..7edab3004d 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreTab.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreTab.java @@ -38,7 +38,6 @@ public abstract class SaveAndRestoreTab extends Tab implements WebSocketMessageHandler { protected SaveAndRestoreBaseController controller; - protected WebSocketClientService webSocketClientService; public SaveAndRestoreTab() { ContextMenu contextMenu = new ContextMenu(); @@ -63,17 +62,13 @@ public SaveAndRestoreTab() { contextMenu.getItems().addAll(closeAll, closeOthers); setContextMenu(contextMenu); - webSocketClientService = WebSocketClientService.getInstance(); - setOnCloseRequest(event -> { - if (!controller.handleTabClosed()) { - event.consume(); + if (doCloseCheck()) { + handleTabClosed(); } else { - webSocketClientService.removeWebSocketMessageHandler(this); + event.consume(); } }); - - webSocketClientService.addWebSocketMessageHandler(this); } /** @@ -89,4 +84,19 @@ public void secureStoreChanged(List validTokens) { public void handleWebSocketMessage(SaveAndRestoreWebSocketMessage saveAndRestoreWebSocketMessage) { } + + /** + * Performs suitable cleanup, e.g. close web socket and PVs (where applicable). + */ + public void handleTabClosed() { + controller.handleTabClosed(); + } + + /** + * Checks if the tab may be closed, e.g. if data managed in the UI has been saved. + * @return false if tab contains unsaved data, otherwise true + */ + public boolean doCloseCheck() { + return controller.doCloseCheck(); + } } diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/configuration/ConfigurationController.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/configuration/ConfigurationController.java index e62ca06684..9e252b8738 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/configuration/ConfigurationController.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/configuration/ConfigurationController.java @@ -67,6 +67,7 @@ import org.phoebus.framework.jobs.JobManager; import org.phoebus.framework.selection.SelectionService; import org.phoebus.ui.application.ContextMenuHelper; +import org.phoebus.ui.dialog.DialogHelper; import org.phoebus.ui.dialog.ExceptionDetailsErrorDialog; import org.phoebus.ui.javafx.FocusUtil; import org.phoebus.ui.javafx.ImageCache; @@ -86,7 +87,7 @@ public class ConfigurationController extends SaveAndRestoreBaseController implem @FXML @SuppressWarnings("unused") - private BorderPane root; + private BorderPane borderPane; @FXML @SuppressWarnings("unused") @@ -485,7 +486,7 @@ public void loadConfiguration(final Node node) { try { configurationData = saveAndRestoreService.getConfiguration(node.getUniqueId()); } catch (Exception e) { - Platform.runLater(() -> ExceptionDetailsErrorDialog.openError(root, Messages.errorGeneric, Messages.errorUnableToRetrieveData, e)); + Platform.runLater(() -> ExceptionDetailsErrorDialog.openError(borderPane, Messages.errorGeneric, Messages.errorUnableToRetrieveData, e)); return; } @@ -513,24 +514,28 @@ public void loadConfiguration(final Node node) { } /** - * Handles clean-up when the associated {@link ConfigurationTab} is closed. * A check is made if content is dirty, in which case user is prompted to cancel or close anyway. * * @return true if content is not dirty or user chooses to close anyway, * otherwise false. */ @Override - public boolean handleTabClosed() { + public boolean doCloseCheck() { if (dirty.get()) { Alert alert = new Alert(Alert.AlertType.CONFIRMATION); - alert.setTitle(Messages.closeTabPrompt); + alert.setTitle(Messages.closeConfigurationTabPrompt); alert.setContentText(Messages.closeConfigurationWarning); + DialogHelper.positionDialog(alert, borderPane, -200, -200); Optional result = alert.showAndWait(); return result.isPresent() && result.get().equals(ButtonType.OK); - } else { - webSocketClientService.removeWebSocketMessageHandler(this); - return true; } + + return true; + } + + @Override + public void handleTabClosed(){ + webSocketClientService.removeWebSocketMessageHandler(this); } @Override diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchAndFilterTab.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchAndFilterTab.java index 7001537dde..5d0005f063 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchAndFilterTab.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchAndFilterTab.java @@ -75,12 +75,4 @@ public SearchAndFilterTab(SaveAndRestoreController saveAndRestoreController) { setText(Messages.search); setGraphic(new ImageView(ImageCache.getImage(ImageCache.class, "/icons/sar-search_18x18.png"))); } - - public void filterActivated(String filterName){ - ((SearchAndFilterViewController)controller).filterActivated(filterName); - } - - public void filterDeactivated(String filterName){ - ((SearchAndFilterViewController)controller).filterDeactivated(filterName); - } } diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchAndFilterViewController.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchAndFilterViewController.java index 25f7d53bbe..ec45d6bf19 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchAndFilterViewController.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchAndFilterViewController.java @@ -648,11 +648,13 @@ public void handleWebSocketMessage(SaveAndRestoreWebSocketMessage saveAndRest } } - public void filterActivated(String filterName){ - + @Override + public void handleTabClosed(){ + webSocketClientService.removeWebSocketMessageHandler(this); } - public void filterDeactivated(String filterName){ - + @Override + public boolean doCloseCheck(){ + return true; } } diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchResultTableViewController.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchResultTableViewController.java index d593898a7e..661c564843 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchResultTableViewController.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchResultTableViewController.java @@ -419,8 +419,13 @@ public void handleWebSocketMessage(SaveAndRestoreWebSocketMessage saveAndRest } } - public boolean handleTabClosed() { + @Override + public void handleTabClosed() { webSocketClientService.removeWebSocketMessageHandler(this); + } + + @Override + public boolean doCloseCheck(){ return true; } } diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/CompositeSnapshotController.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/CompositeSnapshotController.java index e8440c49f4..733f6fef2c 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/CompositeSnapshotController.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/CompositeSnapshotController.java @@ -430,17 +430,21 @@ public void loadCompositeSnapshot(final Node node) { } @Override - public boolean handleTabClosed() { + public boolean doCloseCheck() { if (dirty.get()) { Alert alert = new Alert(Alert.AlertType.CONFIRMATION); - alert.setTitle(Messages.closeTabPrompt); + alert.setTitle(Messages.closeCompositeSnapshotTabPrompt); alert.setContentText(Messages.closeCompositeSnapshotWarning); + DialogHelper.positionDialog(alert, borderPane, -200, -200); Optional result = alert.showAndWait(); return result.isPresent() && result.get().equals(ButtonType.OK); - } else { - webSocketClientService.removeWebSocketMessageHandler(this); - return true; } + return true; + } + + @Override + public void handleTabClosed(){ + webSocketClientService.removeWebSocketMessageHandler(this); } /** diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/SaveAndRestorePV.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/SaveAndRestorePV.java deleted file mode 100644 index 1c3c00c910..0000000000 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/SaveAndRestorePV.java +++ /dev/null @@ -1,135 +0,0 @@ -/* - * Copyright (C) 2020 European Spallation Source ERIC. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * as published by the Free Software Foundation; either version 2 - * of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. - * - */ - -package org.phoebus.applications.saveandrestore.ui.snapshot; - -import org.epics.vtype.VType; -import org.phoebus.saveandrestore.util.VNoData; -import org.phoebus.core.vtypes.VDisconnectedData; -import org.phoebus.pv.PV; -import org.phoebus.pv.PVPool; - -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; -import java.util.logging.Level; -import java.util.logging.Logger; - -/** - * Utility class binding the snapshot table views to a {@link PV} for each row such - * that the view can show the live values. A hardcoded throttling time of 500ms keeps - * update rate of PV values under control. - */ -public class SaveAndRestorePV { - - private final String pvName; - - private final String readbackPvName; - private CountDownLatch countDownLatch; - private PV pv; - private PV readbackPv; - private VType pvValue = VDisconnectedData.INSTANCE; - private VType readbackValue = VDisconnectedData.INSTANCE; - private TableEntry snapshotTableEntry; - - /** - * The time between updates of dynamic data in the table, in ms. - */ - private static final long TABLE_UPDATE_INTERVAL = 500; - - protected SaveAndRestorePV(TableEntry snapshotTableEntry) { - this.snapshotTableEntry = snapshotTableEntry; - this.pvName = patchPvName(snapshotTableEntry.pvNameProperty().get()); - this.readbackPvName = patchPvName(snapshotTableEntry.readbackNameProperty().get()); - - try { - pv = PVPool.getPV(pvName); - pv.onValueEvent().throttleLatest(TABLE_UPDATE_INTERVAL, TimeUnit.MILLISECONDS).subscribe(value -> { - pvValue = org.phoebus.pv.PV.isDisconnected(value) ? VDisconnectedData.INSTANCE : value; - this.snapshotTableEntry.setLiveValue(pvValue); - }); - - if (readbackPvName != null && !readbackPvName.isEmpty()) { - readbackPv = PVPool.getPV(readbackPvName); - readbackPv.onValueEvent() - .throttleLatest(TABLE_UPDATE_INTERVAL, TimeUnit.MILLISECONDS) - .subscribe(value -> { - this.readbackValue = org.phoebus.pv.PV.isDisconnected(value) ? VDisconnectedData.INSTANCE : value; - this.snapshotTableEntry.setReadbackValue(this.readbackValue); - }); - } else { - // If configuration does not define read-back PV, then UI should show "no data" rather than "disconnected" - this.snapshotTableEntry.setReadbackValue(VNoData.INSTANCE); - } - } catch (Exception e) { - Logger.getLogger(SaveAndRestorePV.class.getName()).log(Level.INFO, "Error connecting to PV", e); - } - } - - private String patchPvName(String pvName) { - if (pvName == null || pvName.isEmpty()) { - return null; - } else if (pvName.startsWith("ca://") || pvName.startsWith("pva://")) { - return pvName.substring(pvName.lastIndexOf('/') + 1); - } else { - return pvName; - } - } - - public void setCountDownLatch(CountDownLatch countDownLatch) { - this.countDownLatch = countDownLatch; - } - - public void countDown() { - this.countDownLatch.countDown(); - } - - public void setSnapshotTableEntry(TableEntry snapshotTableEntry) { - this.snapshotTableEntry = snapshotTableEntry; - this.snapshotTableEntry.setLiveValue(pv.read()); - } - - void dispose() { - if (pv != null) { - PVPool.releasePV(pv); - } - if (readbackPv != null) { - PVPool.releasePV(readbackPv); - } - } - - public PV getPv() { - return pv; - } - - public PV getReadbackPv() { - return readbackPv; - } - - public String getPvName() { - return pvName; - } - - public String getReadbackPvName() { - return readbackPvName; - } - - public VType getReadbackValue() { - return readbackValue; - } -} diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/SnapshotController.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/SnapshotController.java index 1619a33834..fc79edaba4 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/SnapshotController.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/SnapshotController.java @@ -5,11 +5,13 @@ import javafx.application.Platform; import javafx.beans.binding.Bindings; +import javafx.beans.property.BooleanProperty; import javafx.beans.property.ObjectProperty; import javafx.beans.property.ReadOnlyObjectWrapper; import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.SimpleObjectProperty; import javafx.beans.property.SimpleStringProperty; +import javafx.beans.property.StringProperty; import javafx.collections.ObservableList; import javafx.event.ActionEvent; import javafx.fxml.FXML; @@ -106,10 +108,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.Date; -import java.util.HashMap; -import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.ServiceLoader; import java.util.concurrent.atomic.AtomicBoolean; @@ -287,8 +286,6 @@ public class SnapshotController extends SaveAndRestoreBaseController implements @FXML protected VBox progressIndicator; - protected final Map pvs = new HashMap<>(); - protected Node configurationNode; public static final Logger LOGGER = Logger.getLogger(SnapshotController.class.getName()); @@ -299,7 +296,6 @@ public class SnapshotController extends SaveAndRestoreBaseController implements private final SimpleStringProperty tabIdProperty = new SimpleStringProperty(); private final SimpleObjectProperty tabGraphicImageProperty = new SimpleObjectProperty<>(); - private List> regexPatterns = new ArrayList<>(); protected final SimpleStringProperty snapshotNameProperty = new SimpleStringProperty(); private final SimpleStringProperty snapshotCommentProperty = new SimpleStringProperty(); @@ -319,10 +315,9 @@ public class SnapshotController extends SaveAndRestoreBaseController implements private final SimpleObjectProperty restoreModeProperty = new SimpleObjectProperty<>(RestoreMode.CLIENT_RESTORE); /** - * List of snapshots used managed in this controller. Index 0 is always the base snapshot, - * all others are snapshots added in the compare use-case. + * List of snapshots added when user chooses to compare base snapshot with other snapshots. */ - protected List snapshots = new ArrayList<>(); + protected List additionalSnapshots = new ArrayList<>(); private final SimpleBooleanProperty selectionInverted = new SimpleBooleanProperty(false); private final SimpleBooleanProperty showReadbacks = new SimpleBooleanProperty(false); @@ -333,6 +328,16 @@ public class SnapshotController extends SaveAndRestoreBaseController implements */ private final SimpleObjectProperty nodeTypeProperty = new SimpleObjectProperty<>(NodeType.SNAPSHOT); + /** + * {@link StringProperty} holding the text of the filter {@link TextField}. + */ + private final StringProperty filterTextProperty = new SimpleStringProperty(""); + + /** + * {@link BooleanProperty} holding state of the Preserve selection checkbox + */ + private final BooleanProperty preserveSelectionProperty = new SimpleBooleanProperty(false); + private SnapshotUtil snapshotUtil; /** * Used to disable portions of the UI when long-lasting operations are in progress, e.g. @@ -341,19 +346,22 @@ public class SnapshotController extends SaveAndRestoreBaseController implements protected final SimpleBooleanProperty disabledUi = new SimpleBooleanProperty(false); /** - * The final {@link Snapshot} object holding the data for this controller. When user performs operations (loading, - * taking and saving snapshots), the fields of this object are updated accordingly. + * The {@link Snapshot} object holding the data for this controller. */ - private final Snapshot snapshot = new Snapshot(); - + private Snapshot snapshot; /** - * {@link Map} of {@link TableEntry} items corresponding to the snapshot data, i.e. - * one per PV as defined in the snapshot's configuration. This map is used to + * {@link List} of {@link TableEntry} items corresponding to the snapshot data, i.e. + * one per PV as defined in the snapshot's configuration. This {@link List} is used to * populate the {@link TableView}, but other parameters (e.g. hideEqualItems) may - * determine which elements in the {@link Map} to actually represent. + * determine which elements in the {@link List} to actually represent. + * + *

+ * Note that the list is cleared and recreated whenever snapshot data has changed, i.e. + * when retrieved from service or when taking a snapshot. + *

*/ - protected final Map tableEntryItems = new LinkedHashMap<>(); + protected final List tableEntryItems = new ArrayList<>(); public SnapshotController(SnapshotTab snapshotTab) { snapshotTab.textProperty().bind(tabTitleProperty); @@ -363,13 +371,9 @@ public SnapshotController(SnapshotTab snapshotTab) { snapshotTab.setGraphic(imageView); } - @FXML public void initialize() { - Node snapshotNode = Node.builder().nodeType(NodeType.SNAPSHOT).build(); - snapshot.setSnapshotNode(snapshotNode); - // Locate registered SaveAndRestoreEventReceivers eventReceivers = ServiceLoader.load(SaveAndRestoreEventReceiver.class); progressIndicator.visibleProperty().bind(disabledUi); @@ -449,28 +453,9 @@ public void initialize() { String filterShortcutName = (new KeyCodeCombination(KeyCode.F, KeyCombination.SHORTCUT_DOWN)).getDisplayText(); filterTextField.setPromptText("* for all matching and , as or separator, & as and separator. Start with / for regex. All if empty. (" + filterShortcutName + ")"); - - filterTextField.addEventHandler(KeyEvent.ANY, event -> { - String filterText = filterTextField.getText().trim(); - - List filters = Arrays.asList(filterText.split(",")); - regexPatterns = filters.stream() - .map(item -> { - if (item.startsWith("/")) { - return List.of(Pattern.compile(item.substring(1, item.length() - 1).trim())); - } else { - return Arrays.stream(item.split("&")) - .map(andItem -> andItem.replaceAll("\\*", ".*")) - .map(andItem -> Pattern.compile(andItem.trim())) - .collect(Collectors.toList()); - } - }).collect(Collectors.toList()); - - applyFilter(filterText, preserveSelectionCheckBox.isSelected(), regexPatterns); - }); - - preserveSelectionCheckBox.selectedProperty() - .addListener((observableValue, aBoolean, isSelected) -> applyPreserveSelection(isSelected)); + filterTextField.textProperty().bindBidirectional(filterTextProperty); + filterTextProperty.addListener((obs, o, n) -> applyFilter()); + preserveSelectionCheckBox.selectedProperty().bindBidirectional(preserveSelectionProperty); showLiveReadbackButton.setGraphic(new ImageView(new Image(getClass().getResourceAsStream("/icons/show_live_readback_column.png")))); showLiveReadbackButton.selectedProperty() @@ -490,10 +475,8 @@ public void initialize() { this.showDeltaPercentage.set(n)); hideEqualItemsButton.setGraphic(new ImageView(new Image(getClass().getResourceAsStream("/icons/hide_show_equal_items.png")))); - hideEqualItemsProperty.bind(hideEqualItemsButton.selectedProperty()); - hideEqualItemsButton.selectedProperty() - .addListener((a, o, n) -> - hideEqualItems()); + hideEqualItemsButton.selectedProperty().bindBidirectional(hideEqualItemsProperty); + hideEqualItemsProperty.addListener((obs, o, n) -> updateTable()); logAction.selectedProperty().bindBidirectional(logActionProperty); @@ -740,6 +723,7 @@ public void initializeViewForNewSnapshot(Node configurationNode) { List configPvs = configurationData.getPvList(); SnapshotData snapshotData = new SnapshotData(); snapshotData.setSnapshotItems(configurationToSnapshotItems(configPvs)); + this.snapshot = new Snapshot(); this.snapshot.setSnapshotData(snapshotData); updateUi(); Platform.runLater(() -> actionResultReadbackColumn.visibleProperty().setValue(false)); @@ -761,8 +745,7 @@ public void takeSnapshot() { actionResultColumn.visibleProperty().set(true); actionResultReadbackColumn.visibleProperty().set(true); }); - this.snapshot.setSnapshotNode(snapshot.get().getSnapshotNode()); - this.snapshot.setSnapshotData(snapshot.get().getSnapshotData()); + this.snapshot = snapshot.get(); updateUi(); } }); @@ -859,33 +842,35 @@ private void updateLoadedSnapshot(TableEntry rowValue, VType newValue) { } /** - * Handles clean-up when the associated {@link SnapshotTab} is closed. * A check is made if content is dirty, in which case user is prompted to cancel or close anyway. * * @return true if content is not dirty or user chooses to close anyway, * otherwise false. */ @Override - public boolean handleTabClosed() { + public boolean doCloseCheck() { if (snapshotDataDirty.get()) { Alert alert = new Alert(Alert.AlertType.CONFIRMATION); - alert.setTitle(Messages.closeTabPrompt); - alert.setContentText(Messages.promptCloseSnapshotTabContent); - DialogHelper.positionDialog(alert, borderPane, -150, -150); + alert.setTitle(Messages.closeSnapshotWarning); + alert.setContentText(Messages.closeSnapshotWarning); + DialogHelper.positionDialog(alert, borderPane, -200, -200); Optional result = alert.showAndWait(); return result.isPresent() && result.get().equals(ButtonType.OK); - } else { - webSocketClientService.removeWebSocketMessageHandler(this); - dispose(); - return true; } + return true; + } + + @Override + public void handleTabClosed(){ + webSocketClientService.removeWebSocketMessageHandler(this); + dispose(); } /** * Releases PV resources. */ private void dispose() { - pvs.values().forEach(SaveAndRestorePV::dispose); + tableEntryItems.forEach(TableEntry::dispose); } private void showLoggingError(String cause) { @@ -909,16 +894,13 @@ public void loadSnapshot(Node snapshotNode) { disabledUi.set(true); JobManager.schedule("Load snapshot items", monitor -> { try { - Snapshot snapshot = getSnapshotFromService(snapshotNode); - this.snapshot.setSnapshotNode(snapshot.getSnapshotNode()); - this.snapshot.setSnapshotData(snapshot.getSnapshotData()); + this.snapshot = getSnapshotFromService(snapshotNode); boolean configurationHasReadbacks = configurationHasReadbackPvs(snapshot.getSnapshotData()); Platform.runLater(() -> { nodeTypeProperty.set(snapshot.getSnapshotNode().getNodeType()); showLiveReadbackButton.setSelected(configurationHasReadbacks); actionResultColumn.visibleProperty().setValue(false); actionResultReadbackColumn.visibleProperty().setValue(false); - snapshotRestorableProperty.set(true); selectedColumn.visibleProperty().set(true); tabTitleProperty.setValue(snapshotNode.getName()); tabIdProperty.setValue(snapshotNode.getUniqueId()); @@ -935,6 +917,7 @@ public void loadSnapshot(Node snapshotNode) { @FXML public void restore() { disabledUi.setValue(true); + tableEntryItems.forEach(tableEntry -> tableEntry.setActionResult(ActionResult.PENDING)); restore(restoreModeProperty.get(), restoreResultList -> { disabledUi.setValue(false); if (logActionProperty.get()) { @@ -1089,7 +1072,7 @@ private void takeSnapshotFromArchiver(Consumer> consumer) { private void takeSnapshotReadPVs(Consumer> consumer) { JobManager.schedule("Take snapshot", monitor -> { // Clear snapshots array - snapshots.clear(); + additionalSnapshots.clear(); List snapshotItems; try { snapshotItems = SaveAndRestoreService.getInstance().takeSnapshot(configurationNode.getUniqueId()); @@ -1144,9 +1127,8 @@ private void showTakeSnapshotResult(List snapshotItems) { for (SnapshotItem snapshotItem : snapshotItems) { if (snapshotItem.getValue().equals(VDisconnectedData.INSTANCE)) { disconnectedPvEncountered.set(true); - Platform.runLater(() -> { - actionResultColumn.setGraphic(new ImageView(ImageCache.getImage(SnapshotController.class, "/icons/error.png"))); - }); + Platform.runLater(() -> + actionResultColumn.setGraphic(new ImageView(ImageCache.getImage(SnapshotController.class, "/icons/error.png")))); break; } } @@ -1154,9 +1136,9 @@ private void showTakeSnapshotResult(List snapshotItems) { if (snapshotItem.getConfigPv().getReadbackPvName() != null && snapshotItem.getReadbackValue() != null && snapshotItem.getReadbackValue().equals(VDisconnectedData.INSTANCE)) { disconnectedReadbackPvEncountered.set(true); - Platform.runLater(() -> { - actionResultReadbackColumn.setGraphic(new ImageView(ImageCache.getImage(SnapshotController.class, "/icons/error.png"))); - }); + Platform.runLater(() -> + actionResultReadbackColumn.setGraphic(new ImageView(ImageCache.getImage(SnapshotController.class, "/icons/error.png")))); + break; } } @@ -1170,27 +1152,20 @@ private void showTakeSnapshotResult(List snapshotItems) { }); } + /** + * Computes thresholds on scalar data types. The threshold is used to indicate that a delta value within threshold + * should not decorate the delta column, i.e. consider saved and live values equal. + * + * @param threshold Threshold in percent + */ private void updateThreshold(double threshold) { - snapshot.getSnapshotData().getSnapshotItems().forEach(item -> { - VType vtype = item.getValue(); - VNumber diffVType; - - double ratio = threshold / 100; - - TableEntry tableEntry = tableEntryItems.get(item.getConfigPv().getPvName()); - if (tableEntry == null) { - tableEntry = tableEntryItems.get(item.getConfigPv().getPvName()); - } - - if (!item.getConfigPv().equals(tableEntry.getConfigPv())) { - return; - } - + double ratio = threshold / 100; + tableEntryItems.forEach(tableEntry -> { + VType vtype = tableEntry.getSnapshotVal().get(); + // Only scalars considered if (vtype instanceof VNumber) { - diffVType = SafeMultiply.multiply((VNumber) vtype, ratio); - VNumber vNumber = diffVType; + VNumber vNumber = SafeMultiply.multiply((VNumber) vtype, ratio); boolean isNegative = vNumber.getValue().doubleValue() < 0; - tableEntry.setThreshold(Optional.of(new Threshold<>(isNegative ? SafeMultiply.multiply(vNumber.getValue(), -1.0) : vNumber.getValue()))); } }); @@ -1202,83 +1177,77 @@ private void updateThreshold(double threshold) { * @param multiplier The (double) factor used to change the snapshot set-points used in restore operation. */ private void updateSnapshotValues(double multiplier) { - snapshot.getSnapshotData().getSnapshotItems() - .forEach(item -> { - TableEntry tableEntry = tableEntryItems.get(item.getConfigPv().getPvName()); - VType vtype = tableEntry.storedSnapshotValue().get(); - VType newVType; - - if (vtype instanceof VNumber) { - newVType = SafeMultiply.multiply((VNumber) vtype, multiplier); - } else if (vtype instanceof VNumberArray) { - newVType = SafeMultiply.multiply((VNumberArray) vtype, multiplier); - } else { - return; - } + tableEntryItems.forEach(tableEntry -> { + VType vtype = tableEntry.storedSnapshotValue().get(); + VType newVType; - item.setValue(newVType); + if (vtype instanceof VNumber) { + newVType = SafeMultiply.multiply((VNumber) vtype, multiplier); + } else if (vtype instanceof VNumberArray) { + newVType = SafeMultiply.multiply((VNumberArray) vtype, multiplier); + } else { + return; + } - tableEntry.snapshotValProperty().set(newVType); + tableEntry.getSnapshotItem().setValue(newVType); + tableEntry.snapshotValProperty().set(newVType); - ObjectProperty value = tableEntry.valueProperty(); - value.setValue(new VTypePair(value.get().base, newVType, value.get().threshold)); - }); + ObjectProperty value = tableEntry.valueProperty(); + value.setValue(new VTypePair(value.get().base, newVType, value.get().threshold)); + }); } - private void applyFilter(String filterText, boolean preserveSelection, List> regexPatterns) { - if (filterText.isEmpty()) { - List arrayList = tableEntryItems.values().stream() + /** + * Applies the filter pattern, if any, to compute which entries to hide/show in the table. + * PV names matching user specified patterns (comma separated) will be maintained in the view. + * Only entries in the view will be subject to restore. + * If however user has ticked the Preserve selection... checkbox, non-matching entries will be hidden, + * but still considered as selected and hence subject to restore. + */ + private void applyFilter() { + if (filterTextProperty.isEmpty().get() && preserveSelectionProperty.not().get()) { + List arrayList = tableEntryItems.stream() .peek(item -> { - if (!preserveSelection) { - if (!item.readOnlyProperty().get()) { - item.selectedProperty().set(true); - } + if (!item.readOnlyProperty().get()) { + item.selectedProperty().set(true); } }).collect(Collectors.toList()); - Platform.runLater(() -> updateTable(arrayList)); - return; - } - - List filteredEntries = tableEntryItems.values().stream() - .filter(item -> { - boolean matchEither = false; - for (List andPatternList : regexPatterns) { - boolean matchAnd = true; - for (Pattern pattern : andPatternList) { - matchAnd &= pattern.matcher(item.pvNameProperty().get()).find(); + } else { + List filters = Arrays.asList(filterTextProperty.get().split(",")); + List> regexPatterns = filters.stream() + .map(item -> { + if (item.startsWith("/")) { + return List.of(Pattern.compile(item.substring(1, item.length() - 1).trim())); + } else { + return Arrays.stream(item.split("&")) + .map(andItem -> andItem.replaceAll("\\*", ".*")) + .map(andItem -> Pattern.compile(andItem.trim())) + .collect(Collectors.toList()); + } + }).toList(); + List filteredEntries = tableEntryItems.stream() + .filter(item -> { + boolean matchEither = false; + for (List andPatternList : regexPatterns) { + boolean matchAnd = true; + for (Pattern pattern : andPatternList) { + matchAnd &= pattern.matcher(item.pvNameProperty().get()).find(); + } + matchEither |= matchAnd; } - matchEither |= matchAnd; - } - - if (!preserveSelection) { - item.selectedProperty().setValue(matchEither); - } else { - matchEither |= item.selectedProperty().get(); - } - - return matchEither; - }).collect(Collectors.toList()); + if (preserveSelectionProperty.not().get()) { + item.selectedProperty().setValue(matchEither); + } - Platform.runLater(() -> updateTable(filteredEntries)); - } + return matchEither; + }).collect(Collectors.toList()); - private void applyPreserveSelection(boolean preserve) { - if (preserve) { - boolean allSelected = tableEntryItems.values().stream().allMatch(item -> item.selectedProperty().get()); - if (allSelected) { - tableEntryItems.values() - .forEach(item -> item.selectedProperty().set(false)); - } + Platform.runLater(() -> updateTable(filteredEntries)); } } - private void hideEqualItems() { - ArrayList arrayList = new ArrayList<>(tableEntryItems.values()); - Platform.runLater(() -> updateTable(arrayList)); - } - /** * Restores a snapshot from client or service. * @@ -1292,10 +1261,9 @@ private void restore(RestoreMode restoreMode, Consumer> comp List restoreResultList = null; try { switch (restoreMode) { - case CLIENT_RESTORE -> - restoreResultList = snapshotUtil.restore(getSnapshotItemsToRestore(snapshot)); + case CLIENT_RESTORE -> restoreResultList = snapshotUtil.restore(getSnapshotItemsToRestore()); case SERVICE_RESTORE -> - restoreResultList = SaveAndRestoreService.getInstance().restore(getSnapshotItemsToRestore(snapshot)); + restoreResultList = SaveAndRestoreService.getInstance().restore(getSnapshotItemsToRestore()); } } catch (Exception e) { Platform.runLater(() -> { @@ -1322,9 +1290,8 @@ private void restore(RestoreMode restoreMode, Consumer> comp * @param restoreResultList Data created through a restore operation. */ private void showRestoreResult(List restoreResultList) { - List tableEntries = snapshotTableView.getItems(); AtomicBoolean disconnectedPvEncountered = new AtomicBoolean(false); - for (TableEntry tableEntry : tableEntries) { + for (TableEntry tableEntry : tableEntryItems) { Optional tableEntryOptional = restoreResultList.stream().filter(r -> r.getSnapshotItem().getConfigPv().getPvName().equals(tableEntry.getConfigPv().getPvName())).findFirst(); if (tableEntryOptional.isPresent()) { disconnectedPvEncountered.set(true); @@ -1346,31 +1313,26 @@ private void showRestoreResult(List restoreResultList) { /** * Compiles a list of {@link SnapshotItem}s based on the snapshot's PVs (and potential read-only property setting) - * as well as user's choice to exclude items in the UI. + * as well as user's choice to exclude items in the UI using a filter * - * @param snapshot {@link Snapshot} contents. * @return A list of {@link SnapshotItem}s to be subject to a restore operation. */ - private List getSnapshotItemsToRestore(Snapshot snapshot) { + private List getSnapshotItemsToRestore() { List itemsToRestore = new ArrayList<>(); - - for (SnapshotItem entry : snapshot.getSnapshotData().getSnapshotItems()) { - TableEntry e = tableEntryItems.get(entry.getConfigPv().getPvName()); - - boolean restorable = e.selectedProperty().get() && - !e.readOnlyProperty().get() && - entry.getValue() != null && - !entry.getValue().equals(VNoData.INSTANCE); - + tableEntryItems.forEach(tableEntry -> { + boolean restorable = tableEntry.selectedProperty().get() && + tableEntry.readOnlyProperty().not().get() && + tableEntry.getSnapshotVal().get() != null && + !tableEntry.getSnapshotVal().get().equals(VNoData.INSTANCE); if (restorable) { - itemsToRestore.add(entry); + itemsToRestore.add(tableEntry.getSnapshotItem()); } - } + }); return itemsToRestore; } private void addSnapshot(Snapshot snapshot) { - snapshots.add(snapshot); + additionalSnapshots.add(snapshot); snapshotTableView.getColumns().clear(); @@ -1387,10 +1349,10 @@ private void addSnapshot(Snapshot snapshot) { compareColumn = new TableColumn<>(Messages.storedValues); compareColumn.getStyleClass().add("snapshot-table-centered"); - String baseSnapshotTimeStamp = snapshots.get(0).getSnapshotNode().getCreated() == null ? + String baseSnapshotTimeStamp = this.snapshot.getSnapshotNode().getCreated() == null ? "" : - " (" + TimestampFormats.SECONDS_FORMAT.format(snapshots.get(0).getSnapshotNode().getCreated().toInstant()) + ")"; - String snapshotName = snapshots.get(0).getSnapshotNode().getName() + baseSnapshotTimeStamp; + " (" + TimestampFormats.SECONDS_FORMAT.format(this.snapshot.getSnapshotNode().getCreated().toInstant()) + ")"; + String snapshotName = this.snapshot.getSnapshotNode().getName() + baseSnapshotTimeStamp; baseSnapshotColumn = new TableColumn<>(snapshotName); baseSnapshotColumn.getStyleClass().add("snapshot-table-centered"); @@ -1405,8 +1367,8 @@ private void addSnapshot(Snapshot snapshot) { ObjectProperty value = e.getRowValue().valueProperty(); value.setValue(new VTypePair(value.get().base, updatedValue, value.get().threshold)); updateLoadedSnapshot(e.getRowValue(), updatedValue); - for (int i = 1; i < snapshots.size(); i++) { - ObjectProperty compareValue = e.getRowValue().compareValueProperty(i); + for (int i = 0; i < additionalSnapshots.size(); i++) { + ObjectProperty compareValue = e.getRowValue().compareValueProperty(i + 1); compareValue.setValue(new VTypePair(updatedValue, compareValue.get().value, compareValue.get().threshold)); } }); @@ -1423,38 +1385,36 @@ private void addSnapshot(Snapshot snapshot) { compareColumn.getColumns().add(0, baseSnapshotColumn); - for (int s = 1; s < snapshots.size(); s++) { - Node snapshotNode = snapshots.get(s).getSnapshotNode(); + for (int s = 0; s < additionalSnapshots.size(); s++) { + Node snapshotNode = additionalSnapshots.get(s).getSnapshotNode(); String snapshotName = snapshotNode.getName(); - List entries = snapshot.getSnapshotData().getSnapshotItems(); - String nodeName; - TableEntry tableEntry; - // Base snapshot data - List baseSnapshotTableEntries = new ArrayList<>(tableEntryItems.values()); - SnapshotItem entry; + // Base snapshot data. Create a copy as tableEntryItems should always contain full list. + List baseSnapshotTableEntries = new ArrayList<>(tableEntryItems); + SnapshotItem snpshotItem; for (int i = 0; i < entries.size(); i++) { - entry = entries.get(i); - nodeName = entry.getConfigPv().getPvName(); - tableEntry = tableEntryItems.get(nodeName); + snpshotItem = entries.get(i); + String pvName = snpshotItem.getConfigPv().getPvName(); + Optional tableEntryOptional = + tableEntryItems.stream().filter(t -> t.getConfigPv().getPvName().equals(pvName)).findFirst(); // tableEntry is null if the added snapshot has more items than the base snapshot. - if (tableEntry == null) { - tableEntry = new TableEntry(); + TableEntry tableEntry; + if (tableEntryOptional.isEmpty()) { + tableEntry = new TableEntry(snpshotItem); tableEntry.idProperty().setValue(tableEntryItems.size() + i + 1); - tableEntry.pvNameProperty().setValue(nodeName); - tableEntry.setConfigPv(entry.getConfigPv()); - tableEntryItems.put(nodeName, tableEntry); - tableEntry.readbackNameProperty().set(entry.getConfigPv().getReadbackPvName()); + tableEntryItems.add(tableEntry); + tableEntry.connect(); + } else { + tableEntry = tableEntryOptional.get(); } - tableEntry.setSnapshotValue(entry.getValue(), snapshots.size()); - tableEntry.setStoredReadbackValue(entry.getReadbackValue(), snapshots.size()); - tableEntry.readOnlyProperty().set(entry.getConfigPv().isReadOnly()); + tableEntry.setSnapshotValue(snpshotItem.getValue(), additionalSnapshots.size()); + tableEntry.setStoredReadbackValue(snpshotItem.getReadbackValue(), additionalSnapshots.size()); baseSnapshotTableEntries.remove(tableEntry); } // If added snapshot has more items than base snapshot, the base snapshot's values for those // table rows need to be set to DISCONNECTED. for (TableEntry te : baseSnapshotTableEntries) { - te.setSnapshotValue(VDisconnectedData.INSTANCE, snapshots.size()); + te.setSnapshotValue(VDisconnectedData.INSTANCE, additionalSnapshots.size()); } TableColumn headerColumn = new TableColumn<>(snapshotName + " (" + @@ -1465,7 +1425,7 @@ private void addSnapshot(Snapshot snapshot) { Messages.setpoint, Messages.toolTipTableColumnSetpointPVValue, minWidth); - setpointValueCol.setCellValueFactory(e -> e.getValue().compareValueProperty(snapshots.size())); + setpointValueCol.setCellValueFactory(e -> e.getValue().compareValueProperty(additionalSnapshots.size())); setpointValueCol.setCellFactory(e -> new VTypeCellEditor<>()); setpointValueCol.setEditable(false); setpointValueCol.setSortable(false); @@ -1474,7 +1434,7 @@ private void addSnapshot(Snapshot snapshot) { TooltipTableColumn deltaCol = new TooltipTableColumn<>( Utilities.DELTA_CHAR + " " + Messages.baseSetpoint, "", minWidth); - deltaCol.setCellValueFactory(e -> e.getValue().compareValueProperty(snapshots.size())); + deltaCol.setCellValueFactory(e -> e.getValue().compareValueProperty(additionalSnapshots.size())); deltaCol.setCellFactory(e -> { VDeltaCellEditor vDeltaCellEditor = new VDeltaCellEditor<>(); vDeltaCellEditor.setShowDeltaPercentage(showDeltaPercentage.get()); @@ -1486,7 +1446,7 @@ private void addSnapshot(Snapshot snapshot) { headerColumn.getColumns().addAll(setpointValueCol, deltaCol, new DividerTableColumn()); - compareColumn.getColumns().add(s, headerColumn); + compareColumn.getColumns().add(s + 1, headerColumn); } columns.add(compareColumn); @@ -1495,57 +1455,52 @@ private void addSnapshot(Snapshot snapshot) { snapshotTableView.getColumns().addAll(columns); - connectPVs(); - updateTable(null); + updateTable(); } + /** + * This clears the list of {@link TableEntry}s in the view and creates new objects based + * on the contents of the current {@link Snapshot}. + */ private void showSnapshotInTable() { - if (snapshots.isEmpty()) { - snapshots.add(snapshot); - } else { - snapshots.set(0, snapshot); - } AtomicInteger counter = new AtomicInteger(0); - snapshot.getSnapshotData().getSnapshotItems().forEach(entry -> { - TableEntry tableEntry = new TableEntry(); - String name = entry.getConfigPv().getPvName(); + tableEntryItems.forEach(TableEntry::dispose); + tableEntryItems.clear(); + snapshot.getSnapshotData().getSnapshotItems().forEach(snapshotItem -> { + TableEntry tableEntry = new TableEntry(snapshotItem); tableEntry.idProperty().setValue(counter.incrementAndGet()); - tableEntry.pvNameProperty().setValue(name); - tableEntry.setConfigPv(entry.getConfigPv()); - tableEntry.setSnapshotValue(entry.getValue(), 0); - tableEntry.setStoredReadbackValue(entry.getReadbackValue(), 0); - if (entry.getValue() == null || entry.getValue().equals(VDisconnectedData.INSTANCE)) { - tableEntry.setActionResult(ActionResult.FAILED); - } else { - tableEntry.setActionResult(ActionResult.OK); - } - if (entry.getConfigPv().getReadbackPvName() != null) { - if (entry.getReadbackValue() == null || entry.getReadbackValue().equals(VDisconnectedData.INSTANCE)) { - tableEntry.setActionResultReadback(ActionResult.FAILED); - } else { - tableEntry.setActionResultReadback(ActionResult.OK); - } - } - tableEntry.readbackNameProperty().set(entry.getConfigPv().getReadbackPvName()); - tableEntry.readOnlyProperty().set(entry.getConfigPv().isReadOnly()); - tableEntryItems.put(name, tableEntry); + tableEntry.setSnapshotValue(snapshotItem.getValue(), 0); + tableEntry.setStoredReadbackValue(snapshotItem.getReadbackValue(), 0); + tableEntryItems.add(tableEntry); }); - updateTable(null); - connectPVs(); + JobManager.schedule("Connect to PVs", monitor -> + tableEntryItems.forEach(TableEntry::connect)); + + updateTable(); + } + + /** + * Updates the {@link TableView} with the full list of {@link TableEntry} objects as created from + * the {@link Snapshot} data. + */ + private void updateTable() { + updateTable(tableEntryItems); } /** - * Sets new table entries for this table, but do not change the structure of the table. + * Updates the table showing the {@link TableEntry}s. Note though that while the full list of {@link TableEntry}s + * associated with a snapshot is maintained in this class, the supplied {@link List} of {@link TableEntry}s + * may be a subset, e.g. if user selects to filter or hide items where store setpoint and live value are equal. * - * @param entries the entries to set + * @param entries The entries to show in the table. */ private void updateTable(List entries) { final ObservableList items = snapshotTableView.getItems(); final boolean notHide = hideEqualItemsProperty.not().get(); Platform.runLater(() -> { items.clear(); - tableEntryItems.forEach((key, value) -> { + entries.forEach(value -> { // there is no harm if this is executed more than once, because only one line is allowed for these // two properties (see SingleListenerBooleanProperty for more details) value.liveStoredEqualProperty().addListener((a, o, n) -> { @@ -1580,21 +1535,6 @@ private int measureStringWidth(String text, Font font) { return (int) mText.getLayoutBounds().getWidth(); } - /** - * Attempts to connect to all the PVs of the configuration/snapshot and binds the created {@link SaveAndRestorePV} objects - * to the {@link TableEntry} objects matched on PV name. - */ - private void connectPVs() { - JobManager.schedule("Connect PVs", monitor -> tableEntryItems.values().forEach(e -> { - SaveAndRestorePV pv = pvs.get(e.getConfigPv().getPvName()); - if (pv == null) { - pvs.put(e.getConfigPv().getPvName(), new SaveAndRestorePV(e)); - } else { - pv.setSnapshotTableEntry(e); - } - })); - } - /** * @param configurationData {@link ConfigurationData} obejct of a {@link org.phoebus.applications.saveandrestore.model.Configuration} * @return true if any if the {@link ConfigPv} items in {@link ConfigurationData#getPvList()} defines a non-null read-back @@ -1648,7 +1588,7 @@ protected void updateItem(Instant item, boolean empty) { /** * {@link TableCell} implementation for the action result columns. */ - private class ActionResultTableCell extends TableCell { + private static class ActionResultTableCell extends TableCell { @Override public void updateItem(org.phoebus.applications.saveandrestore.ui.snapshot.ActionResult actionResult, boolean empty) { if (empty) { diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/TableEntry.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/TableEntry.java index 0f5f88b420..6367ef5be8 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/TableEntry.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/TableEntry.java @@ -28,17 +28,23 @@ import org.epics.vtype.VNumberArray; import org.epics.vtype.VType; import org.phoebus.applications.saveandrestore.model.ConfigPv; +import org.phoebus.applications.saveandrestore.model.SnapshotItem; import org.phoebus.applications.saveandrestore.ui.SingleListenerBooleanProperty; +import org.phoebus.applications.saveandrestore.ui.VTypePair; +import org.phoebus.core.vtypes.VDisconnectedData; +import org.phoebus.pv.PV; +import org.phoebus.pv.PVPool; import org.phoebus.saveandrestore.util.Threshold; import org.phoebus.saveandrestore.util.Utilities; import org.phoebus.saveandrestore.util.VNoData; -import org.phoebus.applications.saveandrestore.ui.VTypePair; -import org.phoebus.core.vtypes.VDisconnectedData; import java.time.Instant; import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.concurrent.TimeUnit; +import java.util.logging.Level; +import java.util.logging.Logger; /** * TableEntry represents a single line in the snapshot viewer table. It provides values for all columns in @@ -101,13 +107,21 @@ public class TableEntry { */ private final ObjectProperty actionResultReadback = new SimpleObjectProperty<>(this, "actionResultReadback", ActionResult.PENDING); + private final ConfigPv configPv; + private final SnapshotItem snapshotItem; - private ConfigPv configPv; + private PV pv; + private PV readbackPv; + /** + * The time between updates of dynamic data in the table, in ms. + */ + private static final long TABLE_UPDATE_INTERVAL = 500; /** * Construct a new table entry. */ - public TableEntry() { + public TableEntry(SnapshotItem snapshotItem) { + this.snapshotItem = snapshotItem; //when read only is set to true, unselect this PV readOnly.addListener((a, o, n) -> { if (n) { @@ -120,12 +134,27 @@ public TableEntry() { selected.set(false); } }); + readOnlyProperty().setValue(snapshotItem.getConfigPv().isReadOnly()); + pvNameProperty().set(snapshotItem.getConfigPv().getPvName()); + readbackNameProperty().set(snapshotItem.getConfigPv().getReadbackPvName()); + setReadbackValue(snapshotItem.getReadbackValue()); + if (snapshotItem.getValue() == null || snapshotItem.getValue().equals(VDisconnectedData.INSTANCE)) { + setActionResult(ActionResult.FAILED); + } else { + setActionResult(ActionResult.OK); + } + if (snapshotItem.getConfigPv().getReadbackPvName() != null) { + if (snapshotItem.getReadbackValue() == null || snapshotItem.getReadbackValue().equals(VDisconnectedData.INSTANCE)) { + setActionResultReadback(ActionResult.FAILED); + } else { + setActionResultReadback(ActionResult.OK); + } + } + this.configPv = snapshotItem.getConfigPv(); } - public void setConfigPv(ConfigPv configPv) { - this.configPv = configPv; - pvName.setValue(configPv.getPvName()); - readbackName.setValue(configPv.getReadbackPvName()); + public SnapshotItem getSnapshotItem() { + return snapshotItem; } public ConfigPv getConfigPv() { @@ -437,21 +466,54 @@ public ObjectProperty getSnapshotVal() { return snapshotVal; } - public ObjectProperty actionResultProperty(){ + + + @SuppressWarnings("unused") + public ObjectProperty actionResultProperty() { return actionResult; } - public void setActionResult(ActionResult actionResult){ + public void setActionResult(ActionResult actionResult) { this.actionResult.set(actionResult); } - public ObjectProperty actionResultReadbackProperty(){ + public ObjectProperty actionResultReadbackProperty() { return actionResultReadback; } - public void setActionResultReadback(ActionResult actionResult){ + public void setActionResultReadback(ActionResult actionResult) { this.actionResultReadback.set(actionResult); } + /** + * Connects to PV and read-back PV (if defined). + */ + public void connect() { + try { + pv = PVPool.getPV(pvNameProperty().get()); + pv.onValueEvent().throttleLatest(TABLE_UPDATE_INTERVAL, TimeUnit.MILLISECONDS) + .subscribe(value -> setLiveValue(PV.isDisconnected(value) ? VDisconnectedData.INSTANCE : value)); + if(readbackName.isNotNull().get() && !readbackName.get().isEmpty()) { + readbackPv = PVPool.getPV(readbackName.get()); + readbackPv.onValueEvent() + .throttleLatest(TABLE_UPDATE_INTERVAL, TimeUnit.MILLISECONDS) + .subscribe(value -> setReadbackValue(PV.isDisconnected(value) ? VDisconnectedData.INSTANCE : value)); + } + else { + // If configuration does not define read-back PV, then UI should show "no data" rather than "disconnected" + setReadbackValue(VNoData.INSTANCE); + } + } catch (Exception e) { + Logger.getLogger(TableEntry.class.getName()).log(Level.INFO, "Error connecting to PV", e); + } + } + void dispose() { + if (pv != null) { + PVPool.releasePV(pv); + } + if (readbackPv != null) { + PVPool.releasePV(readbackPv); + } + } } diff --git a/app/save-and-restore/app/src/main/resources/org/phoebus/applications/saveandrestore/messages.properties b/app/save-and-restore/app/src/main/resources/org/phoebus/applications/saveandrestore/messages.properties index d0ba22cd63..7371d54ca7 100644 --- a/app/save-and-restore/app/src/main/resources/org/phoebus/applications/saveandrestore/messages.properties +++ b/app/save-and-restore/app/src/main/resources/org/phoebus/applications/saveandrestore/messages.properties @@ -15,9 +15,12 @@ cancel=Cancel cannotCompareHeader=No snapshot data available for comparison. cannotCompareTitle=Cannot Compare choose=Choose -closeConfigurationWarning=Configuration modified, but not saved. Do you wish to continue? +closeConfigurationWarning=Save&restore configuration modified, but not saved. Do you wish to continue? closeCompositeSnapshotWarning=Composite snapshot modified, but not saved. Do you wish to continue? -closeTabPrompt=Close tab? +closeSnapshotWarning=Save&restore snapshot created or modified, but not saved. Do you wish to continue? +closeCompositeSnapshotTabPrompt=Close composite snapshot tab? +closeConfigurationTabPrompt=Close configuration tab? +closeSnapshotTabPrompt=Close snapshot tab? comment=Comment comparisonMode=Comparison Mode comparisonTolerance=Tolerance @@ -144,7 +147,6 @@ openSearchView=Open Search View overwrite=Overwrite paste=Paste preserveSelection=Preserve selection after -promptCloseSnapshotTabContent=Snapshot data is not saved. Do you wish to continue? promptDeletePVTitle=Delete PV? promptDeletePVFromSaveSet=NOTE: deleting PVs from the save set will also delete stored values in all associated snapshots. \n\nDo you wish to continue? promptDeleteSelectedTitle=Delete selected item(s)? diff --git a/app/save-and-restore/app/src/main/resources/org/phoebus/applications/saveandrestore/ui/configuration/ConfigurationEditor.fxml b/app/save-and-restore/app/src/main/resources/org/phoebus/applications/saveandrestore/ui/configuration/ConfigurationEditor.fxml index 70ac3d2d65..60d2d47e01 100644 --- a/app/save-and-restore/app/src/main/resources/org/phoebus/applications/saveandrestore/ui/configuration/ConfigurationEditor.fxml +++ b/app/save-and-restore/app/src/main/resources/org/phoebus/applications/saveandrestore/ui/configuration/ConfigurationEditor.fxml @@ -5,7 +5,7 @@ -