diff --git a/doc/client/coding/Activators.md b/doc/client/coding/Activators.md new file mode 100644 index 000000000..2ddc5fe02 --- /dev/null +++ b/doc/client/coding/Activators.md @@ -0,0 +1,50 @@ +# Activators + +An Activator is the entry-point for a Plug-in - it gets created by the Eclipse framework when the Plug-in is first used. + +Activators are implemented using a singleton pattern; a simplified typical activator looks like: + +```java +import org.osgi.framework.BundleActivator; +import org.osgi.framework.BundleContext; + +public class Help implements BundleActivator { + + private static BundleContext context; + private static Help instance; + + static BundleContext getContext() { + return context; + } + + public Help() { + instance = this; + } + + @Override + public void start(BundleContext bundleContext) throws Exception { + Help.context = bundleContext; + } + + public static Help getInstance() { + return instance; + } + + @Override + public void stop(BundleContext bundleContext) throws Exception { + Help.context = null; + } +} +``` + +Activators may create and hold references to static data - for example models - so that they can later be referenced by +other plugins that need access to those models (for example, views). + +:::{note} +At first glance the above pattern may seem dangerous - in most cases a singleton would need to check whether an instance +already exists and create one if not in `getInstance`, and would also need to use locking for thread safety. + +However, this pattern is safe for Activators, because the Eclipse framework guarantees that `start` will +be called exactly once for a singleton plugin, before any other code can use the plugin, and that `stop` will only be +called when no other plugin will be able to access it afterwards (for example, during platform shutdown). +::: diff --git a/doc/client/coding/Adding-a-Button-to-the-Perspective-Switcher.md b/doc/client/coding/Adding-a-Button-to-the-Perspective-Switcher.md deleted file mode 100644 index 1a4d4378a..000000000 --- a/doc/client/coding/Adding-a-Button-to-the-Perspective-Switcher.md +++ /dev/null @@ -1,352 +0,0 @@ -# Adding a perspective - -![Perspective Switcher](IBEX_complete_perspective_switcher_highlighted.png) - - -### Creating a plug-in - -* In Eclipse, add a new UI plug-in via File->New->Other->Plug-in Development->Plug-in Project - -* Give it a name, for example: uk.ac.stfc.isis.ibex.ui.myperspective - -* Do not use the default location, instead set it to C:\\Instrument\\Dev\\client\\ISIS\\base\\ plus the plug-in name. - * for example: C:\\Instrument\\Dev\\ibex_gui\\base\\uk.ac.stfc.isis.ibex.ui.myperspective - -* The dialog should look like this: - -![Add Perspective](eclipse_add_perspective_plugin1.png) - -* Click 'Next' - -* On the next page of the dialog check it looks like the following and click 'Finish': - -![Add Perspective](eclipse_add_perspective_plugin2.png) - -Note: Generate an activator is unchecked as generally we do not need one for UI plug-ins, but it does not really matter if there is one. - -### Creating a Perspective - -Now the plug-in has been created: - -* Add a source package via File->New->Other->Java->Package - -* Set the source folder to the path of the source folder in the newly created plug-in - -* Set the name to something sensible prefixed by the plug-in name (or just the plug-in itself) - -* The dialog should look something like this: - -![Add a package](eclipse_adding_a_package.png) - -Note: Package names should be in the format of uk.ac.stfc.isis.ibex.ui.packagename, not org.csstudio.isis.ui.packagename as shown in all images. - -We now need to add a Perspective class to the new package; the easiest way to do this is to copy and paste an existing one, for example: the one in uk.ac.stfc.isis.ibex.ui.scripting, and edit it. - -The first thing you will notice is that there are numerous red errors. - -![Eclipse errors](eclipse_perspective_copy_errors.png) - -These are easily fixed: - -* Hover the mouse over `BasePerspective` on the `public class Perspective` line and select `Fix project setup...` - -* Delete the `Consoles.getDefault().createConsole();` line - -* Open the MANIFEST.MF file in META_INF and select the `Dependencies` tab; on that tab click the `Add` button under `Required Plug-ins`. From the list select uk.ac.stfc.isis.ibex.ui and click `OK`. Add org.eclipse.ui and org.eclipse.core.runtime with the same method. - -* There are likely to be other dependencies missing, fix any other errors. - -* Save all the changes. - -The errors should now have disappeared, but there are a few more things to do: - -* Change the value returned by the name function to be what you want shown on the Perspective Switcher. Put an ampersand before the character that you want to use as the perspective shortcut making sure that the chosen character is not in use already. (More on this later) - -* Add a new icon (we will leave that as it is for now and fix it later!) - -### Creating a View - -With the Perspective now in place we need to add a View: - -* Add a new class via File->New->Other->Java->Class - -* Make sure the Package is the same as one created earlier - -* Enter a sensible name - -* Click the 'Browse' button next to the Superclass and select ViewPart - -The dialog should look something like this: - -![Add a View](eclipse_adding_a_View.png) - -The new class file should open in the editor. For this example I am just going to add a web browser to the view: - -* Right-click on the View in the Package Explorer in Eclipse and select Open With->WindowBuilder Editor. - -* In the window that pops up select the Design tab - -* Select the FillLayout and click on the mock-up of the View to add it - -* Select the Browser control and click on the mock-up to add it - -* Now click on the Source tab - -* If the `createPartControl` method has a Composite argument called arg0 change it to parent - -* Delete the TODO comments - -* Add an ID for the View which is the full-name of the class in lower-case; for example: uk.ac.stfc.isis.ibex.ui.myperspective.myview - -* It should now look something like: - -```java - package uk.ac.stfc.isis.ibex.ui.myperspective; - - import org.eclipse.swt.widgets.Composite; - import org.eclipse.ui.part.ViewPart; - import org.eclipse.swt.layout.FillLayout; - import org.eclipse.swt.SWT; - import org.eclipse.swt.browser.Browser; - - public class MyView extends ViewPart { - public static final String ID = "uk.ac.stfc.isis.ibex.ui.myperspective.myview"; - - public MyView() { - } - - @Override - public void createPartControl(Composite parent) { - parent.setLayout(new FillLayout(SWT.HORIZONTAL)); - - Browser browser = new Browser(parent, SWT.NONE); - } - - @Override - public void setFocus() { - } - } -``` - -### Adding the Perspective and View to the GUI - -Note: The extensions are defined in XML inside plugin.xml, so example XML is included below if you would prefer to edit that directly. - -Note: Sometimes Eclipse cannot find the schema for the extensions, so when trying to select, say, New->contribution contribution does not appear in the list rather it says Generic; in this case, it is necessary to edit the XML directly. - -To add both the new Perspective and View to the main GUI we use extensions. First let's add the Perspective: - -* Open the MANIFEST.MF file in META_INF and select the 'Extensions' tab, it should be empty like this: - -![Extensions](eclipse_no_extensions.png) - -* Click the 'Add' button and select uk.ac.stfc.isis.ibex.ui.perspectives extension point and click 'Finish' - -![Extension points](eclipse_select_extension_point.png) - -* It should now look like this: - -![Extension points](eclipse_extensions_added1.png) - -* Right-click on the uk.ac.stfc.isis.ibex.ui.perspectives extension point and select New->contribution - -* The contribution should appear below the extension point - -* Using the 'Browse' button select the Perspective class created earlier, the screen should now look like like this: - -![Extension points](eclipse_extensions_added2.png) - -* Using the 'Add' button as before we need to add org.eclipse.ui.perspectives extension point - -* Right-click on the new extension point and select New->perspective; the new perspective should appear below the extension point - -* Select the newly added item and fill in the required details, it should look something like this: - -![Extension points](eclipse_extensions_added3.png) - -The XML in plugin.xml for what we have done so far is: -```xml - - - - - - - - -``` - -Now we add the extensions for the View: - -* Using the 'Add' button as before we need to add org.eclipse.ui.perspectiveExtensions extension point - -* Right-click on the new extension point and select New->perspectiveExtension; the new perspectiveExtension should appear below the extension point - -* For the new perspectiveExtension set the targetID to the ID of your perspective, for this example it is uk.ac.stfc.isis.ibex.ui.myperspective.perspective - -* Right-click on the perspectiveExtension and select New->view; the new view should appear - -* Select the view and change the ID to the ID of your View, for example: uk.ac.stfc.isis.ibex.ui.myperspective.myview - -* Change the relative to uk.ac.stfc.isis.ibex.ui.perspectives.PerspectiveSwitcher - -* The remaining setting determine how the View will appear and behave, it is recommended that you set the following values: - - * closeable: false - * minimized: false - * moveable: false - * showTitle: false - * standalone: true - * visible: true - * relationship: right - -* It should look something like this: - -![Extension points](eclipse_extensions_added4.png) - -* Next, using the 'Add' button we need to add org.eclipse.ui.views extension point - -* Right-click on the new item and select New->view; the new view should appear below the extension point - -* Set the class and id to the name and id of your View class respectively; it should look something like this: - -![Extension points](eclipse_extensions_added5.png) - -The XML in the plugin.xml for the View related stuff is: -```xml - - - - - - - - - - -``` - -Finally, the last step is to add the plug-in we created to uk.ac.stfc.isis.ibex.feature.base: - -* Open the feature.xml file in uk.ac.stfc.isis.ibex.feature.base and select the Plug-ins tab - -* Click the 'Add' button and select the new plug-in we created - -* Save everything and run the main GUI - hopefully, the new Perspective will appear - - -### Adding a new icon - -* Grab a nice png icon which is appropriately sized (`24x24` pixels) from somewhere like http://www.flaticon.com/ - -* Create a icons folder in the top-level of the plug-in - -* Drag the icon into it - -* Open the MANIFEST.MF file and select the Build tab - -* Tick the icons box under Binary Build to include the icons folder in the build - -![Add an icon](eclipse_add_icons_to_build.png) - -* Open the Perspective class created earlier - -* Change the image method to return the new icon from the correct plug-in by changing the plug-in name and icon name, like so: -```java - package uk.ac.stfc.isis.ibex.ui.myperspective; - - import uk.ac.stfc.isis.ibex.ui.perspectives.BasePerspective; - import org.eclipse.swt.graphics.Image; - import org.eclipse.ui.IPageLayout; - import org.eclipse.wb.swt.ResourceManager; - - public class Perspective extends BasePerspective { - - public static final String ID = "uk.ac.stfc.isis.ibex.ui.myperspective.perspective"; //$NON-NLS-1$ - - @Override - public void createInitialLayout(IPageLayout layout) { - super.createInitialLayout(layout); - } - - @Override - public String ID() { - return ID; - } - - @Override - public String name() { - return "My Perspective"; - } - - @Override - public Image image() { - return ResourceManager.getPluginImage("uk.ac.stfc.isis.ibex.myperspective", "icons/myperspective.png"); - } - } -``` - -* Finally, start the GUI to check the new icon is shown - - -### Adding a keyboard shortcut - -* In the plugin xml for your new perspective add the following: - -```xml - - - - - - -``` - -* Run the GUI and hit ALT + SHIFT + _chosen char_ and confirm that the perspective is switched to. - -### Setting the Perspective to be invisible - -If you are creating the perspective for testing a new feature that you do not want displayed to the user by default then add the following code to your perspective class: -``` - @Override - public boolean isVisibleDefault() { - return false; - } -``` -by default this value is set to true and so perspectives are displayed. - -To subsequently display the perspective run IBEX and go to the perspective window (CTRL + ALT + P) then enable the checkbox in ISIS Perspectives. On the next restart of the GUI your perspective should be displayed. - -### Troubleshooting - -If the perspective is not being shown in the switcher at the side it may be Eclipse being silly or you may not be running the right product. Be sure to re-run by going selecting the client product, rather than using the drop-down (which will run the same product as you used previously) Finally, try clearing the workspace and resetting the target platform etc. \ No newline at end of file diff --git a/doc/client/coding/Adding-a-Plugin-or-Feature-to-Maven-Build.md b/doc/client/coding/Adding-a-Plugin-or-Feature-to-Maven-Build.md index b3ea650ab..44ee2671a 100644 --- a/doc/client/coding/Adding-a-Plugin-or-Feature-to-Maven-Build.md +++ b/doc/client/coding/Adding-a-Plugin-or-Feature-to-Maven-Build.md @@ -1,56 +1,29 @@ -# Adding a plugin or feature to Maven +# Adding a plugin or feature -The steps for adding a plug-in (one small part of IBEX, such as the blocks view) or feature (a larger collection of plug-ins, such as CSS) to the maven build are below: +The steps for adding a plug-in (one small part of IBEX, such as the blocks view) or feature (a larger collection of +plug-ins, such as CSS) to IBEX are: -## Step by step: * Add the plug-in to `feature.base`: * Open `feature.xml` in `uk.ac.stfc.isis.ibex.feature.base` * Go to "Included Plug-ins" (or "Included Features") tab and click "Add..." * Find your new plug-in in the list and add it - * Add the plug-in to `ibex.product` * Open `ibex.product` in `uk.ac.stfc.isis.ibex.e4.client.product` * Go to "Configuration" tab and click "Add..." next the "Start Levels" section * Find your new plug-in in the list and add it - -* Add the plug-in to `feature.base`: - * Open `feature.xml` in `uk.ac.stfc.isis.ibex.feature.base` - * Go to "Included Plug-ins" tab and click "Add..." - * Find your new plug-in in the list and add it - * Convert the plug-in to a Maven project. * Right-click on the plug-in and select Configure > Convert to Maven Project * Click "Finish". This should create a `pom.xml` inside the project. - * Add the new plug-in to the Parent POM * Select the `pom.xml` file in `uk.ac.stfc.isis.ibex.client.tycho.parent` * On the overview tab click "Add..." under the Modules section * Select the new plug-in from the list * Enable the "Update POM parent section in selected projects" option and click "OK" * Save it - -* Edit the plug-in pom.xml file +* Edit the plug-in `pom.xml` file * Select the pom.xml file * Open the pom.xml tab * Change/add the packaging to `eclipse-plugin` (or `eclipse-test-plugin` if it's a unit test plugin) * Remove the build section * Remove the `groupID` and version entries outside of parent - * Save it - -## Example POM - -An example of what the plug-in POM should look like: - -``` - - 4.0.0 - tychodemo.bundle.tests - eclipse-plugin - - CSS_ISIS - parent - 1.0.0-SNAPSHOT - ../tychodemo.parent - - -``` + * Verify that the resulting plug-in POM looks like an existing `pom.xml` for a similar plugin diff --git a/doc/client/coding/Connecting-a-View-to-a-PV.md b/doc/client/coding/Connecting-a-View-to-a-PV.md index c1ae8abbf..3ed2ad773 100644 --- a/doc/client/coding/Connecting-a-View-to-a-PV.md +++ b/doc/client/coding/Connecting-a-View-to-a-PV.md @@ -1,55 +1,25 @@ # Connecting a view to a PV -See [An Introduction to Databinding](Databinding) for a explanation of databinding. +:::{seealso} +- [An Introduction to Databinding](Databinding) for an explanation of databinding. +- [PV Switching](PV-Switching) for a detailed description of how PVs are switched when the IBEX GUI is pointed +at a new instrument. +::: -The basic arrangement for the mechanism for connecting a View to a PV is shown below: +The basic arrangement for the mechanism for connecting a View to a PV is: -![Basic principle](basic_principle.png) + -## Adding an Observable +## Reading from a PV -The Observable is responsible for the connection to the PV and monitoring it for changes. +The Observable is responsible for the connection to the PV and monitoring it for changes. The most basic pattern +for creating and observing a PV is: -For this example we are going to connect and read the title PV on the DAE IOC. There is already a way of getting the title in the IBEX GUI, but for demonstration purposes we are going to ignore that! - -First we need to create a new Plug-in in Eclipse: - -* Select File->New->Plug-in Project - -* Enter org.csstudio.isis.title for the project name - -* Un-check 'Use default location' and browse to where the GUI code is located and append the project name to the path. For example, on my system the location would be C:\\Instrument\\Dev\\client\\ISIS\\base\\org.csstudio.isis.title - -* Click 'Next' - -* On the next screen, check the 'Generate an activator ...' check-box and un-check the others. - -* Set 'Would you like to create a 3.x rich client application?' to 'No' - -* Click 'Finish' - -Now the Plug-in is created we can create an Observable: - -* Open the MANIFEST.MF file in META-INF and under the 'Dependencies' tab add org.csstudio.isis.instrument to the list of required plug-ins - -* Don't forget to save the changes - -* Create a new Class called TitleVariable, this will be the class that holds the connection to the PV - -* First we will need to create an observable factory to get our observable, in this case the PV should be updated when the instrument switches so add the following as a field. -```java -private final ObservableFactory obsFactory = new ObservableFactory(OnInstrumentSwitch.SWITCH); -``` - -* Next we get the specific variable for the PV the observable for the PV: ```java -package org.csstudio.isis.title; - import uk.ac.stfc.isis.ibex.epics.observing.ForwardingObservable; import uk.ac.stfc.isis.ibex.epics.switching.ObservableFactory; import uk.ac.stfc.isis.ibex.epics.switching.OnInstrumentSwitch; -import org.csstudio.isis.instrument.Channels; -import org.csstudio.isis.instrument.channels.CharWaveformChannel; +import uk.ac.stfc.isis.ibex.instrument.channels.CharWaveformChannel; import uk.ac.stfc.isis.ibex.instrument.InstrumentUtils; public class TitleVariable { @@ -57,163 +27,14 @@ public class TitleVariable { private final ObservableFactory obsFactory = new ObservableFactory(OnInstrumentSwitch.SWITCH); public final ForwardingObservable titleRBV = obsFactory.getSwitchableObservable(new CharWaveformChannel(), InstrumentUtils.addPrefix("DAE:TITLE")); - - public TitleVariable() { - } - } ``` -Explanation on Channels, `InitialiseOnSubscribeObservable` and reader. - -## Adding a Model interface - -Next we create a Model interface: - -* Create a new Interface called TitleModel`` - -* Implementing the interface will require `getTitle` and `setTitle` methods, so the code needs to look like: -``` -java -package org.csstudio.isis.title; - -public interface TitleModel { - String getTitle(); - void setTitle(String value); -} -``` - -## Adding an ObservableModel - -Next we create an ObservableModel which allows us to bind controls on the View to the PV Observable: - -* Create a new Class called ObservableTitleModel - -* ObservableTitleModel needs to inherit from ModelObject from org.csstudio.isis.model, so change the code to look like: -```java -package org.csstudio.isis.title; - -public class ObservableTitleModel extends ModelObject { - -} -``` -* To fix the errors we need to add org.csstudio.isis.model to the required plug-ins list in the MANIFEST.INF. We can either add it manually or hover over the error and select "Add 'org.csstudio.isis.model' to required bundles" - -* The next error can be fixed via the drop-down by selecting "Import 'ModelObject' (org.csstudio.isis.model)" - -* The class also needs to implement TitleModel, so change the code to implement TitleModel: -```java -public class ObservableTitleModel extends ModelObject implements TitleModel { -``` -* There should now be an error because the methods of TitleModel are not implemented. Hover over the error and select 'Add unimplemented methods'. The code should look like this: -```java -package org.csstudio.isis.title; - -import org.csstudio.isis.epics.observing.BaseObserver; -import org.csstudio.isis.model.ModelObject; - -public class ObservableTitleModel extends ModelObject implements TitleModel { - - private String title; - - @Override - public String getTitle() { - // TODO Auto-generated method stub - return null; - } - - @Override - public void setTitle(String value) { - // TODO Auto-generated method stub - - } - -} -``` -* The correctly cased getter and setter are requirements for the object to be bindable from a View (http://en.wikipedia.org/wiki/JavaBeans). We now need to add our own code to the methods: -```java -package org.csstudio.isis.title; - -import org.csstudio.isis.model.ModelObject; - - -public class ObservableTitleModel extends ModelObject{ - private String title; - - public String getTitle() { - return title; - } - - public void setTitle(String value) { - firePropertyChange("title", this.title, this.title = value); - } -} -``` -The `firePropertyChange` method raises an event when the title changes. - -* Next we create a BaseObserver inside the class, like so: -```java -package org.csstudio.isis.title; - -import org.csstudio.isis.epics.observing.BaseObserver; -import org.csstudio.isis.model.ModelObject; - -public class ObservableTitleModel extends ModelObject implements TitleModel { - - private String title; - - private final BaseObserver titleObserver = new BaseObserver(){ - - @Override - public void onValue(String value) { - setTitle(value); - } - @Override - public void onError(Exception e) { - - } - - @Override - public void onConnectionChanged(boolean isConnected) { - - } - }; - - @Override - public String getTitle() { - return title; - } - - @Override - public void setTitle(String value) { - firePropertyChange("title", this.title, this.title = value); - } -} -``` -* The BaseObserver is responsible for observing changes in the TitleVariable, but for that we need to wire the objects up. Add a private variable for TitleVariable, like so: -```java -... -public class ObservableTitleModel extends ModelObject implements TitleModel { +The above PV is linked to a model class, which derives from `ModelObject`, in the following way: - private String title; - private final TitleVariable titleVar; - - private final BaseObserver titleObserver = new BaseObserver(){ -... -``` -* Now we add a constructor for that allows the wiring up of the BaseObserver and the TitleVariable: -```java -public ObservableTitleModel(TitleVariable titleVar) { - this.titleVar = titleVar; - titleVar.titleRBV.subscribe(titleObserver); -} -``` -* The final class looks like this: ```java -package org.csstudio.isis.title; - -import org.csstudio.isis.epics.observing.BaseObserver; -import org.csstudio.isis.model.ModelObject; +import uk.ac.stfc.isis.ibex.epics.observing.BaseObserver; +import uk.ac.stfc.isis.ibex.model.ModelObject; public class ObservableTitleModel extends ModelObject implements TitleModel { @@ -221,423 +42,112 @@ public class ObservableTitleModel extends ModelObject implements TitleModel { private final TitleVariable titleVar; private final BaseObserver titleObserver = new BaseObserver() { - @Override public void onValue(String value) { setTitle(value); } - - @Override - public void onError(Exception e) { - - } - - @Override - public void onConnectionChanged(boolean isConnected) { - - } + + // Error and connection-status handling omitted }; - - @Override + public String getTitle() { return title; } - - @Override + public void setTitle(String value) { firePropertyChange("title", this.title, this.title = value); } - + public ObservableTitleModel(TitleVariable titleVar) { this.titleVar = titleVar; titleVar.titleRBV.subscribe(titleObserver); } } ``` -## The Activator - -The Activator is the entry-point for the Plug-in - it gets created when the Plug-in is first used. -This is where we wire up the ObservableTitleModel and the TitleVariable: -* First we want to rename Activator because it is not a very descriptive name, let's refactor it to be called Title (okay that is not much better...) - -* We want to make the Activator a singleton, so we need it to contain a static instance of itself, a constructor and a `getInstance` method: -```java -package org.csstudio.isis.title; - -import org.osgi.framework.BundleActivator; -import org.osgi.framework.BundleContext; - -public class Title implements BundleActivator { - - private static BundleContext context; - private static Title instance; - - public Title() { - instance = this; - } - - public static Title getInstance() { - return instance; - } - - static BundleContext getContext() { - return context; - } - - /* - * (non-Javadoc) - * @see org.osgi.framework.BundleActivator#start(org.osgi.framework.BundleContext) - */ - public void start(BundleContext bundleContext) throws Exception { - Title.context = bundleContext; - } - - /* - * (non-Javadoc) - * @see org.osgi.framework.BundleActivator#stop(org.osgi.framework.BundleContext) - */ - public void stop(BundleContext bundleContext) throws Exception { - Title.context = null; - } -} -``` -* Now we need it to wire up our classes from earlier, so we add the wiring up to the constructor: -```java -... -private static BundleContext context; -private static Title instance; - -private TitleVariable titleVar; -private ObservableTitleModel model; - -public Title() { - instance = this; - titleVar = new TitleVariable(Instrument.getInstance().channels()); - model = new ObservableTitleModel(titleVar); -} -... -``` -* Finally we add a method to retrieve the model so we can bind against it. The final code look like this: -```java -package org.csstudio.isis.title; - -import org.csstudio.isis.instrument.Instrument; -import org.osgi.framework.BundleActivator; -import org.osgi.framework.BundleContext; - -public class Title implements BundleActivator { - - private static BundleContext context; - private static Title instance; - - private TitleVariable titleVar; - private ObservableTitleModel model; - - public Title() { - instance = this; - titleVar = new TitleVariable(Instrument.getInstance().channels()); - model = new ObservableTitleModel(titleVar); - } - - public static Title getInstance() { - return instance; - } - - public ObservableTitleModel model() { - return model; - } - - static BundleContext getContext() { - return context; - } - - /* - * (non-Javadoc) - * @see org.osgi.framework.BundleActivator#start(org.osgi.framework.BundleContext) - */ - public void start(BundleContext bundleContext) throws Exception { - Title.context = bundleContext; - } - - /* - * (non-Javadoc) - * @see org.osgi.framework.BundleActivator#stop(org.osgi.framework.BundleContext) - */ - public void stop(BundleContext bundleContext) throws Exception { - Title.context = null; - } -} -``` -## A little tidying up - -By convention we should put any class etc. we don't want exposed outside outside of the Plug-in in a package ending in internal. In this example the package would be called org.csstudio.isis.title.internal. - -## Binding the View to the Model - -The simplest way to do this is to add a method for creating the binding to the View: -```java -private DataBindingContext bindingContext; - -private void doBinding(Label lblTitle){ - bindingContext = new DataBindingContext(); - bindingContext.bindValue(WidgetProperties.text().observe(lblTitle), BeanProperties.value("title").observe(Title.getInstance().model())); -} -``` -* Then the method can be called from the `createPartControl` method of the View +This model class can now be bound to a UI element using [data binding](Databinding). ## Writing to a PV -For this example we will start by adding a writeable PV for writing to the title to the TitleVariable class, so it now looks like this: -```java -package org.csstudio.isis.title; +Similar to an Observable, a Writable is responsible for writing to a PV on demand. The most basic pattern +for creating a writable PV is: +```java import uk.ac.stfc.isis.ibex.epics.observing.ForwardingObservable; -import org.csstudio.isis.epics.writing.Writable; -import org.csstudio.isis.instrument.Channels; -import org.csstudio.isis.instrument.channels.CharWaveformChannel; +import uk.ac.stfc.isis.ibex.epics.writing.Writable; +import uk.ac.stfc.isis.ibex.instrument.channels.CharWaveformChannel; import uk.ac.stfc.isis.ibex.instrument.InstrumentUtils; public class TitleVariable { - private final ObservableFactory obsFactory = new ObservableFactory(OnInstrumentSwitch.SWITCH); - public final ForwardingObservable titleRBV = - obsFactory.getSwitchableObservable(new CharWaveformChannel(), InstrumentUtils.addPrefix("DAE:TITLE")); - private final WritableFactory writeFactory = new WritableFactory(OnInstrumentSwitch.SWITCH); public final Writable titleSP = writeFactory.getSwitchableWritable(new CharWaveformChannel(), InstrumentUtils.addPrefix("DAE:TITLE:SP")); - - public TitleVariable() { - } } ``` -Note that the type of the new PV is Writable and uses writable and not reader. - -Next we open the TitleModel interface and add two new methods for working with the set-point, it now looks like this: -```java -package org.csstudio.isis.title; +The above PV can be linked to a model class, which derives from `ModelObject`, in the following way: -public interface TitleModel { - String getTitle(); - void setTitle(String value); - String getTitleSP(); - void setTitleSP(String value); -} -``` -This interface is implemented in the ObservableTitleModel class, so we need to add implementations for the two new methods. -In fact, the ObservableTitleModel should now be showing an error because these methods are not implemented. -Open the class and hover over the class name, it should give you the option to 'Add unimplemented methods', select this. -Now we need to add code for these new methods: -```java -... -@Override -public String getTitleSP() { - return ""; -} - -@Override -public void setTitleSP(String value) { - titleVar.titleSP.write(value); -} -... -``` -The getter returns an empty string, but this does not matter - we could wire it up to get the current value of the set-point using a BaseObserver in the same way we did for reading the current title earlier, but we will leave that as an exercise for the reader! - -The setter uses the Writable object to write the new value to the IOC via channel access. - -Finally we need to edit the View itself to enable it to bind to titleSP. For this we add a text-box called `txtNewTitle` and set up the binding: ```java -... -bindingContext.bindValue(WidgetProperties.text(SWT.Modify).observe(txtNewTitle), BeanProperties.value("titleSP").observe(org.csstudio.isis.title.Title.getInstance().model())); -... -``` -The key thing to note here is that WidgetProperties.txt takes SWT.Modify; this tells the binding to push the changes back to the model when the text-box is changed. +import uk.ac.stfc.isis.ibex.epics.observing.BaseObserver; +import uk.ac.stfc.isis.ibex.model.ModelObject; -If we start the GUI it can be seen that as we type in the text-box the title changes; however, it is not ideal as the title set-point is updated with every keystroke which seems a bit unnecessary. - -The alternative is to have a set button. First add a string property for the title SP called titleSP like so: -```java -... public class ObservableTitleModel extends ModelObject implements TitleModel { -private String title; -private String titleSP; -private final TitleVariable titleVar; - -private final BaseObserver titleObserver = new BaseObserver() { -... -``` -Now we modify the getter and setter to use this variable for storing the new title before it is sent to the IOC: -```java -... -@Override -public String getTitleSP() { - return titleSP; -} + private final TitleVariable titleVar; + + // getTitle omitted -@Override -public void setTitleSP(String value) { - firePropertyChange("titleSP", this.titleSP, this.titleSP = value); -} -... -``` -We also add a method to the TitleModel interface and the implementation to the ObservableTitleModle for sending the string to the IOC: -```java -... -@Override -public void sendTitleSP() -{ - titleVar.titleSP.write(titleSP); -} -... -``` -The final step is to add a button to the View's `createPartControl` method: -```java -... -Button btnSet = new Button(parent, SWT.NONE); -btnSet.setText("Set"); -btnSet.addSelectionListener(new SelectionAdapter() { - @Override - public void widgetSelected(SelectionEvent e){ - org.csstudio.isis.title.Title.getInstance().model().sendTitleSP(); + public void setTitle(String value) { + this.titleVar.titleSP.write(value); + } + + public ObservableTitleModel(TitleVariable titleVar) { + this.titleVar = titleVar; } -}); -... -``` - -## Using a Read-Write control - -It is fine to have separate controls on the View for reading and writing, but sometimes it is more convenient to have one control for both. - -This is relatively straightforward as there is already a helper class called WritableObservableAdapter that does most of the work. - -The first step though is to create a View Model class in our UI plug-in, so create a new class in org.csstudio.isis.ui.title called ViewModel. -This class is where we will connect up the WritableObservableAdapter, but first we need to make some changes to ObservableTitleModel -and to do that we start by modifying TitleModel to add a methods for accessing the Writeable object and a CachingObservable object (explained later): -```java -package org.csstudio.isis.title; - -import org.csstudio.isis.epics.observing.CachingObservable; -import org.csstudio.isis.epics.writing.Writable; - - -public interface TitleModel { - String getTitle(); - void setTitle(String value); - String getTitleSP(); - void setTitleSP(String value); - void sendTitleSP(); - CachingObservable title(); - Writable titleSetter(); } ``` -Now we implement these methods in ObservableTitleModel: -```java -... -@Override -public CachingObservable title() { - return titleVar.titleRBV; -} - -@Override -public Writable titleSetter() { - return titleVar.titleSP; -} -... -``` -Next we add some code to ViewModel to connect things up: -```java -package org.csstudio.isis.ui.myperspective; +The above class can then be linked, via [data binding](Databinding), to a UI widget, using the `SWT.Modify` flag so +that a notification is received each time the user input changes. -import org.csstudio.isis.title.ObservableTitleModel; -import org.csstudio.isis.ui.widgets.observable.WritableObservableAdapter; +In many cases it is preferable not to send a new value to a PV on every keystroke, but instead to have an explicit +"set" action. To achieve this, the above class is modified to only stash the title setpoint, and to have an explicit +`writeTitle` method: -public class ViewModel { - - public final ObservableTitleModel model = org.csstudio.isis.title.Title.getInstance().model(); - - public final WritableObservableAdapter title = new WritableObservableAdapter(model.titleSetter(), model.title()); - - public ViewModel() { - - } -} -``` -Some error messages will appear relating to WritableObservableAdapter; to fix, hover over WritableObervableAdaptor and select "Add 'org.csstudio.isis.ui.widgets' to required bundles". -Errors will still remain on model.titleSetter() and model.title(), so hover over one of those and select "Add 'org.csstudio.isis.epics' to required bundles". - -Now we need to adjust the View to use the ViewModel, first we strip out all of the code relating to the previous label, text-box and button (including the bindings), and then we add a ViewModel object and a WritableObservingTextBox: ```java -package org.csstudio.isis.ui.myperspective; +import uk.ac.stfc.isis.ibex.epics.observing.BaseObserver; +import uk.ac.stfc.isis.ibex.model.ModelObject; -import org.csstudio.isis.ui.widgets.observable.WritableObservingTextBox; -import org.eclipse.swt.SWT; -import org.eclipse.swt.widgets.Composite; -import org.eclipse.ui.part.ViewPart; -import org.eclipse.swt.layout.GridData; -import org.eclipse.swt.layout.GridLayout; +public class ObservableTitleModel extends ModelObject implements TitleModel { -public class MyView extends ViewPart { - public static final String ID = "org.csstudio.isis.ui.myperspective.myview"; + private String titleSP; + private final TitleVariable titleVar; - private final ViewModel viewModel = new ViewModel(); - private WritableObservingTextBox rbNumber; - - public MyView() { - } + // getTitle omitted - @Override - public void createPartControl(Composite parent) { - parent.setLayout(new GridLayout(1, false)); - rbNumber = new WritableObservingTextBox(parent, SWT.NONE, viewModel.title); - rbNumber.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false, 1, 1)); - + public void setTitleSP(String value) { + firePropertyChange("titleSP", this.titleSP, this.titleSP = value); } - @Override - public void setFocus() { + public void writeTitleSP() { + this.titleVar.titleSP.write(this.titleSP); } - -} -``` -Hopefully once the GUI is started that all works as expected. - -So what is going on and what are these new bits we are using? - -* WritableObservingTextBox is a custom ISIS control for displaying and editing a value where they are different PVs for reading and writing -* WritableObservableAdapter is the object for linking up the two PVs. Essentially it does the same as what we did earlier with reading one PV and writing to another -* View Model - a class for providing presentation logic and state information for a View. The code could live in the View itself, but it is cleaner to put it in a separate class. - -http://blogs.msdn.com/b/dphill/archive/2009/01/31/the-viewmodel-pattern.aspx provides a good explanation of View Models and how they fit in with the View and Model. - -As you have probably noticed there are a number of methods in the ObservableTitlemodel that is no longer used, these can be removed (don't forgot to remove them from the interface as well!). The BaseObserver can also be removed. -```java -package org.csstudio.isis.title; - -import org.csstudio.isis.epics.observing.CachingObservable; -import org.csstudio.isis.epics.writing.Writable; -import org.csstudio.isis.model.ModelObject; - -public class ObservableTitleModel extends ModelObject implements TitleModel { - - private final TitleVariable titleVar; public ObservableTitleModel(TitleVariable titleVar) { this.titleVar = titleVar; - //titleVar.titleRBV.subscribe(titleObserver); } +} +``` - @Override - public CachingObservable title() { - return titleVar.titleRBV; - } +The `writeTitleSP` method is then called from an explicit set button in the view: - @Override - public Writable titleSetter() { - return titleVar.titleSP; +```java +Button btnSet = new Button(parent, SWT.NONE); +btnSet.setText("Set"); +btnSet.addSelectionListener(new SelectionAdapter() { + public void widgetSelected(SelectionEvent e){ + model.writeTitleSP(); } -} -``` \ No newline at end of file +}); +``` diff --git a/doc/client/coding/Databinding---common-mistakes.md b/doc/client/coding/Databinding---common-mistakes.md deleted file mode 100644 index 89f022e7b..000000000 --- a/doc/client/coding/Databinding---common-mistakes.md +++ /dev/null @@ -1,40 +0,0 @@ -# Databinding - common mistakes - -Start with [An introduction to databinding](Databinding) if you are new to this. - -### Missing getter or setter (or incorrectly named) - -The getter and setters (if the value can be set from the UI) must exist and be named correctly. For example: `getName` and `setName` for `BeanProperties.value("name")`. - -Behind the scenes the databinding library automatically changes the first character to upper-case and then prepends "get" and "set". - -### Incorrectly cased string in binding - -With something like the following it is import that the name of the property to bind to is cased correctly: -```java -ctx.bindValue(WidgetProperties.text(SWT.Modify) - .observe(txtAge), BeanProperties.value("age").observe(person)); -``` - -This assumes there is a getter and setter called `getAge` and `setAge`. - -For something like `getFedId` the binding code would look like: - -```java -ctx.bindValue(WidgetProperties.text(SWT.Modify) - .observe(txtId), BeanProperties.value("fedId").observe(person)); -``` -**The important point to note is the 'f' of "fedId" is lower-case. It will not work if it is upper-case.** - -### The getter or setter "silently" throws an exception - -If any code in the getter throws an unhandled exception then the binding won't work because the value cannot be read. -If a setter throws an unhandled exception before the firing the property change then the listeners will not receive the change signal. Both result in the binding being broken. - -If a binding seems to work intermittently then there might be something in the getter or setter causing this, e.g. an object used in a getter that switches between being null and initialised based on something else. - -**The exceptions will appear in the console inside Eclipse and IBEX but won't cause an error pop-up to appear.** - -### The binding works but the initial value is not put in the widget - -When the binding first happens it will call the getter to set the widget properties to whatever is in the model. If this doesn't happen the getter is probably non-existent or misspent. \ No newline at end of file diff --git a/doc/client/coding/Databinding.md b/doc/client/coding/Databinding.md index 515a8500c..f05ffcd8b 100644 --- a/doc/client/coding/Databinding.md +++ b/doc/client/coding/Databinding.md @@ -1,387 +1,163 @@ # Databinding -Note: there are some helpful tips to common errors/mistakes [here](Databinding---common-mistakes). +:::{seealso} +- [How to connect a view to a PV](Connecting-a-View-to-a-PV) for a higher-level description of how to pass data from an EPICS +PV through backend classes (model & view model), and into a UI element. +- [The Eclipse data binding documentation](https://github.com/eclipse-platform/eclipse.platform.ui/blob/master/docs/JFaceDataBinding.md) +has more detailed explanations about advanced data binding options. +::: -Note: an explanation of how to connect a view to a PV is [here](Connecting-a-View-to-a-PV) +## Backend -## Create the example plug-ins ## +The `ModelObject` class is a central helper for data binding in the IBEX GUI, which is implemented by backend classes. +It provides the following key methods: -First create an Eclipse plug-in for the back-end: +### `firePropertyChange` -* Start the plug-in wizard via File->New->Plug-in Project +The `firePropertyChange` method is used to inform the listeners when the value has changed. A typical implementation of +a getter and setter in a backend class uses `firePropertyChange` as follows: -* On the first page of the wizard enter `org.databinding.example.backend` as the name - -* On the second page: - - * Deselect "Generate an activator, a Java class that controls the plug-in's life cycle" - * Deselect "This plug-in will make contributions to the UI" - * Select "No" for "Would you like to create a 3.x rich client application?" - * Click "Finish" - -Next create a plug-in for the UI side: - -* Start the plug-in wizard via File->New->Plug-in Project - -* On the first page of the wizard enter `org.databinding.example.frontend` as the name - -* On the second page: - - * Select "Generate an activator, a Java class that controls the plug-in's life cycle" - * Select "This plug-in will make contributions to the UI" - * Select "Yes" for "Would you like to create a 3.x rich client application?" - * Click "Next" - * Select "RCP application with a view" - * Click "Finish" - -## Creating the Person class (the model) - -In the backend plug-in create two new classes, one called `ModelObject` and one called Person. -Enter the following code for the `ModelObject`: ```java -package org.databinding.example.backend; - -import java.beans.PropertyChangeListener; -import java.beans.PropertyChangeSupport; - -public class ModelObject { - private final PropertyChangeSupport changeSupport = new PropertyChangeSupport(this); + private String foo; - public void addPropertyChangeListener(PropertyChangeListener listener) { - changeSupport.addPropertyChangeListener(listener); + public String getFoo() { + return this.foo; } - public void removePropertyChangeListener(PropertyChangeListener listener) { - changeSupport.removePropertyChangeListener(listener); + public void setFoo(String value) { + firePropertyChange("foo", this.foo, this.foo = value); } - - protected void firePropertyChange(String propertyName, Object oldValue, - Object newValue) { - changeSupport.firePropertyChange(propertyName, oldValue, newValue); - } -} ``` -The ModelObject contains the code that is essential for data-binding. -Behind the scenes when you create a binding the data-binding framework automatically calls `addPropertyChangeListener` for the UI widget being bound -(not strictly true - it is a bean object that is associated with the widget). The `removePropertyChangeListener` is called when the widget binding is disposed, e.g. when the GUI is closed. +If `oldValue` and `newValue` are equal and non-null, then the event is not raised. +If you want a change to always fire, even if `oldValue` and `newValue` are the same, pass `null` as the `oldValue` to +`firePropertyChange`. -The `firePropertyChange` method is used to inform the listeners when the value has changed. -Note: if `oldValue` and `newValue` are the same then the event is not raised. +Not all listeners will use the `newValue` provided to the change event; some listeners will instead only use the event +as a trigger to call the explicit getter. -The Person class is the class that is going to be used as the model the UI binds to. Add the following code: -```java -package org.databinding.example.backend; +### `addPropertyChangeListener` & `removePropertyChangeListener` -public class Person extends ModelObject { - private String name; - private int age; - private String idNumber; +The data-binding framework automatically calls `addPropertyChangeListener` when a UI widget being bound. +The `removePropertyChangeListener` is called when the widget binding is disposed, for example when the GUI is closed. - public String getName() { - return name; - } +## Frontend - public void setName(String name) { - firePropertyChange("name", this.name, this.name = name); - } +A typical frontend class binds to its view model, in a constructor, like: - public int getAge() { - return age; - } - - public void setAge(int age) { - firePropertyChange("age", this.age, this.age = age); - } +```java + Label lblFoo = new Label(this, SWT.NONE); - public String getIdNumber() { - return idNumber; - } + DataBindingContext bindingContext = new DataBindingContext(); - public void setIdNumber(String id) { - firePropertyChange("idNumber", this.idNumber, this.idNumber = id); - } - - public Person(String name, int age) { - setName(name); - setAge(age); - setIdNumber("0000"); - } -} + bindingContext.bindValue(WidgetProperties.text().observe(lblFoo), + BeanProperties.value("foo").observe(viewModel)); ``` -Whenever a setter is called the `firePropertyChange` is called which informs all the listeners that the value has changed. -Other than that it is a pretty standard Java class. - -## Basic data-binding -In the frontend plug-in's MANIFEST.MF add the backend plug-in to the list of dependencies. Also, add the following dependencies: +For a control that a user can write to, use `WidgetProperties.text(SWT.Modify)`. This will notify the model whenever +the user input in a control changes. -* `org.eclipse.core.databinding` -* `org.eclipse.core.databinding.beans` -* `org.eclipse.core.databinding.property` -* `org.eclipse.jface.databinding` +:::{important} +Create the binding context on the thread which should receive events. For UI code, this is the +UI thread - so the binding context is normally created in the view's constructor, alongside instantiating UI elements. +**Do not create the binding context in a static initializer.** +::: -Open the View class and change it to the following: -```java -package org.databinding.example.frontend; - -import org.databinding.example.backend.Person; -import org.eclipse.swt.SWT; -import org.eclipse.swt.layout.GridData; -import org.eclipse.swt.layout.GridLayout; -import org.eclipse.swt.widgets.Composite; -import org.eclipse.swt.widgets.Label; -import org.eclipse.swt.widgets.Text; -import org.eclipse.ui.part.ViewPart; -import org.eclipse.core.databinding.Binding; -import org.eclipse.core.databinding.DataBindingContext; -import org.eclipse.core.databinding.observable.value.IObservableValue; -import org.eclipse.jface.databinding.fieldassist.ControlDecorationSupport; -import org.eclipse.jface.databinding.swt.WidgetProperties; -import org.eclipse.core.databinding.beans.BeanProperties; - -public class View extends ViewPart { - public static final String ID = "org.databinding.example.frontend.view"; - - private Person person; - - public View() { - person = new Person("John", 25); - } - - public void createPartControl(Composite parent) { - GridLayout glParent = new GridLayout(2, false); - glParent.marginHeight = 5; - glParent.marginWidth = 5; - glParent.horizontalSpacing = 1; - glParent.verticalSpacing = 0; - parent.setLayout(glParent); - - Label lblName = new Label(parent, SWT.NONE); - lblName.setText("Name: "); - lblName.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, false, false, 1, 1)); - - Text txtName = new Text(parent, SWT.BORDER); - GridData gdText = new GridData(SWT.LEFT, SWT.CENTER, true, false, 1, 1); - gdText.minimumWidth = 100; - txtName.setLayoutData(gdText); - - Text txtReadName = new Text(parent, SWT.BORDER); - GridData gdReadText = new GridData(SWT.LEFT, SWT.CENTER, true, false, 1, 1); - gdReadText.minimumWidth = 100; - txtReadName.setLayoutData(gdReadText); - txtReadName.setEditable(false); - - Label lblAge = new Label(parent, SWT.NONE); - lblAge.setText("Age: "); - lblAge.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, false, false, 1, 1)); - - Text txtAge = new Text(parent, SWT.BORDER); - GridData gdAge = new GridData(SWT.LEFT, SWT.CENTER, true, false, 1, 1); - gdAge.minimumWidth = 100; - txtAge.setLayoutData(gdAge); - - // DATABINDING START - DataBindingContext ctx = new DataBindingContext(); - - IObservableValue target = WidgetProperties.text(SWT.Modify).observe(txtName); - IObservableValue model = BeanProperties.value("name").observe(person); - ctx.bindValue(target, model); - - // Bind on one line - ctx.bindValue(WidgetProperties.text(SWT.Modify) - .observe(txtReadName), BeanProperties.value("Name").observe(person)); - - // Use default validation - binding code on one line - Binding bindValue = ctx.bindValue(WidgetProperties.text(SWT.Modify) - .observe(txtAge), BeanProperties.value("age").observe(person)); - - ControlDecorationSupport.create(bindValue, SWT.TOP | SWT.RIGHT); - // DATABINDING END - } +## Update strategies - public void setFocus() { +The call to `bindValue` accepts two optional extra arguments, which are update strategies. These allow extra steps to +be performed during the databinding operation, for example validation or conversion. - } -} +In a call like: +```java +UpdateValueStrategy strategy1 = new UpdateValueStrategy(); +UpdateValueStrategy strategy2 = new UpdateValueStrategy(); +bindingContext.bindValue(target, model, strategy1, strategy2); ``` - -The binding of the name is a simple as it gets; an `IObservableValue` is created for both the widget and the name in the Person class -and these are bound together in the `DataBindingContext` object. - -For age, because it is an int it is possible to make the widget show an error message when is contains a value that is not an int -using `ControlDecorationSupport` as shown. - -If the UI is started then it is possible to see the data-binding in action, by using the debugger it is possible to see that the -`setName` method is called every time a change is made to the widget text, i.e. every time a letter is added or removed. +The first strategy is used for data going from the target (UI widget) towards the model; the second strategy is used +for data coming from the model towards the target (UI widget). -## Validators +### Validators -In the previous example, a warning was shown when the age entered was invalid - this is basic validation, it might be that more advanced validation is desired. -For example, it might that we want to ensure a string is only made up of digits. +It is possible to implement validators, which verify that a value is "valid" before passing it onto the model. +Validators implement the `org.eclipse.core.databinding.validation.IValidator` interface and are assigned to an update +strategy: -Create a new class called `NumbersOnlyValidator` and add the following code: ```java -package org.databinding.example.frontend; - -import java.util.regex.Pattern; - -import org.eclipse.core.databinding.validation.IValidator; -import org.eclipse.core.databinding.validation.ValidationStatus; -import org.eclipse.core.runtime.IStatus; - -public class NumbersOnlyValidator implements IValidator { - - private final Pattern numbersOnly = Pattern.compile("\\d*"); - - public IStatus validate(Object value) { - if (value != null && numbersOnly.matcher(value.toString()).matches()) { - return ValidationStatus.ok(); - } - return ValidationStatus.error(value + " contains a non-numeric!"); - } -} +UpdateValueStrategy strategy = new UpdateValueStrategy(); +strategy.setBeforeSetValidator(new MyValidator()); ``` -All custom validators implement `IValidator`. In this example, pattern matching is used to check that the value (in this case a string) contains only digits. -Otherwise it returns an error status. -Now modify View by adding the following directly before DATABINDING END: -```java -Label lblId = new Label(parent, SWT.NONE); -lblId.setText("ID: "); -lblId.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, false, false, 1, 1)); +[The `uk.ac.stfc.isis.ibex.validators` package](https://github.com/ISISComputingGroup/ibex_gui/tree/master/base/uk.ac.stfc.isis.ibex.validators/src/uk/ac/stfc/isis/ibex/validators) +in the GUI contains commonly-used data binding validators which may be used as reference implementations. -Text txtId = new Text(parent, SWT.BORDER); -GridData gdId = new GridData(SWT.LEFT, SWT.CENTER, true, false, 1, 1); -gdId.minimumWidth = 100; -txtId.setLayoutData(gdId); +### Converters -IValidator validator = new NumbersOnlyValidator(); +Converters are similar to validators in that they are used as part of the update strategy; however, they are used to +convert either the data from the model or the data sent to the model by the widget. They implement the +`org.eclipse.core.databinding.conversion.Converter` interface. Similar to validators, they are attached to an update +strategy: +```java UpdateValueStrategy strategy = new UpdateValueStrategy(); -strategy.setBeforeSetValidator(validator); -Binding bindValue2 = ctx.bindValue(WidgetProperties.text(SWT.Modify) - .observe(txtId), - BeanProperties.value("idNumber").observe(person), strategy, - null); -ControlDecorationSupport.create(bindValue2, SWT.TOP | SWT.RIGHT); - +strategy.setConverter(new MyConverter()); ``` -This code creates a label and text-box for the ID field of the Person class. -An instance of `NumbersOnlyValidator` is created and added to a `UpdateValueStrategy` object. -The `UpdateValueStrategy` object defines how a data-binding responses to value changes - in this case it calls the validator before setting the value. -The strategy is added as the third parameter of the `bindValue` method call, this is because the third parameter represents the strategy for updating the model based on the widget. -The fourth parameter is null because there is currently no update strategy for updating the widget based on the model (the default is used). -By running this example it can be seen that a warning appears if a non-numeric character is entered for the ID. +## Databinding lists -## Converters +Most of this is taken care of by the databinding library via `ListViewer` and `ObservableListContentProvider`: -Converters are similar to validators in that they are used as part of the update strategy; however, they are used to convert either the data from the model or the data send to the model by the widget. -In this example, two converters are created: one to convert the name to upper-case; and, one to convert to lower-case. - -Create two new classes called `ToLowerConverter` and `ToUpperConverter`. - -Add the following code to `ToLowerConverter`: ```java -package org.databinding.example.frontend; - -import org.eclipse.core.databinding.conversion.Converter; - -public class ToLowerConverter extends Converter { - - public ToLowerConverter() { - super(String.class, String.class); - } +ListViewer myViewer = new ListViewer(parent, SWT.BORDER | SWT.V_SCROLL | SWT.SINGLE); +ObservableListContentProvider contentProvider = new ObservableListContentProvider(); +myViewer.setContentProvider(contentProvider); +myViewer.setInput(BeanProperties.list("names").observe(myViewModel)); - public Object convert(Object fromObject) { - return fromObject.toString().toLowerCase(); - } -} +// To get the List Control itself +org.eclipse.swt.widget.List myList = myViewer.getList(); ``` - -Likewise for ToUpperConverter: -```java -package org.databinding.example.frontend; -import org.eclipse.core.databinding.conversion.Converter; +`ListViewer` is a wrapper around the List control that provides extra functionality and `ObservableListContentProvider` +makes the databinding work. -public class ToUpperConverter extends Converter { +## Troubleshooting - public ToUpperConverter() { - super(String.class, String.class); - } +### Missing or incorrectly named getter or setter (or incorrectly named) - public Object convert(Object fromObject) { - return fromObject.toString().toUpperCase(); - } -} +Behind the scenes the databinding library automatically changes the first character to upper-case and then prepends +"get" and "set". -``` -Now replace the code between the DATABINDING START and the DATABINDING FINISH with: +With something like the following it is import that the name of the property to bind to is cased correctly: ```java -DataBindingContext ctx = new DataBindingContext(); - -UpdateValueStrategy strategy1 = new UpdateValueStrategy(); -strategy1.setConverter(new ToLowerConverter()); - -UpdateValueStrategy strategy2 = new UpdateValueStrategy(); -strategy2.setConverter(new ToUpperConverter()); - -IObservableValue target = WidgetProperties.text(SWT.Modify).observe(txtName); -IObservableValue model = BeanProperties.value("name").observe(person); - -// First strategy goes towards the model, second goes towards the widget (target) -ctx.bindValue(target, model, strategy1, strategy2); - +bindingContext.bindValue(WidgetProperties.text(SWT.Modify) + .observe(txtAge), BeanProperties.value("age").observe(person)); ``` -Note: The data-binding for age has been deleted, so the age text-box will be empty when this is run. -Running this example should show the name in upper-case and it ran through the debugger it can be seen that if the value is modified the `setName` method in Person receives a lower-case string. +This assumes there is a getter and setter called `getAge` and `setAge`. -## Update strategies - -There are other things that can be done with update strategies, in this example the model is only updated when the update button is clicked (unlike in the first example where the person's name changes each time it is modified). +For something like `getFedId` the binding code would look like: -Replace the code between the DATABINDING START and the DATABINDING FINISH with: ```java -final DataBindingContext ctx = new DataBindingContext(); - -UpdateValueStrategy strategyOnPress = new UpdateValueStrategy(UpdateValueStrategy.POLICY_ON_REQUEST); - -IObservableValue target = WidgetProperties.text(SWT.Modify).observe(txtName); -IObservableValue model = BeanProperties.value("name").observe(person); -ctx.bindValue(target, model, strategyOnPress, null); - -Button btnUpdate = new Button(parent, SWT.NONE); -btnUpdate.setText("Update"); -txtAge.setLayoutData(new GridData(SWT.LEFT, SWT.CENTER, true, false, 1, 1)); - -btnUpdate.addSelectionListener(new SelectionAdapter() { - @Override - public void widgetSelected(SelectionEvent arg0) { - ctx.updateModels(); - } -}); - +bindingContext.bindValue(WidgetProperties.text(SWT.Modify) + .observe(txtId), BeanProperties.value("fedId").observe(person)); ``` -In this example, the Update strategy is initialised with POLICY_ON_REQUEST. This means the updates are only send to the model (the Person object) when told to. -The models are updated when `updateModels` is called by the `DataBindingContext` which in this example is on a button click. -Running this example in the debugger shows that the `setName` method is only called when the button is clicked rather than on every modification. +:::{important} +The 'f' of `fedId` is lower-case in this example. It will not work if it is upper-case. +::: -## Binding a Java List to a List Control +### The getter or setter "silently" throws an exception -Most of this is taken care of by the databinding library via `ListViewer` and `ObservableListContentProvider`. Here is an example of it in action: +If any code in the getter throws an unhandled exception then the binding won't work because the value cannot be read. +If a setter throws an unhandled exception before the firing the property change then the listeners will not receive the +change signal. Both result in the binding being broken. -```java -ListViewer myViewer = new ListViewer(parent, SWT.BORDER | SWT.V_SCROLL | SWT.SINGLE); -ObservableListContentProvider contentProvider = new ObservableListContentProvider(); -myViewer.setContentProvider(contentProvider); -myViewer.setInput(BeanProperties.list("names").observe(myViewModel)); +If a binding seems to work intermittently then there might be something in the getter or setter causing this, e.g. an +object used in a getter that switches between being null and initialised based on something else. -// To get the List Control itself - it is org.eclipse.swt.widget.List not a standard Java List -List myList = myViewer.getList(); -``` +The exceptions will appear in the console inside Eclipse and IBEX, but won't cause an error pop-up to appear. + +### The binding works but the initial value is not put in the widget -`ListViewer` is a wrapper around the List control that provides extra functionality and `ObservableListContentProvider` make the databinding work. \ No newline at end of file +When the binding first happens it will call the getter to set the widget properties to whatever is in the model. +If this doesn't happen, the getter is probably non-existent or not implemented correctly. diff --git a/doc/client/coding/GUI-Coding-Conventions.md b/doc/client/coding/GUI-Coding-Conventions.md index e9a4e11e2..92a8de3b1 100644 --- a/doc/client/coding/GUI-Coding-Conventions.md +++ b/doc/client/coding/GUI-Coding-Conventions.md @@ -1,14 +1,15 @@ -# Conventions +# GUI Coding Conventions -Contains the style and coding conventions for the IBEX GUI. +:::{note} +New conventions should not be added to this document without first being discussed at a +[code chat](/processes/meetings/Code-Chats-and-Lightning-Talks) or a group code-review session. +::: -**New conventions should not be added to this document without first being discussed at a 'Code Chat'.** +## Style Conventions -## Style Conventions # +Unless stated otherwise below, we should follow the standard Java conventions for style where possible. -Unless stated otherwise below we should follow the standard Java conventions for style where possible. - -## Code documentation ## +## Documentation Classes and public methods should be documented using the Javadoc syntax. For example: @@ -46,7 +47,7 @@ public class CustomPizza extends Pizza { } } ``` -For Getters it is okay to skip the first sentence and do the following as it saves repetition: +For getters, it is acceptable to skip the first sentence and do the following as it saves repetition: ```java /** * @return true if the block is enabled @@ -56,9 +57,16 @@ public boolean getEnabled() { } ``` -## Code formatting ## +Inline comments should have a space between the // and the text, and start with a capital letter: +```java +// This is a good comment + +//this is a bad comment +``` -For Java use the standard conventions built in to the IBEX developer's version of Eclipse. +## Code formatting + +For Java use the standard conventions built in to Eclipse. An example of what it looks like: ```java @@ -78,58 +86,18 @@ void foo2() { ``` In Eclipse, a quick way to auto-format the code correctly is to use Ctrl+Shift+F. -## Code comments ## - -Comments should have a space between the // and the text, and start with a capital letter: -```java -// This is a good comment - -//this is a bad comment -``` - -## Checkstyle ## - -Code should be run through Checkstyle via Eclipse and corrected (within reason) before being committed. -The Checkstyle plug-in is installed as part of the IBEX developer's version of Eclipse. - -The Checkstyle configuration file for Eclipse is more picky that the one used on Jenkins as it warns about 'magic numbers' and 'Java-docs'. - -By right-clicking on a file one can tell Checkstyle to check that file. +## Naming conventions -Warnings must be fixed where possible. +### General -Checkstyle has a suppress warning flag that tells it to ignore certain warning; warnings that are allowed to be suppressed are: +We use the [standard Java naming conventions](https://www.oracle.com/java/technologies/javase/codeconventions-namingconventions.html): +- Class names are CamelCase and usually nouns, e.g. `FileReader` not `ReadsFile` +- Method names are Mixed Case (also known as Lower CamelCase), e.g. `sayHello` +- Package names are lowercase, e.g. `uk.ac.stfc.isis.dae` +- Variable names are Mixed Case, e.g. `calculateMeanSpeed` +- Constants are uppercase spaced by underscores, e.g. `SECONDS_PER_FORTNIGHT` -* Magic numbers - if it is related to a GUI layout then suppress them. - -* Name must match pattern - ignore GUI names that don't match the recommended pattern (e.g. `gd_switchToCombo`) - -Suppression example: - -```java -@SuppressWarnings({ "checkstyle:magicnumber", "checkstyle:localvariablename" }) -public void getSecondsInHours(int hours) { - int seconds_per_hour = 60 * 60; // Magic numbers and a bad variable name - return hours * seconds_per_hour; -} -``` -## Naming conventions ## - -### General ### - -In general we use the standard Java naming conventions, e.g.: - -* Class names are CamelCase and usually nouns, e.g. `FileReader` not `ReadsFile` - -* Method names are Mixed Case (also known as Lower CamelCase), e.g. `sayHello` - -* Package names are lowercase, e.g. `uk.ac.stfc.isis.dae` - -* Variable names are Mixed Case, e.g. `calculateMeanSpeed` - -* Constants are uppercase spaced by underscores, e.g. `SECONDS_PER_FORTNIGHT` - -### Getters and Setters ### +### Getters and Setters Getters and Setters should follow the JavaBeans convention, namely: @@ -163,45 +131,34 @@ class Point { } ``` -## Coding Conventions ## +## Coding Conventions -A mix of IBEX specific and general Java coding conventions. - -### GUI code must use a View Model ### +### GUI code should use a model-view-viewmodel (MVVM) pattern This maintains a separation between pure GUI code and the back-end. It also makes it easier for us to test our code. -See the previous GUI Chat slides for more information. - -Some legacy code does not have a View Model, this is on the list to fix. +See the [previous GUI Chat slides](/processes/meetings/Code-Chats-and-Lightning-Talks) for more information. -### Use data-binding for GUI items ### +### Use data-binding for GUI items -~~For connecting UI elements to data from the View Model use data-binding. -It seems that if a mix of data-binding and more traditional SWT wiring up is used (e.g. AddPropertyChangeListener) then the data-binding will stop working*, so always using data-binding should avoid this problem.~~ +For connecting UI elements to data from the View Model, prefer to use [databinding](Databinding) where possible. -~~*This does need more investigation to find out why it occurs.~~ +### Don't mess with finalizers -This no longer seems to apply. - -### Don't mess with finalizers ### -From the Google Java Style Guide: - -``` -It is extremely rare to override Object.finalize. - -Tip: Don't do it. If you absolutely must, first read and understand Effective Java Item 7, "Avoid Finalizers," very carefully, and then don't do it. -``` - -As of Java 9, `finalize` is officially deprecated in java. The IBEX checkstyle configuration is configured to disallow finalizers - this check should not be disabled or overridden. +In Java versions >=9, `finalize` is deprecated and marked for removal from Java. The IBEX checkstyle configuration +is configured to disallow finalizers - this check must not be disabled or overridden. The remaining options for supported clean-up mechanisms (in preference order) are: -- Implement `autocloseable` and use the class in a try-with-resources statement to ensure the relevant resource is closed -- Use a `closeable` and manually call `close` to ensure the relevant resource is closed +- Implement `AutoCloseable` and use the class in a try-with-resources statement to ensure the relevant resource is closed +- Use a `Closeable` and manually call `close` to ensure the relevant resource is closed - Refactor to avoid needing to close the resource at all -- Use a *supported* automatic clean-up mechanism such as `PhantomReference` or `Cleaner` only as a last resort. While these are *better* than finalizers, they still suffer from high complexity and are tricky to get right. In particular it is easy to accidentally create reference loops meaning that objects will never be cleaned. +- Use a *supported* automatic clean-up mechanism such as `PhantomReference` or `Cleaner` only as a last resort. +While these are *better* than finalizers, they still suffer from high complexity and are tricky to get right. +In particular, it is easy to accidentally create reference loops meaning that objects will never be cleaned. + +### Return a empty collection or stream, not null -### Return a empty collection or stream, not null ### -For methods that return arrays/lists/maps/sets/streams etc. don't return null. It is cleaner to return an empty instance as the calling code does not need to check for null. +For methods that return arrays/lists/maps/sets/streams etc. don't return null. It is cleaner to return an empty +instance as the calling code does not need to check for null. ```java // Avoids this extra check in the caller @@ -211,14 +168,18 @@ if (cars != null && cars.contains("mustang") { ``` See Effective Java Item 43 "Return empty arrays or collections, not nulls" -### Prefer StringBuilder for string concatenation ### -Using `+` is fine for, say, joining two or three short strings but it is inefficient for larger numbers of strings and longer strings. Use StringBuilder instead. +### Prefer StringBuilder for string concatenation + +Using `+` is fine for, say, joining two or three short strings, but it is inefficient for larger numbers of strings and +longer strings. Use `StringBuilder` instead. See Effective Java Item 51 "Beware the performance of string concatenation" ### Prefer `Optional` over `null` -New APIs should not return null to indicate a missing value. Instead, return an Optional wrapping the value which may not exist. Where external code returns null to indicate a missing value, this should be wrapped in an optional as soon as reasonable. +New APIs should not return null to indicate a missing value. Instead, return an `Optional` wrapping the value which may +not exist. Where external code returns null to indicate a missing value, this should be wrapped in an optional as soon +as reasonable. To convert a maybe-null value to an optional, use: ```java @@ -236,7 +197,7 @@ String str = mightNotExist.orElse(null); Streams should be used where they make an algorithm clearer. -When using streams, put each operation on it's own line. +When using streams, put each operation on its own line. Good: ```java @@ -254,5 +215,3 @@ public Stream getNames() { return getThings().map(thing -> thing.getName()).filter(name -> name != "").sorted(); } ``` - - diff --git a/doc/client/coding/IBEX_complete_DAE_readonly.png b/doc/client/coding/IBEX_complete_DAE_readonly.png deleted file mode 100644 index 6a7c9117f..000000000 Binary files a/doc/client/coding/IBEX_complete_DAE_readonly.png and /dev/null differ diff --git a/doc/client/coding/IBEX_complete_clean.png b/doc/client/coding/IBEX_complete_clean.png deleted file mode 100644 index e6c9c0fe7..000000000 Binary files a/doc/client/coding/IBEX_complete_clean.png and /dev/null differ diff --git a/doc/client/coding/IBEX_complete_perspective_switcher_highlighted.png b/doc/client/coding/IBEX_complete_perspective_switcher_highlighted.png deleted file mode 100644 index e9e857a22..000000000 Binary files a/doc/client/coding/IBEX_complete_perspective_switcher_highlighted.png and /dev/null differ diff --git a/doc/client/coding/Instrument-switching.md b/doc/client/coding/Instrument-switching.md deleted file mode 100644 index 710b68bba..000000000 --- a/doc/client/coding/Instrument-switching.md +++ /dev/null @@ -1,15 +0,0 @@ -# Instrument switching - -Instrument switching in the GUI uses an extension point. This means that the switch can take place in a central place but then each plugin which is interested can sign up to the switching event. This keeps a separation between the plugins and the instrument switching module; i.e. a plugin can be removed without changing the instrument switching code. - -The instrument switching in E4 is performed through the E3 compatibility layer, as E4 has no native support for extension points. The equivalent behaviour in E4 is provided through services, which might be a necessary transition as services and extensions points are not cross-compatible. - -This extension point is setup in `uk.ac.stfc.isis.ibex.instrument/META-INF/MANIFEST.MF` (see the extension Points tab). This sets up the name of the extension point and the schema. The schema is in `/uk.ac.stfc.isis.ibex.instrument/schema/uk.ac.stfc.isis.ibex.instrument.info.exsd` (click Schema on previous page). This defines the methods that should be fulfilled by the plugin that want to sign up to this extension. In this case there are three methods: - - `preSetInstrument` - this will be called before the instrument is switched. This is useful for closing resources using the old instrument. - `setInstrument` - this will be called to set the instrument and should actually perform the change. - `postSetInstrument` - this will be called after the instrument is set. It can be used, for example, to perform final clean up of resources or reopening of perspectives. - -The instrument handler is responsible for calling this method on each registered plugin. It does this in the method `uk.ac.stfc.isis.ibex.instrument.Instrument.setInstrumentForAllPlugins` in the private method `updateExtendingPlugins`. Notice that it call all preSetInstruments methods before setInstrument. - -To sign up to this event the plugin must create a class which implements the interface `uk.ac.stfc.isis.ibex.instrument.InstrumentInfoReceiver`. This will be instantiated when the instrument is switched to. This class is registered in the plugin's extensions (this is similar to the perspectives see [adding a perspective](Adding-a-Button-to-the-Perspective-Switcher) ). diff --git a/doc/client/coding/Migrating-or-adding-a-button-to-the-E4-perspective-switcher.md b/doc/client/coding/Migrating-or-adding-a-button-to-the-E4-perspective-switcher.md deleted file mode 100644 index d2210c75e..000000000 --- a/doc/client/coding/Migrating-or-adding-a-button-to-the-E4-perspective-switcher.md +++ /dev/null @@ -1,38 +0,0 @@ -# Adding a button to the E4 Perspective switcher - -## Migrating an E3 perspective to E4 - -1. Open the Application.e4xmi from `uk.ac.stfc.isis.ibex.e4.client` -1. Go to `Snippets` -1. Click `Add` to add a new perspective -1. Set the perspective up using an existing migrated perspective as a template - 1. Set a sensible ID - 1. Give it a label - 1. If you want your perspective to be invisible toggle the visible checkbox - 1. Set the icon - 1. Add controls. This should be a hierarchy of part sash containers. You can see how it should be set up from the existing perspectives. Don't forget to set the container data where appropriate; it sets the relative size of sibling components. -1. Add the perspective-specific parts - 1. In the alarms perspective, you'll see one part in the final part sash container called alarms. Do the same thing in your new perspective, but give it an appropriate name - 1. Change the ID of your new part to the ID of the view class you want the perspective to open -1. Add the dependency of the view you've added to the `plugin.xml` in the `...e4.client` plugin -1. Add the new dependency to `...feature.xml` (in `uk.ac.stfc.isis.ibex.feature.base`) -1. Synchronize `ibex.product` (in `...e4.client.product`) -1. Open IBEX -1. Check the new perspective scales appropriately and change the layout accordingly if needed - -## Creating a brand new E4 perspective - -Making a brand new E4 perspective would probably look similar to the steps above, minus the E3 steps. However, a new E4 perspective has yet to been attempted. - -## Hiding Perspectives - -Perspectives can be hidden by adding perspective IDs to the Eclipse preference store at the preference key `uk.ac.stfc.isis.ibex.preferences/perspectives_not_shown` (at `/uk.ac.stfc.isis.ibex.e4.client/plugin_customization.ini`). e.g. - -``` -uk.ac.stfc.isis.ibex.preferences/perspectives_not_shown=uk.ac.stfc.isis.ibex.client.e4.product.perspective.scriptGenerator -``` -Note: Multiple perspectives can be hidden using a comma separated list of perspective IDs. e.g. - -``` -uk.ac.stfc.isis.ibex.preferences/perspectives_not_shown=uk.ac.stfc.isis.ibex.client.e4.product.perspective.scriptGenerator,uk.ac.stfc.isis.ibex.client.e4.product.perspective.dae -``` \ No newline at end of file diff --git a/doc/client/coding/PV-Switching.md b/doc/client/coding/PV-Switching.md index 1e0cde652..8926cc352 100644 --- a/doc/client/coding/PV-Switching.md +++ b/doc/client/coding/PV-Switching.md @@ -1,49 +1,80 @@ -# PV switching +# Instrument & PV switching -If you are only interested in how to create PVs in IBEX with proper switching behaviour go to "Using the PV Switching". +To create a PV in the GUI which switches instrument, or closes, on instrument switch, use: -## Background ## - -PV switching occurs when a user in IBEX changes from viewing one instrument to another. It is then required that PVs 'switch' prefix and instead of, for example, looking at IN:LARMOR:CS:BLOCKSERVER:CONFIGS the GUI will now point to IN:DEMO:CS:BLOCKSERVER:CONFIGS. This means that information about the new instrument will be displayed. - -This was originally done in IBEX by maintaining a PVAddressBook class that kept a map of all PV-connected observables that had been registered with it and reset their prefixes when the instrument was changed. This created a number of issues: - -* Some PVs might not need to be switched (e.g. the beam status), this was solved by bypassing the `PVAddressBook` and creating PVs in other plugins +```java +ObservableFactory closingObsFactory = new ObservableFactory(OnInstrumentSwitch.CLOSE); +ForwardingObservable pv + = closingObsFactory.getSwitchableObservable(new StringChannel(), "A_PV_ADDRESS")); +``` -* There was no way to remove PVs from this address book, which led to an ever increasing map, and subsequent channel access connection attempts, when configurations or instruments were changed +Subscriptions can then be attached to the PV and will be called when PVs change value. -* Some PVs will need to be closed all together when instruments are switched, such as those relating to specific configurations or synoptics. There was no mechanism for this to happen within the PVAddressBook +## Design -* PV creation was messy, requiring inheriting from specific classes or some knowledge of how the underlying PVs were created +### Extension point -This was further complicated by the large number of Observable, Observer, Writable and Writer classes that were used to observe PVs in different situations. +Instrument switching in the GUI uses an extension point. This means that the switch can take place in a central place, +and each plugin which is interested can sign up to receive a callback on the switching event. +This keeps a separation between the plugins and the instrument switching module; a plugin can be removed without +changing the instrument switching code. -## Solution Design +The instrument switching in E4 is performed through the E3 compatibility layer, as E4 has no native support for +extension points. +The equivalent behaviour in E4 is provided through services, which might be a necessary transition +as services and extensions points are not cross-compatible. -The proposed solution was to remove the central PVAddressBook class and instead use a factory for creating PV Observables, which are passed the switching behaviour as a switcher class. Each of these switcher classes provides a different switching functionality and is switched using the extension point that is globally used for instrument switching in the GUI. +This extension point is setup in `uk.ac.stfc.isis.ibex.instrument/META-INF/MANIFEST.MF` (see the extension Points tab). +This sets up the name of the extension point and the schema. +The schema is in `/uk.ac.stfc.isis.ibex.instrument/schema/uk.ac.stfc.isis.ibex.instrument.info.exsd` +(click Schema on previous page). +This defines the methods that should be fulfilled by the plugin that want to sign up to this extension. -![Switching](new_switching.jpg) - -1. When an observable PV is required an instance of ObservableFactory is first created and passed an OnInstrumentSwitch enum to describe which switching behaviour is required +In this case there are three methods: +- `preSetInstrument` - this will be called before the instrument is switched. This is useful for closing resources +using the old instrument. +- `setInstrument` - this will be called to set the instrument and should actually perform the change. +- `postSetInstrument` - this will be called after the instrument is set. It can be used, for example, to perform final +clean up of resources or reopening of perspectives. -1. On initialisation the factory will then query the InstrumentSwitchers class for the specific Switcher class that handles that type of switching +The instrument handler is responsible for calling this method on each registered plugin. +It does this in the method +`uk.ac.stfc.isis.ibex.instrument.Instrument.setInstrumentForAllPlugins` in the private method `updateExtendingPlugins`. +All `preSetInstruments` methods are called before any `setInstrument` methods. -1. The original class that wanted the PV will subsequently ask the factory for PVs, providing the channel type and PV address. This will be provided as a `SwitchableObservable` that can be subscribed to for value changes. +To sign up to this event, the plugin must create a class which implements the interface +`uk.ac.stfc.isis.ibex.instrument.InstrumentInfoReceiver`. +This will be instantiated when the instrument is switched to. +This class is registered in the plugin's extensions (this is similar to +[adding a perspective](Perspectives)). -1. Before providing the new Observable object the factory will register it with the relevant Switcher class, each of which holds a list of all `Switchable` objects that it is required to switch. The `SwitchableObservable` is also provided with a reference to the switcher so that it can remove itself from the relevant list if it is closed for any reason. +### Observables -1. When the instrument is changed the InstrumentSwitchers class will call the `switchInstruments` method on each of the switchers, which will then go on to perform the relevant switching behaviour. E.g. Changing a `Switchable`'s source, closing it or doing nothing. +The design of switchable PVs in the GUI uses a factory to create PV Observables, which are passed the switching +behaviour as a switcher class. Each of these switcher classes provides a different switching functionality and is +switched using the Eclipse extension point that is globally used for instrument switching in the GUI. -A similar process also occurs when switching writable PVs, as can be seen in the UML diagram below. The differences being that a `WritableFactory` is used, this can create a Writable that inherits from `Switchable` and can write values to PVs. Both the `Switchable` interface and the abstract `PrefixChangingSwitcher` were created so that the switching process is as similar as possible when dealing with `Writables` and `Observables` +![Switching](new_switching.jpg) + +- When an observable PV is required an instance of `ObservableFactory` is first created and passed an +`OnInstrumentSwitch` enum to describe which switching behaviour is required +- On initialisation the factory will then query the `InstrumentSwitchers` class for the specific Switcher class that +handles that type of switching. +- The original class that wanted the PV will subsequently ask the factory for PVs, providing the channel type and PV +address. This will be provided as a `SwitchableObservable` that can be subscribed to for value changes. +- Before providing the new `Observable` object the factory will register it with the relevant `Switcher` class, each of +which holds a list of all `Switchable` objects that it is required to switch. The `SwitchableObservable` is also +provided with a reference to the switcher so that it can remove itself from the relevant list if it is closed for any +reason. +- When the instrument is changed the `InstrumentSwitchers` class will call the `switchInstruments` method on each of the +switchers, which will then go on to perform the relevant switching behaviour - for example changing a `Switchable`'s +source, closing it, or doing nothing. + +### Writables + +A similar process also occurs when switching writable PVs, as can be seen in the UML diagram below. The differences +being that a `WritableFactory` is used, this can create a `Writable` that inherits from `Switchable` and can write +values to PVs. Both the `Switchable` interface and the abstract `PrefixChangingSwitcher` are provided, so that +the switching process is as similar as possible when dealing with `Writables` and `Observables`. ![Writables](new_switching_writables.jpg) - -## Using the PV Switching - -The inner workings of the switching code need not be understood to create PVs that switch with the instrument. Only steps 1 and 3 in the previous list need be performed by a class that wants a new PV. Specifically the following code must be used -```java -ObservableFactory closingObsFactory = new ObservableFactory(OnInstrumentSwitch.CLOSE); -ForwardingObservable pv - = closingObsFactory.getSwitchableObservable(new StringChannel(), "A_PV_ADDRESS")); -``` -The above code will create a String type PV observable that will close when the instrument is changed. Subscriptions can now be attached to the PV and will be called when PVs change value. diff --git a/doc/client/coding/Perspectives.md b/doc/client/coding/Perspectives.md new file mode 100644 index 000000000..d83cf320c --- /dev/null +++ b/doc/client/coding/Perspectives.md @@ -0,0 +1,53 @@ +# Perspectives + +:::{note} +If you are implementing a brand new perspective, first follow the +[adding a plugin or feature guide](Adding-a-Plugin-or-Feature-to-Maven-Build) to define the new plugin. +::: + +## Defining a perspective + +Perspectives are defined in `base\uk.ac.stfc.isis.ibex.e4.client\Application.e4xmi`, under the "snippets" section. + +The `Application.e4xmi` file controls how a perspective is laid out. This is done using a hierarchy of: +- **Part Sash Containers**: these allow multiple items to appear side-by-side, either vertically or horizontally. +- **Part Stacks**: these allow multiple items to appear in a tabbed view. +- **Parts**: these are an individual view. Use a part if the view appears in only one perspective (for example, a DAE +configuration view, which only appears in the DAE perspective). +- **Placeholders**: an individual view which is shared between multiple perspectives (for example the blocks view). + +Part stacks can be defined as "not rendered" in the `Application.e4xmi` - this means they will not be shown at first, +but the perspective still defines where the part will appear once some other code renders it. Example of this are: +- The plotting view in the scripting perspective, which is only rendered after plotting using matplotlib in the +scripting console. +- OPIs in the device screens perspective, which are only rendered once a device screen is clicked on. + +## Keyboard shortcuts + +Keyboard shortcuts for changing perspective are also defined in `base\uk.ac.stfc.isis.ibex.e4.client\Application.e4xmi`, +under "Binding Tables". These key bindings are linked to commands, which then execute handlers. Both commands and +handlers are also specified in the `Application.e4xmi`. + +## Icons + +Find or create a PNG icon which is appropriately sized (`24x24` pixels). The icon path is specified in the +`Application.e4xmi` file along with the other properties of a perspective. + +:::{important} +If you find an icon online, you must ensure it is licensed appropriately and added to the +[list of icon licenses](https://github.com/ISISComputingGroup/ibex_gui/blob/master/base/uk.ac.stfc.isis.ibex.ui.help/resources/iconlicences.txt) +which appears using help -> "icon licenses" in the IBEX GUI. +::: + +## Hiding Perspectives + +Perspectives can be hidden using the preferences -> "select visible perspectives" menu in IBEX. These settings can be +changed either locally, for a given client session, or remotely, where the set of perspectives to show by default will +be stored by the IBEX server. If you cannot set remote perspectives, you probably do not have write access to the +instrument. + +## Troubleshooting + +If a new perspective is not being shown in the switcher during development, the GUI may have cached a set of +perspectives in its workspace. To clear this, go to run -> "run configurations" in Eclipse, and +ensure that the "clear workspace" option is selected for the IBEX product. diff --git a/doc/client/coding/Static-analysis.md b/doc/client/coding/Static-analysis.md index 05e5b2c7d..79c57c2b0 100644 --- a/doc/client/coding/Static-analysis.md +++ b/doc/client/coding/Static-analysis.md @@ -1,95 +1,43 @@ # Static analysis -We are currently making use of 3 static analysis tools to help ensure the quality of our code: [PMD](https://pmd.github.io/), [FindBugs](http://findbugs.sourceforge.net/), and [CheckStyle](https://checkstyle.sourceforge.io/). These particular tools are mostly intended for use with Java programs and as such our main use for them is the Eclipse GUI project, though they are also used on some Java code in the main EPICS project (specifically the IOC log server). - -All of these tools have maven plugins which allow the analyses to be run as part of the maven build process, with the results being output to an XML file. Additionally, they all have Jenkins plugins which can consume the XML results files, producing reports and graphs within Jenkins and allowing you to track code quality trends over time. The Jenkins reports list all the specific violations which can be categorised in a number of ways and are listed with file name and line number as well as a detailed description of the problem. +We are currently making use of 2 static analysis tools in the IBEX GUI: +[Checkstyle](https://checkstyle.sourceforge.io/) and [CodeQL](https://codeql.github.com/). ## Checkstyle -Checkstyle is designed to enforce a set of highly configurable coding standards. It supports a very large number of rules including ones relating to naming conventions, annotations, javadoc comments, poor coding practices, etc. That rules that Checkstyle will check for violations of may be configured in an XML file. - -The style rules imposed by Checkstyle are very demanding and it may be helpful to consider them as guidelines rather than as hard rules. At our first analysis of the Eclipse GUI project, Checkstyle showed over 30000 errors and it probably isn't worth the effort to fix all of these. However moving forward, when writing new code, it is a good idea to make at least some effort to conform to the Checkstyle 'rules'. - -Checkstyle configuration is done by XML file called 'checkstyle.xml' which is located in the Tycho parent directory. This is then set up as a global rule set called IBEX Checks. The rule set used is the default Sun ruleset with some rules disabled (by commenting them out in the XML file). In order to reduce the number of needless Checkstyle warnings, the following specific rules, which are enabled by default, have been disabled for Jenkins: +Checkstyle is designed to enforce a set of highly configurable coding standards. It is run as part of the Maven build +process, and will fail the GUI build if violations are detected. Eclipse can also be +[configured to highlight Checkstyle violations](../eclipse/Checkstyle-setup). -* FileTabCharacterCheck - using a tab character anywhere in the code -* RegexpSinglelineCheck - line has trailing white space -* LineLengthCheck - more than 80 characters in a line -* JavaDoc - checks for JavaDoc comments -* LineLength - checks that lines are not longer than 80 chars -* AvoidInlineConditionals - checks for ? : style conditionals -* HiddenField - checks that method parameter names do not mirror class parameter names -* InnerAssignment - checks that inner assignment is not used -* DesignForExtension - enforces a programming style where superclasses provide empty "hooks" that can be implemented by subclasses. -* FinalParameters - checks for method parameters that should be declared "final" -* NewlineAtEndOfFile - checks for a newline at the end of the code file -* MagicNumber - checks for numbers that could perhaps be variables instead +Checkstyle supports a very large number of rules including ones relating to naming conventions, annotations, javadoc +comments, poor coding practices, etc. Checkstyle configuration is done by +[an XML file called 'checkstyle.xml'](https://github.com/ISISComputingGroup/ibex_gui/blob/master/base/uk.ac.stfc.isis.ibex.client.tycho.parent/checkstyle.xml) +in the IBEX GUI repository. -The ISIS standard Checkstyle rules for the IDE is stored in the repository for our client, so that can just be imported rather than setting the rules up by hand. For the Eclipse IDE the "MagicNumber" rule is not disabled as it does occasionally provide some useful warnings, but can be ignored for cases where it is being pernickety. +In cases where Checkstyle has flagged code as non-compliant, but is being overzealous, a `@SuppressWarnings` annotation +can be used to tell Checkstyle to ignore certain warnings for specific classes or methods. For example: -An example of a "useful" warning: -``` - @Override - public int hashCode() { - return displayName.hashCode() ^ (isWritable ? 1231 : 1237); // It is not clear what these numbers represent - } -``` - -An example of an overzealous warning: -``` - @Override - public String getColumnText(Object element, int columnIndex) { - IocState ioc = (IocState) element; - switch (columnIndex) { - case 0: - //The hidden column - case 1: - //The IOC Name column - return ioc.getName(); - case 2: - //The IOC Name column - return ioc.getDescription(); - case 3: // MagicNumber flags this up - //The current state column - //This is handled in getColumnImage - case 4: // MagicNumber flags this up too - // Dummy column that is hidden by the scrollbar - return ""; - default: - return "Unknown column"; - } - } -``` -There is also the option of using the `@SuppressWarnings` qualifier to tell Checkstyle to ignore certain warnings for specific classes or methods. For example: -``` +```java @SuppressWarnings("checkstyle:magicnumber") public void getSecondsInHours(int hours) { return hours * 60 * 60; // Magic numbers! } ``` + Or for multiple warnings: -``` +```java @SuppressWarnings({"checkstyle:magicnumber", "checkstyle:localvariablename"}) public void getSecondsInHours(int hours) { int seconds_per_hour = 60 * 60; // Magic numbers and a variable name that does not conform to the recommended style! return hours * seconds_per_hour; } ``` -## PMD - -PMD focuses on potential coding problems such as unused or suboptimal code, code size and complexity, and good coding practices. Some typical rules include "Empty If Statement", "Broken Null Check", "Avoid Deeply Nested If Statements", "Switch Statements Should Have Defaults", etc. PMD will produce far fewer warnings/errors than checkstyle, however they are typically far more important, indicating technical problems with the code rather than merely stylistic ones, and should therefore be fixed whenever possible/sensible. - -## FindBugs - -FindBugs detects similar types of problems as PMD, though it can detect many additional serious problems such as potential null pointer exceptions, infinite loops, and unintentional access of the internal state of an object (to name a few). Unlike PMD, which checks through the source code, FindBugs actually checks through the applications bytecode for potential issues. FindBugs tends to find a smaller number of more important issues. - -For both PMD and FindBugs, there are certain rules which, while generally sensible, may not apply to the project at hand. For example, on the Eclipse GUI project, FindBugs throws up a lot of problems involving writing to a static member from an instance method, however the Activator class in most Eclipse plugins make use of this construct. -## Maven +## CodeQL -The tools may be run as part of a maven build by including them as plugins in the project's pom.xml. In the case of the Eclipse GUI, each plugin is included in the `tycho.parent` pom.xml file, and is therefore used in every other project (Eclipse plugin) during the build. +CodeQL is a security-focused linting tool. It aims to find coding patterns which are known to be insecure, though it +also has a smaller number of stylistic lint rules. -* [FindBugs maven plugin](https://gleclaire.github.io/findbugs-maven-plugin/index.html) -* [CheckStyle maven plugin](http://maven.apache.org/plugins/maven-checkstyle-plugin/) -* [PMD maven plugin ](http://maven.apache.org/plugins/maven-pmd-plugin/) \ No newline at end of file +CodeQL is set to run on Github pull requests. It will automatically comment on pull requests with any findings. If not +relevant, these findings can be dismissed via the Github interface. CodeQL is currently not set up to run locally. diff --git a/doc/client/coding/basic_principle.png b/doc/client/coding/connect_view_to_pv_basic_principle.png similarity index 100% rename from doc/client/coding/basic_principle.png rename to doc/client/coding/connect_view_to_pv_basic_principle.png diff --git a/doc/client/coding/eclipse_add_icons_to_build.png b/doc/client/coding/eclipse_add_icons_to_build.png deleted file mode 100644 index 6c50c464b..000000000 Binary files a/doc/client/coding/eclipse_add_icons_to_build.png and /dev/null differ diff --git a/doc/client/coding/eclipse_add_perspective_plugin1.png b/doc/client/coding/eclipse_add_perspective_plugin1.png deleted file mode 100644 index 18e285415..000000000 Binary files a/doc/client/coding/eclipse_add_perspective_plugin1.png and /dev/null differ diff --git a/doc/client/coding/eclipse_add_perspective_plugin2.png b/doc/client/coding/eclipse_add_perspective_plugin2.png deleted file mode 100644 index acc258b63..000000000 Binary files a/doc/client/coding/eclipse_add_perspective_plugin2.png and /dev/null differ diff --git a/doc/client/coding/eclipse_adding_a_View.png b/doc/client/coding/eclipse_adding_a_View.png deleted file mode 100644 index d01077ca0..000000000 Binary files a/doc/client/coding/eclipse_adding_a_View.png and /dev/null differ diff --git a/doc/client/coding/eclipse_adding_a_package.png b/doc/client/coding/eclipse_adding_a_package.png deleted file mode 100644 index 97f293277..000000000 Binary files a/doc/client/coding/eclipse_adding_a_package.png and /dev/null differ diff --git a/doc/client/coding/eclipse_extensions_added1.png b/doc/client/coding/eclipse_extensions_added1.png deleted file mode 100644 index 8eff1532a..000000000 Binary files a/doc/client/coding/eclipse_extensions_added1.png and /dev/null differ diff --git a/doc/client/coding/eclipse_extensions_added2.png b/doc/client/coding/eclipse_extensions_added2.png deleted file mode 100644 index 3b9d06a3a..000000000 Binary files a/doc/client/coding/eclipse_extensions_added2.png and /dev/null differ diff --git a/doc/client/coding/eclipse_extensions_added3.png b/doc/client/coding/eclipse_extensions_added3.png deleted file mode 100644 index 5c0c12d10..000000000 Binary files a/doc/client/coding/eclipse_extensions_added3.png and /dev/null differ diff --git a/doc/client/coding/eclipse_extensions_added4.png b/doc/client/coding/eclipse_extensions_added4.png deleted file mode 100644 index 3fb1442ae..000000000 Binary files a/doc/client/coding/eclipse_extensions_added4.png and /dev/null differ diff --git a/doc/client/coding/eclipse_extensions_added5.png b/doc/client/coding/eclipse_extensions_added5.png deleted file mode 100644 index 87b3c5bc5..000000000 Binary files a/doc/client/coding/eclipse_extensions_added5.png and /dev/null differ diff --git a/doc/client/coding/eclipse_no_extensions.png b/doc/client/coding/eclipse_no_extensions.png deleted file mode 100644 index e5c6046af..000000000 Binary files a/doc/client/coding/eclipse_no_extensions.png and /dev/null differ diff --git a/doc/client/coding/eclipse_perspective_copy_errors.png b/doc/client/coding/eclipse_perspective_copy_errors.png deleted file mode 100644 index e1f79d74f..000000000 Binary files a/doc/client/coding/eclipse_perspective_copy_errors.png and /dev/null differ diff --git a/doc/client/coding/eclipse_select_extension_point.png b/doc/client/coding/eclipse_select_extension_point.png deleted file mode 100644 index d39514776..000000000 Binary files a/doc/client/coding/eclipse_select_extension_point.png and /dev/null differ diff --git a/doc/client/coding/parent_pom.png b/doc/client/coding/parent_pom.png deleted file mode 100644 index f7e9ca651..000000000 Binary files a/doc/client/coding/parent_pom.png and /dev/null differ diff --git a/doc/client/eclipse/Common-Eclipse-Tasks.md b/doc/client/eclipse/Common-Eclipse-Tasks.md index 1d8596191..980bc0f02 100644 --- a/doc/client/eclipse/Common-Eclipse-Tasks.md +++ b/doc/client/eclipse/Common-Eclipse-Tasks.md @@ -125,7 +125,7 @@ public class FooDisplay extends Canvas ## Add A New Perspective -Instructions on adding a new perspective can be found [here](../coding/Adding-a-Button-to-the-Perspective-Switcher). +Instructions on adding a new perspective can be found [here](../coding/Perspectives). ## Add A Preference Page diff --git a/doc/conf.py b/doc/conf.py index 14a46339d..b20bb4553 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -92,4 +92,8 @@ "client/eclipse/Creating-the-IBEX-Developer-Version-of-Eclipse": "../compiling/Building-the-GUI.html#gui-build-install-eclipse", # noqa E501 "client/eclipse/Dictionary-setup": "Common-Eclipse-Issues.html#adding-a-dictionary-to-eclipse-s-spelling-checker", # noqa E501 "client/getting_started/GUI-Development-Workflow": "../../processes/git_and_github/Git-workflow.html", # noqa E501 + "client/coding/Instrument-switching": "PV-Switching.html", + "client/coding/Databinding---common-mistakes": "Databinding.html", + "client/coding/Migrating-or-adding-a-button-to-the-E4-perspective-switcher": "Perspectives.html", # noqa E501 + "client/coding/Adding-a-Button-to-the-Perspective-Switcher": "Perspectives.html", } diff --git a/doc/spelling_wordlist.txt b/doc/spelling_wordlist.txt index 2d307be69..d2632f512 100644 --- a/doc/spelling_wordlist.txt +++ b/doc/spelling_wordlist.txt @@ -534,7 +534,6 @@ mvn mW mx myexample -myperspective MySQL mysql mysqladmin @@ -604,9 +603,6 @@ pdf Peopleware performant pernickety -perspectiveExtension -perspectiveExtensions -PerspectiveSwitcher Pfeiffer Physica Physik @@ -641,7 +637,6 @@ prepended prepends preplanning prescale -preSetInstruments printf prisma ProcServ @@ -754,7 +749,6 @@ searchable seci sendSlackMessage Sensirion -setInstrument setIOCName setParam setpoint