fixes for Meter Service

Change-Id: I83d5b8a2e0a955c050f7afe96761d5709d4f9f18
diff --git a/cli/src/main/java/org/onosproject/cli/net/AddMeter.java b/cli/src/main/java/org/onosproject/cli/net/AddMeter.java
new file mode 100644
index 0000000..25a7726
--- /dev/null
+++ b/cli/src/main/java/org/onosproject/cli/net/AddMeter.java
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2015 Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.onosproject.cli.net;
+
+import org.apache.karaf.shell.commands.Argument;
+import org.apache.karaf.shell.commands.Command;
+import org.onosproject.cli.AbstractShellCommand;
+import org.onosproject.core.CoreService;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.meter.Band;
+import org.onosproject.net.meter.DefaultBand;
+import org.onosproject.net.meter.DefaultMeter;
+import org.onosproject.net.meter.Meter;
+import org.onosproject.net.meter.MeterId;
+import org.onosproject.net.meter.MeterOperation;
+import org.onosproject.net.meter.MeterService;
+
+import java.util.Collections;
+
+/**
+ * Add a meter.
+ */
+@Command(scope = "onos", name = "add-meter",
+        description = "Adds a meter to a device (currently for testing)")
+public class AddMeter extends AbstractShellCommand {
+
+    @Argument(index = 0, name = "uri", description = "Device ID",
+            required = true, multiValued = false)
+    String uri = null;
+
+    private final String appId = "org.onosproject.cli.addMeter";
+
+    @Override
+    protected void execute() {
+        MeterService service = get(MeterService.class);
+        CoreService coreService = get(CoreService.class);
+
+        DeviceId deviceId = DeviceId.deviceId(uri);
+
+        MeterId meterId = service.allocateMeterId();
+
+        Band band = DefaultBand.builder()
+                        .ofType(Band.Type.DROP)
+                        .withRate(500)
+                        .build();
+
+
+        Meter meter = DefaultMeter.builder()
+                .forDevice(deviceId)
+                .fromApp(coreService.registerApplication(appId))
+                .withId(meterId)
+                .withUnit(Meter.Unit.KB_PER_SEC)
+                .withBands(Collections.singleton(band))
+                .build();
+
+        MeterOperation op = new MeterOperation(meter, MeterOperation.Type.ADD, null);
+
+        service.addMeter(op);
+
+    }
+}
diff --git a/cli/src/main/java/org/onosproject/cli/net/Meters.java b/cli/src/main/java/org/onosproject/cli/net/Meters.java
new file mode 100644
index 0000000..76f3813
--- /dev/null
+++ b/cli/src/main/java/org/onosproject/cli/net/Meters.java
@@ -0,0 +1,58 @@
+/*
+ * Copyright 2015 Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.onosproject.cli.net;
+
+import com.google.common.collect.Collections2;
+import org.apache.karaf.shell.commands.Argument;
+import org.apache.karaf.shell.commands.Command;
+import org.onosproject.cli.AbstractShellCommand;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.meter.Meter;
+import org.onosproject.net.meter.MeterService;
+
+import java.util.Collection;
+
+/**
+ * Add a meter.
+ */
+@Command(scope = "onos", name = "meters",
+        description = "Shows meters")
+public class Meters extends AbstractShellCommand {
+
+    @Argument(index = 0, name = "uri", description = "Device ID",
+            required = false, multiValued = false)
+    String uri = null;
+
+
+    @Override
+    protected void execute() {
+        MeterService service = get(MeterService.class);
+
+        Collection<Meter> meters = service.getAllMeters();
+        if (uri != null) {
+            DeviceId deviceId = DeviceId.deviceId(uri);
+            Collection<Meter> devMeters = Collections2.filter(meters,
+                                                              m -> m.deviceId().equals(deviceId));
+            printMeters(devMeters);
+        } else {
+            printMeters(meters);
+        }
+    }
+
+    private void printMeters(Collection<Meter> meters) {
+        meters.forEach(m -> print(" %s", m));
+    }
+}
diff --git a/cli/src/main/resources/OSGI-INF/blueprint/shell-config.xml b/cli/src/main/resources/OSGI-INF/blueprint/shell-config.xml
index 4912c82..75680d8 100644
--- a/cli/src/main/resources/OSGI-INF/blueprint/shell-config.xml
+++ b/cli/src/main/resources/OSGI-INF/blueprint/shell-config.xml
@@ -113,6 +113,18 @@
             </completers>
         </command>
         <command>
