Index: doc/options.txt =================================================================== --- doc/options.txt (revision 4642) +++ doc/options.txt (working copy) @@ -637,7 +637,7 @@ problems. Default value is 0 (disabled) because it's not a completely reliable heuristic. -;--check-routing-island-len=INTEGER +;--report-routing-islands : Routing islands are small road networks which are not connected to other roads. A typical case is a footway that is not connected to the main road network, or a small set of ways on the inner courtyard of a large building. @@ -644,30 +644,19 @@ : These islands can cause problems if you try to calculate a route and the GPS selects a point on the island as a start or end. It will fail to calculate the route even if a major road is only a few steps away. If this option is -specified, then mkgmap will detect these islands. If the value is set to zero, -mkgmap will simply report the islands (you will need to set -uk.me.parabola.imgfmt.app.net.RoadNetwork.level=INFO to activate logging of -the message). If the value is greater than zero, mkgmap will mark islands with -a total length less than the specified value in metres as not routable. -Reasonable values are 500 or higher. The default is for the check to not take -place. If any of the roads forming the island touches a tile boundary or a -country border the island is ignored, as it may be connected to other roads in -a different tile. -: See also option --add-boundary-nodes-at-admin-boundaries. -: This option seems to cause routing problems in BaseCamp. +specified, then mkgmap will report these islands. +: See also --max-routing-island-len. ;--report-similar-arcs : Issue a warning when more than one arc connects two nodes and the ways that the arcs are derived from contain identical -points. It doesn't make sense to use this option at the same -time as using the cycleway creating options. +points. -;--report-dead-ends=LEVEL -: Set the dead end road warning level. The value of LEVEL (which -defaults to 1 if this option is not specified) determines +;--report-dead-ends[=LEVEL] +: Set the dead end road warning level. The value of LEVEL determines those roads to report: -:* 0 = none -:* 1 = report on connected one-way roads that go nowhere +:* 0 = none (the default) +:* 1 = report on connected one-way roads that go nowhere (default if no LEVEL specified) :* 2 = also report on individual one-way roads that go nowhere. ;--dead-ends[=key[=value]][,key[=value]...] @@ -981,12 +970,34 @@ The tag mkgmap:drawLevel can be used to override the natural area of a polygon, so forcing changes to the rendering order. +;--max-routing-island-len=integer +: Routing islands are small road networks which are not connected to other +roads. A typical case is a footway that is not connected to the main road +network, or a small set of ways on the inner courtyard of a large building. +: These islands can cause problems if you try to calculate a route and the GPS +selects a point on the island as a start or end. It will fail to calculate the +route even if a major road is only a few steps away. If a positive value is +specified, then mkgmap will mark islands with a total length less than the +specified value in metres as not routable. +Reasonable values are 500 or higher. The default is to not to mark any islands +as unroutable. If any of the roads forming the island touches a tile boundary or a +country border the island is ignored, as it may be connected to other roads in +a different tile. +: See also options --add-boundary-nodes-at-admin-boundaries and +--report-routing-islands. +: This option seems to cause routing problems in BaseCamp. + === Deprecated and Obsolete Options === +;--check-routing-island-len=integer +: Deprecated; use --report-routing-islands and --max-routing-island-len instead. +Translated to --report-routing-islands if info level logging is enabled, plus +--max-routing-island-len=integer. + ;--drive-on-left ;--drive-on-right -: Deprecated; use drive-on instead. -The options are translated to drive-on=left|right. +: Deprecated; use --drive-on instead. +The options are translated to --drive-on=left|right. ;--make-all-cycleways : Deprecated; use --make-opposite-cycleways instead. Former meaning: Index: src/uk/me/parabola/imgfmt/app/net/RoadNetwork.java =================================================================== --- src/uk/me/parabola/imgfmt/app/net/RoadNetwork.java (revision 4642) +++ src/uk/me/parabola/imgfmt/app/net/RoadNetwork.java (working copy) @@ -61,8 +61,8 @@ private int maxFlareLengthRatio ; private boolean reportSimilarArcs; private boolean routable; - - private long maxSumRoadLenghts; + private boolean reportRoutingIslands; + private long maxSumRoadLengths; /** for route island search */ private int visitId; @@ -71,7 +71,14 @@ checkRoundaboutFlares = props.getProperty("check-roundabout-flares", false); maxFlareLengthRatio = props.getProperty("max-flare-length-ratio", 0); reportSimilarArcs = props.getProperty("report-similar-arcs", false); - maxSumRoadLenghts = props.getProperty("check-routing-island-len", -1); + int checkRoutingIslandLengths = props.getProperty("check-routing-island-len", -1); + if (checkRoutingIslandLengths >= 0) { + Logger.defaultLogger.warn("The --check-routing-island-len option is deprecated. Please use --report-routing-islands and/or max-routing-island-len"); + maxSumRoadLengths = checkRoutingIslandLengths; + reportRoutingIslands = log.isInfoEnabled(); + } + maxSumRoadLengths = props.getProperty("max-routing-island-len", -1); + reportRoutingIslands = props.getProperty("report-routing-islands", false); routable = props.containsKey("route"); angleChecker.config(props); } @@ -303,8 +310,9 @@ * report routing islands and maybe remove them from NOD. */ private void checkRoutingIslands() { - if (maxSumRoadLenghts < 0) + if (maxSumRoadLengths <= 0 && !reportRoutingIslands) return; // island check is disabled + long t1 = System.currentTimeMillis(); // calculate all islands @@ -314,7 +322,7 @@ if (!islands.isEmpty()) { analyseIslands(islands); } - if (maxSumRoadLenghts > 0) { + if (maxSumRoadLengths > 0) { long t3 = System.currentTimeMillis(); log.info("routing island removal took", (t3 - t2), "ms"); } @@ -350,10 +358,11 @@ for (List island : islands) { // compute size of island as sum of road lengths Set visitedRoads = new HashSet<>(); - long sumOfRoadLenghts = calcIslandSize(island, nodeToRoadMap, visitedRoads); - log.info("Routing island at", island.get(0).getCoord().toDegreeString(), "with", island.size(), - "routing node(s) and total length of", sumOfRoadLenghts, "m"); - if (sumOfRoadLenghts < maxSumRoadLenghts) { + long sumOfRoadLengths = calcIslandSize(island, nodeToRoadMap, visitedRoads); + if (reportRoutingIslands) + log.diagnostic("Routing island " + visitedRoads.iterator().next() + " at " + island.get(0).getCoord().toDegreeString() + " with " + island.size() + + " routing node(s) and total length of " + sumOfRoadLengths + "m"); + if (sumOfRoadLengths < maxSumRoadLengths) { // set discarded flag for all nodes of the island island.forEach(RouteNode::discard); visitedRoads.forEach(rd -> rd.skipAddToNOD(true)); Index: src/uk/me/parabola/imgfmt/app/net/RouteNode.java =================================================================== --- src/uk/me/parabola/imgfmt/app/net/RouteNode.java (revision 4642) +++ src/uk/me/parabola/imgfmt/app/net/RouteNode.java (working copy) @@ -388,7 +388,7 @@ // non roundabout highway overlaps roundabout nonRoundaboutArcs.remove(ra1); if(!ra.getRoadDef().messagePreviouslyIssued("roundabout forks/overlaps")) - log.warn("Highway",ra1.getRoadDef(), "overlaps roundabout", ra.getRoadDef(), "at",coord.toOSMURL()); + log.diagnostic("Highway " + ra1.getRoadDef() + " overlaps roundabout " + ra.getRoadDef() + " at " + coord.toOSMURL()); break; } } @@ -461,27 +461,27 @@ RouteArc roundaboutArc = roundaboutArcs.get(0); if (arcs.size() > 1 && roundaboutArcs.size() == 1) - log.warn("Roundabout",roundaboutArc.getRoadDef(),roundaboutArc.isForward() ? "starts at" : "ends at", coord.toOSMURL()); + log.diagnostic("Roundabout " + roundaboutArc.getRoadDef() + (roundaboutArc.isForward() ? " starts at " : " ends at ") + coord.toOSMURL()); if (countNonRoundaboutRoads > 1) - log.warn("Roundabout",roundaboutArc.getRoadDef(),"is connected to more than one road at",coord.toOSMURL()); + log.diagnostic("Roundabout " + roundaboutArc.getRoadDef() + " is connected to more than one road at " + coord.toOSMURL()); else if (countNonRoundaboutRoads == 1) { if (countNonRoundaboutOtherHighways > 0) { if (countHighwaysInsideRoundabout > 0) - log.warn("Roundabout",roundaboutArc.getRoadDef(),"is connected to a road",countNonRoundaboutOtherHighways,"other highway(s) and",countHighwaysInsideRoundabout,"highways inside the roundabout at",coord.toOSMURL()); + log.diagnostic("Roundabout " + roundaboutArc.getRoadDef() + " is connected to a road, " + countNonRoundaboutOtherHighways + " other highway(s) and " + countHighwaysInsideRoundabout + " highways inside the roundabout at " + coord.toOSMURL()); else - log.warn("Roundabout",roundaboutArc.getRoadDef(),"is connected to a road and",countNonRoundaboutOtherHighways,"other highway(s) at",coord.toOSMURL()); + log.diagnostic("Roundabout " + roundaboutArc.getRoadDef() + " is connected to a road and " + countNonRoundaboutOtherHighways + " other highway(s) at " + coord.toOSMURL()); } else if (countHighwaysInsideRoundabout > 0) - log.warn("Roundabout",roundaboutArc.getRoadDef(),"is connected to a road and",countHighwaysInsideRoundabout,"highway(s) inside the roundabout at",coord.toOSMURL()); + log.diagnostic("Roundabout " + roundaboutArc.getRoadDef() + " is connected to a road and " + countHighwaysInsideRoundabout + " highway(s) inside the roundabout at " + coord.toOSMURL()); } else if (countNonRoundaboutOtherHighways > 0) { if (countHighwaysInsideRoundabout > 0) - log.warn("Roundabout",roundaboutArc.getRoadDef(),"is connected to",countNonRoundaboutOtherHighways,"highway(s) and",countHighwaysInsideRoundabout,"inside the roundabout at",coord.toOSMURL()); + log.diagnostic("Roundabout " + roundaboutArc.getRoadDef() + " is connected to " + countNonRoundaboutOtherHighways+ " highway(s) and " + countHighwaysInsideRoundabout + " inside the roundabout at " + coord.toOSMURL()); else if (countNonRoundaboutOtherHighways > 1) - log.warn("Roundabout",roundaboutArc.getRoadDef(),"is connected to",countNonRoundaboutOtherHighways,"highways at",coord.toOSMURL()); + log.diagnostic("Roundabout " + roundaboutArc.getRoadDef() + " is connected to " + countNonRoundaboutOtherHighways + " highways at " + coord.toOSMURL()); } else if (countHighwaysInsideRoundabout > 1) - log.warn("Roundabout",roundaboutArc.getRoadDef(),"is connected to",countHighwaysInsideRoundabout,"highways inside the roundabout at",coord.toOSMURL()); + log.diagnostic("Roundabout " +roundaboutArc.getRoadDef() + " is connected to " + countHighwaysInsideRoundabout + " highways inside the roundabout at " + coord.toOSMURL()); if(roundaboutArcs.size() > 2) { for(RouteArc fa : roundaboutArcs) { if(fa.isForward()) { @@ -492,12 +492,12 @@ ((fb.isForward() && fb.getDest() == fa.getDest()) || (!fb.isForward() && fb.getSource() == fa.getDest()))) { if(!rd.messagePreviouslyIssued("roundabout forks/overlaps")) { - log.warn("Roundabout " + rd + " overlaps " + fb.getRoadDef() + " at " + coord.toOSMURL()); + log.diagnostic("Roundabout " + rd + " overlaps " + fb.getRoadDef() + " at " + coord.toOSMURL()); } } else if (fb.isForward() && !rd.messagePreviouslyIssued("roundabout forks/overlaps")) { - log.warn("Roundabout " + rd + " forks at " + coord.toOSMURL()); + log.diagnostic("Roundabout " + rd + " forks at " + coord.toOSMURL()); } } } @@ -641,14 +641,14 @@ // only issue one warning per flare if(!fa.isForward()) - log.warn("Outgoing roundabout flare road " + fa.getRoadDef() + " points in wrong direction? " + fa.getSource().coord.toOSMURL()); + log.diagnostic("Outgoing roundabout flare road " + fa.getRoadDef() + " points in wrong direction? " + fa.getSource().coord.toOSMURL()); else if(fb.isForward()) - log.warn("Incoming roundabout flare road " + fb.getRoadDef() + " points in wrong direction? " + fb.getSource().coord.toOSMURL()); + log.diagnostic("Incoming roundabout flare road " + fb.getRoadDef() + " points in wrong direction? " + fb.getSource().coord.toOSMURL()); else if(!fa.getRoadDef().isOneway()) - log.warn("Outgoing roundabout flare road " + fa.getRoadDef() + " is not oneway? " + fa.getSource().coord.toOSMURL()); + log.diagnostic("Outgoing roundabout flare road " + fa.getRoadDef() + " is not oneway? " + fa.getSource().coord.toOSMURL()); else if(!fb.getRoadDef().isOneway()) - log.warn("Incoming roundabout flare road " + fb.getRoadDef() + " is not oneway? " + fb.getDest().coord.toOSMURL()); + log.diagnostic("Incoming roundabout flare road " + fb.getRoadDef() + " is not oneway? " + fb.getDest().coord.toOSMURL()); else { // check that the flare road arcs are not // part of a longer way @@ -655,9 +655,9 @@ for(RouteArc a : fa.getDest().arcs) { if(a.isDirect() && a.getDest() != this && a.getDest() != nb) { if(a.getRoadDef() == fa.getRoadDef()) - log.warn("Outgoing roundabout flare road " + fb.getRoadDef() + " does not finish at flare? " + fa.getDest().coord.toOSMURL()); + log.diagnostic("Outgoing roundabout flare road " + fb.getRoadDef() + " does not finish at flare? " + fa.getDest().coord.toOSMURL()); else if(a.getRoadDef() == fb.getRoadDef()) - log.warn("Incoming roundabout flare road " + fb.getRoadDef() + " does not start at flare? " + fb.getDest().coord.toOSMURL()); + log.diagnostic("Incoming roundabout flare road " + fb.getRoadDef() + " does not start at flare? " + fb.getDest().coord.toOSMURL()); } } } @@ -670,16 +670,19 @@ public void reportSimilarArcs() { for(int i = 0; i < arcs.size(); ++i) { RouteArc arci = arcs.get(i); - if (!arci.isDirect()) + RoadDef rdi = arci.getRoadDef(); + if (!arci.isDirect() || rdi.isSynthesised()) continue; for(int j = i + 1; j < arcs.size(); ++j) { RouteArc arcj = arcs.get(j); - if (!arcj.isDirect()) + RoadDef rdj = arcj.getRoadDef(); + if (!arcj.isDirect() || rdj.isSynthesised()) continue; if(arci.getDest() == arcj.getDest() && arci.getLength() == arcj.getLength() && - arci.getPointsHash() == arcj.getPointsHash()) { - log.warn("Similar arcs (" + arci.getRoadDef() + " and " + arcj.getRoadDef() + ") from " + coord.toOSMURL()); + arci.getPointsHash() == arcj.getPointsHash() && + !rdi.messagePreviouslyIssued("Similar arcs")) { + log.diagnostic("Similar arcs " + rdi + " and " + rdj + " found at " + coord.toOSMURL()); } } } Index: src/uk/me/parabola/log/Logger.java =================================================================== --- src/uk/me/parabola/log/Logger.java (revision 4642) +++ src/uk/me/parabola/log/Logger.java (working copy) @@ -35,6 +35,8 @@ */ public class Logger { private final java.util.logging.Logger log; + private final boolean addPrefix; + public static final Logger defaultLogger = new Logger(java.util.logging.Logger.GLOBAL_LOGGER_NAME, false); private static final ThreadLocal threadTags = new ThreadLocal<>(); @@ -42,8 +44,9 @@ initLogging(); } - private Logger(String name) { + private Logger(String name, boolean addPrefix) { this.log = java.util.logging.Logger.getLogger(name); + this.addPrefix = addPrefix; } /** @@ -55,7 +58,7 @@ * @return The logger. */ public static Logger getLogger(String name) { - return new Logger(name); + return new Logger(name, true); } /** @@ -83,6 +86,8 @@ else { staticSetup(); } + if (!defaultLogger.isLoggable(Level.WARNING)) + defaultLogger.log.setLevel(Level.WARNING); } private static void initLoggingFromFile(String logconf) { @@ -114,10 +119,10 @@ f.setShowTime(false); handler.setFormatter(f); - handler.setLevel(Level.SEVERE); + handler.setLevel(Level.FINE); l.addHandler(handler); - l.setLevel(Level.WARNING); + l.setLevel(Level.SEVERE); } public boolean isLoggable(Level level) { @@ -177,7 +182,8 @@ } public void warn(Object o) { - log.warning(tagMessage(o == null? "null" : o.toString())); + if (log.isLoggable(Level.WARNING)) + log.warning(tagMessage(o == null? "null" : o.toString())); } public void warn(Object ... olist) { @@ -195,8 +201,9 @@ } public void error(Object ... olist) { - arrayFormat(Level.SEVERE, olist); + arrayFormat(Level.SEVERE, olist); } + public void errorf(String fmt, Object... args) { printf(Level.SEVERE, fmt, args); } @@ -204,7 +211,22 @@ public void error(Object o, Throwable e) { log.log(Level.SEVERE, tagMessage(o == null? "null" : o.toString()), e); } + + // output a requested diagnostic message + public void diagnostic(String msg) { + log.log(LogLevel.DIAGNOSTIC, tagMessage(msg)); + } + // output an echo or echotags message + public void echo(String msg) { + log.log(LogLevel.ECHO, tagMessage(msg)); + } + + // an information message that is always output + public void write(String msg) { + log.log(LogLevel.OVERRIDE, tagMessage(msg)); + } + public void log(Level level, Object o) { if (log.isLoggable(level)) log.log(level, tagMessage(o == null? "null" : o.toString())); @@ -226,23 +248,28 @@ * @param olist The argument list as objects. */ private void arrayFormat(Level type, Object... olist) { - StringBuilder sb = new StringBuilder(); - - for (Object o : olist) { - sb.append(o); - sb.append(' '); + if (log.isLoggable(type)) { + StringBuilder sb = new StringBuilder(); + for (Object o : olist) { + sb.append(o); + sb.append(' '); + } + sb.setLength(sb.length()-1); + log.log(type, tagMessage(sb.toString())); } - sb.setLength(sb.length()-1); - - log.log(type, tagMessage(sb.toString())); } private void printf(Level type, String fmt, Object... args) { - String msg = String.format(fmt, args); - log.log(type, tagMessage(msg)); + if (log.isLoggable(type)) { + String msg = String.format(fmt, args); + log.log(type, tagMessage(msg)); + } } - private static String tagMessage(String message) { + private String tagMessage(String message) { + if (!addPrefix) + return message; + String threadTag = threadTags.get(); return (threadTag != null) ? threadTag + ": " + message : message; } Index: src/uk/me/parabola/log/LogLevel.java =================================================================== --- src/uk/me/parabola/log/LogLevel.java (nonexistent) +++ src/uk/me/parabola/log/LogLevel.java (working copy) @@ -0,0 +1,17 @@ +package uk.me.parabola.log; + +import java.util.logging.Level; + +public class LogLevel extends Level { + + public static final LogLevel DIAGNOSTIC = new LogLevel("DIAGNOSTIC", 1100); + + public static final LogLevel ECHO = new LogLevel("ECHO", 1200); + + public static final LogLevel OVERRIDE = new LogLevel("OVERRIDE", 1300); + + protected LogLevel(String name, int value) { + super(name, value); + } + +} Index: src/uk/me/parabola/log/UsefulFormatter.java =================================================================== --- src/uk/me/parabola/log/UsefulFormatter.java (revision 4642) +++ src/uk/me/parabola/log/UsefulFormatter.java (working copy) @@ -20,6 +20,7 @@ import java.io.StringWriter; import java.util.Calendar; import java.util.logging.Formatter; +import java.util.logging.Level; import java.util.logging.LogRecord; /** @@ -38,29 +39,30 @@ public String format(LogRecord record) { StringBuffer sb = new StringBuffer(); - if (showTime) { - long millis = record.getMillis(); - Calendar cal = Calendar.getInstance(); - cal.setTimeInMillis(millis); - sb.append(cal.get(Calendar.YEAR)); - sb.append('/'); - sb.append(fmt2(cal.get(Calendar.MONTH)+1)); - sb.append('/'); - sb.append(fmt2(cal.get(Calendar.DAY_OF_MONTH))); - sb.append(' '); - sb.append(fmt2(cal.get(Calendar.HOUR_OF_DAY))); - sb.append(':'); - sb.append(fmt2(cal.get(Calendar.MINUTE))); - sb.append(':'); - sb.append(fmt2(cal.get(Calendar.SECOND))); - sb.append(' '); + if (record.getLevel().intValue() <= Level.SEVERE.intValue()) { + if (showTime) { + long millis = record.getMillis(); + Calendar cal = Calendar.getInstance(); + cal.setTimeInMillis(millis); + sb.append(cal.get(Calendar.YEAR)); + sb.append('/'); + sb.append(fmt2(cal.get(Calendar.MONTH)+1)); + sb.append('/'); + sb.append(fmt2(cal.get(Calendar.DAY_OF_MONTH))); + sb.append(' '); + sb.append(fmt2(cal.get(Calendar.HOUR_OF_DAY))); + sb.append(':'); + sb.append(fmt2(cal.get(Calendar.MINUTE))); + sb.append(':'); + sb.append(fmt2(cal.get(Calendar.SECOND))); + sb.append(' '); + } + + sb.append(record.getLevel().getLocalizedName()); + sb.append(" ("); + sb.append(shortName(record.getLoggerName())); + sb.append("): "); } - - sb.append(record.getLevel().getLocalizedName()); - sb.append(" ("); - sb.append(shortName(record.getLoggerName())); - sb.append("): "); - sb.append(record.getMessage()); sb.append(lineSeparator); Index: src/uk/me/parabola/mkgmap/osmstyle/StyledConverter.java =================================================================== --- src/uk/me/parabola/mkgmap/osmstyle/StyledConverter.java (revision 4642) +++ src/uk/me/parabola/mkgmap/osmstyle/StyledConverter.java (working copy) @@ -204,7 +204,7 @@ countryAbbr = countryAbbr.toUpperCase(); checkRoundabouts = props.getProperty("check-roundabouts",false); - reportDeadEnds = props.getProperty("report-dead-ends", 1); + reportDeadEnds = (props.getProperty("report-dead-ends") != null) ? props.getProperty("report-dead-ends", 1) : 0; prefixSuffixFilter = new PrefixSuffixFilter(props); lineAdder = line -> { @@ -348,9 +348,9 @@ numDriveOnSideUnknown++; } } - if (cw.isRoundabout() && wasReversed) { - log.warn("Roundabout", way.getId(), - "has reverse oneway tag (" + way.getFirstPoint().toOSMURL() + ")"); + if (cw.isRoundabout() && wasReversed && checkRoundabouts) { + log.diagnostic("Roundabout " + way.getId() + + " has reverse oneway tag (" + way.getFirstPoint().toOSMURL() + ")"); } lastRoadId = way.getId(); } else { @@ -1075,13 +1075,13 @@ if (points.get(0) == points.get(points.size() - 1)) { // roundabout is a loop if (dirIsWrong) { - log.warn("Roundabout " + way.getId() + " direction is wrong - reversing it (see " + log.diagnostic("Roundabout " + way.getId() + " direction is wrong - reversing it (see " + centre.toOSMURL() + ")"); way.reverse(); } } else if (dirIsWrong) { // roundabout is a line - log.warn("Roundabout segment " + way.getId() + " direction looks wrong (see " + log.diagnostic("Roundabout segment " + way.getId() + " direction looks wrong (see " + points.get(0).toOSMURL() + ")"); } } @@ -1232,7 +1232,6 @@ line.setType(replType); line.setPoints(points); - if (way.tagIsLikeYes(TK_ONEWAY)) line.setDirection(true); @@ -2231,7 +2230,7 @@ } if (isDeadEnd && (isDeadEndOfMultipleWays || reportDeadEnds > 1)) { - log.warn("Oneway road " + way.getId() + " with tags " + way.toTagString() + log.diagnostic("Oneway road " + way.getId() + " with tags " + way.toTagString() + ((pos == 0) ? " comes from" : " goes to") + " nowhere at " + p.toOSMURL()); } }