diff --git a/core/api/src/main/java/org/onosproject/net/config/basics/BasicElementConfig.java b/core/api/src/main/java/org/onosproject/net/config/basics/BasicElementConfig.java index 3033b3cb0c..74257e6567 100644 --- a/core/api/src/main/java/org/onosproject/net/config/basics/BasicElementConfig.java +++ b/core/api/src/main/java/org/onosproject/net/config/basics/BasicElementConfig.java @@ -30,12 +30,13 @@ public abstract class BasicElementConfig extends AllowedEntityConfig { protected static final String RACK_ADDRESS = "rackAddress"; protected static final String OWNER = "owner"; - protected static final double DEFAULT_COORD = -1.0; + protected static final double ZERO_THRESHOLD = Double.MIN_VALUE * 2.0; + private static final double DEFAULT_COORD = 0.0; /** * Returns friendly label for the element. * - * @return friendly label or element id itself if not set + * @return friendly label or element identifier itself if not set */ public String name() { return get(NAME, subject.toString()); @@ -51,10 +52,25 @@ public abstract class BasicElementConfig extends AllowedEntityConfig { return (BasicElementConfig) setOrClear(NAME, name); } + private static boolean doubleIsZero(double value) { + return value >= -ZERO_THRESHOLD && value <= ZERO_THRESHOLD; + } + + /** + * Returns true if the geographical coordinates (latitude and longitude) + * are set on this element. + * + * @return true if geo-coordinates are set + */ + public boolean geoCoordsSet() { + return !doubleIsZero(latitude()) || !doubleIsZero(longitude()); + } + /** * Returns element latitude. * - * @return element latitude; -1 if not set + * @return element latitude; 0.0 if (possibly) not set + * @see #geoCoordsSet() */ public double latitude() { return get(LATITUDE, DEFAULT_COORD); @@ -71,9 +87,10 @@ public abstract class BasicElementConfig extends AllowedEntityConfig { } /** - * Returns element latitude. + * Returns element longitude. * - * @return element latitude; -1 if not set + * @return element longitude; 0 if (possibly) not set + * @see #geoCoordsSet() */ public double longitude() { return get(LONGITUDE, DEFAULT_COORD); diff --git a/core/api/src/test/java/org/onosproject/net/config/basics/BasicElementConfigTest.java b/core/api/src/test/java/org/onosproject/net/config/basics/BasicElementConfigTest.java new file mode 100644 index 0000000000..63040af602 --- /dev/null +++ b/core/api/src/test/java/org/onosproject/net/config/basics/BasicElementConfigTest.java @@ -0,0 +1,98 @@ +/* + * Copyright 2016-present 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.net.config.basics; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.onosproject.net.config.basics.BasicElementConfig.ZERO_THRESHOLD; + +/** + * Unit tests for {@link BasicElementConfig}. + */ +public class BasicElementConfigTest { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + private static final String E1 = "e1"; + + // concrete subclass of abstract class we are testing + private static class ElmCfg extends BasicElementConfig { + ElmCfg() { + object = MAPPER.createObjectNode(); + } + + @Override + public String toString() { + return object.toString(); + } + } + + private static void print(String fmt, Object... args) { + System.out.println(String.format(fmt, args)); + } + + private static void print(Object o) { + print("%s", o); + } + + private BasicElementConfig cfg; + + @Before + public void setUp() { + cfg = new ElmCfg().name(E1); + } + + @Test + public void basicNoGeo() { + print(cfg); + assertFalse("geo set?", cfg.geoCoordsSet()); + assertEquals("lat", 0.0, cfg.latitude(), ZERO_THRESHOLD); + assertEquals("lon", 0.0, cfg.longitude(), ZERO_THRESHOLD); + } + + @Test + public void geoLatitudeOnly() { + cfg.latitude(0.1); + print(cfg); + assertTrue("geo NOT set", cfg.geoCoordsSet()); + assertEquals("lat", 0.1, cfg.latitude(), ZERO_THRESHOLD); + assertEquals("lon", 0.0, cfg.longitude(), ZERO_THRESHOLD); + } + + @Test + public void geoLongitudeOnly() { + cfg.longitude(-0.1); + print(cfg); + assertTrue("geo NOT set", cfg.geoCoordsSet()); + assertEquals("lat", 0.0, cfg.latitude(), ZERO_THRESHOLD); + assertEquals("lon", -0.1, cfg.longitude(), ZERO_THRESHOLD); + } + + @Test + public void geoLatLong() { + cfg.latitude(3.1415).longitude(2.71828); + print(cfg); + assertTrue("geo NOT set", cfg.geoCoordsSet()); + assertEquals("lat", 3.1415, cfg.latitude(), ZERO_THRESHOLD); + assertEquals("lon", 2.71828, cfg.longitude(), ZERO_THRESHOLD); + } +} diff --git a/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java b/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java index 9645c5fba9..255ee46858 100644 --- a/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java +++ b/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java @@ -34,8 +34,6 @@ import java.util.Set; */ public final class BasicHostOperator implements ConfigOperator { - protected static final double DEFAULT_COORD = -1.0; - private BasicHostOperator() { } @@ -81,10 +79,8 @@ public final class BasicHostOperator implements ConfigOperator { if (cfg.name() != null) { newBuilder.set(AnnotationKeys.NAME, cfg.name()); } - if (cfg.latitude() != DEFAULT_COORD) { + if (cfg.geoCoordsSet()) { newBuilder.set(AnnotationKeys.LATITUDE, Double.toString(cfg.latitude())); - } - if (cfg.longitude() != DEFAULT_COORD) { newBuilder.set(AnnotationKeys.LONGITUDE, Double.toString(cfg.longitude())); } if (cfg.rackAddress() != null) { diff --git a/core/net/src/test/java/org/onosproject/net/host/impl/BasicHostOperatorTest.java b/core/net/src/test/java/org/onosproject/net/host/impl/BasicHostOperatorTest.java index caf36d144d..3d1331a24b 100644 --- a/core/net/src/test/java/org/onosproject/net/host/impl/BasicHostOperatorTest.java +++ b/core/net/src/test/java/org/onosproject/net/host/impl/BasicHostOperatorTest.java @@ -54,18 +54,20 @@ public class BasicHostOperatorTest { private static final BasicHostConfig BHC = new BasicHostConfig(); private static final String NAME = "testhost"; private static final double LAT = 40.96; + private static final double LON = 0.0; @Before public void setUp() { BHC.init(ID, "test", JsonNodeFactory.instance.objectNode(), mapper, delegate); BHC.name(NAME).latitude(40.96); + // if you set lat or long, the other becomes valid as 0.0 (not null) } @Test public void testDescOps() { HostDescription desc = BasicHostOperator.combine(BHC, HOST); assertEquals(NAME, desc.annotations().value(AnnotationKeys.NAME)); - assertEquals(null, desc.annotations().value(AnnotationKeys.LONGITUDE)); + assertEquals(String.valueOf(LON), desc.annotations().value(AnnotationKeys.LONGITUDE)); assertEquals(String.valueOf(LAT), desc.annotations().value(AnnotationKeys.LATITUDE)); } } diff --git a/tools/test/topos/regions-topo-2 b/tools/test/topos/regions-topo-2 index befed3b7c5..297bc657c2 100755 --- a/tools/test/topos/regions-topo-2 +++ b/tools/test/topos/regions-topo-2 @@ -7,12 +7,15 @@ onos ${host} null-simulation stop custom onos ${host} wipe-out please onos ${host} null-simulation start custom +# null-create-device <#ports> +# null-create-link + onos ${host} <<-EOF -null-create-device switch s1 10 0 0 -null-create-device switch s2 10 0 0 -null-create-device switch s3 10 0 0 -null-create-device switch s4 10 0 0 +null-create-device switch s1-Bristol 10 51.4500 -2.5833 +null-create-device switch s2-London 10 51.5072 -0.1275 +null-create-device switch s3-Dover 10 51.1295 1.3089 +null-create-device switch s4-Brighton 10 50.8429 -0.1313 null-create-device switch s5 10 0 0 null-create-device switch s6 10 0 0 null-create-device switch s7 10 0 0 @@ -20,11 +23,11 @@ null-create-device switch s8 10 0 0 null-create-device switch s9 10 0 0 # null-create-device switch s10 10 0 0 -null-create-link direct s1 s2 -null-create-link direct s2 s3 -null-create-link direct s2 s4 -null-create-link direct s3 s4 -null-create-link direct s3 s5 +null-create-link direct s1-Bristol s2-London +null-create-link direct s2-London s3-Dover +null-create-link direct s2-London s4-Brighton +null-create-link direct s3-Dover s4-Brighton +null-create-link direct s3-Dover s5 null-create-link direct s6 s5 null-create-link direct s6 s8 null-create-link direct s7 s9 diff --git a/web/gui/src/main/java/org/onosproject/ui/impl/TopologyViewMessageHandlerBase.java b/web/gui/src/main/java/org/onosproject/ui/impl/TopologyViewMessageHandlerBase.java index 872a0998b3..985cb17e13 100644 --- a/web/gui/src/main/java/org/onosproject/ui/impl/TopologyViewMessageHandlerBase.java +++ b/web/gui/src/main/java/org/onosproject/ui/impl/TopologyViewMessageHandlerBase.java @@ -31,7 +31,6 @@ import org.onosproject.incubator.net.tunnel.OpticalTunnelEndPoint; import org.onosproject.incubator.net.tunnel.Tunnel; import org.onosproject.incubator.net.tunnel.TunnelService; import org.onosproject.mastership.MastershipService; -import org.onosproject.net.ElementId; import org.onosproject.net.Annotated; import org.onosproject.net.AnnotationKeys; import org.onosproject.net.Annotations; @@ -40,6 +39,7 @@ import org.onosproject.net.DefaultEdgeLink; import org.onosproject.net.Device; import org.onosproject.net.DeviceId; import org.onosproject.net.EdgeLink; +import org.onosproject.net.ElementId; import org.onosproject.net.Host; import org.onosproject.net.HostId; import org.onosproject.net.HostLocation; @@ -77,8 +77,8 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.Optional; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import static com.google.common.base.Preconditions.checkNotNull; @@ -94,6 +94,8 @@ import static org.onosproject.ui.topo.TopoUtils.compactLinkString; */ public abstract class TopologyViewMessageHandlerBase extends UiMessageHandler { + private static final String NO_GEO_VALUE = "0.0"; + // default to an "add" event... private static final DefaultHashMap CLUSTER_EVENT = new DefaultHashMap<>("addInstance"); @@ -361,10 +363,10 @@ public abstract class TopologyViewMessageHandlerBase extends UiMessageHandler { String slng = annotations.value(AnnotationKeys.LONGITUDE); String slat = annotations.value(AnnotationKeys.LATITUDE); - boolean haveLng = slng != null && !slng.isEmpty(); - boolean haveLat = slat != null && !slat.isEmpty(); - try { - if (haveLng && haveLat) { + boolean validLng = slng != null && !slng.equals(NO_GEO_VALUE); + boolean validLat = slat != null && !slat.equals(NO_GEO_VALUE); + if (validLat && validLng) { + try { double lng = Double.parseDouble(slng); double lat = Double.parseDouble(slat); ObjectNode loc = objectNode() @@ -372,11 +374,9 @@ public abstract class TopologyViewMessageHandlerBase extends UiMessageHandler { .put("lng", lng) .put("lat", lat); payload.set("location", loc); - } else { - log.trace("missing Lng/Lat: lng={}, lat={}", slng, slat); + } catch (NumberFormatException e) { + log.warn("Invalid geo data: longitude={}, latitude={}", slng, slat); } - } catch (NumberFormatException e) { - log.warn("Invalid geo data: longitude={}, latitude={}", slng, slat); } }