+            <action class="org.onosproject.cli.net.AddMeter"/>
+            <completers>
+                <ref component-id="deviceIdCompleter"/>
+            </completers>
+        </command>
+        <command>
+            <action class="org.onosproject.cli.net.Meters"/>
+            <completers>
+                <ref component-id="deviceIdCompleter"/>
+            </completers>
+        </command>
+        <command>
             <action class="org.onosproject.cli.net.DeviceRoleCommand"/>
             <completers>
                 <ref component-id="deviceIdCompleter"/>
diff --git a/core/api/src/main/java/org/onosproject/net/meter/DefaultBand.java b/core/api/src/main/java/org/onosproject/net/meter/DefaultBand.java
index a3a4fc2..58a2476 100644
--- a/core/api/src/main/java/org/onosproject/net/meter/DefaultBand.java
+++ b/core/api/src/main/java/org/onosproject/net/meter/DefaultBand.java
@@ -15,6 +15,7 @@
  */
 package org.onosproject.net.meter;
 
+import static com.google.common.base.MoreObjects.toStringHelper;
 import static com.google.common.base.Preconditions.checkArgument;
 
 /**
@@ -24,13 +25,14 @@
 
     private final Type type;
     private final long rate;
-    private final long burstSize;
-    private final short prec;
+    //TODO: should be made optional
+    private final Long burstSize;
+    private final Short prec;
     private long packets;
     private long bytes;
 
     public DefaultBand(Type type, long rate,
-                       long burstSize, short prec) {
+                       Long burstSize, Short prec) {
         this.type = type;
         this.rate = rate;
         this.burstSize = burstSize;
@@ -77,6 +79,15 @@
         this.bytes = bytes;
     }
 
+    @Override
+    public String toString() {
+        return toStringHelper(this)
+                .add("rate", rate)
+                .add("burst-size", burstSize)
+                .add("type", type)
+                .add("drop-precedence", prec).toString();
+    }
+
     public static Builder builder() {
         return new Builder();
     }
@@ -84,7 +95,7 @@
     public static final class Builder implements Band.Builder {
 
         private long rate;
-        private long burstSize;
+        private Long burstSize;
         private Short prec;
         private Type type;
 
@@ -114,7 +125,7 @@
 
         @Override
         public DefaultBand build() {
-            checkArgument(prec != null && type == Type.REMARK,
+            checkArgument(type != Type.REMARK && prec == null,
                           "Only REMARK bands can have a precendence.");
 
             return new DefaultBand(type, rate, burstSize, prec);
diff --git a/core/api/src/main/java/org/onosproject/net/meter/DefaultMeter.java b/core/api/src/main/java/org/onosproject/net/meter/DefaultMeter.java
index 611a23e..ab6b444 100644
--- a/core/api/src/main/java/org/onosproject/net/meter/DefaultMeter.java
+++ b/core/api/src/main/java/org/onosproject/net/meter/DefaultMeter.java
@@ -15,12 +15,13 @@
  */
 package org.onosproject.net.meter;
 
+import com.google.common.collect.ImmutableSet;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.net.DeviceId;
 
 import java.util.Collection;
-import java.util.Collections;
 
+import static com.google.common.base.MoreObjects.toStringHelper;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 
@@ -138,6 +139,18 @@
         this.bytes = bytes;
     }
 
+    @Override
+    public String toString() {
+        return toStringHelper(this)
+                .add("device", deviceId)
+                .add("id", id)
+                .add("appId", appId.name())
+                .add("unit", unit)
+                .add("isBurst", burst)
+                .add("state", state)
+                .add("bands", bands).toString();
+    }
+
     public static final class Builder implements Meter.Builder {
 
         private MeterId id;
@@ -155,8 +168,8 @@
         }
 
         @Override
