Ticket #20716: josm_20716_power_v7_reupload.patch

src/org/openstreetmap/josm/data/osm/Way.java

@@ -650 +650 @@
         return length;
     }
 
+    /**
+     * Calculates the segment lengths of the way as computed by {@link LatLon#greatCircleDistance}.
+     * @return The segment lengths of a way in metres, from the first to last node
+     * @since xxx
+     */
+    public double[] getSegmentLengths() {
+        double[] segmentLengths = new double[nodes.length - 1];
+        Node prevNode = this.firstNode();
+
+        for (int i = 1; i < nodes.length; i++) {
+            Node n = nodes[i];
+
+            double distance = n.getCoor().greatCircleDistance(prevNode.getCoor());
+            segmentLengths[i - 1] = distance;
+            prevNode = n;
+        }
+
+        return segmentLengths;
+    }
+
     /**
      * Replies the length of the longest segment of the way, in metres, as computed by {@link LatLon#greatCircleDistance}.
      * @return The length of the segment, in metres

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

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.data.validation.tests;
 
+import static org.openstreetmap.josm.gui.MainApplication.getLayerManager;
 import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.LinkedHashSet;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.stream.Collectors;
 
+import org.openstreetmap.josm.data.coor.EastNorth;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationMember;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.data.osm.WaySegment;
 import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon;
 import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.JoinedWay;
 import org.openstreetmap.josm.data.osm.visitor.paint.relations.MultipolygonCache;
@@ -23 +28 @@
 import org.openstreetmap.josm.data.validation.TestError;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.openstreetmap.josm.tools.Geometry;
+import org.openstreetmap.josm.tools.Pair;
 
 /**
- * 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
+ * <ul>
+ * <li>nodes in power lines/minor_lines that do not have a power=tower/pole/portal tag
+ * <li>nodes where the reference numbering not consistent
+ * <li>ways where are unusually long segments without line support feature
+ * <li>ways where the line type is possibly misused
+ * </ul>
+ * See #7812 and #20716 for discussions about this test.
  */
 public class PowerLines extends Test {
 
-    /** Test identifier */
-    protected static final int POWER_LINES = 2501;
+    /** Test identifiers */
+    protected static final int POWER_SUPPORT = 2501;
     protected static final int POWER_CONNECTION = 2502;
+    protected static final int POWER_SEGMENT_LENGTH = 2503;
+    protected static final int POWER_REF = 2504;
+    protected static final int POWER_LINE_TYPE = 2505;
 
     /** 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("catenary_mast", "pole", "portal", "tower");
     /** 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> badConnections = new HashSet<>();
+    private final Set<Node> missingTags = new HashSet<>();
+    private final Set<Node> refDiscontinuities = new HashSet<>();
+    private final Set<Way> wrongLineType = new HashSet<>();
+    private final Set<WaySegment> missingNodes = new HashSet<>();
 
     private final List<OsmPrimitive> powerStations = new ArrayList<>();
 
+    private final Collection<Way> datasetWaterways = new HashSet<>(64);
+
     /**
      * Constructs a new {@code PowerLines} test.
      */
     public PowerLines() {
-        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);
-            }
-        }
+        super(tr("Power lines"), tr("Checks if power line missing a support node and " +
+                "for nodes in power lines that do not have a power=tower/pole tag"));
+    }
+
+    /**
+     * Power line support features ref=* numbering direction.
+     */
+    private enum NumberingDirection {
+        /** No direction */
+        NONE,
+        /** Numbering follows way direction */
+        SAME,
+        /** Numbering goes opposite way direction */
+        OPPOSITE,
+        /** Not distinguishable numbering direction */
+        MIXED
     }
 
     @Override
@@ -89 +108 @@
             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 (!isPrimitiveUsable(w)) return;
