Index: doc/logging.txt =================================================================== --- doc/logging.txt (revision 4642) +++ doc/logging.txt (working copy) @@ -45,3 +45,10 @@ Further information can be found at [https://docs.oracle.com/javase/8/docs/technotes/guides/logging/overview.html https://docs.oracle.com/javase/8/docs/technotes/guides/logging/overview.html] +If you see messages that contain non-ASCII characters are not displaying +properly in the console, you may need use the Java -Dfile.encoding= +option and/or precede the mkgmap command line with a chcp command to make +sure that the Java environment has the same code page as the shell it is +being run from. This is especially likely on the Windows platform where the +Java environment uses the Windows default code page, but the Windows command +shell normally uses a different code page. 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/FileBackedImgFileWriter.java =================================================================== --- src/uk/me/parabola/imgfmt/app/FileBackedImgFileWriter.java (revision 4642) +++ src/uk/me/parabola/imgfmt/app/FileBackedImgFileWriter.java (working copy) @@ -24,6 +24,7 @@ import uk.me.parabola.imgfmt.Sized; import uk.me.parabola.imgfmt.fs.ImgChannel; import uk.me.parabola.imgfmt.sys.FileLink; +import uk.me.parabola.log.Logger; /** * Write img file data to a temporary file. On a call to sync() the data @@ -70,7 +71,7 @@ channel.transferTo(0, channel.size(), outputChan); } finally { if (!tmpFile.delete()) - System.err.println("could not delete temporary file " + tmpFile.getPath()); + Logger.defaultLogger.error("could not delete temporary file " + tmpFile.getPath()); } } Index: src/uk/me/parabola/imgfmt/app/lbl/LBLFileReader.java =================================================================== --- src/uk/me/parabola/imgfmt/app/lbl/LBLFileReader.java (revision 4642) +++ src/uk/me/parabola/imgfmt/app/lbl/LBLFileReader.java (working copy) @@ -29,6 +29,7 @@ import uk.me.parabola.imgfmt.app.labelenc.DecodedText; import uk.me.parabola.imgfmt.app.trergn.Subdivision; import uk.me.parabola.imgfmt.fs.ImgChannel; +import uk.me.parabola.log.Logger; import static uk.me.parabola.imgfmt.app.Label.NULL_LABEL; @@ -444,7 +445,7 @@ } if (hasTides) { - System.out.println("Map has tide prediction, please implement!"); + Logger.defaultLogger.error("Map has tide prediction, please implement!"); } pois.put(poiOffset, poi); Index: src/uk/me/parabola/imgfmt/app/mdr/MDRFile.java =================================================================== --- src/uk/me/parabola/imgfmt/app/mdr/MDRFile.java (revision 4642) +++ src/uk/me/parabola/imgfmt/app/mdr/MDRFile.java (working copy) @@ -28,6 +28,7 @@ import uk.me.parabola.imgfmt.app.srt.Sort; import uk.me.parabola.imgfmt.app.trergn.Point; import uk.me.parabola.imgfmt.fs.ImgChannel; +import uk.me.parabola.log.Logger; /** * The MDR file. This is embedded into a .img file, either its own @@ -150,7 +151,7 @@ Sort sort = mdrHeader.getSort(); if (sort.getCodepage() != codePage) - System.err.println("WARNING: input files have different code pages"); + Logger.defaultLogger.warn("Input files have different code pages"); } public Mdr14Record addCountry(Country country) { Index: src/uk/me/parabola/imgfmt/app/net/NumberStyle.java =================================================================== --- src/uk/me/parabola/imgfmt/app/net/NumberStyle.java (revision 4642) +++ src/uk/me/parabola/imgfmt/app/net/NumberStyle.java (working copy) @@ -1,5 +1,7 @@ package uk.me.parabola.imgfmt.app.net; +import uk.me.parabola.log.Logger; + /** * The number style down one side of a side of a road. * @@ -63,7 +65,7 @@ case 'O': return ODD; case 'B': return BOTH; case '0': - System.err.println("zero instead of capital O in number spec"); + Logger.defaultLogger.error("zero instead of capital O in number spec"); return ODD; default: return NONE; } 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/imgfmt/mdxfmt/MdxFile.java =================================================================== --- src/uk/me/parabola/imgfmt/mdxfmt/MdxFile.java (revision 4642) +++ src/uk/me/parabola/imgfmt/mdxfmt/MdxFile.java (working copy) @@ -23,6 +23,8 @@ import java.util.ArrayList; import java.util.List; +import uk.me.parabola.log.Logger; + /** * The MDX index file. Used with the global index. This is located * at the family level in the windows registry and can perhaps index @@ -103,7 +105,7 @@ // Although its not necessarily wrong for them to be zero, it probably // sign that something is wrong. if (info.getHexMapname() == 0 || info.getMapname() == 0) - System.err.println("Invalid mapname for " + info.getFilename() + ", perhaps it is not a .img file"); + Logger.defaultLogger.warn("Invalid mapname for " + info.getFilename() + ", perhaps it is not a .img file"); buf.compact(); info.write(buf); Index: src/uk/me/parabola/imgfmt/sys/ImgFS.java =================================================================== --- src/uk/me/parabola/imgfmt/sys/ImgFS.java (revision 4642) +++ src/uk/me/parabola/imgfmt/sys/ImgFS.java (working copy) @@ -101,7 +101,7 @@ FileChannel chan = FileChannel.open(Paths.get(filename), OPEN_CREATE_RW); return createFs(chan, params); } catch (IOException e) { - throw new FileNotWritableException("Could not create file: " + params.getFilename(), e); + throw new FileNotWritableException("Could not create file", e); } } Index: src/uk/me/parabola/imgfmt/Utils.java =================================================================== --- src/uk/me/parabola/imgfmt/Utils.java (revision 4642) +++ src/uk/me/parabola/imgfmt/Utils.java (working copy) @@ -31,6 +31,7 @@ import uk.me.parabola.imgfmt.app.Coord; import uk.me.parabola.imgfmt.app.ImgFileWriter; +import uk.me.parabola.log.Logger; /** * Some miscellaneous functions that are used within the .img code. * @@ -189,7 +190,7 @@ try { f.close(); } catch (IOException e) { - e.printStackTrace(); + Logger.defaultLogger.error("Error closing file", e); } } } 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/build/LocatorConfig.java =================================================================== --- src/uk/me/parabola/mkgmap/build/LocatorConfig.java (revision 4642) +++ src/uk/me/parabola/mkgmap/build/LocatorConfig.java (working copy) @@ -180,13 +180,12 @@ } else { - System.out.println(fileName + "contains invalid root tag " + rootNode.getNodeName()); + log.error(fileName + "contains invalid root tag " + rootNode.getNodeName()); } - } + } catch (Exception ex) { - ex.printStackTrace(); - //System.out.println("Something is wrong here"); + Logger.defaultLogger.error("Unexpected error reading " + fileName, ex); } } Index: src/uk/me/parabola/mkgmap/combiners/FileInfo.java =================================================================== --- src/uk/me/parabola/mkgmap/combiners/FileInfo.java (revision 4642) +++ src/uk/me/parabola/mkgmap/combiners/FileInfo.java (working copy) @@ -205,7 +205,7 @@ fr.position(0x15); info.setCodePage(fr.get2u()); } catch (IOException e) { - e.printStackTrace(); + Logger.defaultLogger.error("Unexpected error reading " + filename, e); } } Index: src/uk/me/parabola/mkgmap/combiners/GmapsuppBuilder.java =================================================================== --- src/uk/me/parabola/mkgmap/combiners/GmapsuppBuilder.java (revision 4642) +++ src/uk/me/parabola/mkgmap/combiners/GmapsuppBuilder.java (working copy) @@ -60,8 +60,6 @@ * @author Steve Ratcliffe */ public class GmapsuppBuilder implements Combiner { - private static final Logger log = Logger.getLogger(GmapsuppBuilder.class); - private static final String GMAPSUPP = "gmapsupp.img"; private final Map files = new LinkedHashMap<>(); @@ -121,7 +119,7 @@ ImgChannel chan = imgFs.create(imgname); mdrBuilder.initForDevice(chan, sort, mdrConfig); } catch (FileExistsException e) { - System.err.println("Could not create duplicate MDR file"); + Logger.defaultLogger.error("Could not create duplicate MDR file"); } mdrBuilderMap.put(familyId, mdrBuilder); @@ -140,11 +138,9 @@ } } else { if (prevSort.getCodepage() != sort.getCodepage()) - System.err.printf("WARNING: input file '%s' has a different code page (%d rather than %d)\n", - info.getFilename(), sort.getCodepage(), prevSort.getCodepage()); + Logger.defaultLogger.warn("Input file '" + info.getFilename() + "' has a different code page (" + sort.getCodepage() + " rather than " + prevSort.getCodepage() + ")"); if (info.hasSortOrder() && prevSort.getSortOrderId() != sort.getSortOrderId()) - System.err.printf("WARNING: input file '%s' has a different sort order (%x rather than %x\n", - info.getFilename(), sort.getSortOrderId(), prevSort.getSortOrderId()); + Logger.defaultLogger.warn("Input file '" + info.getFilename() + "' has a different sort order (" + sort.getSortOrderId() + " rather than " + prevSort.getSortOrderId() + ")"); } } @@ -186,8 +182,7 @@ writeMpsFile(); } catch (FileNotWritableException e) { - log.warn("Could not create gmapsupp file"); - System.err.println("Could not create gmapsupp file"); + Logger.defaultLogger.error("Could not create gmapsupp file"); } finally { Utils.closeFile(imgFs); } @@ -215,7 +210,7 @@ // Do not close srtFile here } catch (FileExistsException e) { // well it shouldn't exist! - log.error("could not create SRT file as it exists already"); + Logger.defaultLogger.error("could not create SRT file as it exists already"); throw new FileNotWritableException("already existed", e); } } @@ -286,7 +281,7 @@ ((FileLink)chan).link(sf, sync); } catch (FileExistsException e) { - log.warn("Could not copy " + sf.getName(), e); + Logger.defaultLogger.warn("Could not copy " + sf.getName(), e); } } } @@ -299,7 +294,7 @@ ImgChannel chan = outfs.create(createImgFilename(filename)); ((FileLink) chan).link(info.subFiles().get(0), fc.file(chan)); } catch (FileExistsException e) { - log.warn("Counld not copy " + filename, e); + Logger.defaultLogger.warn("Could not copy " + filename, e); } } @@ -319,7 +314,7 @@ mpsFile.addProduct(b); mr.close(); } catch (IOException e) { - log.error("Could not read MPS file from gmapsupp", e); + Logger.defaultLogger.error("Could not read MPS file from gmapsupp", e); } } @@ -340,7 +335,7 @@ return new MpsFile(channel); } catch (FileExistsException e) { // well it shouldn't exist! - log.error("could not create MPS file as it already exists"); + Logger.defaultLogger.error("could not create MPS file as it already exists"); throw new FileNotWritableException("already existed", e); } } Index: src/uk/me/parabola/mkgmap/combiners/NsisBuilder.java =================================================================== --- src/uk/me/parabola/mkgmap/combiners/NsisBuilder.java (revision 4642) +++ src/uk/me/parabola/mkgmap/combiners/NsisBuilder.java (working copy) @@ -27,6 +27,7 @@ import java.util.Map; import uk.me.parabola.imgfmt.Utils; +import uk.me.parabola.log.Logger; import uk.me.parabola.mkgmap.CommandArgs; import uk.me.parabola.mkgmap.Version; @@ -95,7 +96,7 @@ } catch (Exception ex) { inStream = this.getClass().getResourceAsStream("/installer/installer_template.nsi"); if (inStream == null) { - System.err.println("Could not find the installer template."); + Logger.defaultLogger.error("Could not find the installer template."); return; } } @@ -118,7 +119,7 @@ } } catch (IOException e) { - System.err.println("Could not write NSIS file"); + Logger.defaultLogger.error("Could not write NSIS file"); } finally { Utils.closeFile(inStream); } @@ -182,7 +183,7 @@ } catch (Exception ex) { inStream = this.getClass().getResourceAsStream("/installer/license_template.txt"); if (inStream == null) { - System.err.println("Could not find the license template."); + Logger.defaultLogger.error("Could not find the license template."); return; } } @@ -198,7 +199,7 @@ } } catch (IOException e) { - System.err.println("Could not write license file"); + Logger.defaultLogger.error("Could not write license file"); } finally { Utils.closeFile(inStream); } Index: src/uk/me/parabola/mkgmap/combiners/OverviewBuilder.java =================================================================== --- src/uk/me/parabola/mkgmap/combiners/OverviewBuilder.java (revision 4642) +++ src/uk/me/parabola/mkgmap/combiners/OverviewBuilder.java (working copy) @@ -215,7 +215,7 @@ codepage = finfo.getCodePage(); } if (codepage != finfo.getCodePage()){ - System.err.println("WARNING: input file " + filename + " has different code page " + finfo.getCodePage()); + Logger.defaultLogger.warn("Input file " + filename + " has different code page " + finfo.getCodePage()); } try { @@ -225,7 +225,7 @@ encodingType = mapReader.getEncodingType(); } if (encodingType != mapReader.getEncodingType()){ - System.err.println("WARNING: input file " + filename + " has different charset type " + encodingType); + Logger.defaultLogger.warn("Input file " + filename + " has different charset type " + encodingType); } String[] msgs = mapReader.getCopyrights(); @@ -324,7 +324,6 @@ int min = levels[l].getLevel(); int res = levels[l].getResolution(); List lineList = mapReader.linesForLevel(min); - //System.out.println(lineList.size() + " lines in lowest resolution " + levels[1].getResolution()); for (Polyline line : lineList) { if (log.isDebugEnabled()) log.debug("got line", line); Index: src/uk/me/parabola/mkgmap/combiners/TdbBuilder.java =================================================================== --- src/uk/me/parabola/mkgmap/combiners/TdbBuilder.java (revision 4642) +++ src/uk/me/parabola/mkgmap/combiners/TdbBuilder.java (working copy) @@ -190,7 +190,6 @@ try { tdb.write(Utils.joinPath(outputDir, overviewMapname, "tdb")); } catch (IOException e) { - log.error("tdb write", e); throw new ExitException("Could not write the TDB file", e); } } Index: src/uk/me/parabola/mkgmap/CommandArgs.java =================================================================== --- src/uk/me/parabola/mkgmap/CommandArgs.java (revision 4642) +++ src/uk/me/parabola/mkgmap/CommandArgs.java (working copy) @@ -11,6 +11,7 @@ import uk.me.parabola.imgfmt.ExitException; import uk.me.parabola.imgfmt.app.srt.Sort; +import uk.me.parabola.log.Logger; import uk.me.parabola.util.EnhancedProperties; public class CommandArgs { @@ -97,14 +98,14 @@ // Test if directory exists File outputDir = new File(fileOutputDir); if (!outputDir.exists()) { - System.out.println("Output directory not found. Creating directory '" + fileOutputDir + "'"); + Logger.defaultLogger.warn("Output directory not found. Creating directory '" + fileOutputDir + "'"); outputDir.mkdirs(); if (!outputDir.exists()) { - System.err.println("Unable to create output directory! Using default directory instead"); + Logger.defaultLogger.warn("Unable to create output directory! Using default directory instead"); fileOutputDir = DEFAULT_DIR; } } else if (!outputDir.isDirectory()) { - System.err.println("The --output-dir parameter must specify a directory. The parameter is being ignored, writing to default directory instead."); + Logger.defaultLogger.warn("The --output-dir parameter must specify a directory. The parameter is being ignored, writing to default directory instead."); fileOutputDir = DEFAULT_DIR; } Index: src/uk/me/parabola/mkgmap/CommandArgsReader.java =================================================================== --- src/uk/me/parabola/mkgmap/CommandArgsReader.java (revision 4642) +++ src/uk/me/parabola/mkgmap/CommandArgsReader.java (working copy) @@ -17,7 +17,6 @@ package uk.me.parabola.mkgmap; import java.io.File; -import java.io.IOException; import java.util.ArrayList; import java.util.Formatter; import java.util.Iterator; @@ -47,6 +46,7 @@ private final ArgumentProcessor proc; private boolean mapnameWasSet; + private boolean hasFiles = false; private final ArgList arglist = new ArgList(); @@ -109,6 +109,7 @@ } else { log.debug("adding filename:", arg); + hasFiles = true; add(new Filename(arg)); } } @@ -125,6 +126,9 @@ proc.endOptions(new CommandArgs(this.args)); } + public boolean getHasFiles() { + return hasFiles; + } /** * Add an option based on the option and value separately. @@ -202,6 +206,7 @@ case "input-file": if (value != null){ log.debug("adding filename", value); + hasFiles = true; add(new Filename(value)); } break; Index: src/uk/me/parabola/mkgmap/main/Main.java =================================================================== --- src/uk/me/parabola/mkgmap/main/Main.java (revision 4642) +++ src/uk/me/parabola/mkgmap/main/Main.java (working copy) @@ -81,6 +81,7 @@ */ public class Main implements ArgumentProcessor { private static final Logger log = Logger.getLogger(Main.class); + private static final Date StartTime = new Date(); // Final .img file combiners. private final List combiners = new ArrayList<>(); @@ -103,6 +104,7 @@ private volatile int programRC = 0; private final Map combinerMap = new HashMap<>(); + private boolean informationDisplayed = false; /** * Used for unit tests @@ -126,7 +128,7 @@ */ private static int mainStart(String... args) { Instant start = Instant.now(); - System.out.println("Time started: " + new Date()); + // We need at least one argument. if (args.length < 1) { printUsage(); @@ -137,18 +139,19 @@ Main mm = new Main(); int numExitExceptions = 0; + CommandArgsReader commandArgs = new CommandArgsReader(mm); try { // Read the command line arguments and process each filename found. - CommandArgsReader commandArgs = new CommandArgsReader(mm); commandArgs.setValidOptions(getValidOptions(System.err)); commandArgs.readArgs(args); } catch (OutOfMemoryError e) { ++numExitExceptions; - System.err.println(e); + String message = "Out of memory.\r\n"; if (mm.maxJobs > 1) - System.err.println("Try using the mkgmap --max-jobs option with a value less than " + mm.maxJobs + " to reduce the memory requirement, or use the Java -Xmx option to increase the available heap memory."); + message += "Try using the mkgmap --max-jobs option with a value less than " + mm.maxJobs + " to reduce the memory requirement, or use the Java -Xmx option to increase the available heap memory."; else - System.err.println("Try using the Java -Xmx option to increase the available heap memory."); + message += "Try using the Java -Xmx option to increase the available heap memory."; + Logger.defaultLogger.error(message); } catch (MapFailedException | ExitException e) { // one of the combiners failed ++numExitExceptions; @@ -158,32 +161,33 @@ message += "\r\n" + cause.toString(); cause = cause.getCause(); } - System.err.println(message); + Logger.defaultLogger.error(message); } - - System.out.println("Number of ExitExceptions: " + numExitExceptions); - - System.out.println("Time finished: " + new Date()); - Duration duration = Duration.between(start, Instant.now()); - long seconds = duration.getSeconds(); - if (seconds > 0) { - long hours = seconds / 3600; - seconds -= hours * 3600; - long minutes = seconds / 60; - seconds -= minutes * 60; - System.out.println("Total time taken: " + - (hours > 0 ? hours + (hours > 1 ? " hours " : " hour ") : "") + - (minutes > 0 ? minutes + (minutes > 1 ? " minutes " : " minute ") : "") + - (seconds > 0 ? seconds + (seconds > 1 ? " seconds" : " second") : "")); + + if(commandArgs.getHasFiles()) { + Logger.defaultLogger.write("Number of ExitExceptions: " + numExitExceptions); + + Logger.defaultLogger.write("Time finished: " + new Date()); + Duration duration = Duration.between(start, Instant.now()); + long seconds = duration.getSeconds(); + if (seconds > 0) { + long hours = seconds / 3600; + seconds -= hours * 3600; + long minutes = seconds / 60; + seconds -= minutes * 60; + Logger.defaultLogger.write("Total time taken: " + + (hours > 0 ? hours + (hours > 1 ? " hours " : " hour ") : "") + + (minutes > 0 ? minutes + (minutes > 1 ? " minutes " : " minute ") : "") + + (seconds > 0 ? seconds + (seconds > 1 ? " seconds" : " second") : "")); + } + else + Logger.defaultLogger.write("Total time taken: " + duration.getNano() / 1000000 + " ms"); } - else - System.out.println("Total time taken: " + duration.getNano() / 1000000 + " ms"); - if (numExitExceptions > 0 || mm.getProgramRC() != 0){ - return 1; - } - return 0; + else if (numExitExceptions == 0 && !mm.informationDisplayed) + System.err.println("The command line does not appear to require mkgmap to do anything."); + return (numExitExceptions > 0 || mm.getProgramRC() != 0) ? 1 : 0; } - + private static void printUsage (){ System.err.println("Usage: mkgmap [options...] "); } @@ -318,6 +322,7 @@ break; case "help": + informationDisplayed = true; printHelp(System.out, getLang(), (!val.isEmpty()) ? val : "help"); break; case "style-file": @@ -331,9 +336,11 @@ verbose = true; break; case "list-styles": + informationDisplayed = true; listStyles(); break; case "check-styles": + informationDisplayed = true; checkStyles(); break; case "max-jobs": @@ -342,15 +349,17 @@ else { maxJobs = Integer.parseInt(val); if (maxJobs < 1) { - log.warn("max-jobs has to be at least 1"); + Logger.defaultLogger.warn("max-jobs has to be at least 1"); maxJobs = 1; } if (maxJobs > Runtime.getRuntime().availableProcessors()) - log.warn("It is recommended that max-jobs be no greater that the number of processor cores"); + Logger.defaultLogger.warn("It is recommended that max-jobs be no greater that the number of processor cores"); } break; case "version": - System.err.println(Version.VERSION); + informationDisplayed = true; + System.out.println("Mkgmap version " + Version.VERSION); + System.out.println(Version.VERSION); System.exit(0); } } @@ -458,7 +467,7 @@ try { style = new StyleImpl(styleFile, name, new EnhancedProperties(), performChecks); } catch (SyntaxException e) { - System.err.println("Error in style: " + e.getMessage()); + Logger.defaultLogger.error("Error in style: " + e.getMessage()); } catch (FileNotFoundException e) { log.debug("could not find style", name); try { @@ -465,7 +474,7 @@ searchedStyleName = new File(styleFile).getName(); style = new StyleImpl(styleFile, null, new EnhancedProperties(), performChecks); } catch (SyntaxException e1) { - System.err.println("Error in style: " + e1.getMessage()); + Logger.defaultLogger.error("Error in style: " + e1.getMessage()); } catch (FileNotFoundException e1) { log.debug("could not find style", styleFile); } @@ -485,9 +494,13 @@ public void endOptions(CommandArgs args) { fileOptions(args); + int taskCount = futures.size(); + if (taskCount > 0) { + Logger.defaultLogger.write("Mkgmap version " + Version.VERSION); + Logger.defaultLogger.write("Time started: " + StartTime); + } log.info("Start tile processors"); int threadCount = maxJobs; - int taskCount = futures.size(); Runtime runtime = Runtime.getRuntime(); if (threadPool == null) { if (threadCount == 0) { @@ -511,7 +524,7 @@ } threadCount = Math.max(threadCount, 1); threadCount = Math.min(threadCount, runtime.availableProcessors()); - System.out.println("Setting max-jobs to " + threadCount); + Logger.defaultLogger.warn("Setting max-jobs to " + threadCount); } } @@ -566,14 +579,14 @@ throw new ExitException("Exiting - if you want to carry on regardless, use the --keep-going option"); } } catch (Exception e) { - e.printStackTrace(); + Logger.defaultLogger.error("Unexpected error", e); throw new ExitException("Exiting due to unexpected error"); } } } - System.out.println("Number of MapFailedExceptions: " + numMapFailedExceptions); + Logger.defaultLogger.write("Number of MapFailedExceptions: " + numMapFailedExceptions); if ((taskCount > threadCount + 1) && (maxJobs == 0) && (threadCount < runtime.availableProcessors())) { - System.out.println("To reduce the run time, consider increasing the amnount of memory available for use by mkgmap by using the Java -Xmx flag to set the memory to more than " + 100* (1 + ((runtime.maxMemory() * runtime.availableProcessors()) / (threadCount * 1024 * 1024 * 100))) + " MB, providing this is less than the amount of physical memory installed."); + Logger.defaultLogger.warn("To reduce the run time, consider increasing the amnount of memory available for use by mkgmap by using the Java -Xmx flag to set the memory to more than " + 100* (1 + ((runtime.maxMemory() * runtime.availableProcessors()) / (threadCount * 1024 * 1024 * 100))) + " MB, providing this is less than the amount of physical memory installed."); } if (combiners.isEmpty()) @@ -589,7 +602,7 @@ hasFiles = true; } if (!hasFiles){ - log.warn("nothing to do for combiners."); + log.info("nothing to do for combiners."); return; } log.info("Combining maps"); @@ -679,7 +692,7 @@ if (f.exists() && f.isFile()) { try { Files.delete(f.toPath()); - log.warn("removed " + f); + log.info("removed " + f); } catch (IOException e) { log.warn("removing " + f + "failed with " + e.getMessage()); } Index: src/uk/me/parabola/mkgmap/main/MapMaker.java =================================================================== --- src/uk/me/parabola/mkgmap/main/MapMaker.java (revision 4642) +++ src/uk/me/parabola/mkgmap/main/MapMaker.java (working copy) @@ -51,7 +51,7 @@ public String makeMap(CommandArgs args, String filename) { if (new File(filename).isDirectory()) { - System.err.println("Need a single file, not a directory: " + filename); + Logger.defaultLogger.error("Need a single file, not a directory: " + filename); return filename; } try { @@ -69,11 +69,10 @@ } return makeMap(args, src, ""); } catch (FormatException e) { - System.err.println("Bad file format: " + filename); - System.err.println(e.getMessage()); + Logger.defaultLogger.error("Bad file format: " + filename); return filename; } catch (FileNotFoundException e) { - System.err.println("Could not open file: " + filename); + Logger.defaultLogger.error("Could not open file: " + filename); return filename; } } @@ -120,14 +119,14 @@ map.close(); return outName; } catch (FileExistsException e) { - log.error("File exists already"); + Logger.defaultLogger.error(e.getMessage()); throw new MapFailedException("File exists already", e); } catch (FileNotWritableException e) { - log.error("Could not create or write to file"); + Logger.defaultLogger.error(e.getMessage()); throw new MapFailedException("Could not create or write to file", e); } catch (MapFailedException e) { - log.error(e.getMessage()); // make sure the filename is logged + Logger.defaultLogger.error(e.getMessage()); // make sure the filename is logged throw e; } } Index: src/uk/me/parabola/mkgmap/osmstyle/actions/EchoAction.java =================================================================== --- src/uk/me/parabola/mkgmap/osmstyle/actions/EchoAction.java (revision 4642) +++ src/uk/me/parabola/mkgmap/osmstyle/actions/EchoAction.java (working copy) @@ -16,9 +16,10 @@ package uk.me.parabola.mkgmap.osmstyle.actions; import uk.me.parabola.mkgmap.reader.osm.Element; +import uk.me.parabola.log.Logger; /** - * Sends a message to the console. + * Logs a message. * * @author Richard Fairhurst */ @@ -30,7 +31,7 @@ } public boolean perform(Element el) { - System.err.println(el.getBasicLogInformation() + " " + value.build(el, el)); + Logger.defaultLogger.echo(el.getBasicLogInformation() + " " + value.build(el, el)); return false; } } Index: src/uk/me/parabola/mkgmap/osmstyle/actions/EchoTagsAction.java =================================================================== --- src/uk/me/parabola/mkgmap/osmstyle/actions/EchoTagsAction.java (revision 4642) +++ src/uk/me/parabola/mkgmap/osmstyle/actions/EchoTagsAction.java (working copy) @@ -14,9 +14,10 @@ package uk.me.parabola.mkgmap.osmstyle.actions; import uk.me.parabola.mkgmap.reader.osm.Element; +import uk.me.parabola.log.Logger; /** - * Sends a message including the tags of an element to System.err. + * Logs a message including the tags of an element. * * @author WanMil */ @@ -28,7 +29,7 @@ } public boolean perform(Element el) { - System.err.println(el.getBasicLogInformation() + " " + el.toTagString() + " " + value.build(el, el)); + Logger.defaultLogger.echo(el.getBasicLogInformation() + " " + el.toTagString() + " " + value.build(el, el)); return false; } Index: src/uk/me/parabola/mkgmap/osmstyle/eval/AbstractOp.java =================================================================== --- src/uk/me/parabola/mkgmap/osmstyle/eval/AbstractOp.java (revision 4642) +++ src/uk/me/parabola/mkgmap/osmstyle/eval/AbstractOp.java (working copy) @@ -20,6 +20,7 @@ import java.util.Set; import uk.me.parabola.imgfmt.ExitException; +import uk.me.parabola.log.Logger; import uk.me.parabola.mkgmap.osmstyle.function.GetTagFunction; import uk.me.parabola.mkgmap.reader.osm.Element; import uk.me.parabola.mkgmap.scan.SyntaxException; @@ -210,7 +211,7 @@ } else if (this.isType(NodeType.EXISTS) || this.isType(NodeType.NOT_EXISTS) || this.isType(NodeType.NOT)) { set.addAll(getFirst().getEvaluatedTagKeys()); } else if (this.getFirst() != null) { - System.err.println("Unhandled type of Op"); + Logger.defaultLogger.error("Unhandled type of Op"); } } Index: src/uk/me/parabola/mkgmap/osmstyle/housenumber/HousenumberGenerator.java =================================================================== --- src/uk/me/parabola/mkgmap/osmstyle/housenumber/HousenumberGenerator.java (revision 4642) +++ src/uk/me/parabola/mkgmap/osmstyle/housenumber/HousenumberGenerator.java (working copy) @@ -106,7 +106,7 @@ if (n != nameSearchDepth) { nameSearchDepth = Math.min(25, Math.max(0, n)); if (nameSearchDepth != n) { - System.err.println("name-service-roads=" + n + " was changed to name-service-roads=" + nameSearchDepth); + Logger.defaultLogger.warn("name-service-roads=" + n + " was changed to name-service-roads=" + nameSearchDepth); } } } 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) @@ -165,6 +165,10 @@ private LineAdder lineAdder; private NearbyPoiHandler nearbyPoiHandler; + + static List unusedStyleOptions = new ArrayList<>(); + static List duplicateKeys = new ArrayList<>(); + static List unspecifiedStyleOptions = new ArrayList<>(); public StyledConverter(Style style, MapCollector collector, EnhancedProperties props) { this.collector = collector; @@ -204,7 +208,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 -> { @@ -250,14 +254,24 @@ String optionKey = pair[0]; String tagKey = STYLE_OPTION_PREF + optionKey; if (!style.getUsedTags().contains(tagKey)) { - System.err.println("Warning: Option style-options sets tag not used in style: '" - + optionKey + "' (gives " + tagKey + ")"); - } else { - String val = (pair.length == 1) ? "true" : pair[1]; - String old = styleTags.put(tagKey, val); - if (old != null) - log.error("duplicate tag key", optionKey, "in style option", styleOption); + synchronized(unusedStyleOptions) { + if (!unusedStyleOptions.contains(optionKey)) { + unusedStyleOptions.add(optionKey); + Logger.defaultLogger.warn("Option style-options sets tag not used in style: '" + + optionKey + "' (gives " + tagKey + ")"); + } + } } + String val = (pair.length == 1) ? "true" : pair[1]; + String old = styleTags.put(tagKey, val); + if (old != null) { + synchronized(duplicateKeys) { + if (!duplicateKeys.contains(optionKey)) { + duplicateKeys.add(optionKey); + Logger.defaultLogger.error("duplicate tag key", optionKey, "in style option", styleOption); + } + } + } } } // flag options used in style but not specified in --style-option @@ -264,8 +278,13 @@ if (style.getUsedTags() != null) { for (String s : style.getUsedTags()) { if (s != null && s.startsWith(STYLE_OPTION_PREF) && styleTags.get(s) == null) { - System.err.println("Warning: Option style-options doesn't specify '" - + s.replaceFirst(STYLE_OPTION_PREF, "") + "' (for " + s + ")"); + synchronized(unspecifiedStyleOptions) { + if (!unspecifiedStyleOptions.contains(s)) { + unspecifiedStyleOptions.add(s); + Logger.defaultLogger.warn("Option style-options doesn't specify '" + + s.replaceFirst(STYLE_OPTION_PREF, "") + "' (for " + s + ")"); + } + } } } } @@ -348,9 +367,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 +1094,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 +1251,6 @@ line.setType(replType); line.setPoints(points); - if (way.tagIsLikeYes(TK_ONEWAY)) line.setDirection(true); @@ -2231,7 +2249,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()); } } Index: src/uk/me/parabola/mkgmap/osmstyle/StyleImpl.java =================================================================== --- src/uk/me/parabola/mkgmap/osmstyle/StyleImpl.java (revision 4642) +++ src/uk/me/parabola/mkgmap/osmstyle/StyleImpl.java (working copy) @@ -239,19 +239,18 @@ try (InputStream is = this.getClass().getResourceAsStream("/styles/builtin-tag-list");) { if (is != null) { BufferedReader br = new BufferedReader(new InputStreamReader(is, StandardCharsets.UTF_8)); - // System.out.println("Got built in list"); String line; while ((line = br.readLine()) != null) { line = line.trim(); if (line.startsWith("#")) continue; - // System.out.println("adding " + line); + set.add(line); } } } catch (IOException e) { // the file doesn't exist, this is ok but unlikely - System.err.println("warning: built in tag list not found"); + Logger.defaultLogger.warn("built in tag list not found"); } return set; } @@ -262,7 +261,7 @@ l = LevelInfo.DEFAULT_LEVELS; LevelInfo[] levels = LevelInfo.createFromString(l); if (performChecks && levels[0].getBits() <= 10) { - System.err.println("Warning: Resolution values <= 10 may confuse MapSource: " + l); + Logger.defaultLogger.warn("Resolution values <= 10 may confuse MapSource: " + l); } l = generalOptions.get("overview-levels"); if (l != null){ @@ -270,10 +269,10 @@ // TODO: make sure that the combination of the two level strings makes sense if (performChecks){ if (ovLevels[0].getBits() <= 10){ - System.err.println("Warning: Resolution values <= 10 may confuse MapSource: " + l); + Logger.defaultLogger.warn("Resolution values <= 10 may confuse MapSource: " + l); } if (levels[0].getLevel() >= ovLevels[ovLevels.length-1].getLevel()){ - System.err.println("Warning: Overview level not higher than highest normal level. " + l); + Logger.defaultLogger.warn("Overview level not higher than highest normal level. " + l); } } List tmp = new ArrayList<>(); @@ -336,7 +335,7 @@ String val = opt.getValue(); if ("name-tag-list".equals(key)) { if (!"name".equals(val)) { - System.err.println("Warning: option name-tag-list used in the style options is ignored. " + Logger.defaultLogger.warn("Option name-tag-list used in the style options is ignored. " + "Please use only the command line option to specify this value."); } } else if (OPTION_LIST.contains(key)) { @@ -410,7 +409,7 @@ try { baseStyles.add(new StyleImpl(location, name, props, performChecks)); } catch (SyntaxException e) { - System.err.println("Error in style: " + e.getMessage()); + Logger.defaultLogger.error("Error in style: " + e.getMessage()); } catch (FileNotFoundException e) { // not found, try on the classpath. This is the common // case where you have an external style, but want to @@ -420,9 +419,9 @@ try { baseStyles.add(new StyleImpl(null, name, props, performChecks)); } catch (SyntaxException se) { - System.err.println("Error in style: " + se.getMessage()); + Logger.defaultLogger.error("Error in style: " + se.getMessage()); } catch (FileNotFoundException e1) { - log.error("Could not find base style", e); + Logger.defaultLogger.error("Could not find base style", e); } } } @@ -478,7 +477,7 @@ } if (version > VERSION) { - System.err.println("Warning: unrecognised style version " + version + + Logger.defaultLogger.warn("Unrecognised style version " + version + ", but only versions up to " + VERSION + " are understood"); } } @@ -546,7 +545,7 @@ try { style = new StyleImpl(loc, name, props, WITHOUT_CHECKS); } catch (SyntaxException e) { - System.err.println("Error in style: " + e.getMessage()); + Logger.defaultLogger.error("Error in style: " + e.getMessage()); throw new ExitException("Could not open style " + (name == null? "":name)); } catch (FileNotFoundException e) { String msg = "Could not open style "; Index: src/uk/me/parabola/mkgmap/reader/hgt/HGTList.java =================================================================== --- src/uk/me/parabola/mkgmap/reader/hgt/HGTList.java (revision 4642) +++ src/uk/me/parabola/mkgmap/reader/hgt/HGTList.java (working copy) @@ -38,7 +38,7 @@ try { knownHgt = loadConfig(); } catch (IOException e) { - e.printStackTrace(); + Logger.defaultLogger.error("Error reading hgt config", e); } } @@ -114,7 +114,7 @@ bs.set(getBitSetPos(lat, lon)); } catch (NumberFormatException e) { - e.printStackTrace(); + Logger.defaultLogger.error("Error reading latitude/longitude", e); } } return bs; Index: src/uk/me/parabola/mkgmap/reader/hgt/HGTReader.java =================================================================== --- src/uk/me/parabola/mkgmap/reader/hgt/HGTReader.java (revision 4642) +++ src/uk/me/parabola/mkgmap/reader/hgt/HGTReader.java (working copy) @@ -130,7 +130,7 @@ HGTList hgtList = HGTList.get(); if (hgtList != null) { if (hgtList.shouldExist(lat, lon)) - System.err.println(this.getClass().getSimpleName() + ": file " + fileName + " not found but it should exist. Height values will be 0."); + Logger.defaultLogger.warn(this.getClass().getSimpleName() + ": file " + fileName + " not found but it should exist. Height values will be 0."); } else { log.warn("file " + fileName + " not found. Is expected to cover sea."); } Index: src/uk/me/parabola/mkgmap/reader/osm/CoastlineFileLoader.java =================================================================== --- src/uk/me/parabola/mkgmap/reader/osm/CoastlineFileLoader.java (revision 4642) +++ src/uk/me/parabola/mkgmap/reader/osm/CoastlineFileLoader.java (working copy) @@ -106,8 +106,7 @@ } catch (FileNotFoundException exp) { log.error("Coastline file " + coastlineFile + " not found."); } catch (Exception exp) { - log.error(exp); - exp.printStackTrace(); + log.error("Unexpected exception reading " + coastlineFile, exp); } } coastlinesLoaded.set(true); Index: src/uk/me/parabola/mkgmap/reader/osm/o5m/O5mBinHandler.java =================================================================== --- src/uk/me/parabola/mkgmap/reader/osm/o5m/O5mBinHandler.java (revision 4642) +++ src/uk/me/parabola/mkgmap/reader/osm/o5m/O5mBinHandler.java (working copy) @@ -18,7 +18,9 @@ import java.io.InputStream; import java.nio.charset.StandardCharsets; +import uk.me.parabola.imgfmt.FormatException; import uk.me.parabola.imgfmt.app.Coord; +import uk.me.parabola.log.Logger; import uk.me.parabola.mkgmap.reader.osm.Element; import uk.me.parabola.mkgmap.reader.osm.GeneralRelation; import uk.me.parabola.mkgmap.reader.osm.Node; @@ -107,12 +109,13 @@ try { int start = is.read(); ++countBytes; - if (start != RESET_FLAG) - throw new IOException("wrong header byte " + start); + if (start != RESET_FLAG) { + Logger.defaultLogger.error("wrong header byte " + start); + throw new FormatException("wrong header byte " + start); + } readFile(); } catch (IOException e) { - System.err.println("exception after " + countBytes + " bytes"); - e.printStackTrace(); + Logger.defaultLogger.error("exception after " + countBytes + " bytes", e); } } Index: src/uk/me/parabola/mkgmap/reader/osm/OsmMapDataSource.java =================================================================== --- src/uk/me/parabola/mkgmap/reader/osm/OsmMapDataSource.java (revision 4642) +++ src/uk/me/parabola/mkgmap/reader/osm/OsmMapDataSource.java (working copy) @@ -167,7 +167,7 @@ break; } catch (InstantiationException | IllegalAccessException e) { // TODO Auto-generated catch block - e.printStackTrace(); + log.error("Unexpected error", e); } } } Index: src/uk/me/parabola/mkgmap/reader/osm/SeaGenerator.java =================================================================== --- src/uk/me/parabola/mkgmap/reader/osm/SeaGenerator.java (revision 4642) +++ src/uk/me/parabola/mkgmap/reader/osm/SeaGenerator.java (working copy) @@ -690,8 +690,7 @@ } catch (FileNotFoundException exp) { log.error("Preompiled sea tile " + tileName + " not found."); } catch (Exception exp) { - log.error(exp); - exp.printStackTrace(); + Logger.defaultLogger.error("Unexpected error reading "+ tileName, exp); } } Index: src/uk/me/parabola/mkgmap/reader/osm/xml/OsmXmlHandler.java =================================================================== --- src/uk/me/parabola/mkgmap/reader/osm/xml/OsmXmlHandler.java (revision 4642) +++ src/uk/me/parabola/mkgmap/reader/osm/xml/OsmXmlHandler.java (working copy) @@ -221,7 +221,7 @@ */ @Override public void fatalError(SAXParseException e) throws SAXException { - System.err.println("Error at line " + e.getLineNumber() + ", col " + Logger.defaultLogger.error("Error at line " + e.getLineNumber() + ", col " + e.getColumnNumber()); super.fatalError(e); } Index: src/uk/me/parabola/mkgmap/reader/polish/RestrictionHelper.java =================================================================== --- src/uk/me/parabola/mkgmap/reader/polish/RestrictionHelper.java (revision 4642) +++ src/uk/me/parabola/mkgmap/reader/polish/RestrictionHelper.java (working copy) @@ -14,6 +14,7 @@ import uk.me.parabola.imgfmt.app.CoordNode; import uk.me.parabola.imgfmt.app.net.GeneralRouteRestriction; +import uk.me.parabola.log.Logger; import uk.me.parabola.mkgmap.general.MapDetails; import java.util.ArrayList; @@ -56,7 +57,7 @@ if (restriction.isValid()) allRestrictions.add(restriction); else { - System.err.println(restriction); + Logger.defaultLogger.warn("Invalid restriction " + restriction.toString()); } } } Index: test/func/ArgsTest.java =================================================================== --- test/func/ArgsTest.java (revision 4642) +++ test/func/ArgsTest.java (working copy) @@ -38,7 +38,7 @@ public class ArgsTest extends Base { @Test public void testHelp() { - Outputs outputs = TestUtils.run("--help"); + Outputs outputs = TestUtils.runAsProcess("--help"); outputs.checkOutput("--help=options", "--help=links"); outputs.checkNoError(); checkNoStdFile(); @@ -46,7 +46,7 @@ @Test public void testHelpOptions() { - Outputs outputs = TestUtils.run("--help=options"); + Outputs outputs = TestUtils.runAsProcess("--help=options"); outputs.checkNoError(); outputs.checkOutput("--mapname=name", "--latin1", "--list-styles"); checkNoStdFile(); @@ -54,7 +54,7 @@ @Test public void testHelpUnknown() { - Outputs outputs = TestUtils.run("--help=unknown-help-option"); + Outputs outputs = TestUtils.runAsProcess("--help=unknown-help-option"); outputs.checkNoError(); outputs.checkOutput("Could not find", "unknown-help-option"); checkNoStdFile(); @@ -62,7 +62,7 @@ @Test public void testListStyles() { - Outputs op = TestUtils.run("--style-file=test/resources/teststyles", "--list-styles"); + Outputs op = TestUtils.runAsProcess("--style-file=test/resources/teststyles", "--list-styles"); op.checkNoError(); op.checkOutput("empty", "main", "simple", "derived", "2.2: A simple test style"); checkNoStdFile(); @@ -70,7 +70,7 @@ @Test public void testListStylesVerbose() { - Outputs op = TestUtils.run("--style-file=test/resources/teststyles", + Outputs op = TestUtils.runAsProcess("--style-file=test/resources/teststyles", "--verbose", "--list-styles"); op.checkNoError(); op.checkOutput("empty", "main", "simple", "derived", @@ -83,9 +83,9 @@ TestUtils.registerFile("osmmap.img"); int pri = 42; - Outputs op = TestUtils.run("--draw-priority=" + pri, + Outputs op = TestUtils.runAsProcess("--draw-priority=" + pri, Args.TEST_RESOURCE_OSM + "uk-test-1.osm.gz"); - op.checkNoError(); + op.checkError("Number of ExitExceptions: 0"); FileSystem fs = openFs(Args.DEF_MAP_FILENAME); ImgChannel chan = fs.open(Args.DEF_MAP_ID + ".TRE", "r"); @@ -96,7 +96,7 @@ @Test public void testNoDescription() { - Outputs op = TestUtils.run("--description", Args.TEST_RESOURCE_OSM + "uk-test-1.osm.gz"); - op.checkNoError(); + Outputs op = TestUtils.runAsProcess("--description", Args.TEST_RESOURCE_OSM + "uk-test-1.osm.gz"); + op.checkError("Number of ExitExceptions: 0"); } } Index: test/func/files/GmapsuppTest.java =================================================================== --- test/func/files/GmapsuppTest.java (revision 4642) +++ test/func/files/GmapsuppTest.java (working copy) @@ -401,7 +401,7 @@ "--latin1", Args.TEST_RESOURCE_OSM + "uk-test-2.osm.gz"); - Outputs outputs = TestUtils.run(Args.TEST_STYLE_ARG, + Outputs outputs = TestUtils.runAsProcess(Args.TEST_STYLE_ARG, "--gmapsupp", "--index", Index: test/func/files/IndexTest.java =================================================================== --- test/func/files/IndexTest.java (revision 4642) +++ test/func/files/IndexTest.java (working copy) @@ -36,7 +36,7 @@ f.delete(); assertFalse("does not pre-exist", f.exists()); - Outputs outputs = TestUtils.run( + Outputs outputs = TestUtils.runAsProcess( Args.TEST_STYLE_ARG, "--index", "--latin1", @@ -45,7 +45,7 @@ Args.TEST_RESOURCE_IMG + "63240001.img", Args.TEST_RESOURCE_IMG + "63240002.img" ); - outputs.checkNoError(); + outputs.checkError("Number of ExitExceptions: 0"); TestUtils.registerFile(MDR_IMG); TestUtils.registerFile(OVERVIEW_NAME+".tdb"); Index: test/func/lib/TestUtils.java =================================================================== --- test/func/lib/TestUtils.java (revision 4642) +++ test/func/lib/TestUtils.java (working copy) @@ -16,12 +16,16 @@ */ package func.lib; +import java.io.BufferedReader; import java.io.ByteArrayOutputStream; import java.io.Closeable; import java.io.File; import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStreamReader; import java.io.OutputStream; import java.io.PrintStream; +import java.lang.ProcessBuilder.Redirect; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; @@ -28,6 +32,7 @@ import java.util.Collections; import java.util.Deque; import java.util.List; +import java.util.concurrent.TimeUnit; import uk.me.parabola.imgfmt.Utils; import uk.me.parabola.mkgmap.general.LevelInfo; @@ -136,6 +141,52 @@ } /** + * Run with the given args as a new process. Some standard arguments are added first. + * + * @param in The arguments to use. + */ + public static Outputs runAsProcess(String ... in) { + List args = new ArrayList<>(Arrays.asList(in)); + args.add(0, Args.TEST_STYLE_ARG); + args.add(0, "dist\\mkgmap.jar"); + args.add(0, "-jar"); + args.add(0, "java.exe"); + + ProcessBuilder pb = new ProcessBuilder(args); + StringBuilder outBuilder = new StringBuilder(); + StringBuilder errBuilder = new StringBuilder(); + try { + Process process = pb.start(); + try (BufferedReader errorStream = new BufferedReader(new InputStreamReader(process.getErrorStream())); + BufferedReader outputStream = new BufferedReader(new InputStreamReader(process.getInputStream()))) { + while (true) { + getStreamOutput(outputStream, outBuilder); + getStreamOutput(errorStream, errBuilder); + if (process.waitFor(1, TimeUnit.SECONDS)) { + getStreamOutput(outputStream, outBuilder); + getStreamOutput(errorStream, errBuilder); + break; + } + } + } + } catch (IOException e) { + // do nothing + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + return new Outputs(outBuilder.toString(), errBuilder.toString()); + } + + private static void getStreamOutput(BufferedReader stream, StringBuilder stringBuilder) throws IOException { + char[] buff = new char[100000]; + while (stream.ready()) { + int count = stream.read(buff); + if (count > 0) + stringBuilder.append(buff, 0, count); + } + } + + /** * Create a rule set out of a string. The string is processed * as if it were in a file and the levels spec had been set. */