From 68ecad5b98e4f6f7298efde08fd4923aeaecc6b6 Mon Sep 17 00:00:00 2001 From: georgweiss Date: Tue, 21 Jan 2025 14:41:00 +0100 Subject: [PATCH 01/16] Update of save&restore config pv data model to include comparison data --- .../saveandrestore/model/Comparison.java | 9 +++++ .../saveandrestore/model/ConfigPv.java | 34 +++++++++++++++++++ .../saveandrestore/model/PvCompareMode.java | 11 ++++++ .../saveandrestore/model/ConfigPvTest.java | 5 +++ 4 files changed, 59 insertions(+) create mode 100644 app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/Comparison.java create mode 100644 app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/PvCompareMode.java diff --git a/app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/Comparison.java b/app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/Comparison.java new file mode 100644 index 0000000000..75eaa73c89 --- /dev/null +++ b/app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/Comparison.java @@ -0,0 +1,9 @@ +/* + * Copyright (C) 2024 European Spallation Source ERIC. + */ + +package org.phoebus.applications.saveandrestore.model; + +public record Comparison(PvCompareMode pvCompareMode, double tolerance) { + +} diff --git a/app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/ConfigPv.java b/app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/ConfigPv.java index 369708d9d7..c0b975224a 100644 --- a/app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/ConfigPv.java +++ b/app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/ConfigPv.java @@ -24,15 +24,36 @@ * Class encapsulating data to describe a PV subject to a save operation. A read-back PV name * is optionally associated with the PV name. A PV record is uniquely identified by both the PV name * and the read-back PV name (if it has been specified). + *

+ * If a read-back PV name has been specified, a {@link Comparison} can be specified to + * indicate if and how the stored read-back value is compared to the live read-back value + * if a client requests it. + *

