diff --git a/src/main/java/net/onrc/onos/registry/controller/IControllerRegistryService.java b/src/main/java/net/onrc/onos/registry/controller/IControllerRegistryService.java
index afd9dc6..dadee65 100644
--- a/src/main/java/net/onrc/onos/registry/controller/IControllerRegistryService.java
+++ b/src/main/java/net/onrc/onos/registry/controller/IControllerRegistryService.java
@@ -6,6 +6,23 @@
 
 import net.floodlightcontroller.core.module.IFloodlightService;
 
+/**
+ * A registry service that allows ONOS to register controllers and switches
+ * in a way that is global to the entire ONOS cluster. The registry is the
+ * arbiter for allowing controllers to control switches. 
+ * 
+ * The OVS/OF1.{2,3} fault tolerance model is a switch connects to multiple 
+ * controllers, and the controllers send role requests to determine what their
+ * role is in controlling the switch.
+ * 
+ * The ONOS fault tolerance model allows only a single controller to have 
+ * control of a switch (MASTER role) at once. Controllers therefore need a 
+ * mechanism that enables them to decide who should control a each switch.
+ * The registry service provides this mechanism.
+ * 
+ * @author jono
+ *
+ */
 public interface IControllerRegistryService extends IFloodlightService {
 	
 	/**
@@ -54,19 +71,11 @@
 	 */
 	public boolean hasControl(long dpid);
 	
-	
-	/**
-	 * Superseded by registerController
-	 * @param id
-	 */
-	@Deprecated
-	public void setMastershipId (String id);
-	
 	/**
 	 * Get the unique ID used to identify this controller in the cluster
 	 * @return
 	 */
-	public String getMastershipId ();
+	public String getControllerId ();
 	
 	/**
 	 * Register a controller to the ONOS cluster. Must be called before
@@ -92,7 +101,18 @@
 	 */
 	public Map<String, List<ControllerRegistryEntry>> getAllSwitches();
 	
+	/**
+	 * Get the controller that has control of a given switch
+	 * @param dpid Switch to find controller for
+	 * @return controller ID
+	 * @throws RegistryException Errors contacting registry service
+	 */
 	public String getControllerForSwitch(long dpid) throws RegistryException;
 	
+	/**
+	 * Get all switches controlled by a given controller
+	 * @param controllerId 
+	 * @return Collection of dpids
+	 */
 	public Collection<Long> getSwitchesControlledByController(String controllerId);
 }
diff --git a/src/main/java/net/onrc/onos/registry/controller/StandaloneRegistry.java b/src/main/java/net/onrc/onos/registry/controller/StandaloneRegistry.java
index 8a468bc..7345084 100644
--- a/src/main/java/net/onrc/onos/registry/controller/StandaloneRegistry.java
+++ b/src/main/java/net/onrc/onos/registry/controller/StandaloneRegistry.java
@@ -16,14 +16,19 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+/**
+ * Implementation of a registry that doesn't rely on any external registry
+ * service. This is designed to be used only in single-node setups (e.g. for
+ * development). All registry data is stored in local memory.
+ * @author jono
+ *
+ */
 public class StandaloneRegistry implements IFloodlightModule,
 		IControllerRegistryService {
 	protected static Logger log = LoggerFactory.getLogger(StandaloneRegistry.class);
 	
 	protected IRestApiService restApi;
 	
-	//protected String controllerId;
-	
 	protected String controllerId = null;
 	protected Map<String, ControlChangeCallback> switchCallbacks;
 	
@@ -63,13 +68,7 @@
 	}
 
 	@Override
-	public void setMastershipId(String id) {
-		// TODO Auto-generated method stub
-
-	}
-
-	@Override
-	public String getMastershipId() {
+	public String getControllerId() {
 		return controllerId;
 	}
 
@@ -147,16 +146,6 @@
 		restApi = context.getServiceImpl(IRestApiService.class);
 		
 		switchCallbacks = new HashMap<String, ControlChangeCallback>();
-		
-		//Put some data in for testing
-		/*
-		try {
-			registerController("hurro");
-			requestControl(2L, null);
-		} catch (RegistryException e1) {
-			// TODO Auto-generated catch block
-			e1.printStackTrace();
-		}*/
 	}
 
 	@Override
diff --git a/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java b/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java
index 468bb07..51335fe 100644
--- a/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java
+++ b/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java
@@ -36,6 +36,12 @@
 import com.netflix.curator.framework.recipes.leader.Participant;
 import com.netflix.curator.retry.ExponentialBackoffRetry;
 