-        public Meter.Builder withId(long id) {
-            this.id = MeterId.meterId(id);
+        public Meter.Builder withId(MeterId id) {
+            this.id = id;
             return this;
         }
 
@@ -180,7 +193,7 @@
 
         @Override
         public Meter.Builder withBands(Collection<Band> bands) {
-            this.bands = Collections.unmodifiableCollection(bands);
+            this.bands = ImmutableSet.copyOf(bands);
             return this;
         }
 
diff --git a/core/api/src/main/java/org/onosproject/net/meter/Meter.java b/core/api/src/main/java/org/onosproject/net/meter/Meter.java
index 5181c92..8ea04b0 100644
--- a/core/api/src/main/java/org/onosproject/net/meter/Meter.java
+++ b/core/api/src/main/java/org/onosproject/net/meter/Meter.java
@@ -130,10 +130,10 @@
         /**
          * Assigns the id to this meter.
          *
-         * @param id a long
+         * @param id a meter id
          * @return this
          */
-        Builder withId(long id);
+        Builder withId(MeterId id);
 
         /**
          * Assigns the application that built this meter.
diff --git a/core/api/src/main/java/org/onosproject/net/meter/MeterId.java b/core/api/src/main/java/org/onosproject/net/meter/MeterId.java
index f479ac5..872de2d 100644
--- a/core/api/src/main/java/org/onosproject/net/meter/MeterId.java
+++ b/core/api/src/main/java/org/onosproject/net/meter/MeterId.java
@@ -32,7 +32,7 @@
     public static final MeterId ALL = new MeterId(0xFFFFFFFF);
 
     private MeterId(long id) {
-        checkArgument(id <= MAX, "id cannot be larger than 0xFFFF0000");
+        checkArgument(id >= MAX, "id cannot be larger than 0xFFFF0000");
         this.id = id;
     }
 
@@ -65,6 +65,11 @@
         return Long.hashCode(id);
     }
 
+    @Override
+    public String toString() {
+        return Long.toHexString(this.id);
+    }
+
     public static MeterId meterId(long id) {
         return new MeterId(id);
 
diff --git a/core/api/src/main/java/org/onosproject/net/meter/MeterService.java b/core/api/src/main/java/org/onosproject/net/meter/MeterService.java
index eaa2a50..31aa17f 100644
--- a/core/api/src/main/java/org/onosproject/net/meter/MeterService.java
+++ b/core/api/src/main/java/org/onosproject/net/meter/MeterService.java
@@ -17,6 +17,8 @@
 
 import org.onosproject.event.ListenerService;
 
+import java.util.Collection;
+
 /**
  * Service for add/updating and removing meters. Meters are
  * are assigned to flow to rate limit them and provide a certain
@@ -55,6 +57,13 @@
     Meter getMeter(MeterId id);
 
     /**
+     * Fetches all the meters.
+     *
+     * @return a collection of meters
+     */
+    Collection<Meter> getAllMeters();
+
+    /**
      * Allocate a meter id which must be used to create the meter.
      *
      * @return a meter id
diff --git a/incubator/net/src/main/java/org/onosproject/incubator/net/meter/impl/MeterManager.java b/incubator/net/src/main/java/org/onosproject/incubator/net/meter/impl/MeterManager.java
index a0c1dfc..4e75301 100644
--- a/incubator/net/src/main/java/org/onosproject/incubator/net/meter/impl/MeterManager.java
+++ b/incubator/net/src/main/java/org/onosproject/incubator/net/meter/impl/MeterManager.java
@@ -52,7 +52,7 @@
 /**
  * Provides implementation of the meter service APIs.
  */
-@Component(immediate = true)
+@Component(immediate = true, enabled = true)
 @Service
 public class MeterManager extends AbstractListenerProviderRegistry<MeterEvent, MeterListener,
         MeterProvider, MeterProviderService>
@@ -66,7 +66,7 @@
     protected StorageService storageService;
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
-    MeterStore store;
+    protected MeterStore store;
 
     private AtomicCounter meterIdCounter;
 
@@ -78,6 +78,8 @@
                 .withName(meterIdentifier)
                 .build();
 
+        store.setDelegate(delegate);
+
         onComplete = (op, result, error) ->
             {
                 op.context().ifPresent(c -> {
@@ -98,6 +100,7 @@
 
     @Deactivate
     public void deactivate() {
+        store.unsetDelegate(delegate);
         log.info("Stopped");
     }
 
@@ -136,6 +139,11 @@
     }
 
     @Override
+    public Collection<Meter> getAllMeters() {
+        return store.getAllMeters();
+    }
+
+    @Override
     public MeterId allocateMeterId() {
         // FIXME: This will break one day.
         return MeterId.meterId((int) meterIdCounter.getAndIncrement());
diff --git a/incubator/store/src/main/java/org/onosproject/incubator/store/meter/impl/DistributedMeterStore.java b/incubator/store/src/main/java/org/onosproject/incubator/store/meter/impl/DistributedMeterStore.java
index ed7c9bf..bc8456e 100644
--- a/incubator/store/src/main/java/org/onosproject/incubator/store/meter/impl/DistributedMeterStore.java
+++ b/incubator/store/src/main/java/org/onosproject/incubator/store/meter/impl/DistributedMeterStore.java
@@ -18,11 +18,16 @@
 import com.google.common.collect.Collections2;
 import com.google.common.collect.Maps;
 import org.apache.felix.scr.annotations.Activate;
+import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
+import org.apache.felix.scr.annotations.Service;
 import org.onosproject.cluster.ClusterService;
 import org.onosproject.cluster.NodeId;
+import org.onosproject.mastership.MastershipService;
+import org.onosproject.net.meter.Band;
+import org.onosproject.net.meter.DefaultBand;
 import org.onosproject.net.meter.DefaultMeter;
 import org.onosproject.net.meter.Meter;
 import org.onosproject.net.meter.MeterEvent;
@@ -33,7 +38,6 @@
 import org.onosproject.net.meter.MeterStore;
 import org.onosproject.net.meter.MeterStoreDelegate;
 import org.onosproject.net.meter.MeterStoreResult;
-import org.onosproject.mastership.MastershipService;
 import org.onosproject.store.AbstractStore;
 import org.onosproject.store.serializers.KryoNamespaces;
 import org.onosproject.store.service.ConsistentMap;
@@ -56,6 +60,8 @@
  * A distributed meter store implementation. Meters are stored consistently
  * across the cluster.
  */
+@Component(immediate = true)
+@Service
 public class DistributedMeterStore extends AbstractStore<MeterEvent, MeterStoreDelegate>
                     implements MeterStore {
 
@@ -89,8 +95,14 @@
         meters = storageService.<MeterId, MeterData>consistentMapBuilder()
                     .withName(METERSTORE)
                     .withSerializer(Serializer.using(Arrays.asList(KryoNamespaces.API),
-                                                     MeterData.class))
-                    .build();
+                                                     MeterData.class,
+                                                     DefaultMeter.class,
+                                                     DefaultBand.class,
+                                                     Band.Type.class,
+                                                     MeterState.class,
+                                                     Meter.Unit.class,
+                                                     MeterFailReason.class,
+                                                     MeterId.class)).build();
 
         meters.addListener(mapListener);
 
@@ -205,13 +217,13 @@
                                 } else if (data.reason().isPresent() && local.equals(data.origin())) {
                                     MeterStoreResult msr = MeterStoreResult.fail(data.reason().get());
                                     //TODO: No future -> no friend
-                                    futures.get(data.meter().id()).complete(msr);
+                                    futures.remove(data.meter().id()).complete(msr);
                                 }
                                 break;
                             case ADDED:
                             case REMOVED:
                                 if (local.equals(data.origin())) {
-                                    futures.get(data.meter().id()).complete(MeterStoreResult.success());
+                                    futures.remove(data.meter().id()).complete(MeterStoreResult.success());
                                 }
                                 break;
                             default:
diff --git a/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/impl/MeterModBuilder.java b/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/impl/MeterModBuilder.java
index 930042b..c07354b 100644
--- a/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/impl/MeterModBuilder.java
+++ b/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/impl/MeterModBuilder.java
@@ -116,7 +116,8 @@
             default:
                 log.warn("Unknown unit type {}", unit);
         }
-        builder.setBands(buildBands());
+        //FIXME: THIS WILL CHANGE IN OF1.4 to setBands.
+        builder.setMeters(buildBands());
         builder.setFlags(flags)
                 .setMeterId(id)
                 .setXid(xid);
diff --git a/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/impl/OpenFlowMeterProvider.java b/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/impl/OpenFlowMeterProvider.java
index 2e1611d..f5a777b 100644
--- a/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/impl/OpenFlowMeterProvider.java
+++ b/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/impl/OpenFlowMeterProvider.java
@@ -33,6 +33,7 @@
 import org.onosproject.net.meter.DefaultMeter;
 import org.onosproject.net.meter.Meter;
 import org.onosproject.net.meter.MeterFailReason;
+import org.onosproject.net.meter.MeterId;
 import org.onosproject.net.meter.MeterOperation;
 import org.onosproject.net.meter.MeterOperations;
 import org.onosproject.net.meter.MeterProvider;
@@ -74,7 +75,7 @@
 /**
  * Provider which uses an OpenFlow controller to handle meters.
  */
-@Component(immediate = true, enabled = false)
+@Component(immediate = true, enabled = true)
 public class OpenFlowMeterProvider extends AbstractProvider implements MeterProvider {
 
 
@@ -245,7 +246,7 @@
             DefaultMeter.Builder builder = DefaultMeter.builder();
             Collection<Band> bands = buildBands(stat.getBandStats());
             builder.forDevice(deviceId)
-                    .withId(stat.getMeterId())
+                    .withId(MeterId.meterId(stat.getMeterId()))
                     //FIXME: need to encode appId in meter id, but that makes
                     // things a little annoying for debugging
                     .fromApp(coreService.getAppId("org.onosproject.core"))