Bug 539328 - Fix NPE in NewArduinoTargetWizard

Arduino-Connections can be created and edited using Preferences->Remote
Development->Remote Connections.

Change-Id: Ie92330689bdad4987c12d2bd17828dbe5e862b90
Signed-off-by: Lutz Hamann <lhamann@planettime.de>

4 files changed, 106 insertions, 10 deletions

diff --git a/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/ArduinoRemoteServicesUI.java b/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/ArduinoRemoteServicesUI.java
index 77287f5d243..561e4d70742 100644
--- a/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/ArduinoRemoteServicesUI.java
+++ b/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/ArduinoRemoteServicesUI.java
@@ -63,7 +63,7 @@ public class ArduinoRemoteServicesUI extends AbstractRemoteUIConnectionService {
 
 	@Override
 	public IRemoteUIConnectionWizard getConnectionWizard(Shell shell) {
-		return new NewArduinoTargetWizard();
+		return new NewArduinoTargetWizard(shell, connectionType);
 	}
 
 	@Override
diff --git a/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/BoardPropertyControl.java b/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/BoardPropertyControl.java
index b9e73bca18e..a8953940510 100644
--- a/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/BoardPropertyControl.java
+++ b/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/BoardPropertyControl.java
@@ -167,7 +167,7 @@ public class BoardPropertyControl extends Composite {
 			HierarchicalProperties programmers = board.getPlatform().getProgrammers();
 			if (programmers != null && programmers.getChildren() != null) {
 				programmerLabel = new Label(this, SWT.NONE);
-				programmerLabel.setText("Programmer:");
+				programmerLabel.setText("Programmer:"); //$NON-NLS-1$
 
 				programmerCombo = new Combo(this, SWT.READ_ONLY);
 				programmerCombo.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false));
@@ -243,4 +243,44 @@ public class BoardPropertyControl extends Composite {
 		}
 	}
 
+	public void updateFromOriginal(ArduinoRemoteConnection arduinoService) throws CoreException {
+		// Set and select the portname
+		portCombo.setText(arduinoService.getPortName());
+		portName = arduinoService.getPortName();
+
+		// Set and select the board
+		boardCombo.setText(arduinoService.getBoard().getName());
+		boardChanged();
+
+		// Lock changing of board
+		boardCombo.setEnabled(false);
+
+		// Set the programmer
+		if (!arduinoService.getProgrammer().isEmpty() && programmerCombo != null && !programmerCombo.isDisposed()) {
+			programmerCombo.setText(board.getPlatform().getProgrammers().getChild(arduinoService.getProgrammer())
+					.getChild("name").getValue()); //$NON-NLS-1$
+		}
+
+		// Set all properties based on the displayed Controls.
+		String key = null;
+		// Loop over all controls. Labels and corresponding combos are saved in order
+		for (Control control : menuControls) {
+			if (control instanceof Label) {
+				// the label is the key
+				key = (String) control.getData();
+			} else if (control instanceof Combo) {
+				if (key != null && !arduinoService.getMenuValue(key).isEmpty()) {
+					String selectableValue = board.getMenus().getChild(key).getChild(arduinoService.getMenuValue(key))
+							.getValue();
+					((Combo) control).setText(selectableValue);
+				}
+				// reset key
+				key = null;
+			} else {
+				// unexpected order - reset key
+				key = null;
+			}
+		}
+	}
+
 }
diff --git a/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/NewArduinoTargetWizard.java b/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/NewArduinoTargetWizard.java
index 43ba1d88e08..94f252fa944 100644
--- a/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/NewArduinoTargetWizard.java
+++ b/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/NewArduinoTargetWizard.java
@@ -16,11 +16,13 @@ import org.eclipse.cdt.arduino.core.internal.remote.ArduinoRemoteConnection;
 import org.eclipse.cdt.arduino.ui.internal.Activator;
 import org.eclipse.jface.viewers.IStructuredSelection;
 import org.eclipse.jface.wizard.Wizard;
+import org.eclipse.jface.wizard.WizardDialog;
 import org.eclipse.remote.core.IRemoteConnectionType;
 import org.eclipse.remote.core.IRemoteConnectionWorkingCopy;
 import org.eclipse.remote.core.IRemoteServicesManager;
 import org.eclipse.remote.core.exception.RemoteConnectionException;
 import org.eclipse.remote.ui.IRemoteUIConnectionWizard;
+import org.eclipse.swt.widgets.Shell;
 import org.eclipse.ui.INewWizard;
 import org.eclipse.ui.IWorkbench;
 
@@ -29,6 +31,18 @@ public class NewArduinoTargetWizard extends Wizard implements IRemoteUIConnectio
 	private NewArduinoTargetWizardPage page;
 	private IRemoteConnectionWorkingCopy workingCopy;
 	private boolean isNewWizard;
