Deprecating PortDiscovery in favour of DeviceDescritpionDiscovery

Change-Id: Ie9cff7937412c62c8a5a3b75b87a43952017f146

@@ -26,12 +26,18 @@ import java.util.List;
 /**
  * Discovers the set of ports from a device through a device specific protocol.
  * The returned ports are not retrieved from the information stored in ONOS.
+ *
+ * @deprecated 1.6.0 Goldeneye. Use DeviceDescriptionDiscovery instead
  */
+@Deprecated
 public interface PortDiscovery extends HandlerBehaviour {
 
     /**
      * Retrieves the set of ports from a device.
+     *
      * @return a set of port descriptions.
+     * @deprecated 1.6.0 Goldeneye. Use DeviceDescriptionDiscovery instead
      */
+    @Deprecated
     List<PortDescription> getPorts();
 }
\ No newline at end of file

@@ -28,7 +28,7 @@ import java.util.List;
 public interface DeviceDescriptionDiscovery extends HandlerBehaviour {
 
     /**
-     * Returns device description appropriately annotated to support
+     * Returns a device description appropriately annotated to support
      * downstream model extension via projections of the resulting device,
      * as in the following example.
      * <pre>
@@ -40,7 +40,7 @@ public interface DeviceDescriptionDiscovery extends HandlerBehaviour {
     DeviceDescription discoverDeviceDetails();
 
     /**
-     * Returns list of port descriptions appropriately annotated to support
+     * Returns a list of port descriptions appropriately annotated to support
      * downstream model extension via projections of their parent device,
      * as in the following example.
      * <pre>

@@ -22,6 +22,7 @@ import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.onosproject.net.DeviceId;
+import org.onosproject.net.device.DeviceDescription;
 import org.onosproject.net.device.DeviceDescriptionDiscovery;
 import org.onosproject.net.device.DeviceProvider;
 import org.onosproject.net.device.DeviceProviderRegistry;
@@ -93,10 +94,15 @@ public abstract class AbstractDeviceProvider extends AbstractProvider
     protected void discoverDevice(DriverHandler handler) {
         DeviceId deviceId = handler.data().deviceId();
         DeviceDescriptionDiscovery discovery = handler.behaviour(DeviceDescriptionDiscovery.class);
-        providerService.deviceConnected(deviceId, discovery.discoverDeviceDetails());
+        DeviceDescription description = discovery.discoverDeviceDetails();
+        if (description != null) {
+            providerService.deviceConnected(deviceId, description);
+        } else {
+            log.info("No other description given for device {}", deviceId);
+        }
         providerService.updatePorts(deviceId, discovery.discoverPortDetails());
-    }
 
+    }
     // TODO: inspect NETCONF, SNMP, RESTSB device providers for additional common patterns
     // TODO: provide base for port status update
     // TODO: integrate with network config for learning about management addresses to probe

@@ -18,6 +18,7 @@
 
 package org.onosproject.drivers.ciena;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import org.apache.commons.configuration.HierarchicalConfiguration;
 import org.onosproject.drivers.utilities.XmlConfigParser;
@@ -31,11 +32,13 @@ import org.onosproject.net.OchSignal;
 import org.onosproject.net.OduSignalType;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.SparseAnnotations;
-import org.onosproject.net.behaviour.PortDiscovery;
+import org.onosproject.net.device.DeviceDescription;
+import org.onosproject.net.device.DeviceDescriptionDiscovery;
 import org.onosproject.net.device.PortDescription;
 import org.onosproject.net.driver.AbstractHandlerBehaviour;
 import org.onosproject.net.driver.DriverHandler;
 import org.onosproject.protocol.rest.RestSBController;
+import org.slf4j.Logger;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -43,12 +46,15 @@ import java.util.List;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.onosproject.net.optical.device.OchPortHelper.ochPortDescription;
 import static org.onosproject.net.optical.device.OduCltPortHelper.oduCltPortDescription;
+import static org.slf4j.LoggerFactory.getLogger;
 
 /**
  * Discovers the ports from a Ciena WaveServer Rest device.
  */
-public class PortDiscoveryCienaWaveserverImpl extends AbstractHandlerBehaviour
-        implements PortDiscovery {
+public class CienaWaveserverDeviceDescription extends AbstractHandlerBehaviour
+        implements DeviceDescriptionDiscovery {
+
+    private final Logger log = getLogger(getClass());
 
     private static final String SPEED = "speed";
     private static final String GBPS = "Gbps";
@@ -74,9 +80,19 @@ public class PortDiscoveryCienaWaveserverImpl extends AbstractHandlerBehaviour
 //    private static final String SPECIFIC_PORT_CONFIG =
 //            "/ptp-config?config=true&format=xml&depth=unbounded";
 
+    @Override
+    public DeviceDescription discoverDeviceDetails() {
+        log.info("No description to be added for device");
+        //TODO to be implemented if needed.
+        return null;
+    }
 
     @Override
-    public List<PortDescription> getPorts() {
+    public List<PortDescription> discoverPortDetails() {
+        return getPorts();
+    }
+
+    private List<PortDescription> getPorts() {
         List<PortDescription> ports = Lists.newArrayList();
         DriverHandler handler = handler();
         RestSBController controller = checkNotNull(handler.get(RestSBController.class));
@@ -121,11 +137,11 @@ public class PortDiscoveryCienaWaveserverImpl extends AbstractHandlerBehaviour
                                 .replace(" ", EMPTY_STRING))) == speed100GbpsinMbps ?
                         CltSignalType.CLT_100GBE : null;
                 ports.add(oduCltPortDescription(PortNumber.portNumber(sub.getLong(PORT_ID)),
-                                                    sub.getString(ADMIN_STATE).equals(ENABLED),
-                                                    cltType, annotations.build()));
+                                                sub.getString(ADMIN_STATE).equals(ENABLED),
+                                                cltType, annotations.build()));
             }
         });
