removing some TODOs

Change-Id: Ic98c811573a1b37813993eedddec512ec85729f3

@@ -58,7 +58,6 @@ public class ElectionTest {
 
     private ControllerNode localControllerNode;
 
-    // TODO write cli command to get leader
 
     @Activate
     protected void activate() {

@@ -174,7 +174,6 @@ public class FooComponent {
             //try {
                 //Thread.sleep(5000);
             //} catch (InterruptedException e) {
-                // TODO Auto-generated catch block
                 //e.printStackTrace();
             //}
             log.info("Acquired Lock");

@@ -129,10 +129,6 @@ public class IntentMetrics implements IntentMetricsService,
     @Override
     public void event(IntentEvent event) {
         synchronized (lastEvents) {
-            //
-            // TODO: The processing below is incomplete: we don't have
-            // an event equivalent of "Withdraw Requested"
-            //
             switch (event.type()) {
             case INSTALL_REQ:
                 intentSubmittedEventMetric.eventReceived();
@@ -141,7 +137,7 @@ public class IntentMetrics implements IntentMetricsService,
                 intentInstalledEventMetric.eventReceived();
                 break;
             case FAILED:
-                // TODO: Just ignore?
+                // ignore
                 break;
             case WITHDRAW_REQ:
                 intentWithdrawRequestedEventMetric.eventReceived();

@@ -129,7 +129,7 @@ public class HostMobility {
                     }
                 }
             }
-            //TODO: handle ip cleanup
+
             return flowRules;
         }
 

@@ -364,7 +364,6 @@ public class OpticalConfigProvider extends AbstractProvider implements DevicePro
 
     @Override
     public void roleChanged(DeviceId device, MastershipRole newRole) {
-        // TODO Auto-generated method stub.
     }
 
     @Override

@@ -36,7 +36,6 @@ class PktOptLink {
     }
 
     public PktOptLink() {
-        // TODO Auto-generated constructor stub
     }
 
     public void setSrcNodeName(String name) {

@@ -37,7 +37,6 @@ class WdmLink {
     }
 
     public WdmLink() {
-        // TODO Auto-generated constructor stub
     }
 
     public void setSrcNodeName(String name) {

@@ -49,13 +49,11 @@ public class MetricsListCommand extends AbstractShellCommand {
     protected void execute() {
         MetricsService metricsService = get(MetricsService.class);
 
-        // TODO support filter condition
         MetricFilter filter = MetricFilter.ALL;
 
         TreeMultimap<String, Metric> matched = listMetrics(metricsService, filter);
         matched.asMap().forEach((name, metrics) -> {
             for (Metric metric : metrics) {
-                // TODO JSON version
                 printMetric(name, metric);
             }
         });

@@ -17,11 +17,11 @@ package org.onosproject.cli;
 
 import org.apache.karaf.shell.commands.Argument;
 import org.apache.karaf.shell.commands.Command;
+import org.onlab.packet.IpAddress;
 import org.onosproject.cluster.ControllerNode;
 import org.onosproject.cluster.DefaultControllerNode;
 import org.onosproject.cluster.NodeId;
 import org.onosproject.store.service.DatabaseAdminService;
-import org.onlab.packet.IpAddress;
 
 /**
  * Adds a new controller cluster node.
@@ -34,7 +34,6 @@ public class TabletAddCommand extends AbstractShellCommand {
               required = true, multiValued = false)
     String nodeId = null;
 
-    // TODO context aware completer to get IP from ClusterService?
     @Argument(index = 1, name = "ip", description = "Node IP address",
               required = true, multiValued = false)
     String ip = null;

@@ -30,7 +30,6 @@ import org.onosproject.net.topology.TopologyService;
          description = "Lists summary of the current topology")
 public class TopologyCommand extends AbstractShellCommand {
 
-    // TODO: format the time-stamp
     private static final String FMT =
             "time=%s, devices=%d, links=%d, clusters=%d, paths=%d";
 

@@ -51,7 +51,6 @@ public class DefaultEventSinkRegistry implements EventSinkRegistry {
     @Override
     @SuppressWarnings("unchecked")
     public <E extends Event> EventSink<E> getSink(Class<E> eventClass) {
-        // TODO: add implicit registration of descendant classes
         return (EventSink<E>) sinks.get(eventClass);
     }
 

@@ -296,7 +296,6 @@ public class MastershipManager
         @Override
         public void event(ClusterEvent event) {
             switch (event.type()) {
-                //FIXME: worry about addition when the time comes
                 case INSTANCE_ADDED:
                 case INSTANCE_ACTIVATED:
                     clusterSize.incrementAndGet();
@@ -341,14 +340,9 @@ public class MastershipManager
             if (clusterService.getNodes().size() > (clusterSize.intValue() / 2)) {
                 return true;
             }
-//            else {
-                //FIXME: break tie for equal-sized clusters, by number of
-                //       connected switches, then masters, then nodeId hash
-                //       problem is, how do we get at channel info cleanly here?
-                //       Also, what's the time hit for a distributed store look-up
-                //       versus channel re-negotiation? bet on the latter being worse.
-
-//            }
+
+             //FIXME: break tie for equal-sized clusters,
+
             return false;
         }
 

@@ -81,7 +81,6 @@ public class CoreManager implements CoreService {
 
     @Override
     public IdGenerator getIdGenerator(String topic) {
-        // FIXME this should be created lazily (once per topic)
         IdBlockAllocator allocator = new StoreBasedIdBlockAllocator(topic, idBlockStore);
         return new BlockAllocatorBasedIdGenerator(allocator);
     }

@@ -26,8 +26,6 @@ import org.projectfloodlight.openflow.types.OFPort;
  */
 public interface OpenFlowPacketContext {
 
-    //TODO: may want to support sending packet out other switches than
-    // the one it came in on.
     /**
      * Blocks further responses (ie. send() calls) on this
      * packet in event.

@@ -275,8 +275,8 @@ public abstract class AbstractOpenFlowSwitch implements OpenFlowSwitchDriver {
                 this.transitionToEqualSwitch();
             }
         }  else {
+            log.warn("Failed to set role for {}", this.getStringId());
             return;
-            //TODO: tell people that we failed.
         }
     }
 

@@ -81,13 +81,9 @@ public class OpenflowControllerAdapter implements OpenFlowController {
 
     @Override
     public void addEventListener(OpenFlowEventListener listener) {
-        // TODO Auto-generated method stub
-
     }
 
     @Override
     public void removeEventListener(OpenFlowEventListener listener) {
-        // TODO Auto-generated method stub
-
     }
 }

@@ -137,7 +137,6 @@ public class OFOpticalSwitchImplLINC13 extends AbstractOpenFlowSwitch {
         }
     }
 
-    //Todo
     public void processOFPortStatus(OFCircuitPortStatus ps) {
         log.debug("LINC-OE ..OF Port Status :", ps);
 

@@ -672,9 +672,7 @@ public class OFSwitchImplCPqD13 extends AbstractOpenFlowSwitch {
             subnetIps.add("10.0.3.0");
             subnetIps.add("10.0.1.0");
         }
-        // TODO needed?
-        //if (getId() == 0x2) {
-        //}
+
         if (getId() == 0x3) {
             subnetIps.add("7.7.7.0");
         }
@@ -740,9 +738,6 @@ public class OFSwitchImplCPqD13 extends AbstractOpenFlowSwitch {
             hostNextHopIps.add(new RouteEntry("10.0.2.1", 4, "00:00:00:00:02:01"));
             hostNextHopIps.add(new RouteEntry("10.0.3.1", 5, "00:00:00:00:03:01"));
         }
-        // TODO needed?
-        //if (getId() == 0x2) {
-        //}
         if (getId() == 0x3) {
             hostNextHopIps.add(new RouteEntry("7.7.7.7", 1, "00:00:07:07:07:07"));
         }

@@ -152,7 +152,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
             }
             synchronized (discoverers) {
                 if (!discoverers.containsKey(deviceId)) {
-                    // TODO: ideally, should never reach here
+                    // ideally, should never reach here
                     log.debug("Device mastership changed ({}) {}",
                             event.type(), deviceId);
                     discoverers.put(deviceId, new LinkDiscovery(device,
@@ -220,7 +220,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
                     ConnectPoint point = new ConnectPoint(deviceId,
                                                           port.number());
                     providerService.linksVanished(point);
-                    // TODO: Don't we need to removePort from ld?
+
                     break;
                 case DEVICE_REMOVED:
                 case DEVICE_SUSPENDED:

@@ -344,7 +344,6 @@ public class LinkDiscovery implements TimerTask {
     }
 
     private void sendProbes(Long portNumber) {
-        // TODO: should have suppression port configuration, not by type
         if (device.type() != Device.Type.ROADM) {
             log.trace("Sending probes out to {}@{}", portNumber, device.id());
             OutboundPacket pkt = this.createOutBoundLLDP(portNumber);

@@ -271,8 +271,6 @@ public class LLDPLinkProviderTest {
 
         @Override
         public void linkVanished(LinkDescription linkDescription) {
-            // TODO Auto-generated method stub
-
         }
 
         @Override

@@ -137,18 +137,6 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr
             providerService.deviceDisconnected(deviceId);
         } else {
             LOG.trace("Confirmed device {} connection", deviceId);
-            // FIXME require something like below to match javadoc description
-            // but this starts infinite loop with current DeviceManager
-//            final ChassisId cId = new ChassisId(dpid.value());
-//            final Type deviceType = device.type();
-//            DeviceDescription description =
-//                    new DefaultDeviceDescription(deviceId.uri(), deviceType,
-//                                                 sw.manfacturerDescription(),
-//                                                 sw.hardwareDescription(),
-//                                                 sw.softwareDescription(),
-//                                                 sw.serialNumber(),
-//                                                 cId);
-//            providerService.deviceConnected(deviceId, description);
         }
 
         // Prompt an update of port information. We can use any XID for this.
@@ -165,16 +153,6 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr
         }
     }
 
-    // Checks if the OF channel is connected.
-    //private boolean checkChannel(Device device, OpenFlowSwitch sw) {
-    // FIXME if possible, we might want this to be part of
-    // OpenFlowSwitch interface so the driver interface isn't misused.
-    //    if (sw == null || !((OpenFlowSwitchDriver) sw).isConnected()) {
-    //      return false;
-    //      }
-    //    return true;
-    // }
-
     @Override
     public void roleChanged(DeviceId deviceId, MastershipRole newRole) {
         switch (newRole) {

@@ -114,7 +114,7 @@ public class OpenFlowDeviceProviderTest {
 
     @Test
     public void triggerProbe() {
-        // TODO
+
     }
 
     @Test

@@ -186,7 +186,6 @@ public class FlowEntryBuilder {
                 }
                 break;
             case SET_FIELD:
-                // TODO handle 1.3 set field actions correctly
                 break;
             case SET_TP_DST:
             case SET_TP_SRC:

@@ -78,7 +78,7 @@ public class FlowModBuilderVer10 extends FlowModBuilder {
 
         long cookie = flowRule().id().value();
 
-        //TODO: what to do without bufferid? do we assume that there will be a pktout as well?
+
         OFFlowAdd fm = factory().buildFlowAdd()
                 .setXid(xid)
                 .setCookie(U64.of(cookie))
@@ -99,7 +99,7 @@ public class FlowModBuilderVer10 extends FlowModBuilder {
 
         long cookie = flowRule().id().value();
 
-        //TODO: what to do without bufferid? do we assume that there will be a pktout as well?
+
         OFFlowMod fm = factory().buildFlowModify()
                 .setXid(xid)
                 .setCookie(U64.of(cookie))

@@ -93,7 +93,7 @@ public class FlowModBuilderVer13 extends FlowModBuilder {
 
         long cookie = flowRule().id().value();
 
-        //TODO: what to do without bufferid? do we assume that there will be a pktout as well?
+
         OFFlowAdd fm = factory().buildFlowAdd()
                 .setXid(xid)
                 .setCookie(U64.of(cookie))
@@ -117,7 +117,7 @@ public class FlowModBuilderVer13 extends FlowModBuilder {
 
         long cookie = flowRule().id().value();
 
-        //TODO: what to do without bufferid? do we assume that there will be a pktout as well?
+
         OFFlowMod fm = factory().buildFlowModify()
                 .setXid(xid)
                 .setCookie(U64.of(cookie))
@@ -145,8 +145,6 @@ public class FlowModBuilderVer13 extends FlowModBuilder {
                 .setXid(xid)
                 .setCookie(U64.of(cookie))
                 .setBufferId(OFBufferId.NO_BUFFER)
-                //.setActions(actions) //FIXME do we want to send actions in flowdel?
-                //.setInstructions(Collections.singletonList(writeActions))
                 .setMatch(match)
                 .setFlags(Collections.singleton(OFFlowModFlags.SEND_FLOW_REM))
                 .setPriority(flowRule().priority())

@@ -142,7 +142,6 @@ public class OpenFlowHostProvider extends AbstractProvider implements HostProvid
 
             }
 
-            // TODO: Use DHCP packets as well later...
         }
 
     }

@@ -258,8 +258,6 @@ public class OpenFlowLinkProviderTest {
 
         @Override
         public void linkVanished(LinkDescription linkDescription) {
-            // TODO Auto-generated method stub
-
         }
 
         @Override

@@ -111,7 +111,6 @@ implements MutableGraph<V, E> {
 
     @Override
     public void removeVertex(V vertex) {
-        // TODO Auto-generated method stub
         if (vertexes != null && edges != null) {
             if (vertexes.contains(vertex)) {
                 vertexes.remove(vertex);
@@ -149,7 +148,6 @@ implements MutableGraph<V, E> {
 
     @Override
     public Graph<V, E> toImmutable() {
-        // TODO Auto-generated method stub
         return null;
     }
 

@@ -118,9 +118,7 @@ public abstract class BasePacket implements IPacket {
         } catch (final Exception e) {
             throw new RuntimeException("Could not clone packet");
         }
-        // TODO: we are using serialize()/deserialize() to perform the
-        // cloning. Not the most efficient way but simple. We can revisit
-        // if we hit performance problems.
+
         final byte[] data = this.serialize();
         pkt.deserialize(this.serialize(), 0, data.length);
         pkt.setParent(this.parent);

@@ -19,7 +19,6 @@ package org.onlab.packet;
  * The class representing a network device chassisId.
  * This class is immutable.
  */
-// TODO: Move this to a reasonable place.
 public final class ChassisId {
 
     private static final long UNKNOWN = 0;

@@ -262,7 +262,6 @@ public class IpAddress implements Comparable<IpAddress> {
      */
     public static IpAddress makeMaskedAddress(final IpAddress address,
                                               int prefixLength) {
-        // TODO: The code below should go away and replaced with generics
         if (address instanceof Ip4Address) {
             Ip4Address ip4a = (Ip4Address) address;
             return Ip4Address.makeMaskedAddress(ip4a, prefixLength);

@@ -86,7 +86,6 @@ public class NettyMessagingService implements MessagingService {
 
     private void initEventLoopGroup() {
         // try Epoll first and if that does work, use nio.
-        // TODO: make this configurable.
         try {
             clientGroup = new EpollEventLoopGroup();
             serverGroup = new EpollEventLoopGroup();
@@ -107,11 +106,9 @@ public class NettyMessagingService implements MessagingService {
     }
 
     public NettyMessagingService() {
-        // TODO: Default port should be configurable.
         this(8080);
     }
 
-    // FIXME: Constructor should not throw exceptions.
     public NettyMessagingService(int port) {
         try {
             localEp = new Endpoint(java.net.InetAddress.getLocalHost().getHostName(), port);
@@ -193,7 +190,6 @@ public class NettyMessagingService implements MessagingService {
 
     @Override
     public void registerHandler(String type, MessageHandler handler) {
-        // TODO: Is this the right semantics for handler registration?
         handlers.putIfAbsent(type, handler);
     }
 
@@ -210,7 +206,6 @@ public class NettyMessagingService implements MessagingService {
         ServerBootstrap b = new ServerBootstrap();
         b.option(ChannelOption.WRITE_BUFFER_HIGH_WATER_MARK, 32 * 1024);
         b.option(ChannelOption.WRITE_BUFFER_LOW_WATER_MARK, 8 * 1024);
-        // TODO: Need JVM options to configure PooledByteBufAllocator.
         b.option(ChannelOption.ALLOCATOR, PooledByteBufAllocator.DEFAULT);
         b.group(serverGroup, clientGroup)
             .channel(serverChannelClass)

@@ -19,7 +19,7 @@ package org.onlab.thirdparty;
 /**
  * Empty class required to get the onlab-thirdparty module to build properly.
  *
- * TODO Figure out how to remove this.
+ * NOTE Required for shade plugin to operate.
  */
 public class OnlabThirdparty {
 

@@ -54,7 +54,6 @@ import org.onosproject.net.intent.MultiPointToSinglePointIntent;
 import org.onosproject.net.link.LinkEvent;
 import org.onosproject.net.link.LinkListener;
 import org.onlab.osgi.ServiceDirectory;
-import org.onlab.packet.Ethernet;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -369,7 +368,6 @@ public class TopologyViewWebSocket
 
         // FIXME: clearly, this is not enough
         TrafficSelector selector = DefaultTrafficSelector.builder()
-                .matchEthType(Ethernet.TYPE_IPV4)
                 .matchEthDst(dstHost.mac()).build();
         TrafficTreatment treatment = DefaultTrafficTreatment.builder().build();