Changed argument from Device -> DeviceId

- roleChanged(), isReachable()
   Eventually consistent nature of Device store was
  interfering with mastership control.

Change-Id: I9c0dd846a4e30863f922f6706c6cb62fd7c83f29

@@ -368,12 +368,12 @@ public class OpticalConfigProvider extends AbstractProvider implements DevicePro
     }
     }
 
 
     @Override
     @Override
-    public void roleChanged(Device device, MastershipRole newRole) {
+    public void roleChanged(DeviceId device, MastershipRole newRole) {
         // TODO Auto-generated method stub.
         // TODO Auto-generated method stub.
     }
     }
 
 
     @Override
     @Override
-    public boolean isReachable(Device device) {
+    public boolean isReachable(DeviceId device) {
         return false;
         return false;
     }
     }
 }
 }

@@ -16,6 +16,7 @@
 package org.onlab.onos.net.device;
 package org.onlab.onos.net.device;
 
 
 import org.onlab.onos.net.Device;
 import org.onlab.onos.net.Device;
+import org.onlab.onos.net.DeviceId;
 import org.onlab.onos.net.MastershipRole;
 import org.onlab.onos.net.MastershipRole;
 import org.onlab.onos.net.provider.Provider;
 import org.onlab.onos.net.provider.Provider;
 
 
@@ -42,16 +43,16 @@ public interface DeviceProvider extends Provider {
      * Notifies the provider of a mastership role change for the specified
      * Notifies the provider of a mastership role change for the specified
      * device as decided by the core.
      * device as decided by the core.
      *
      *
-     * @param device  affected device
+     * @param deviceId  device identifier
      * @param newRole newly determined mastership role
      * @param newRole newly determined mastership role
      */
      */
-    void roleChanged(Device device, MastershipRole newRole);
+    void roleChanged(DeviceId deviceId, MastershipRole newRole);
 
 
     /**
     /**
      * Checks the reachability (connectivity) of a device from this provider.
      * Checks the reachability (connectivity) of a device from this provider.
      *
      *
-     * @param device device to check
+     * @param deviceId  device identifier
      * @return true if reachable, false otherwise
      * @return true if reachable, false otherwise
      */
      */
-    boolean isReachable(Device device);
+    boolean isReachable(DeviceId deviceId);
 }
 }

@@ -159,13 +159,14 @@ public class DeviceManager
     }
     }
 
 
     // Check a device for control channel connectivity.
     // Check a device for control channel connectivity.
-    private boolean isReachable(Device device) {
-        // FIXME: Device might not be there yet. (eventual consistent)
-        if (device == null) {
+    private boolean isReachable(DeviceId deviceId) {
+        DeviceProvider provider = getProvider(deviceId);
+        if (provider != null) {
+            return provider.isReachable(deviceId);
+        } else {
+            log.error("Provider not found for {}", deviceId);
             return false;
             return false;
         }
         }
-        DeviceProvider provider = getProvider(device.providerId());
-        return provider.isReachable(device);
     }
     }
 
 
     @Override
     @Override
@@ -226,15 +227,9 @@ public class DeviceManager
                 log.warn("Provider for {} was not found. Cannot apply role {}", deviceId, newRole);
                 log.warn("Provider for {} was not found. Cannot apply role {}", deviceId, newRole);
                 return false;
                 return false;
             }
             }
-            // FIXME roleChanged should take DeviceId instead of Device
-            Device device = store.getDevice(deviceId);
-            if (device == null) {
-                log.warn("{} was not there. Cannot apply role {}", deviceId, newRole);
-                return false;
-            }
-            provider.roleChanged(device, newRole);
+            provider.roleChanged(deviceId, newRole);
+            // not triggering probe when triggered by provider service event
 
 
-            // not triggering prove when triggered by provider service
             return true;
             return true;
         }
         }
 
 
@@ -442,6 +437,7 @@ public class DeviceManager
 
 
             Device device = store.getDevice(deviceId);
             Device device = store.getDevice(deviceId);
             // FIXME: Device might not be there yet. (eventual consistent)
             // FIXME: Device might not be there yet. (eventual consistent)
+            // FIXME relinquish role
             if (device == null) {
             if (device == null) {
                 log.warn("{} was not there. Cannot apply role {}", deviceId, newRole);
                 log.warn("{} was not there. Cannot apply role {}", deviceId, newRole);
                 return false;
                 return false;
@@ -452,8 +448,7 @@ public class DeviceManager
                 log.warn("Provider for {} was not found. Cannot apply role {}", deviceId, newRole);
                 log.warn("Provider for {} was not found. Cannot apply role {}", deviceId, newRole);
                 return false;
                 return false;
             }
             }
-            // FIXME roleChanged should take DeviceId instead of Device
-            provider.roleChanged(device, newRole);
+            provider.roleChanged(deviceId, newRole);
 
 
             if (newRole.equals(MastershipRole.MASTER)) {
             if (newRole.equals(MastershipRole.MASTER)) {
                 // only trigger event when request was sent to provider
                 // only trigger event when request was sent to provider
@@ -495,8 +490,7 @@ public class DeviceManager
             }
             }
 
 
 
 
-            final Device device = getDevice(did);
-            final boolean isReachable = isReachable(device);
+            final boolean isReachable = isReachable(did);
             if (!isReachable) {
             if (!isReachable) {
                 // device is not connected to this node
                 // device is not connected to this node
                 if (myNextRole != NONE) {
                 if (myNextRole != NONE) {
@@ -505,6 +499,7 @@ public class DeviceManager
                             + "Relinquishing role.  ",
                             + "Relinquishing role.  ",
                              myNextRole, did);
                              myNextRole, did);
                     mastershipService.relinquishMastership(did);
                     mastershipService.relinquishMastership(did);
+                    // FIXME disconnect?
                 }
                 }
                 return;
                 return;
             }
             }