+
+        if (isPowerLine(w) && !w.hasKey("line") && !w.hasTag("location", "underground")) {
+            final int segmentCount = w.getNodesCount() - 1;
+            final double mean = w.getLength() / segmentCount;
+            final double stdDev = calculateStdDev(w.getSegmentLengths(), mean);
+            int poleCount = 0;
+            int towerCount = 0;
+            Node prevNode = w.firstNode();
+
+            double threshold = w.hasTag("power", "line") ? 1.6 : 1.8;
+            if (mean / stdDev < 4) {
+                //compensate for possibly hilly areas where towers can't be put anywhere
+                threshold += 0.2;
+            }
+
+            for (int i = 1; i < w.getRealNodesCount(); i++) {
+                final Node n = w.getNode(i);
+
+                /// handle power station line connections (eg. power=line + line=*)
+                if (isConnectedToStationLine(n, w)) {
+                    prevNode = n;
+                    continue;   // skip, it would be false positive
                 }
+
+                /// handle missing power line support tags (eg. tower)
+                if (!isPowerTower(n) && !isPowerInfrastructure(n) && IN_DOWNLOADED_AREA.test(n)
+                        && (!w.isFirstLastNode(n) || !isPowerStation(n)))
+                    missingTags.add(n);
+
+                /// handle missing nodes
+                double segmentLen = n.getCoor().greatCircleDistance(prevNode.getCoor());
+
+                // check power=line waterway crossing
+                final Pair<Node, Node> pair = Pair.create(prevNode, n);
+                final Set<Way> crossingWaterWays = new HashSet<>(8);
+                final Set<Node> crossingPositions = new HashSet<>(8);
+                findCrossings(datasetWaterways, w, pair, crossingWaterWays, crossingPositions);
+
+                if (!crossingWaterWays.isEmpty()) {
+                    double compensation = calculateIntersectingLen(prevNode, crossingPositions);
+                    segmentLen -= compensation;
+                }
+
+                if (segmentCount > 4 && segmentLen > mean * threshold && !isPowerInfrastructure(n))
+                    missingNodes.add(WaySegment.forNodePair(w, prevNode, n));
+
+                /// handle wrong line types
+                if (n.hasTag("power", "pole"))
+                    poleCount++;
+                else if (n.hasTag("power", "tower"))
+                    towerCount++;
+
+                prevNode = n;
             }
+
+            /// handle ref=* numbering discontinuities
+            checkDiscontinuities(w);
+
+            /// handle wrong line types
+            if ((poleCount > towerCount && w.hasTag("power", "line")) ||
+                    (poleCount < towerCount && w.hasTag("power", "minor_line")))
+                wrongLineType.add(w);
+
+        } else if (w.isClosed() && isPowerStation(w)) {
+            powerStations.add(w);
         }
-        return false;
     }
 
     @Override