+/**
+ * A registry service that uses Zookeeper. All data is stored in Zookeeper,
+ * so this can be used as a global registry in a multi-node ONOS cluster.
+ * @author jono
+ *
+ */
 public class ZookeeperRegistry implements IFloodlightModule, IControllerRegistryService {
 
 	protected static Logger log = LoggerFactory.getLogger(ZookeeperRegistry.class);
@@ -43,10 +49,9 @@
 	
 	protected IRestApiService restApi;
 	
-	//TODO read this from configuration
+	//This is the default, it's overwritten by the connectionString configuration parameter
 	protected String connectionString = "localhost:2181";
 	
-	
 	private final String namespace = "onos";
 	private final String switchLatchesPath = "/switches";
 	private final String controllerPath = "/controllers";
@@ -64,6 +69,19 @@
 	protected static final int sessionTimeout = 2000;
 	protected static final int connectionTimeout = 4000;
 	
+	/**
+	 * Watches for changes in switch leadership election. The Curator
+	 * LeaderLatch doesn't notify us when leadership changes so we set a watch
+	 * on the election znodes to get leadership change events. The process
+	 * method will be called whenever the switches children change in 
+	 * Zookeeper. We then have to work out whether to send a control-changed
+	 * event to our clients and reset the watch.
+	 * 
+	 * TODO I think it's possible to miss events that happen while we're 
+	 * processing the watch and before we've set a new watch. Need to think
+	 * of a safer way to implement leader change notifications.
+	 *
+	 */
 	protected class ParamaterizedCuratorWatcher implements CuratorWatcher {
 		private String dpid;
 		private boolean isLeader = false;
@@ -78,7 +96,6 @@
 		public synchronized void process(WatchedEvent event) throws Exception {
 			log.debug("Watch Event: {}", event);
 
-			LeaderLatch latch = switchLatches.get(dpid);
 			
 			if (event.getState() == KeeperState.Disconnected){
 				if (isLeader) {
@@ -92,6 +109,14 @@
 					}
 				}
 				return;
+				//TODO Watcher is never reset once we reconnect to Zookeeper
+			}
+			
+			LeaderLatch latch = switchLatches.get(dpid);
+			if (latch == null){
+				log.debug("In watcher process, looks like control was released for {}",
+						dpid);
+				return;
 			}
 			
 			try {
@@ -112,24 +137,24 @@
 				}
 			} catch (Exception e){
 				if (isLeader){
-					log.debug("Exception checking leadership status. Assume leadship lost for {}",
+					log.debug("Exception checking leadership status. Assume leadership lost for {}",
 							dpid);
 					
 					isLeader = false;
 					switchCallbacks.get(dpid).controlChanged(HexString.toLong(dpid), false);
 				}
+			} finally {
+				client.getChildren().usingWatcher(this).inBackground().forPath(latchPath);
 			}
-			
-			client.getChildren().usingWatcher(this).inBackground().forPath(latchPath);
 			//client.getChildren().usingWatcher(this).forPath(latchPath);
 		}
 	}
 	
 	
-	/*
-	 * Listens for changes to the switch znodes in Zookeeper. This maintains the second level of
-	 * PathChildrenCaches that hold the controllers contending for each switch - there's one for
-	 * each switch.
+	/**
+	 * Listens for changes to the switch znodes in Zookeeper. This maintains
+	 * the second level of PathChildrenCaches that hold the controllers 
+	 * contending for each switch - there's one for each switch.
 	 */
 	PathChildrenCacheListener switchPathCacheListener = new PathChildrenCacheListener() {
 		@Override
@@ -173,6 +198,7 @@
 	
 	@Override
 	public void requestControl(long dpid, ControlChangeCallback cb) throws RegistryException {
+		log.info("Requesting control for {}", HexString.toHexString(dpid));
 		
 		if (controllerId == null){
 			throw new RuntimeException("Must register a controller before calling requestControl");
@@ -205,20 +231,21 @@
 
 	@Override
 	public void releaseControl(long dpid) {
+		log.info("Releasing control for {}", HexString.toHexString(dpid));
 		
 		String dpidStr = HexString.toHexString(dpid);
 		
 		LeaderLatch latch = switchLatches.get(dpidStr);
 		if (latch == null) {
-			log.debug("Trying to release mastership for switch we are not contesting");
+			log.debug("Trying to release control of a switch we are not contesting");
 			return;
 		}
 		
 		try {
 			latch.close();
 		} catch (IOException e) {
-			//I think it's OK not to do anything here. Either the node got deleted correctly,
-			//or the connection went down and the node got deleted.
+			//I think it's OK not to do anything here. Either the node got 
+			//deleted correctly, or the connection went down and the node got deleted.
 		} finally {
 			switchLatches.remove(dpidStr);
 			switchCallbacks.remove(dpidStr);
@@ -244,13 +271,7 @@
 	}
 
 	@Override
-	public void setMastershipId(String id) {
-		//TODO remove this method if not needed
-		//controllerId = id;
-	}
-
-	@Override
-	public String getMastershipId() {
+	public String getControllerId() {
 		return controllerId;
 	}
 	
@@ -423,8 +444,8 @@
 			//Don't prime the cache, we want a notification for each child node in the path
 			switchCache.start(StartMode.NORMAL);
 		} catch (Exception e) {
-			// TODO Auto-generated catch block
-			e.printStackTrace();
+			throw new FloodlightModuleException("Error initialising ZookeeperRegistry: " 
+					+ e.getMessage());
 		}
 	}
 	
@@ -432,5 +453,4 @@
 	public void startUp (FloodlightModuleContext context) {
 		restApi.addRestletRoutable(new RegistryWebRoutable());
 	}
-
 }