* @author georgweiss * Created 1 Oct 2018 */ public class ConfigPv implements Comparable{ + /** + * Set-point PV name. + */ private String pvName; + + /** + * Optional read-back PV name + */ private String readbackPvName; + + /** + * Flag indicating if set-point PV value should be restored or not. + */ private boolean readOnly = false; + /** + * Optional comparison data for the read-back PV. + */ + private Comparison comparison; + public String getPvName() { return pvName; } @@ -57,6 +78,14 @@ public void setReadOnly(boolean readOnly) { this.readOnly = readOnly; } + public Comparison getComparison() { + return comparison; + } + + public void setComparison(Comparison comparison) { + this.comparison = comparison; + } + @Override public boolean equals(Object other) { if(other instanceof ConfigPv) { @@ -118,6 +147,11 @@ public Builder readOnly(boolean readOnly){ return this; } + public Builder comparison(Comparison comparison){ + configPv.setComparison(comparison); + return this; + } + public ConfigPv build(){ return configPv; } diff --git a/app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/PvCompareMode.java b/app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/PvCompareMode.java new file mode 100644 index 0000000000..d5f5c021ba --- /dev/null +++ b/app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/PvCompareMode.java @@ -0,0 +1,11 @@ +/* + * Copyright (C) 2024 European Spallation Source ERIC. + */ + +package org.phoebus.applications.saveandrestore.model; + +public enum PvCompareMode { + NONE, + RELATIVE, + ABSOLUTE; +} diff --git a/app/save-and-restore/model/src/test/java/org/phoebus/applications/saveandrestore/model/ConfigPvTest.java b/app/save-and-restore/model/src/test/java/org/phoebus/applications/saveandrestore/model/ConfigPvTest.java index 2912f7c266..dc46e2a7af 100644 --- a/app/save-and-restore/model/src/test/java/org/phoebus/applications/saveandrestore/model/ConfigPvTest.java +++ b/app/save-and-restore/model/src/test/java/org/phoebus/applications/saveandrestore/model/ConfigPvTest.java @@ -42,6 +42,11 @@ public void testConfigPv() { assertEquals("b", configPV.getReadbackPvName()); assertTrue(configPV.isReadOnly()); + assertNull(configPV.getComparison()); + + configPV = ConfigPv.builder().pvName("a").readbackPvName("b").readOnly(true).comparison(new Comparison(PvCompareMode.ABSOLUTE, 1.0)).build(); + assertEquals(PvCompareMode.ABSOLUTE, configPV.getComparison().pvCompareMode()); + assertEquals(1.0, configPV.getComparison().tolerance()); } @Test From 3f9b98201c84a79fc29377cfc20917a5e988174b Mon Sep 17 00:00:00 2001 From: georgweiss Date: Tue, 21 Jan 2025 14:58:56 +0100 Subject: [PATCH 02/16] Disallow comparison data if read-back PV name is not set --- .../controllers/ConfigurationController.java | 7 ++++++ .../ConfigurationControllerTest.java | 23 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/web/controllers/ConfigurationController.java b/services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/web/controllers/ConfigurationController.java index 4e832bfcc3..66f9f242c9 100644 --- a/services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/web/controllers/ConfigurationController.java +++ b/services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/web/controllers/ConfigurationController.java @@ -17,6 +17,7 @@ */ package org.phoebus.service.saveandrestore.web.controllers; +import org.phoebus.applications.saveandrestore.model.ConfigPv; import org.phoebus.applications.saveandrestore.model.Configuration; import org.phoebus.applications.saveandrestore.model.ConfigurationData; import org.phoebus.applications.saveandrestore.model.Node; @@ -58,6 +59,12 @@ public class ConfigurationController extends BaseController { public Configuration createConfiguration(@RequestParam(value = "parentNodeId") String parentNodeId, @RequestBody Configuration configuration, Principal principal) { + // Validation: a ConfigPV cannot specify non-null Comparison unless read-back PV is set. + for(ConfigPv configPv : configuration.getConfigurationData().getPvList()){ + if((configPv.getReadbackPvName() == null || configPv.getReadbackPvName().isEmpty()) && configPv.getComparison() != null){ + throw new IllegalArgumentException("PV item \"" + configPv.getPvName() + "\" specifies non-null comparison, but read-back PV is not set"); + } + } configuration.getConfigurationNode().setUserName(principal.getName()); return nodeDAO.createConfiguration(parentNodeId, configuration); } diff --git a/services/save-and-restore/src/test/java/org/phoebus/service/saveandrestore/web/controllers/ConfigurationControllerTest.java b/services/save-and-restore/src/test/java/org/phoebus/service/saveandrestore/web/controllers/ConfigurationControllerTest.java index 4ab5544360..f24ab3eaa7 100644 --- a/services/save-and-restore/src/test/java/org/phoebus/service/saveandrestore/web/controllers/ConfigurationControllerTest.java +++ b/services/save-and-restore/src/test/java/org/phoebus/service/saveandrestore/web/controllers/ConfigurationControllerTest.java @@ -22,9 +22,13 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.phoebus.applications.saveandrestore.model.Comparison; +import org.phoebus.applications.saveandrestore.model.ConfigPv; import org.phoebus.applications.saveandrestore.model.Configuration; +import org.phoebus.applications.saveandrestore.model.ConfigurationData; import org.phoebus.applications.saveandrestore.model.Node; import org.phoebus.applications.saveandrestore.model.NodeType; +import org.phoebus.applications.saveandrestore.model.PvCompareMode; import org.phoebus.service.saveandrestore.persistence.dao.NodeDAO; import org.phoebus.service.saveandrestore.web.config.ControllersTestConfig; import org.springframework.beans.factory.annotation.Autowired; @@ -36,6 +40,8 @@ import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; +import java.util.List; + import static org.mockito.Mockito.reset; import static org.mockito.Mockito.when; import static org.phoebus.service.saveandrestore.web.controllers.BaseController.JSON; @@ -155,6 +161,23 @@ public void testUpdateConfiguration() throws Exception { mockMvc.perform(request).andExpect(status().isUnauthorized() ); + } + + @Test + public void testCreateInvalidConfiguration() throws Exception { + + reset(nodeDAO); + + Configuration configuration = new Configuration(); + configuration.setConfigurationNode(Node.builder().build()); + ConfigurationData configurationData = new ConfigurationData(); + configuration.setConfigurationData(configurationData); + configurationData.setPvList(List.of(ConfigPv.builder().pvName("foo").build(), + ConfigPv.builder().pvName("fooo").comparison(new Comparison(PvCompareMode.ABSOLUTE, 1.0)).build())); + MockHttpServletRequestBuilder request = put("/config?parentNodeId=a") + .header(HttpHeaders.AUTHORIZATION, adminAuthorization) + .contentType(JSON).content(objectMapper.writeValueAsString(configuration)); + mockMvc.perform(request).andExpect(status().isBadRequest()); } } From 4304070a9a921b8c30731644faef921e07d3d64b Mon Sep 17 00:00:00 2001 From: georgweiss Date: Wed, 29 Jan 2025 15:31:08 +0100 Subject: [PATCH 03/16] Save&restore configuration editor: support for comparison data --- .../ui/configuration/ConfigPvEntry.java | 114 +++++++ .../ConfigurationController.java | 255 ++++++++++++---- .../ui/configuration/ConfigurationTab.java | 14 +- .../saveandrestore/messages.properties | 2 + .../ui/configuration/ConfigurationEditor.fxml | 25 +- .../saveandrestore/model/Comparison.java | 9 - .../saveandrestore/model/ConfigPv.java | 278 +++++++++--------- .../saveandrestore/model/PvCompareMode.java | 4 +- .../saveandrestore/model/ConfigPvTest.java | 171 +++++------ .../controllers/ConfigurationController.java | 18 +- .../ConfigurationControllerTest.java | 30 +- 11 files changed, 603 insertions(+), 317 deletions(-) create mode 100644 app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/configuration/ConfigPvEntry.java delete mode 100644 app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/Comparison.java diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/configuration/ConfigPvEntry.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/configuration/ConfigPvEntry.java new file mode 100644 index 0000000000..ae515ebbf9 --- /dev/null +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/configuration/ConfigPvEntry.java @@ -0,0 +1,114 @@ +/* + * Copyright (C) 2024 European Spallation Source ERIC. + */ + +package org.phoebus.applications.saveandrestore.ui.configuration; + +import javafx.beans.property.BooleanProperty; +import javafx.beans.property.DoubleProperty; +import javafx.beans.property.ObjectProperty; +import javafx.beans.property.SimpleBooleanProperty; +import javafx.beans.property.SimpleDoubleProperty; +import javafx.beans.property.SimpleObjectProperty; +import javafx.beans.property.SimpleStringProperty; +import javafx.beans.property.StringProperty; +import org.phoebus.applications.saveandrestore.model.ConfigPv; +import org.phoebus.applications.saveandrestore.model.PvCompareMode; + +import java.util.Objects; + +/** + * Wrapper around a {@link ConfigPv} instance for the purpose of facilitating + * configuration and data binding in (for instance) a {@link javafx.scene.control.TableView}. + */ +public class ConfigPvEntry implements Comparable { + + private final StringProperty pvNameProperty; + private final StringProperty readBackPvNameProperty; + private final BooleanProperty readOnlyProperty; + private final ObjectProperty pvCompareModeProperty; + private ObjectProperty toleranceProperty; + + public ConfigPvEntry(ConfigPv configPv) { + this.pvNameProperty = new SimpleStringProperty(this, "pvNameProperty", configPv.getPvName()); + this.readBackPvNameProperty = new SimpleStringProperty(configPv.getReadbackPvName()); + this.readOnlyProperty = new SimpleBooleanProperty(configPv.isReadOnly()); + this.pvCompareModeProperty = new SimpleObjectProperty(configPv.getPvCompareMode()); + this.toleranceProperty = configPv.getTolerance() != null ? + new SimpleObjectProperty<>(configPv.getTolerance()) : new SimpleObjectProperty<>(null); + } + + public StringProperty getPvNameProperty() { + return pvNameProperty; + } + + public StringProperty getReadBackPvNameProperty() { + return readBackPvNameProperty; + } + + public BooleanProperty getReadOnlyProperty() { + return readOnlyProperty; + } + + public ObjectProperty getPvCompareModeProperty() { + return pvCompareModeProperty; + } + + public ObjectProperty getToleranceProperty() { + return toleranceProperty; + } + + public void setPvNameProperty(String pvNameProperty) { + this.pvNameProperty.set(pvNameProperty); + } + + public void setReadBackPvNameProperty(String readBackPvNameProperty) { + this.readBackPvNameProperty.set(readBackPvNameProperty); + } + + public void setReadOnlyProperty(boolean readOnlyProperty) { + this.readOnlyProperty.set(readOnlyProperty); + } + + public void setPvCompareModeProperty(PvCompareMode pvCompareModeProperty) { + this.pvCompareModeProperty.set(pvCompareModeProperty); + } + + public void setToleranceProperty(Double toleranceProperty) { + this.toleranceProperty.set(toleranceProperty ); + } + + public ConfigPv toConfigPv() { + ConfigPv configPv = ConfigPv.builder() + .pvName(pvNameProperty.get()) + .readbackPvName(readBackPvNameProperty.get()) + .readOnly(readOnlyProperty.get()) + .build(); + if (!readBackPvNameProperty.isEmpty().get() && + pvCompareModeProperty.get() != null) { + configPv.setPvCompareMode(pvCompareModeProperty.get()); + configPv.setTolerance(toleranceProperty.get()); + } + return configPv; + } + + @Override + public boolean equals(Object other) { + if (other instanceof ConfigPvEntry otherConfigPv) { + return Objects.equals(pvNameProperty, otherConfigPv.getPvNameProperty()) && + Objects.equals(readBackPvNameProperty, otherConfigPv.getReadBackPvNameProperty()) && + Objects.equals(readOnlyProperty.get(), otherConfigPv.getReadOnlyProperty().get()); + } + return false; + } + + @Override + public int hashCode() { + return Objects.hash(pvNameProperty, readBackPvNameProperty, readOnlyProperty); + } + + @Override + public int compareTo(ConfigPvEntry other) { + return pvNameProperty.get().compareTo(other.getPvNameProperty().get()); + } +} 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 602a4c8c79..de87fcf544 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 @@ -21,6 +21,7 @@ import javafx.application.Platform; import javafx.beans.binding.Bindings; +import javafx.beans.property.BooleanProperty; import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.SimpleObjectProperty; import javafx.beans.property.SimpleStringProperty; @@ -37,16 +38,18 @@ import javafx.scene.control.MenuItem; import javafx.scene.control.SelectionMode; import javafx.scene.control.SeparatorMenuItem; -import javafx.scene.control.TableCell; import javafx.scene.control.TableColumn; import javafx.scene.control.TableView; import javafx.scene.control.TextArea; import javafx.scene.control.TextField; -import javafx.scene.control.cell.PropertyValueFactory; +import javafx.scene.control.cell.CheckBoxTableCell; +import javafx.scene.control.cell.ComboBoxTableCell; +import javafx.scene.control.cell.TextFieldTableCell; import javafx.scene.image.ImageView; import javafx.scene.layout.BorderPane; import javafx.scene.layout.Pane; -import javafx.util.Callback; +import javafx.util.StringConverter; +import javafx.util.converter.DoubleStringConverter; import org.phoebus.applications.saveandrestore.Messages; import org.phoebus.applications.saveandrestore.SaveAndRestoreApplication; import org.phoebus.applications.saveandrestore.model.ConfigPv; @@ -54,6 +57,7 @@ import org.phoebus.applications.saveandrestore.model.ConfigurationData; import org.phoebus.applications.saveandrestore.model.Node; import org.phoebus.applications.saveandrestore.model.NodeType; +import org.phoebus.applications.saveandrestore.model.PvCompareMode; import org.phoebus.applications.saveandrestore.ui.NodeChangedListener; import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreBaseController; import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreService; @@ -67,6 +71,7 @@ import java.time.Instant; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -78,30 +83,55 @@ public class ConfigurationController extends SaveAndRestoreBaseController implements NodeChangedListener { @FXML + @SuppressWarnings("unused") private BorderPane root; @FXML - private TableColumn pvNameColumn; + @SuppressWarnings("unused") + private TableColumn pvNameColumn; @FXML - private TableView pvTable; + @SuppressWarnings("unused") + private TableColumn readbackPvNameColumn; @FXML + @SuppressWarnings("unused") + private TableColumn comparisonModeColumn; + + @FXML + @SuppressWarnings("unused") + private TableColumn toleranceColumn; + + @FXML + @SuppressWarnings("unused") + private TableColumn readOnlyColumn; + + @FXML + @SuppressWarnings("unused") + private TableView pvTable; + + @FXML + @SuppressWarnings("unused") private TextArea descriptionTextArea; @FXML + @SuppressWarnings("unused") private Button saveButton; @FXML + @SuppressWarnings("unused") private TextField pvNameField; @FXML + @SuppressWarnings("unused") private TextField readbackPvNameField; @FXML + @SuppressWarnings("unused") private Button addPvButton; @FXML + @SuppressWarnings("unused") private CheckBox readOnlyCheckBox; @FXML @@ -113,25 +143,28 @@ public class ConfigurationController extends SaveAndRestoreBaseController implem private final SimpleBooleanProperty readOnlyProperty = new SimpleBooleanProperty(false); @FXML + @SuppressWarnings("unused") private TextField configurationNameField; @FXML + @SuppressWarnings("unused") private Label configurationCreatedDateField; @FXML + @SuppressWarnings("unused") private Label configurationLastModifiedDateField; @FXML + @SuppressWarnings("unused") private Label createdByField; @FXML + @SuppressWarnings("unused") private Pane addPVsPane; private SaveAndRestoreService saveAndRestoreService; private static final Executor UI_EXECUTOR = Platform::runLater; - private final SimpleBooleanProperty dirty = new SimpleBooleanProperty(false); - - private final ObservableList configurationEntries = FXCollections.observableArrayList(); + private final ObservableList configurationEntries = FXCollections.observableArrayList(); private final SimpleBooleanProperty selectionEmpty = new SimpleBooleanProperty(false); private final SimpleStringProperty configurationDescriptionProperty = new SimpleStringProperty(); @@ -146,6 +179,9 @@ public class ConfigurationController extends SaveAndRestoreBaseController implem private final Logger logger = Logger.getLogger(ConfigurationController.class.getName()); + private final BooleanProperty loadInProgress = new SimpleBooleanProperty(); + private final BooleanProperty dirty = new SimpleBooleanProperty(); + public ConfigurationController(ConfigurationTab configurationTab) { this.configurationTab = configurationTab; } @@ -155,16 +191,17 @@ public void initialize() { saveAndRestoreService = SaveAndRestoreService.getInstance(); + dirty.addListener((obs, o, n) -> configurationTab.annotateDirty(n)); + + pvTable.editableProperty().bind(userIdentity.isNull().not()); pvTable.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE); pvTable.getSelectionModel().selectedItemProperty().addListener((obs, ov, nv) -> selectionEmpty.set(nv == null)); - ContextMenu pvNameContextMenu = new ContextMenu(); - MenuItem deleteMenuItem = new MenuItem(Messages.menuItemDeleteSelectedPVs, new ImageView(ImageCache.getImage(ConfigurationController.class, "/icons/delete.png"))); deleteMenuItem.setOnAction(ae -> { configurationEntries.removeAll(pvTable.getSelectionModel().getSelectedItems()); - configurationTab.annotateDirty(true); + //configurationTab.annotateDirty(true); pvTable.refresh(); }); @@ -172,50 +209,133 @@ public void initialize() { || userIdentity.isNull().get(), pvTable.getSelectionModel().getSelectedItems(), userIdentity)); - pvNameColumn.setEditable(true); - pvNameColumn.setCellValueFactory(new PropertyValueFactory<>("pvName")); + ContextMenu contextMenu = new ContextMenu(); + pvTable.setOnContextMenuRequested(event -> { + contextMenu.getItems().clear(); + contextMenu.getItems().addAll(deleteMenuItem); + contextMenu.getItems().add(new SeparatorMenuItem()); + ObservableList selectedPVs = pvTable.getSelectionModel().getSelectedItems(); + if (!selectedPVs.isEmpty()) { + List selectedPVList = selectedPVs.stream() + .map(tableEntry -> new ProcessVariable(tableEntry.getPvNameProperty().get())) + .collect(Collectors.toList()); + SelectionService.getInstance().setSelection(SaveAndRestoreApplication.NAME, selectedPVList); + ContextMenuHelper.addSupportedEntries(FocusUtil.setFocusOn(pvTable), contextMenu); + } + }); + pvTable.setContextMenu(contextMenu); - pvNameColumn.setCellFactory(new Callback<>() { - @Override - public TableCell call(TableColumn param) { - final TableCell cell = new TableCell<>() { - @Override - public void updateItem(String item, boolean empty) { - super.updateItem(item, empty); - selectionEmpty.set(empty); - if (empty) { - setText(null); - } else { - if (isEditing()) { - setText(null); - } else { - setText(getItem()); - setGraphic(null); - } - } + pvNameColumn.setCellFactory(TextFieldTableCell.forTableColumn()); + pvNameColumn.setCellValueFactory(cell -> cell.getValue().getPvNameProperty()); + pvNameColumn.setOnEditCommit(t -> { + t.getTableView().getItems().get(t.getTablePosition().getRow()).setPvNameProperty(t.getNewValue()); + setDirty(true); + }); + + readbackPvNameColumn.setCellFactory(TextFieldTableCell.forTableColumn()); + readbackPvNameColumn.setCellValueFactory(cell -> cell.getValue().getReadBackPvNameProperty()); + readbackPvNameColumn.setOnEditCommit(t -> { + t.getTableView().getItems().get(t.getTablePosition().getRow()).setReadBackPvNameProperty(t.getNewValue()); + if (t.getNewValue() == null || t.getNewValue().isEmpty()) { + t.getTableView().getItems().get(t.getTablePosition().getRow()).setPvCompareModeProperty(null); + t.getTableView().getItems().get(t.getTablePosition().getRow()).setToleranceProperty(null); + } + setDirty(true); + }); + + readOnlyColumn.setCellFactory(CheckBoxTableCell.forTableColumn(readOnlyColumn)); + readOnlyColumn.setCellValueFactory(cell -> { + BooleanProperty readOnly = cell.getValue().getReadOnlyProperty(); + readOnly.addListener((obs, o, n) -> setDirty(true)); + return readOnly; + }); + + comparisonModeColumn.setCellValueFactory(cell -> cell.getValue().getPvCompareModeProperty()); + comparisonModeColumn.setCellFactory(callback -> { + ObservableList values = FXCollections.observableArrayList(Arrays.stream(PvCompareMode.values()).toList()); + values.add(0, null); + ComboBoxTableCell tableCell = new ComboBoxTableCell<>(values) { + + @Override + public void startEdit() { + String readBackPvName = getTableView().getItems().get(getIndex()).getReadBackPvNameProperty().get(); + if (readBackPvName == null || readBackPvName.isEmpty()) { + cancelEdit(); + return; } - }; - cell.setOnContextMenuRequested(event -> { - pvNameContextMenu.hide(); - pvNameContextMenu.getItems().clear(); - pvNameContextMenu.getItems().addAll(deleteMenuItem); - pvNameContextMenu.getItems().add(new SeparatorMenuItem()); - ObservableList selectedPVs = pvTable.getSelectionModel().getSelectedItems(); - if (!selectedPVs.isEmpty()) { - List selectedPVList = selectedPVs.stream() - .map(tableEntry -> new ProcessVariable(tableEntry.getPvName())) - .collect(Collectors.toList()); - SelectionService.getInstance().setSelection(SaveAndRestoreApplication.NAME, selectedPVList); - - ContextMenuHelper.addSupportedEntries(FocusUtil.setFocusOn(cell), pvNameContextMenu); + super.startEdit(); + } + + @Override + public void commitEdit(PvCompareMode pvCompareMode) { + if (pvCompareMode == null) { + getTableView().getItems().get(getIndex()).setToleranceProperty(null); + } else if (getTableView().getItems().get(getIndex()).getToleranceProperty().get() == null) { + getTableView().getItems().get(getIndex()).setToleranceProperty(0.0); } - pvNameContextMenu.show(cell, event.getScreenX(), event.getScreenY()); - }); - cell.setContextMenu(pvNameContextMenu); + getTableView().getItems().get(getIndex()).setPvCompareModeProperty(pvCompareMode); + setDirty(true); + super.commitEdit(pvCompareMode); + } + }; + + StringConverter converter = new StringConverter<>() { + @Override + public String toString(PvCompareMode object) { + if (object == null) { + return ""; + } + return object.toString(); + } + + @Override + public PvCompareMode fromString(String string) { + if (string == null) { + return null; + } + return PvCompareMode.valueOf(string); + } + }; + tableCell.setConverter(converter); + return tableCell; + }); - return cell; + toleranceColumn.setCellFactory(callback -> new TextFieldTableCell<>(new DoubleStringConverter() { + @Override + public String toString(Double value) { + return value == null ? null : value.toString(); + } + + @Override + public Double fromString(String string) { + try { + return Double.parseDouble(string); + } catch (Exception e) { + // No logging needed: user has entered text that cannot be parsed as double. + return null; + } + } + }) { + @Override + public void startEdit() { + String readBackPvName = getTableView().getItems().get(getIndex()).getReadBackPvNameProperty().get(); + if (readBackPvName == null || readBackPvName.isEmpty()) { + cancelEdit(); + return; + } + super.startEdit(); + } + + @Override + public void commitEdit(Double value) { + if (value == null) { + return; + } + setDirty(true); + super.commitEdit(value); } }); + toleranceColumn.setCellValueFactory(cell -> cell.getValue().getToleranceProperty()); pvNameField.textProperty().bindBidirectional(pvNameProperty); readbackPvNameField.textProperty().bindBidirectional(readbackPvNameProperty); @@ -224,17 +344,17 @@ public void updateItem(String item, boolean empty) { descriptionTextArea.textProperty().bindBidirectional(configurationDescriptionProperty); descriptionTextArea.disableProperty().bind(userIdentity.isNull()); - configurationEntries.addListener((ListChangeListener) change -> { + configurationEntries.addListener((ListChangeListener) change -> { while (change.next()) { if (change.wasAdded() || change.wasRemoved()) { FXCollections.sort(configurationEntries); - dirty.set(true); + setDirty(true); } } }); - configurationNameProperty.addListener((observableValue, oldValue, newValue) -> dirty.set(!newValue.equals(configurationNode.getName()))); - configurationDescriptionProperty.addListener((observable, oldValue, newValue) -> dirty.set(!newValue.equals(configurationNode.get().getDescription()))); + configurationNameProperty.addListener((observableValue, oldValue, newValue) -> setDirty(!newValue.equals(configurationNode.getName()))); + configurationDescriptionProperty.addListener((observable, oldValue, newValue) -> setDirty(!newValue.equals(configurationNode.get().getDescription()))); saveButton.disableProperty().bind(Bindings.createBooleanBinding(() -> dirty.not().get() || configurationDescriptionProperty.isEmpty().get() || @@ -268,12 +388,13 @@ public void updateItem(String item, boolean empty) { } @FXML + @SuppressWarnings("unused") public void saveConfiguration() { UI_EXECUTOR.execute(() -> { try { configurationNode.get().setName(configurationNameProperty.get()); configurationNode.get().setDescription(configurationDescriptionProperty.get()); - configurationData.setPvList(configurationEntries); + configurationData.setPvList(configurationEntries.stream().map(ConfigPvEntry::toConfigPv).toList()); Configuration configuration = new Configuration(); configuration.setConfigurationNode(configurationNode.get()); configuration.setConfigurationData(configurationData); @@ -281,12 +402,10 @@ public void saveConfiguration() { configuration = saveAndRestoreService.createConfiguration(configurationNodeParent, configuration); configurationTab.setId(configuration.getConfigurationNode().getUniqueId()); - configurationTab.updateTabTitle(configuration.getConfigurationNode().getName()); } else { configuration = saveAndRestoreService.updateConfiguration(configuration); } configurationData = configuration.getConfigurationData(); - dirty.set(false); loadConfiguration(configuration.getConfigurationNode()); } catch (Exception e1) { ExceptionDetailsErrorDialog.openError(pvTable, @@ -298,6 +417,7 @@ public void saveConfiguration() { } @FXML + @SuppressWarnings("unused") public void addPv() { UI_EXECUTOR.execute(() -> { @@ -305,12 +425,12 @@ public void addPv() { String[] pvNames = pvNameProperty.get().trim().split("[\\s;]+"); String[] readbackPvNames = readbackPvNameProperty.get().trim().split("[\\s;]+"); - ArrayList configPVs = new ArrayList<>(); + ArrayList configPVs = new ArrayList<>(); for (int i = 0; i < pvNames.length; i++) { // Disallow duplicate PV names as in a restore operation this would mean that a PV is written // multiple times, possibly with different values. String pvName = pvNames[i].trim(); - if (configurationEntries.stream().anyMatch(s -> s.getPvName().equals(pvName))) { + if (configurationEntries.stream().anyMatch(s -> s.getPvNameProperty().get().equals(pvName))) { continue; } String readbackPV = i >= readbackPvNames.length ? null : readbackPvNames[i] == null || readbackPvNames[i].isEmpty() ? null : readbackPvNames[i].trim(); @@ -319,10 +439,10 @@ public void addPv() { .readOnly(readOnlyProperty.get()) .readbackPvName(readbackPV) .build(); - configPVs.add(configPV); + configPVs.add(new ConfigPvEntry(configPV)); } configurationEntries.addAll(configPVs); - configurationTab.annotateDirty(true); + //configurationTab.annotateDirty(true); resetAddPv(); }); @@ -348,7 +468,7 @@ public void newConfiguration(Node parentNode) { configurationData = new ConfigurationData(); pvTable.setItems(configurationEntries); UI_EXECUTOR.execute(() -> configurationNameField.requestFocus()); - dirty.set(false); + setDirty(false); } /** @@ -363,6 +483,7 @@ public void loadConfiguration(final Node node) { ExceptionDetailsErrorDialog.openError(root, Messages.errorGeneric, Messages.errorUnableToRetrieveData, e); return; } + loadInProgress.set(true); // Create a cloned Node object to avoid changes in the Node object contained in the tree view. configurationNode.set(Node.builder().uniqueId(node.getUniqueId()) .name(node.getName()) @@ -372,16 +493,16 @@ public void loadConfiguration(final Node node) { .created(node.getCreated()) .lastModified(node.getLastModified()) .build()); - loadConfigurationData(); + loadConfigurationData(() -> loadInProgress.set(false)); } - private void loadConfigurationData() { + private void loadConfigurationData(Runnable completion) { UI_EXECUTOR.execute(() -> { try { Collections.sort(configurationData.getPvList()); - configurationEntries.setAll(configurationData.getPvList()); + configurationEntries.setAll(configurationData.getPvList().stream().map(ConfigPvEntry::new).toList()); pvTable.setItems(configurationEntries); - dirty.set(false); + completion.run(); } catch (Exception e) { logger.log(Level.WARNING, "Unable to load existing configuration"); } @@ -412,4 +533,8 @@ public void nodeChanged(Node node) { .build()); } } + + private void setDirty(boolean dirty) { + this.dirty.set(dirty && !loadInProgress.get()); + } } diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/configuration/ConfigurationTab.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/configuration/ConfigurationTab.java index ad4d54cba9..572a7f4a2c 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/configuration/ConfigurationTab.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/configuration/ConfigurationTab.java @@ -95,7 +95,7 @@ public void configureForNewConfiguration(Node parentNode) { @Override public void nodeChanged(Node node) { if (node.getUniqueId().equals(getId())) { - textProperty().set(node.getName()); + Platform.runLater(() -> textProperty().set(node.getName())); } } @@ -104,15 +104,21 @@ public void nodeChanged(Node node) { * * @param tabTitle The wanted tab title. */ - public void updateTabTitle(String tabTitle) { + private void updateTabTitle(String tabTitle) { Platform.runLater(() -> textProperty().set(tabTitle)); } + /** + * Updates the tab to indicate if the data is dirty and needs to be saved. + * @param dirty If true, an asterisk is prepended, otherwise + * only the name {@link org.phoebus.applications.saveandrestore.model.Configuration} + * is rendered. + */ public void annotateDirty(boolean dirty) { String tabTitle = textProperty().get(); - if (dirty && !tabTitle.contains("*")) { + if (dirty && !tabTitle.startsWith("*")) { updateTabTitle("* " + tabTitle); - } else if (!dirty) { + } else if (!dirty && tabTitle.startsWith("*")) { updateTabTitle(tabTitle.substring(2)); } } 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 1789f5a4b6..8908457004 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 @@ -19,6 +19,8 @@ closeConfigurationWarning=Configuration modified, but not saved. Do you wish to closeCompositeSnapshotWarning=Composite snapshot modified, but not saved. Do you wish to continue? closeTabPrompt=Close tab? comment=Comment +comparisonMode=Comparison Mode +comparisonTolerance=Tolerance compositeSnapshotConsistencyCheckFailed=Failed to check consistency for composite snapshot copy=Copy createdDate=Created 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 0080db9c83..70ac3d2d65 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,8 @@ - +
@@ -53,25 +54,15 @@