-        return ports;
+        return ImmutableList.copyOf(ports);
     }
 
     public static List<HierarchicalConfiguration> parseWaveServerCienaPorts(HierarchicalConfiguration cfg) {
@@ -156,7 +172,7 @@ public class PortDiscoveryCienaWaveserverImpl extends AbstractHandlerBehaviour
                 baseFrequency)) / toGbpsFromHz(chSpacing.frequency().asHz())); //FIXME is there a better way ?
 
         return ochPortDescription(PortNumber.portNumber(portNumber), isEnabled, oduSignalType, isTunable,
-                                      new OchSignal(gridType, chSpacing, spacingMult, 1), annotations);
+                                  new OchSignal(gridType, chSpacing, spacingMult, 1), annotations);
     }
 
     //FIXME remove when all optical types have two way information methods, see jira tickets

@@ -20,6 +20,8 @@
                    impl="org.onosproject.drivers.ciena.PortDiscoveryCienaWaveserverImpl"/>
         <behaviour api="org.onosproject.net.optical.OpticalDevice"
                    impl="org.onosproject.net.optical.DefaultOpticalDevice"/>
+        <behaviour api="org.onosproject.net.device.DeviceDescriptionDiscovery"
+                   impl="org.onosproject.drivers.ciena.CienaWaveserverDeviceDescription"/>
     </driver>
 </drivers>
 

@@ -16,6 +16,8 @@
 
 package org.onosproject.drivers.fujitsu;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
 import org.apache.commons.configuration.HierarchicalConfiguration;
 import org.onosproject.drivers.utilities.XmlConfigParser;
 import org.onosproject.net.AnnotationKeys;
@@ -26,7 +28,8 @@ import org.onosproject.net.GridType;
 import org.onosproject.net.OchSignal;
 import org.onosproject.net.OduSignalType;
 import org.onosproject.net.PortNumber;
-import org.onosproject.net.behaviour.PortDiscovery;
+import org.onosproject.net.device.DeviceDescription;
+import org.onosproject.net.device.DeviceDescriptionDiscovery;
 import org.onosproject.net.device.PortDescription;
 import org.onosproject.net.driver.AbstractHandlerBehaviour;
 import org.onosproject.netconf.NetconfController;
@@ -34,8 +37,6 @@ import org.onosproject.netconf.NetconfException;
 import org.onosproject.netconf.NetconfSession;
 import org.slf4j.Logger;
 
-import com.google.common.collect.Lists;
-
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.util.List;
@@ -49,13 +50,20 @@ import static org.slf4j.LoggerFactory.getLogger;
 /**
  * Retrieves the ports from a Fujitsu T100 device via netconf.
  */