+	private Shell shell;
+	private IRemoteConnectionType connectionType;
+
+	public NewArduinoTargetWizard(Shell shell, IRemoteConnectionType connectionType) {
+		this.shell = shell;
+		this.connectionType = connectionType;
+	}
+
+	public NewArduinoTargetWizard() {
+		this.connectionType = Activator.getService(IRemoteServicesManager.class)
+				.getConnectionType(ArduinoRemoteConnection.TYPE_ID);
+	}
 
 	@Override
 	public void init(IWorkbench workbench, IStructuredSelection selection) {
@@ -38,6 +52,10 @@ public class NewArduinoTargetWizard extends Wizard implements IRemoteUIConnectio
 	@Override
 	public void addPages() {
 		page = new NewArduinoTargetWizardPage();
+		// Editing an existing Connection
+		if (workingCopy != null) {
+			page.setWorkingCopy(workingCopy);
+		}
 		addPage(page);
 	}
 
@@ -63,16 +81,19 @@ public class NewArduinoTargetWizard extends Wizard implements IRemoteUIConnectio
 
 	@Override
 	public IRemoteConnectionWorkingCopy open() {
-		return getConnection();
+		WizardDialog dialog = new WizardDialog(shell, this);
+		dialog.setBlockOnOpen(true);
+		if (dialog.open() == WizardDialog.OK) {
+			return getConnection();
+		}
+		return null;
 	}
 
 	@Override
 	public IRemoteConnectionWorkingCopy getConnection() {
 		if (workingCopy == null) {
-			IRemoteServicesManager remoteManager = Activator.getService(IRemoteServicesManager.class);
-			IRemoteConnectionType connectionType = remoteManager.getConnectionType(ArduinoRemoteConnection.TYPE_ID);
 			try {
-				workingCopy = connectionType.newConnection(page.name);
+				workingCopy = connectionType.newConnection(page.getConnectionName());
 			} catch (RemoteConnectionException e) {
 				Activator.getDefault().getLog().log(e.getStatus());
 				return null;
diff --git a/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/NewArduinoTargetWizardPage.java b/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/NewArduinoTargetWizardPage.java
index 94626f0dcbf..3491449ae7e 100644
--- a/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/NewArduinoTargetWizardPage.java
+++ b/toolchains/arduino/org.eclipse.cdt.arduino.ui/src/org/eclipse/cdt/arduino/ui/internal/remote/NewArduinoTargetWizardPage.java
@@ -10,7 +10,10 @@
  *******************************************************************************/
 package org.eclipse.cdt.arduino.ui.internal.remote;
 
+import org.eclipse.cdt.arduino.core.internal.remote.ArduinoRemoteConnection;
+import org.eclipse.cdt.arduino.ui.internal.Activator;
 import org.eclipse.cdt.arduino.ui.internal.Messages;
+import org.eclipse.core.runtime.CoreException;
 import org.eclipse.jface.wizard.WizardPage;
 import org.eclipse.remote.core.IRemoteConnectionWorkingCopy;
 import org.eclipse.swt.SWT;
@@ -26,8 +29,9 @@ import org.eclipse.swt.widgets.Text;
 
 public class NewArduinoTargetWizardPage extends WizardPage {
 
-	String name;
+	private String connectionName;
 	private Text nameText;
+	private IRemoteConnectionWorkingCopy workingCopy;
 
 	BoardPropertyControl boardControl;
 
@@ -55,7 +59,7 @@ public class NewArduinoTargetWizardPage extends WizardPage {
 		nameText.addKeyListener(new KeyListener() {
 			@Override
 			public void keyReleased(KeyEvent e) {
-				name = nameText.getText();
+				connectionName = nameText.getText();
 				updateStatus();
 			}
 
@@ -73,12 +77,18 @@ public class NewArduinoTargetWizardPage extends WizardPage {
 			}
 		});
 
+		try {
+			updateFromWorkingCopy();
+		} catch (CoreException e) {
+			Activator.log(e);
+		}
+
 		setControl(comp);
-		setPageComplete(false);
+		updateStatus();
 	}
 
 	private void updateStatus() {
-		setPageComplete(name != null && !name.isEmpty() && boardControl.getPortName() != null
+		setPageComplete(connectionName != null && !connectionName.isEmpty() && boardControl.getPortName() != null
 				&& boardControl.getSelectedBoard() != null);
 	}
 
@@ -86,4 +96,29 @@ public class NewArduinoTargetWizardPage extends WizardPage {
 		boardControl.apply(workingCopy);
 	}
 
+	public String getConnectionName() {
+		return connectionName;
+	}
+
+	public void setWorkingCopy(IRemoteConnectionWorkingCopy workingCopy) {
+		this.workingCopy = workingCopy;
+	}
+
+	private void updateFromWorkingCopy() throws CoreException {
+		if (null == workingCopy || null == workingCopy.getOriginal())
+			return;
+
+		ArduinoRemoteConnection arduinoService = workingCopy.getService(ArduinoRemoteConnection.class);
+
+		if (null == arduinoService)
+			return;
+
+		// Set the originalName and lock control for it
+		nameText.setText(workingCopy.getOriginal().getName());
+		nameText.setEnabled(false);
+		connectionName = workingCopy.getOriginal().getName();
+
+		// Set all other fields with existing data
+		boardControl.updateFromOriginal(arduinoService);
+	}
 }