@@ -113 +189 @@
     @Override
     public void startTest(ProgressMonitor progressMonitor) {
         super.startTest(progressMonitor);
-        clearCollections();
+        setShowElements(true);
+
+        // collect all waterways
+        getLayerManager()
+                .getActiveDataSet()
+                .getWays()
+                .parallelStream()
+                .filter(way ->
+                        way.hasTag("water", "river", "lake") ||
+                                way.hasKey("waterway") ||
+                                way.referrers(Relation.class)
+                                        .collect(Collectors.toSet())
+                                        // no parallel stream here, just makes the iteration slower
+                                        .stream()
+                                        .anyMatch(relation -> relation.hasTag("water", "river", "lake") ||
+                                                relation.hasKey("waterway")
+                                        ))
+                .forEach(datasetWaterways::add);
     }
 
     @Override
     public void endTest() {
-        for (Node n : missingTowerOrPole) {
+        for (Node n : missingTags) {
             if (!isInPowerStation(n)) {
-                errors.add(TestError.builder(this, Severity.WARNING, POWER_LINES)
-                        .message(tr("Missing power tower/pole within power line"))
+                errors.add(TestError.builder(this, Severity.WARNING, POWER_SUPPORT)
+                        // the "missing tag" grouping can become broken if the MapCSS message get reworded
+                        .message(tr("missing tag"), tr("node without power=*"))
                         .primitives(n)
                         .build());
             }
@@ -130 +224 @@
         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 (WaySegment segment : missingNodes) {
+            errors.add(TestError.builder(this, Severity.WARNING, POWER_SEGMENT_LENGTH)
+                    .message(tr("Missing line support node within power line"))
+                    .primitives(segment.getFirstNode(), segment.getSecondNode())
+                    .highlightWaySegments(new HashSet<>(Collections.singleton(segment)))
+                    .build());
         }
-        clearCollections();
+
+        for (Node n : refDiscontinuities) {
+            errors.add(TestError.builder(this, Severity.WARNING, POWER_REF)
+                    .message(tr("Mixed reference numbering"))
+                    .primitives(n)
+                    .build());
+        }
+
+        for (Way w : wrongLineType) {
+            errors.add(TestError.builder(this, Severity.WARNING, POWER_LINE_TYPE)
+                    .message(tr("Possibly wrong power line type used"))
+                    .primitives(w)
+                    .build());
+        }
+
         super.endTest();
     }
 
+    /**
+     * Checks reference numbering discontinuities on the given way.
+     * @param w A way to be checked for ref=* discontinuities
+     */
+    private void checkDiscontinuities(Way w) {
+        final List<Integer> discontinuityAtIndexes = new ArrayList<>();
+        final NumberingDirection direction = detectDirection(w, discontinuityAtIndexes);
+
+        if (direction == NumberingDirection.MIXED) {
+            for (int disC : discontinuityAtIndexes) {
+                refDiscontinuities.add(w.getNode(disC));
+            }
+        }
+    }
+
+    /**
+     * Calculates the standard deviation from the given way segment lengths.
+     * @param segmentLengths Segment lengths of a way
+     * @param mean Precalculated average segment length of a way
+     * @return standard deviation of the given way segment lengths
+     */
+    private static double calculateStdDev(double[] segmentLengths, double mean) {
+        double standardDeviation = 0;
+        int size = segmentLengths.length;
+
+        for (double length : segmentLengths) {
+            standardDeviation += Math.pow(length - mean, 2);
+        }
+
+        return Math.sqrt(standardDeviation / size);
+    }
+
+    /**
+     * The summarized length (in metres) of a way where a power line hangs over a water area.
+     * @param ref Reference point
+     * @param crossingNodes Crossing nodes, unordered
+     * @return The summarized length (in metres) of a way where a power line hangs over a water area
+     */
+    private static double calculateIntersectingLen(Node ref, Set<Node> crossingNodes) {
+        double min = Double.POSITIVE_INFINITY;
+        double max = Double.NEGATIVE_INFINITY;
+
+        for (Node n : crossingNodes) {
+            double dist = ref.getCoor().greatCircleDistance(n.getCoor());
+
+            if (dist < min)
+                min = dist;
+            if (dist > max)
+                max = dist;
+        }
+        return max - min;
+    }
+
+    /**
+     * Searches for way intersections, which intersect the {@code pair} attribute.
+     * @param ways collection of ways to search
+     * @param parent parent way for {@code pair} param
+     * @param pair {@link Node} pair among which search for another way
+     * @param crossingWays found crossing ways
+     * @param crossingPositions collection of the crossing positions
+     * @implNote Inspired by {@code utilsplugin2/selection/NodeWayUtils.java#addWaysIntersectingWay()}
+     */
+    private static void findCrossings(Collection<Way> ways, Way parent, Pair<Node, Node> pair, Set<Way> crossingWays,
+                                      Set<Node> crossingPositions) {
+        ways.parallelStream()
+                .filter(way -> !way.isDisabled()
+                        && !crossingWays.contains(way)
+                        && way.getBBox().intersects(parent.getBBox()))
+                .forEach(way -> way.getNodePairs(false)
+                        .forEach(pair2 -> {
+                            EastNorth eastNorth = Geometry.getSegmentSegmentIntersection(
+                                    pair.a.getEastNorth(), pair.b.getEastNorth(),
+                                    pair2.a.getEastNorth(), pair2.b.getEastNorth());
+                            if (eastNorth != null) {
+                                crossingWays.add(way);
+                                crossingPositions.add(new Node(eastNorth));
+                            }
+                        })
+                );
+    }
+
+    /**
+     * Detects ref=* numbering direction. Ignores the first and last node, because ways can be connected and the
+     * connection node could have a different numbering.
+     * @param way Way to check
+     * @param discontinuityAtIndex List of node indexes where the discontinuity found
+     * @return way numbering direction
+     */
+    private static NumberingDirection detectDirection(Way way, List<Integer> discontinuityAtIndex) {
+        final Set<NumberingDirection> directions = new HashSet<>(4);
+        final int waySize = way.getNodesCount();
+        int prevRef = -1;
+        int ref;
+
+        // skip first and last node
+        for (int i = 1; i < waySize - 1; i++) {
+            final Node n = way.getNode(i);
+
+            if (n.hasTag("ref")) {
+                try {
+                    ref = Integer.parseInt(n.get("ref"));
+
+                    if (i == 1) {
+                        prevRef = ref;
+                        continue;
+                    }
+
+                    if (ref > prevRef) {
+                        directions.add(NumberingDirection.SAME);
+                    } else if (ref < prevRef) {
+                        directions.add(NumberingDirection.OPPOSITE);
+                    } else {
+                        directions.add(NumberingDirection.MIXED);
+                        discontinuityAtIndex.add(i);
+                    }
+
+                    prevRef = ref;
+                } catch (NumberFormatException ignore) {
+                    prevRef = getGuessedRef(prevRef, directions);
+                }
+            } else if (prevRef != -1) {
+                prevRef = getGuessedRef(prevRef, directions);
+            }
+        }
+
+        if (directions.isEmpty())
+            return NumberingDirection.NONE;
+        else if (directions.size() > 1)
+            return NumberingDirection.MIXED;
+        else
+            return directions.stream().findAny().get();
+    }
+
+    /**
+     * Guesses the ref=* number based on previous value and the recognized direction.
+     * @param prevRef Previous node ref number
+     * @param directions Recognised way numbering direction
+     * @return Guessed ref number of the next power=pole/tower
+     */
+    private static int getGuessedRef(int prevRef, Set<NumberingDirection> directions) {
+        if (directions.size() == 1) {
+            NumberingDirection direction = directions.stream().findFirst().get();
+            if (direction == NumberingDirection.SAME)
+                prevRef += 1;
+            else if (direction == NumberingDirection.OPPOSITE)
+                prevRef -= 1;
+        }
+        return prevRef;
+    }
+
+    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;
+    }
+
+    /**
+     * Determines if the current node connected to a line which used usually used inside power stations.
+     * @param n node to check
+     * @param w parent way of {@code n}
+     * @return {@code true} if {@code n} connected to power=line + line=*
+     */
+    private static boolean isConnectedToStationLine(Node n, Way w) {
+        for (OsmPrimitive p : n.getReferrers()) {
+            if (p instanceof Way && !p.equals(w) && isPowerLine((Way) p) && p.hasKey("line"))
+                return true;
+        }
+        return false;
+    }
+
+    /**
+     * Checks if the given node is inside a power station.
+     * @param n Node to checked
+     */
     protected final boolean isInPowerStation(Node n) {
         for (OsmPrimitive station : powerStations) {
             List<List<Node>> nodesLists = new ArrayList<>();
@@ -171 +470 @@
     /**
      * 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 support feature.
      * @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 pole/tower/portal/catenary_mast
      */
     protected static final boolean isPowerTower(Node n) {
         return isPowerIn(n, POWER_TOWER_TAGS);
@@ -189 +488 @@
     /**
      * 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);
     }
 
     /**
@@ -215 +515 @@
         return p.hasTag("building", values);
     }
 
-    private void clearCollections() {
+    @Override
+    public void clear() {
+        super.clear();
         powerStations.clear();
         badConnections.clear();
-        missingTowerOrPole.clear();
+        missingTags.clear();
+        missingNodes.clear();
+        wrongLineType.clear();
+        refDiscontinuities.clear();
+        datasetWaterways.clear();
     }
 }