-public class PortGetterFujitsuImpl extends AbstractHandlerBehaviour
-        implements PortDiscovery {
+public class FujitsuT100DeviceDescription extends AbstractHandlerBehaviour
+        implements DeviceDescriptionDiscovery {
 
     private final Logger log = getLogger(getClass());
 
     @Override
-    public List<PortDescription> getPorts() {
+    public DeviceDescription discoverDeviceDetails() {
+        log.info("No description to be added for device");
+        //TODO to be implemented if needed.
+        return null;
+    }
+
+    @Override
+    public List<PortDescription> discoverPortDetails() {
         NetconfController controller = checkNotNull(handler().get(NetconfController.class));
         NetconfSession session = controller.getDevicesMap().get(handler().data().deviceId()).getSession();
         String reply;
@@ -67,12 +75,13 @@ public class PortGetterFujitsuImpl extends AbstractHandlerBehaviour
         List<PortDescription> descriptions =
                 parseFujitsuT100Ports(XmlConfigParser.
                         loadXml(new ByteArrayInputStream(reply.getBytes())));
-        return descriptions;
+        return ImmutableList.copyOf(descriptions);
     }
 
     /**
      * Builds a request crafted to get the configuration required to create port
      * descriptions for the device.
+     *
      * @return The request string.
      */
     private String requestBuilder() {
@@ -91,6 +100,7 @@ public class PortGetterFujitsuImpl extends AbstractHandlerBehaviour
 
     /**
      * Parses a configuration and returns a set of ports for the fujitsu T100.
+     *
      * @param cfg a hierarchical configuration
      * @return a list of port descriptions
      */

@@ -20,6 +20,8 @@
                    impl="org.onosproject.drivers.fujitsu.PortGetterFujitsuImpl"/>
         <behaviour api="org.onosproject.net.optical.OpticalDevice"
                    impl="org.onosproject.net.optical.DefaultOpticalDevice"/>
+        <behaviour api="org.onosproject.net.device.DeviceDescriptionDiscovery"
+                   impl="org.onosproject.drivers.fujitsu.FujitsuT100DeviceDescription"/>
     </driver>
 </drivers>
 

@@ -16,6 +16,7 @@
 
 package org.onosproject.drivers.lumentum;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import org.onosproject.net.AnnotationKeys;
 import org.onosproject.net.DefaultAnnotations;
@@ -45,7 +46,7 @@ import static org.slf4j.LoggerFactory.getLogger;
 /**
  * Device description behaviour for Lumentum Snmp devices.
  */
-public class LumentumRoadmDeviceDescription extends AbstractHandlerBehaviour implements DeviceDescriptionDiscovery  {
+public class LumentumRoadmDeviceDescription extends AbstractHandlerBehaviour implements DeviceDescriptionDiscovery {
 
     private final Logger log = getLogger(getClass());
 
@@ -66,7 +67,7 @@ public class LumentumRoadmDeviceDescription extends AbstractHandlerBehaviour imp
 
     @Override
     public List<PortDescription> discoverPortDetails() {
-        return this.getPorts();
+        return ImmutableList.copyOf(this.getPorts());
     }
 
     private List<PortDescription> getPorts() {

@@ -43,6 +43,7 @@ import org.onosproject.net.config.NetworkConfigListener;
 import org.onosproject.net.config.NetworkConfigRegistry;
 import org.onosproject.net.device.DefaultDeviceDescription;
 import org.onosproject.net.device.DeviceDescription;
+import org.onosproject.net.device.DeviceDescriptionDiscovery;
 import org.onosproject.net.device.DeviceEvent;
 import org.onosproject.net.device.DeviceListener;
 import org.onosproject.net.device.DeviceProvider;
@@ -331,10 +332,17 @@ public class NetconfDeviceProvider extends AbstractProvider
 
     private void discoverPorts(DeviceId deviceId) {
         Device device = deviceService.getDevice(deviceId);
+        //TODO remove when PortDiscovery is removed from master
         if (device.is(PortDiscovery.class)) {
             PortDiscovery portConfig = device.as(PortDiscovery.class);
             providerService.updatePorts(deviceId,
                                         portConfig.getPorts());
+        } else if (device.is(DeviceDescriptionDiscovery.class)) {
+            DeviceDescriptionDiscovery deviceDescriptionDiscovery =
+                    device.as(DeviceDescriptionDiscovery.class);
+            providerService.updatePorts(deviceId,
+                                        deviceDescriptionDiscovery.discoverPortDetails());
+
         } else {
             log.warn("No portGetter behaviour for device {}", deviceId);
         }
@@ -343,7 +351,7 @@ public class NetconfDeviceProvider extends AbstractProvider
     /**
      * Return the DeviceId about the device containing the URI.
      *
-     * @param ip IP address
+     * @param ip   IP address
      * @param port port number
      * @return DeviceId
      */

@@ -40,11 +40,11 @@ import org.onosproject.net.config.NetworkConfigListener;
 import org.onosproject.net.config.NetworkConfigRegistry;
 import org.onosproject.net.device.DefaultDeviceDescription;
 import org.onosproject.net.device.DeviceDescription;
+import org.onosproject.net.device.DeviceDescriptionDiscovery;
 import org.onosproject.net.device.DeviceProvider;
 import org.onosproject.net.device.DeviceProviderRegistry;
 import org.onosproject.net.device.DeviceProviderService;
-import org.onosproject.net.driver.DriverHandler;
-import org.onosproject.net.driver.DriverService;
+import org.onosproject.net.device.DeviceService;
 import org.onosproject.net.provider.AbstractProvider;
 import org.onosproject.net.provider.ProviderId;
 import org.onosproject.protocol.rest.RestSBController;
@@ -93,7 +93,7 @@ public class RestDeviceProvider extends AbstractProvider
     protected CoreService coreService;
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
-    protected DriverService driverService;
+    protected DeviceService deviceService;
 
 
     private DeviceProviderService providerService;
@@ -219,19 +219,27 @@ public class RestDeviceProvider extends AbstractProvider
             log.error("Configuration error {}", e);
         }
         log.debug("REST Devices {}", controller.getDevices());
-        addedDevices.forEach(deviceId -> {
-            DriverHandler h = driverService.createHandler(deviceId);
-            PortDiscovery portConfig = h.behaviour(PortDiscovery.class);
-            if (portConfig != null) {
-                providerService.updatePorts(deviceId, portConfig.getPorts());
-            } else {
-                log.warn("No portGetter behaviour for device {}", deviceId);
-            }
-        });
+        addedDevices.forEach(this::discoverPorts);
         addedDevices.clear();
 
     }
 
+    private void discoverPorts(DeviceId deviceId) {
+        Device device = deviceService.getDevice(deviceId);
+        //TODO remove when PortDiscovery is removed from master
+        if (device.is(PortDiscovery.class)) {
+            PortDiscovery portConfig = device.as(PortDiscovery.class);
+            providerService.updatePorts(deviceId,
+                                        portConfig.getPorts());
+        } else if (device.is(DeviceDescriptionDiscovery.class)) {
+            DeviceDescriptionDiscovery deviceDescriptionDiscovery =
+                    device.as(DeviceDescriptionDiscovery.class);
+            providerService.updatePorts(deviceId, deviceDescriptionDiscovery.discoverPortDetails());
+        } else {
+            log.warn("No portGetter behaviour for device {}", deviceId);
+        }
+    }
+
     private boolean testDeviceConnection(RestSBDevice device) {
         try {
             return controller.get(device.deviceId(), "", "json") != null;

@@ -287,9 +287,13 @@ public class SnmpDeviceProvider extends AbstractProvider
                 if (d.is(DeviceDescriptionDiscovery.class)) {
                     DeviceDescriptionDiscovery descriptionDiscovery = d.as(DeviceDescriptionDiscovery.class);
                     DeviceDescription description = descriptionDiscovery.discoverDeviceDetails();
-                    deviceStore.createOrUpdateDevice(
-                            new ProviderId("snmp", "org.onosproject.provider.device"),
-                            did, description);
+                    if (description != null) {
+                        deviceStore.createOrUpdateDevice(
+                                new ProviderId("snmp", "org.onosproject.provider.device"),
+                                did, description);
+                    } else {
+                        log.info("No other description given for device {}", d.id());
+                    }
                     providerService.updatePorts(did, descriptionDiscovery.discoverPortDetails());
                 } else {
                     log.warn("No populate description and ports behaviour for device {}", did);