Ticket #20716: josm_20716_power_v3.patch

src/org/openstreetmap/josm/data/validation/tests/OverlappingWays.java

@@ -47 +47 @@
     /** Bag of all way segments */
     private MultiMap<Pair<Node, Node>, WaySegment> nodePairs;
 
-    private boolean onlyKnwonLinear;
+    private boolean onlyKnownLinear;
     private boolean includeOther;
     private boolean ignoreLayer;
 
@@ -83 +83 @@
         super.startTest(monitor);
         nodePairs = new MultiMap<>(1000);
         includeOther = isBeforeUpload ? ValidatorPrefHelper.PREF_OTHER_UPLOAD.get() : ValidatorPrefHelper.PREF_OTHER.get();
-        onlyKnwonLinear = Config.getPref().getBoolean("overlapping-ways.only-known-linear", true);
+        onlyKnownLinear = Config.getPref().getBoolean("overlapping-ways.only-known-linear", true);
         ignoreLayer = Config.getPref().getBoolean("overlapping-ways.ignore-layer", false);
     }
 
@@ -192 +192 @@
                 errortype = tr("Waterway shares segment with linear way");
                 type = OVERLAPPING_WATERWAY_LINEAR_WAY;
                 severity = Severity.WARNING;
-            } else if (!includeOther || onlyKnwonLinear) {
+            } else if (!includeOther || onlyKnownLinear) {
                 return;
             } else if (countHighway > 0) {
                 errortype = tr("Highway shares segment with other way");
@@ -269 +269 @@
         if (IGNORED.test(w))
             return;
 
-        if (onlyKnwonLinear && (w.concernsArea() || w.getInterestingTags().isEmpty()))
+        if (onlyKnownLinear && (w.concernsArea() || w.getInterestingTags().isEmpty()))
             return;
 
         Node lastN = null;

src/org/openstreetmap/josm/data/validation/tests/PowerLines.java

@@ -11 +11 @@
 import java.util.Set;
 
 import org.openstreetmap.josm.data.osm.Node;
+import org.openstreetmap.josm.data.osm.NodePair;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationMember;
@@ -25 +26 @@
 import org.openstreetmap.josm.tools.Geometry;
 
 /**
- * Checks for nodes in power lines/minor_lines that do not have a power=tower/pole tag.<br>
- * See #7812 for discussions about this test.
+ * Checks for nodes in power lines/minor_lines that do not have a power=tower/pole/portal tag and also for ways where
+ * are unusually long segments without line support feature.<br>
+ * See #7812 and #20716 for discussions about this test.
  */
 public class PowerLines extends Test {
 
     /** Test identifier */
     protected static final int POWER_LINES = 2501;
     protected static final int POWER_CONNECTION = 2502;
+    protected static final int POWER_SEGMENT_LENGTH = 2503;
 
     /** Values for {@code power} key interpreted as power lines */
     static final Collection<String> POWER_LINE_TAGS = Arrays.asList("line", "minor_line");
     /** Values for {@code power} key interpreted as power towers */
-    static final Collection<String> POWER_TOWER_TAGS = Arrays.asList("tower", "pole");
+    static final Collection<String> POWER_TOWER_TAGS = Arrays.asList("tower", "pole", "portal", "catenary_mast");
     /** Values for {@code power} key interpreted as power stations */
-    static final Collection<String> POWER_STATION_TAGS = Arrays.asList("station", "sub_station", "substation", "plant", "generator");
+    static final Collection<String> POWER_STATION_TAGS = Arrays.asList("generator", "plant", "substation");
     /** Values for {@code building} key interpreted as power stations */
     static final Collection<String> BUILDING_STATION_TAGS = Arrays.asList("transformer_tower");
     /** Values for {@code power} key interpreted as allowed power items */
-    static final Collection<String> POWER_ALLOWED_TAGS = Arrays.asList("switch", "transformer", "busbar", "generator", "switchgear",
-            "portal", "terminal", "insulator");
+    static final Collection<String> POWER_INFRASTRUCTURE_TAGS = Arrays.asList("compensator", "converter",
+            "generator", "insulator", "switch", "switchgear", "terminal", "transformer");
 
     private final Set<Node> badConnections = new LinkedHashSet<>();
-    private final Set<Node> missingTowerOrPole = new LinkedHashSet<>();
+    private final Set<Node> missingTag = new LinkedHashSet<>();
+    private final Set<NodePair> missingNode = new LinkedHashSet<>();
 
     private final List<OsmPrimitive> powerStations = new ArrayList<>();
 
@@ -58 +62 @@
         super(tr("Power lines"), tr("Checks for nodes in power lines that do not have a power=tower/pole tag."));
     }
 
-    @Override
-    public void visit(Way w) {
-        if (w.isUsable()) {
-            if (isPowerLine(w) && !w.hasTag("location", "underground")) {
-                for (Node n : w.getNodes()) {
-                    if (!isPowerTower(n) && !isPowerAllowed(n) && IN_DOWNLOADED_AREA.test(n)
-                        && (!w.isFirstLastNode(n) || !isPowerStation(n))) {
-                        missingTowerOrPole.add(n);
-                    }
-                }
-            } else if (w.isClosed() && isPowerStation(w)) {
-                powerStations.add(w);
-            }
-        }
-    }
-
     @Override
     public void visit(Node n) {
         boolean nodeInLineOrCable = false;
@@ -89 +77 @@
             badConnections.add(n);
     }
 
-    private static boolean isRelatedToPower(Way way) {
-        if (way.hasTag("power") || way.hasTag("building"))
-            return true;
-        for (OsmPrimitive ref : way.getReferrers()) {
-            if (ref instanceof Relation && ref.isMultipolygon() && (ref.hasTag("power") || ref.hasTag("building"))) {
-                for (RelationMember rm : ((Relation) ref).getMembers()) {
-                    if (way == rm.getMember())
-                        return true;
+    @Override
+    public void visit(Way w) {
+        if (w.isUsable()) {
+            if (isPowerLine(w) && !w.hasKey("line") && !w.hasTag("location", "underground")) {
+                float avgWayLength = (float) (w.getLength() / w.getRealNodesCount());
+                Node prevNode = w.firstNode();
+
+                for (Node n : w.getNodes()) {
+                    // handle power station power line connections (power=line + line=*)
+                    boolean skip = false;
+                    for (OsmPrimitive p : n.getReferrers()) {
+                        if (p instanceof Way && !p.equals(w) && isPowerLine((Way) p) && p.hasKey("line")) {
+                            skip = true;
+                            break;
+                        }
+                    }
+                    if (skip) {
+                        prevNode = n;
+                        continue;
+                    }
+
+                    // handle missing tags
+                    if (!isPowerTower(n) && !isPowerInfrastructure(n) && IN_DOWNLOADED_AREA.test(n)
+                            && (!w.isFirstLastNode(n) || !isPowerStation(n))) {
+                        missingTag.add(n);
+                    }
+
+                    // handle missing nodes
+                    double distance = n.getCoor().greatCircleDistance(prevNode.getCoor());
+                    double threshold = w.hasTag("power", "line") ? 1.6 : 1.8;
+                    if (w.getRealNodesCount() > 4 && distance > avgWayLength * threshold && !isPowerInfrastructure(n)) {
+                        missingNode.add(new NodePair(prevNode, n));
+                    }
+                    prevNode = n;
                 }
+            } else if (w.isClosed() && isPowerStation(w)) {
+                powerStations.add(w);
             }
         }
-        return false;
     }
 
     @Override
@@ -118 +133 @@
 
     @Override
     public void endTest() {
-        for (Node n : missingTowerOrPole) {
+        for (Node n : missingTag) {
             if (!isInPowerStation(n)) {
                 errors.add(TestError.builder(this, Severity.WARNING, POWER_LINES)
-                        .message(tr("Missing power tower/pole within power line"))
+                        .message(tr("Missing power line support tag from node"))
                         .primitives(n)
                         .build());
             }
@@ -130 +145 @@
         for (Node n : badConnections) {
             errors.add(TestError.builder(this, Severity.WARNING, POWER_CONNECTION)
                     .message(tr("Node connects a power line or cable with an object "
-                            + "which is not related to the power infrastructure."))
-                    .primitives(n).build());
+                            + "which is not related to the power infrastructure"))
+                    .primitives(n)
+                    .build());
         }
+
+        for (NodePair pair : missingNode) {
+            errors.add(TestError.builder(this, Severity.WARNING, POWER_SEGMENT_LENGTH)
+                    .message(tr("Missing line support node within power line"))
+                    .primitives(pair.getA(), pair.getB())
+                    .build());
+        }
+
         clearCollections();
         super.endTest();
     }
 
+    private static boolean isRelatedToPower(Way way) {
+        if (way.hasTag("power") || way.hasTag("building"))
+            return true;
+        for (OsmPrimitive ref : way.getReferrers()) {
+            if (ref instanceof Relation && ref.isMultipolygon() && (ref.hasTag("power") || ref.hasTag("building"))) {
+                for (RelationMember rm : ((Relation) ref).getMembers()) {
+                    if (way == rm.getMember())
+                        return true;
+                }
+            }
+        }
+        return false;
+    }
+
     protected final boolean isInPowerStation(Node n) {
         for (OsmPrimitive station : powerStations) {
             List<List<Node>> nodesLists = new ArrayList<>();
@@ -171 +209 @@
     /**
      * Determines if the specified primitive denotes a power station.
      * @param p The primitive to be tested
-     * @return {@code true} if power key is set and equal to station/sub_station/plant
+     * @return {@code true} if power key is set and equal to generator/substation/plant
      */
     protected static final boolean isPowerStation(OsmPrimitive p) {
         return isPowerIn(p, POWER_STATION_TAGS) || isBuildingIn(p, BUILDING_STATION_TAGS);
     }
 
     /**
-     * Determines if the specified node denotes a power tower/pole.
+     * Determines if the specified node denotes a power tower/pole/portal/catenary_mast.
      * @param n The node to be tested
-     * @return {@code true} if power key is set and equal to tower/pole
+     * @return {@code true} if power key is set and equal to tower/pole/portal/catenary_mast
      */
     protected static final boolean isPowerTower(Node n) {
         return isPowerIn(n, POWER_TOWER_TAGS);
@@ -189 +227 @@
     /**
      * Determines if the specified node denotes a power infrastructure allowed on a power line.
      * @param n The node to be tested
-     * @return True if power key is set and equal to switch/tranformer/busbar/generator
+     * @return True if power key is set and equal to compensator/converter/generator/insulator
+     * /switch/switchgear/terminal/transformer
      */
-    protected static final boolean isPowerAllowed(Node n) {
-        return isPowerIn(n, POWER_ALLOWED_TAGS);
+    protected static final boolean isPowerInfrastructure(Node n) {
+        return isPowerIn(n, POWER_INFRASTRUCTURE_TAGS);
     }
 
     /**
@@ -218 +257 @@
     private void clearCollections() {
         powerStations.clear();
         badConnections.clear();
-        missingTowerOrPole.clear();
+        missingTag.clear();
+        missingNode.clear();
     }
 }