@@ -523,6 +518,7 @@ public class DeviceManager
 
 
             switch (myNextRole) {
             switch (myNextRole) {
             case MASTER:
             case MASTER:
+                final Device device = getDevice(did);
                 if ((device != null) && !isAvailable(did)) {
                 if ((device != null) && !isAvailable(did)) {
                     //flag the device as online. Is there a better way to do this?
                     //flag the device as online. Is there a better way to do this?
                     DefaultDeviceDescription deviceDescription
                     DefaultDeviceDescription deviceDescription

@@ -251,7 +251,7 @@ public class DeviceManagerTest {
 
 
 
 
     private class TestProvider extends AbstractProvider implements DeviceProvider {
     private class TestProvider extends AbstractProvider implements DeviceProvider {
-        private Device deviceReceived;
+        private DeviceId deviceReceived;
         private MastershipRole roleReceived;
         private MastershipRole roleReceived;
 
 
         public TestProvider() {
         public TestProvider() {
@@ -263,13 +263,13 @@ public class DeviceManagerTest {
         }
         }
 
 
         @Override
         @Override
-        public void roleChanged(Device device, MastershipRole newRole) {
+        public void roleChanged(DeviceId device, MastershipRole newRole) {
             deviceReceived = device;
             deviceReceived = device;
             roleReceived = newRole;
             roleReceived = newRole;
         }
         }
 
 
         @Override
         @Override
-        public boolean isReachable(Device device) {
+        public boolean isReachable(DeviceId device) {
             return false;
             return false;
         }
         }
     }
     }

@@ -111,8 +111,8 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr
 
 
 
 
     @Override
     @Override
-    public boolean isReachable(Device device) {
-        OpenFlowSwitch sw = controller.getSwitch(dpid(device.id().uri()));
+    public boolean isReachable(DeviceId deviceId) {
+        OpenFlowSwitch sw = controller.getSwitch(dpid(deviceId.uri()));
         if (sw == null || !sw.isConnected()) {
         if (sw == null || !sw.isConnected()) {
             return false;
             return false;
         }
         }
@@ -170,22 +170,22 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr
     // }
     // }
 
 
     @Override
     @Override
-    public void roleChanged(Device device, MastershipRole newRole) {
+    public void roleChanged(DeviceId deviceId, MastershipRole newRole) {
         switch (newRole) {
         switch (newRole) {
             case MASTER:
             case MASTER:
-                controller.setRole(dpid(device.id().uri()), RoleState.MASTER);
+                controller.setRole(dpid(deviceId.uri()), RoleState.MASTER);
                 break;
                 break;
             case STANDBY:
             case STANDBY:
-                controller.setRole(dpid(device.id().uri()), RoleState.EQUAL);
+                controller.setRole(dpid(deviceId.uri()), RoleState.EQUAL);
                 break;
                 break;
             case NONE:
             case NONE:
-                controller.setRole(dpid(device.id().uri()), RoleState.SLAVE);
+                controller.setRole(dpid(deviceId.uri()), RoleState.SLAVE);
                 break;
                 break;
             default:
             default:
                 LOG.error("Unknown Mastership state : {}", newRole);
                 LOG.error("Unknown Mastership state : {}", newRole);
 
 
         }
         }
-        LOG.info("Accepting mastership role change for device {}", device.id());
+        LOG.info("Accepting mastership role change for device {}", deviceId);
     }
     }
 
 
     private class InternalDeviceProvider implements OpenFlowSwitchListener {
     private class InternalDeviceProvider implements OpenFlowSwitchListener {

@@ -105,11 +105,11 @@ public class OpenFlowDeviceProviderTest {
 
 
     @Test
     @Test
     public void roleChanged() {
     public void roleChanged() {
-        provider.roleChanged(DEV1, MASTER);
+        provider.roleChanged(DID1, MASTER);
         assertEquals("Should be MASTER", RoleState.MASTER, controller.roleMap.get(DPID1));
         assertEquals("Should be MASTER", RoleState.MASTER, controller.roleMap.get(DPID1));
-        provider.roleChanged(DEV1, STANDBY);
+        provider.roleChanged(DID1, STANDBY);
         assertEquals("Should be EQUAL", RoleState.EQUAL, controller.roleMap.get(DPID1));
         assertEquals("Should be EQUAL", RoleState.EQUAL, controller.roleMap.get(DPID1));
-        provider.roleChanged(DEV1, NONE);
+        provider.roleChanged(DID1, NONE);
         assertEquals("Should be SLAVE", RoleState.SLAVE, controller.roleMap.get(DPID1));
         assertEquals("Should be SLAVE", RoleState.SLAVE, controller.roleMap.get(DPID1));
     }
     }
 
 

@@ -244,7 +244,7 @@ class ConfigProvider implements DeviceProvider, LinkProvider, HostProvider {
     }
     }
 
 
     @Override
     @Override
-    public void roleChanged(Device device, MastershipRole newRole) {
+    public void roleChanged(DeviceId device, MastershipRole newRole) {
     }
     }
 
 
     @Override
     @Override
@@ -257,7 +257,7 @@ class ConfigProvider implements DeviceProvider, LinkProvider, HostProvider {
     }
     }
 
 
     @Override
     @Override
-    public boolean isReachable(Device device) {
+    public boolean isReachable(DeviceId device) {
         return false;
         return false;
     }
     }
 }
 }