### Eclipse Workspace Patch 1.0 #P josm-11710 Index: src/org/openstreetmap/josm/data/gpx/GpxImageEntry.java =================================================================== --- src/org/openstreetmap/josm/data/gpx/GpxImageEntry.java (revision 17873) +++ src/org/openstreetmap/josm/data/gpx/GpxImageEntry.java (working copy) @@ -571,7 +571,7 @@ elevation = tmp.elevation; gpsTime = tmp.gpsTime; exifImgDir = tmp.exifImgDir; - isNewGpsData = tmp.isNewGpsData; + isNewGpsData = isNewGpsData || tmp.isNewGpsData; tmp = null; } tmpUpdated(); Index: src/org/openstreetmap/josm/data/gpx/GpxImageCorrelation.java =================================================================== --- src/org/openstreetmap/josm/data/gpx/GpxImageCorrelation.java (revision 17873) +++ src/org/openstreetmap/josm/data/gpx/GpxImageCorrelation.java (working copy) @@ -229,7 +229,7 @@ break; } long tagms = TimeUnit.MINUTES.toMillis(tagTime); - if (curTmp.getPos() == null && + if (!curTmp.hasNewGpsData() && (Math.abs(time - curWpTime) <= tagms || Math.abs(prevWpTime - time) <= tagms)) { if (prevWp != null && time < curWpTime - half) { @@ -255,7 +255,7 @@ if (imgTime < prevWpTime) { break; } - if (curTmp.getPos() == null) { + if (!curTmp.hasNewGpsData()) { // The values of timeDiff are between 0 and 1, it is not seconds but a dimensionless variable double timeDiff = (double) (imgTime - prevWpTime) / Math.abs(curWpTime - prevWpTime); curTmp.setPos(prevWp.getCoor().interpolate(curWp.getCoor(), timeDiff)); Index: src/org/openstreetmap/josm/gui/layer/geoimage/CorrelateGpxWithImages.java =================================================================== --- src/org/openstreetmap/josm/gui/layer/geoimage/CorrelateGpxWithImages.java (revision 17873) +++ src/org/openstreetmap/josm/gui/layer/geoimage/CorrelateGpxWithImages.java (working copy) @@ -1090,10 +1090,10 @@ // Construct a list of images that have a date, and sort them on the date. List dateImgLst = getSortedImgList(); // Create a temporary copy for each image - for (ImageEntry ie : dateImgLst) { - ie.createTmp(); - ie.getTmp().setPos(null); - } + dateImgLst.forEach(i -> { + i.createTmp(); + i.getTmp().unflagNewGpsData(); + }); GpxDataWrapper selGpx = selectedGPX(false); if (selGpx == null) Index: test/unit/org/openstreetmap/josm/data/gpx/GpxImageCorrelationTest.java =================================================================== --- test/unit/org/openstreetmap/josm/data/gpx/GpxImageCorrelationTest.java (revision 17873) +++ test/unit/org/openstreetmap/josm/data/gpx/GpxImageCorrelationTest.java (working copy) @@ -6,11 +6,17 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.io.IOException; import java.util.Arrays; import java.util.List; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.MethodOrderer.Alphanumeric; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.TestInstance.Lifecycle; +import org.junit.jupiter.api.TestMethodOrder; import org.junit.jupiter.api.extension.RegisterExtension; import org.openstreetmap.josm.TestUtils; import org.openstreetmap.josm.data.coor.CachedLatLon; @@ -21,6 +27,7 @@ import org.openstreetmap.josm.testutils.JOSMTestRules; import org.openstreetmap.josm.tools.date.DateUtils; import org.openstreetmap.josm.tools.date.DateUtilsTest; +import org.xml.sax.SAXException; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -27,6 +34,8 @@ /** * Unit tests of {@link GpxImageCorrelation} class. */ +@TestMethodOrder(Alphanumeric.class) +@TestInstance(Lifecycle.PER_CLASS) class GpxImageCorrelationTest { /** @@ -36,69 +45,89 @@ @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") public JOSMTestRules test = new JOSMTestRules(); + GpxData gpx; + GpxImageEntry ib, i0, i1, i2, i3, i4, i5, i6, i7; + List images; + IPreferences s; + /** * Setup test. + * @throws IOException if an error occurs during reading. + * @throws SAXException if a SAX error occurs */ @BeforeAll - public static void setUp() { + public void setUp() throws IOException, SAXException { + s = Config.getPref(); DateUtilsTest.setTimeZone(DateUtils.UTC); - } - /** - * Tests matching of images to a GPX track. - * @throws Exception if the track cannot be parsed - */ - @Test - void testMatchGpxTrack() throws Exception { - IPreferences s = Config.getPref(); - final GpxData gpx = GpxReaderTest.parseGpxData(TestUtils.getTestDataRoot() + "tracks/tracks.gpx"); + gpx = GpxReaderTest.parseGpxData(TestUtils.getTestDataRoot() + "tracks/tracks.gpx"); assertEquals(5, gpx.tracks.size()); assertEquals(1, gpx.tracks.iterator().next().getSegments().size()); assertEquals(128, gpx.tracks.iterator().next().getSegments().iterator().next().getWayPoints().size()); - final GpxImageEntry ib = new GpxImageEntry(); + ib = new GpxImageEntry(); ib.setExifTime(DateUtils.parseInstant("2016:01:03 11:54:58")); // 5 minutes before start of GPX - ib.createTmp(); - final GpxImageEntry i0 = new GpxImageEntry(); + + i0 = new GpxImageEntry(); i0.setExifTime(DateUtils.parseInstant("2016:01:03 11:59:54")); // 4 sec before start of GPX - i0.createTmp(); - final GpxImageEntry i1 = new GpxImageEntry(); + + i1 = new GpxImageEntry(); i1.setExifTime(DateUtils.parseInstant("2016:01:03 12:04:01")); - i1.createTmp(); - final GpxImageEntry i2 = new GpxImageEntry(); + i1.setPos(new CachedLatLon(2, 3)); //existing position inside the track, should always be overridden + + i2 = new GpxImageEntry(); i2.setExifTime(DateUtils.parseInstant("2016:01:03 12:04:57")); - i2.createTmp(); - final GpxImageEntry i3 = new GpxImageEntry(); + + i3 = new GpxImageEntry(); i3.setExifTime(DateUtils.parseInstant("2016:01:03 12:05:05")); - i3.createTmp(); - final GpxImageEntry i4 = new GpxImageEntry(); //Image close to two points with elevation, but without time + + i4 = new GpxImageEntry(); //Image close to two points with elevation, but without time i4.setExifTime(DateUtils.parseInstant("2016:01:03 12:05:20")); - i4.createTmp(); - final GpxImageEntry i5 = new GpxImageEntry(); //between two tracks, closer to first + + i5 = new GpxImageEntry(); //between two tracks, closer to first i5.setExifTime(DateUtils.parseInstant("2016:01:03 12:07:00")); - i5.createTmp(); - final GpxImageEntry i6 = new GpxImageEntry(); //between two tracks, closer to second (more than 1 minute from any track) + + i6 = new GpxImageEntry(); //between two tracks, closer to second (more than 1 minute from any track) i6.setExifTime(DateUtils.parseInstant("2016:01:03 12:07:45")); - i6.createTmp(); - List images = Arrays.asList(ib, i0, i1, i2, i3, i4, i5, i6); + i7 = new GpxImageEntry(); + i7.setExifTime(DateUtils.parseInstant("2021:01:01 00:00:00")); + i7.setPos(new CachedLatLon(1, 2)); //existing position outside the track + // the position should never be null (should keep the old position if not overridden, see #11710) - // TEST #1: default settings - // tag images within 2 minutes to tracks/segments, interpolate between segments only + images = Arrays.asList(ib, i0, i1, i2, i3, i4, i5, i6, i7); + } + + @BeforeEach + void clearTmp() { + for (GpxImageEntry i : images) { + i.discardTmp(); + i.createTmp(); + } + } + + /** + * Tests matching of images to a GPX track. + * + * TEST #1: default settings + * tag images within 2 minutes to tracks/segments, interpolate between segments only + */ + @Test + void testMatchGpxTrack1() { assertEquals(7, GpxImageCorrelation.matchGpxTrack(images, gpx, 0, false)); assertEquals(null, ib.getPos()); assertEquals(new CachedLatLon(47.19286847859621, 8.79732714034617), i0.getPos()); // start of track assertEquals(new CachedLatLon(47.196979885920882, 8.79541271366179), i1.getPos()); // exact match assertEquals(new CachedLatLon(47.197319911792874, 8.792139580473304), i3.getPos()); // exact match - assertEquals(new CachedLatLon((47.197131179273129 + 47.197186248376966) / 2, (8.792974585667253 + 8.792809881269932) / 2), - i2.getPos()); // interpolated - assertEquals(new CachedLatLon(47.197568312311816, 8.790292849679897), - i4.getPos()); // interpolated between points without timestamp - assertEquals(new CachedLatLon(47.19819249585271, 8.78536943346262), - i5.getPos()); // tagged to last WP of first track, because closer and within 2 min (default setting) - assertEquals(new CachedLatLon(47.20138901844621, 8.774476982653141), - i6.getPos()); // tagged to first WP of second track, because closer and within 2 min (default setting) - assertFalse(ib.hasNewGpsData()); + assertEquals(new CachedLatLon((47.197131179273129 + 47.197186248376966) / 2, + (8.792974585667253 + 8.792809881269932) / 2), i2.getPos()); // interpolated + assertEquals(new CachedLatLon(47.197568312311816, 8.790292849679897), i4.getPos()); // interpolated between points without timestamp + // tagged to last WP of first track, because closer and within 2 min (default setting): + assertEquals(new CachedLatLon(47.19819249585271, 8.78536943346262), i5.getPos()); + // tagged to first WP of second track, because closer and within 2 min (default setting): + assertEquals(new CachedLatLon(47.20138901844621, 8.774476982653141), i6.getPos()); + assertEquals(new CachedLatLon(1, 2), i7.getPos()); //existing EXIF data is kept + assertFalse(ib.hasNewGpsData() || i7.hasNewGpsData()); assertTrue(i0.hasNewGpsData() && i1.hasNewGpsData() && i2.hasNewGpsData() && i3.hasNewGpsData() && i4.hasNewGpsData() && i5.hasNewGpsData() && i6.hasNewGpsData()); // First waypoint has no speed in matchGpxTrack(). Speed is calculated @@ -123,11 +152,15 @@ assertEquals(DateUtils.parseInstant("2016:01:03 12:04:01"), i1.getGpsInstant()); assertEquals(DateUtils.parseInstant("2016:01:03 12:04:57"), i2.getGpsInstant()); assertEquals(DateUtils.parseInstant("2016:01:03 12:05:05"), i3.getGpsInstant()); + } - clearTmp(images); - - // TEST #2: Disable all interpolation and tagging close to tracks. Only i1-i4 are tagged - + /** + * Tests matching of images to a GPX track. + * + * TEST #2: Disable all interpolation and tagging close to tracks. Only i1-i4 are tagged + */ + @Test + void testMatchGpxTrack2() { s.putBoolean("geoimage.trk.tag", false); s.putBoolean("geoimage.trk.int", false); s.putBoolean("geoimage.seg.tag", false); @@ -137,21 +170,26 @@ assertEquals(null, ib.getPos()); assertEquals(null, i0.getPos()); assertEquals(new CachedLatLon(47.196979885920882, 8.79541271366179), i1.getPos()); - assertEquals(new CachedLatLon((47.197131179273129 + 47.197186248376966) / 2, (8.792974585667253 + 8.792809881269932) / 2), i2.getPos()); + assertEquals(new CachedLatLon((47.197131179273129 + 47.197186248376966) / 2, + (8.792974585667253 + 8.792809881269932) / 2), i2.getPos()); assertEquals(new CachedLatLon(47.197319911792874, 8.792139580473304), i3.getPos()); assertEquals(new CachedLatLon(47.197568312311816, 8.790292849679897), i4.getPos()); assertEquals(null, i5.getPos()); assertEquals(null, i6.getPos()); + } - clearTmp(images); - - // TEST #3: Disable all interpolation and allow tagging within 1 minute of a track. i0-i5 are tagged. - // i6 will not be tagged, because it's 68 seconds away from the next waypoint in either direction - + /** + * Tests matching of images to a GPX track. + * + * TEST #3: Disable all interpolation and allow tagging within 1 minute of a track. i0-i5 are tagged. + * i6 will not be tagged, because it's 68 seconds away from the next waypoint in either direction + * i7 will keep the old position + */ + @Test + void testMatchGpxTrack3() { s.putBoolean("geoimage.trk.tag", true); s.putBoolean("geoimage.trk.tag.time", true); s.putInt("geoimage.trk.tag.time.val", 1); - s.putBoolean("geoimage.trk.int", false); s.putBoolean("geoimage.seg.tag", false); s.putBoolean("geoimage.seg.int", false); @@ -160,22 +198,30 @@ assertEquals(null, ib.getPos()); assertEquals(new CachedLatLon(47.19286847859621, 8.79732714034617), i0.getPos()); assertEquals(new CachedLatLon(47.196979885920882, 8.79541271366179), i1.getPos()); - assertEquals(new CachedLatLon((47.197131179273129 + 47.197186248376966) / 2, (8.792974585667253 + 8.792809881269932) / 2), i2.getPos()); + assertEquals(new CachedLatLon((47.197131179273129 + 47.197186248376966) / 2, + (8.792974585667253 + 8.792809881269932) / 2), i2.getPos()); assertEquals(new CachedLatLon(47.197319911792874, 8.792139580473304), i3.getPos()); assertEquals(new CachedLatLon(47.197568312311816, 8.790292849679897), i4.getPos()); assertEquals(new CachedLatLon(47.19819249585271, 8.78536943346262), i5.getPos()); assertEquals(null, i6.getPos()); + assertEquals(new CachedLatLon(1, 2), i7.getPos()); + } - clearTmp(images); - - // TEST #4: Force tagging (parameter forceTags=true, no change of configuration). All images will be tagged - // i5-i6 will now be interpolated, therefore it will have an elevation and different coordinates than in tests above - - assertEquals(8, GpxImageCorrelation.matchGpxTrack(images, gpx, 0, true)); + /** + * Tests matching of images to a GPX track. + * + * TEST #4: Force tagging (parameter forceTags=true, no change of configuration). All images will be tagged + * i5-i6 will now be interpolated, therefore it will have an elevation and different coordinates than in tests above + * i7 will be at the end of the track + */ + @Test + void testMatchGpxTrack4() { + assertEquals(9, GpxImageCorrelation.matchGpxTrack(images, gpx, 0, true)); assertEquals(new CachedLatLon(47.19286847859621, 8.79732714034617), ib.getPos()); assertEquals(new CachedLatLon(47.19286847859621, 8.79732714034617), i0.getPos()); assertEquals(new CachedLatLon(47.196979885920882, 8.79541271366179), i1.getPos()); - assertEquals(new CachedLatLon((47.197131179273129 + 47.197186248376966) / 2, (8.792974585667253 + 8.792809881269932) / 2), i2.getPos()); + assertEquals(new CachedLatLon((47.197131179273129 + 47.197186248376966) / 2, + (8.792974585667253 + 8.792809881269932) / 2), i2.getPos()); assertEquals(new CachedLatLon(47.197319911792874, 8.792139580473304), i3.getPos()); assertEquals(new CachedLatLon(47.197568312311816, 8.790292849679897), i4.getPos()); assertEquals(new CachedLatLon(47.198845306804905, 8.783144918860685), i5.getPos()); // interpolated between tracks @@ -184,11 +230,17 @@ assertEquals(Double.valueOf(447.894014085), i5.getElevation(), 0.000001); assertEquals(Double.valueOf(437.395070423), i6.getElevation(), 0.000001); - clearTmp(images); + assertEquals(new CachedLatLon(47.20126815140247, 8.77192972227931), i7.getPos()); + } - // TEST #5: Force tagging (parameter forceTags=false, but configuration changed). - // Results same as #4 - + /** + * Tests matching of images to a GPX track. + * + * TEST #5: Force tagging (parameter forceTags=false, but configuration changed). + * Results same as #4 + */ + @Test + void testMatchGpxTrack5() { s.putBoolean("geoimage.trk.tag", true); s.putBoolean("geoimage.trk.tag.time", false); s.putBoolean("geoimage.trk.int", true); @@ -201,11 +253,12 @@ s.putBoolean("geoimage.seg.int.time", false); s.putBoolean("geoimage.seg.int.dist", false); - assertEquals(8, GpxImageCorrelation.matchGpxTrack(images, gpx, 0, false)); + assertEquals(9, GpxImageCorrelation.matchGpxTrack(images, gpx, 0, false)); assertEquals(new CachedLatLon(47.19286847859621, 8.79732714034617), ib.getPos()); assertEquals(new CachedLatLon(47.19286847859621, 8.79732714034617), i0.getPos()); assertEquals(new CachedLatLon(47.196979885920882, 8.79541271366179), i1.getPos()); - assertEquals(new CachedLatLon((47.197131179273129 + 47.197186248376966) / 2, (8.792974585667253 + 8.792809881269932) / 2), i2.getPos()); + assertEquals(new CachedLatLon((47.197131179273129 + 47.197186248376966) / 2, + (8.792974585667253 + 8.792809881269932) / 2), i2.getPos()); assertEquals(new CachedLatLon(47.197319911792874, 8.792139580473304), i3.getPos()); assertEquals(new CachedLatLon(47.197568312311816, 8.790292849679897), i4.getPos()); assertEquals(new CachedLatLon(47.198845306804905, 8.783144918860685), i5.getPos()); @@ -214,12 +267,18 @@ assertEquals(Double.valueOf(447.894014085), i5.getElevation(), 0.000001); assertEquals(Double.valueOf(437.395070423), i6.getElevation(), 0.000001); - clearTmp(images); + assertEquals(new CachedLatLon(47.20126815140247, 8.77192972227931), i7.getPos()); + } - // TEST #6: Disable tagging but allow interpolation when tracks are less than 500m apart. i0-i4 are tagged. - // i5-i6 will not be tagged, because the tracks are 897m apart. - // not checking all the coordinates again, did that 5 times already, just the number of matched images - + /** + * Tests matching of images to a GPX track. + * + * TEST #6: Disable tagging but allow interpolation when tracks are less than 500m apart. i0-i4 are tagged. + * i5-i6 will not be tagged, because the tracks are 897m apart. + * not checking all the coordinates again, did that 5 times already, just the number of matched images + */ + @Test + void testMatchGpxTrack6() { s.putBoolean("geoimage.trk.tag", false); s.putBoolean("geoimage.trk.int", true); s.putBoolean("geoimage.trk.int.time", false); @@ -229,19 +288,35 @@ s.putBoolean("geoimage.seg.int", false); assertEquals(4, GpxImageCorrelation.matchGpxTrack(images, gpx, 0, false)); - clearTmp(images); + } - // TEST #7: Disable tagging but allow interpolation when tracks are less than 1000m apart. i0-i6 are tagged. - // i5-i6 will be tagged, because the tracks are 897m apart. - + /** + * Tests matching of images to a GPX track. + * + * TEST #7: Disable tagging but allow interpolation when tracks are less than 1000m apart. i0-i6 are tagged. + * i5-i6 will be tagged, because the tracks are 897m apart. + */ + @Test + void testMatchGpxTrack7() { + s.putBoolean("geoimage.trk.tag", false); + s.putBoolean("geoimage.trk.int", true); + s.putBoolean("geoimage.trk.int.time", false); + s.putBoolean("geoimage.trk.int.dist", true); s.putInt("geoimage.trk.int.dist.val", 1000); + s.putBoolean("geoimage.seg.tag", false); + s.putBoolean("geoimage.seg.int", false); assertEquals(6, GpxImageCorrelation.matchGpxTrack(images, gpx, 0, false)); - clearTmp(images); + } - // TEST #8: Disable tagging but allow interpolation when tracks are less than 2 min apart. i0-i4 are tagged. - // i5-i6 will not be tagged, because the tracks are 2.5min apart. - + /** + * Tests matching of images to a GPX track. + * + * TEST #8: Disable tagging but allow interpolation when tracks are less than 2 min apart. i0-i4 are tagged. + * i5-i6 will not be tagged, because the tracks are 2.5min apart. + */ + @Test + void testMatchGpxTrack8() { s.putBoolean("geoimage.trk.tag", false); s.putBoolean("geoimage.trk.int", true); s.putBoolean("geoimage.trk.int.time", true); @@ -251,24 +326,27 @@ s.putBoolean("geoimage.seg.int", false); assertEquals(4, GpxImageCorrelation.matchGpxTrack(images, gpx, 0, false)); - clearTmp(images); + } - // TEST #9: Disable tagging but allow interpolation when tracks are less than 3 min apart. i0-i6 are tagged. - // i5-i6 will be tagged, because the tracks are 2.5min apart. - + /** + * Tests matching of images to a GPX track. + * + * TEST #9: Disable tagging but allow interpolation when tracks are less than 3 min apart. i0-i6 are tagged. + * i5-i6 will be tagged, because the tracks are 2.5min apart. + */ + @Test + void testMatchGpxTrack9() { + s.putBoolean("geoimage.trk.tag", false); + s.putBoolean("geoimage.trk.int", true); + s.putBoolean("geoimage.trk.int.time", true); s.putInt("geoimage.trk.int.time.val", 3); + s.putBoolean("geoimage.trk.int.dist", false); + s.putBoolean("geoimage.seg.tag", false); + s.putBoolean("geoimage.seg.int", false); assertEquals(6, GpxImageCorrelation.matchGpxTrack(images, gpx, 0, false)); - } - private void clearTmp(List imgs) { - for (GpxImageEntry i : imgs) { - i.discardTmp(); - i.createTmp(); - } - } - /** * Unit test of {@link GpxImageCorrelation#getElevation} */