Skip NMEA sentences cut short, rather than misdate a fix by them

README.md

@@ -240,7 +240,9 @@ description-level processing error are included in `processed_file_count`.
 Unsupported entries include their filename, category, stable reason code, and
 normalized extension. Hidden/system files and common GPS or metadata sidecars
 are ignored during directory discovery, but an explicitly supplied unsupported
-file is always reported. The report is written only to the requested local path
+file is always reported. Such a file also fails processing, with a
+`MapillaryUnsupportedFileError` entry in the image description file, whether
+or not the report is requested. The report is written only to the requested local path
 and contains absolute source filenames, so treat it as local capture metadata.
 
 ## Upload

mapillary_tools/exceptions.py

@@ -58,6 +58,13 @@ class MapillaryGeoTaggingError(MapillaryDescriptionError):
     pass
 
 
+class MapillaryUnsupportedFileError(MapillaryDescriptionError):
+    """
+    A file named for processing is neither an image nor a video that can be
+    processed
+    """
+
+
 class MapillaryVideoGPSUnusableError(MapillaryDescriptionError):
     """
     The video has no GPS track that can be used: none at all, an empty one, or

mapillary_tools/geotag/factory.py

@@ -356,20 +356,34 @@ def process(
         sorted(reprocessable_paths - handed_paths), filetypes
     )
 
-    if video_types is not None and (unread_paths or sourceless_videos):
+    # Files that sources were let through, but that none of them described,
+    # nor said why, which they should not do: so that every file given gets
+    # one entry, but those that --filetypes leaves out
+    described_paths = {metadata.filename for metadata in final_metadatas}
+    unhandled_images, unhandled_videos = _filter_images_and_videos(
+        sorted(
+            (reprocessable_paths & handed_paths) - described_paths - set(unread_paths)
+        ),
+        filetypes,
+    )
+
+    if video_types is not None and (
+        unread_paths or sourceless_videos or unhandled_videos
+    ):
         # Limited to some types of video, a video that no source read may not
         # be of any of them. Those of other types are left out, as the videos
         # found to be of other types
         kept_paths = set(
             _videos_not_of_other_types(
-                unread_paths + sourceless_videos,
+                unread_paths + sourceless_videos + unhandled_videos,
                 video_types,
                 # The same for every option (see process_geotag_properties)
                 num_processes=next(iter(options)).num_processes,
             )
         )
         unread_paths = [path for path in unread_paths if path in kept_paths]
         sourceless_videos = [path for path in sourceless_videos if path in kept_paths]
+        unhandled_videos = [path for path in unhandled_videos if path in kept_paths]
 
     final_metadatas.extend(
         _describe_unread_video(path, unread_videos[path]) for path in unread_paths
@@ -378,6 +392,10 @@ def process(
         _describe_sourceless_file(path, options, filetypes)
         for path in sourceless_images + sourceless_videos
     )
+    final_metadatas.extend(
+        _describe_unhandled_file(path, options)
+        for path in unhandled_images + unhandled_videos
+    )
 
     return final_metadatas
 
@@ -461,6 +479,43 @@ def _describe_sourceless_file(
     )
 
 
+def _describe_unhandled_file(
+    path: Path, options: T.Iterable[SourceOption]
+) -> types.ErrorMetadata:
+    """
+    Describe a file that the sources of **options** were let through, but that
+    none of them described, rather than leave it out of the results as if it
+    had not been there
+
+    >>> options = [SourceOption(SourceType.NATIVE), SourceOption(SourceType.EXIFTOOL_RUNTIME)]
+    >>> _describe_unhandled_file(Path("i.jpg"), options).error
+    MapillaryGeoTaggingError('The geotag sources native, exiftool_runtime gave no result for this image')
+    >>> _describe_unhandled_file(Path("v.mp4"), options[:1]).error
+    MapillaryVideoGPSNotFoundError('The geotag source native gave no result for this video')
+    """
+    names = list(dict.fromkeys(option.source.value for option in options))
+    sources = (
+        f"The geotag sources {', '.join(names)}"
+        if len(names) > 1
+        else f"The geotag source {names[0]}"
+    )
+    if utils.is_image_file(path):
+        return types.describe_error_metadata(
+            exceptions.MapillaryGeoTaggingError(
+                f"{sources} gave no result for this image"
+            ),
+            path,
+            filetype=types.FileType.IMAGE,
+        )
+    return types.describe_error_metadata(
+        exceptions.MapillaryVideoGPSNotFoundError(
+            f"{sources} gave no result for this video"
+        ),
+        path,
+        filetype=types.FileType.VIDEO,
+    )
+
+
 def _describe_unread_video(
     video_path: Path, sources: T.Sequence[SourceType]
 ) -> types.ErrorMetadata:

mapillary_tools/geotag/geotag_images_from_gpx.py

@@ -28,6 +28,75 @@
 LOG = logging.getLogger(__name__)
 
 
+def check_within_track(
+    time: float,
+    sorted_points: T.Sequence[geo.Point],
+    gaps: T.Sequence[tuple[geo.Point, geo.Point]] = (),
+) -> None:
+    """
+    Raise MapillaryOutsideGPXTrackError if **time** falls outside the track of
+    **sorted_points**, with a tolerance of 1 ms: before its start, after its
+    end, or in one of its **gaps** (see track_gaps())
+
+    >>> track = [geo.Point(time=t, lat=0, lon=0, alt=None, angle=None) for t in (10, 20, 9000)]
+    >>> check_within_track(15.0, track)
+    >>> check_within_track(9000.0005, track)
+    >>> check_within_track(9.5, track)
+    Traceback (most recent call last):
+    ...
+    mapillary_tools.exceptions.MapillaryOutsideGPXTrackError: The image date time is 0.5 seconds behind the GPX start point
+    >>> check_within_track(4000.0, track, track_gaps(track, 3600))
+    Traceback (most recent call last):
+    ...
+    mapillary_tools.exceptions.MapillaryOutsideGPXTrackError: The GPX track has a gap of 2.5 hours, from 1970-01-01T00:00:20+00:00 at (0, 0) to 1970-01-01T02:30:00+00:00 at (0, 0), which the image time 1970-01-01T01:06:40+00:00 falls in
+    """
+    assert sorted_points, "must have at least one point"
+
+    if time < sorted_points[0].time:
+        delta = sorted_points[0].time - time
+        gpx_start_time = build_capture_time(sorted_points[0].time)
+        gpx_end_time = build_capture_time(sorted_points[-1].time)
+        # with the tolerance of 1ms
+        if 0.001 < delta:
+            raise exceptions.MapillaryOutsideGPXTrackError(
+                f"The image date time is {round(delta, 3)} seconds behind the GPX start point",
+                image_time=build_capture_time(time),
+                gpx_start_time=gpx_start_time,
+                gpx_end_time=gpx_end_time,
+            )
+
+    if sorted_points[-1].time < time:
+        delta = time - sorted_points[-1].time
+        gpx_start_time = build_capture_time(sorted_points[0].time)
+        gpx_end_time = build_capture_time(sorted_points[-1].time)
+        # with the tolerance of 1ms
+        if 0.001 < delta:
+            raise exceptions.MapillaryOutsideGPXTrackError(
+                f"The image time is {round(delta, 3)} seconds beyond the GPX end point",
+                image_time=build_capture_time(time),
+                gpx_start_time=gpx_start_time,
+                gpx_end_time=gpx_end_time,
+            )
+
+    # The last gap that starts at or before the image
+    idx = bisect.bisect_right([before.time for before, _ in gaps], time)
+    if idx:
+        before, after = gaps[idx - 1]
+        delta = min(time - before.time, after.time - time)
+        # with the tolerance of 1ms, as at the ends of the track
+        if 0.001 < delta:
+            captured_at = datetime.datetime.fromtimestamp(
+                time, tz=datetime.timezone.utc
+            ).isoformat()
+            track_gap = describe_gap(before, after, before.time, after.time)
+            raise exceptions.MapillaryOutsideGPXTrackError(
+                f"The GPX track has {track_gap}, which the image time {captured_at} falls in",
+                image_time=build_capture_time(time),
+                gpx_start_time=build_capture_time(sorted_points[0].time),
+                gpx_end_time=build_capture_time(sorted_points[-1].time),
+            )
+
+
 class GeotagImagesFromGPX(GeotagImagesFromGeneric):
     def __init__(
         self,
@@ -55,55 +124,7 @@ def _interpolate_image_metadata_along(
         sorted_points: T.Sequence[geo.Point],
         gaps: T.Sequence[tuple[geo.Point, geo.Point]] = (),
     ) -> types.ImageMetadata:
-        assert sorted_points, "must have at least one point"
-
-        if image_metadata.time < sorted_points[0].time:
-            delta = sorted_points[0].time - image_metadata.time
-            gpx_start_time = build_capture_time(sorted_points[0].time)
-            gpx_end_time = build_capture_time(sorted_points[-1].time)
-            # with the tolerance of 1ms
-            if 0.001 < delta:
-                raise exceptions.MapillaryOutsideGPXTrackError(
-                    f"The image date time is {round(delta, 3)} seconds behind the GPX start point",
-                    image_time=build_capture_time(image_metadata.time),
-                    gpx_start_time=gpx_start_time,
-                    gpx_end_time=gpx_end_time,
-                )
-
-        if sorted_points[-1].time < image_metadata.time:
-            delta = image_metadata.time - sorted_points[-1].time
-            gpx_start_time = build_capture_time(sorted_points[0].time)
-            gpx_end_time = build_capture_time(sorted_points[-1].time)
-            # with the tolerance of 1ms
-            if 0.001 < delta:
-                raise exceptions.MapillaryOutsideGPXTrackError(
-                    f"The image time is {round(delta, 3)} seconds beyond the GPX end point",
-                    image_time=build_capture_time(image_metadata.time),
-                    gpx_start_time=gpx_start_time,
-                    gpx_end_time=gpx_end_time,
-                )
-
-        # The last gap that starts at or before the image
-        idx = bisect.bisect_right(
-            [before.time for before, _ in gaps], image_metadata.time
-        )
-        if idx:
-            before, after = gaps[idx - 1]
-            delta = min(
-                image_metadata.time - before.time, after.time - image_metadata.time
-            )
-            # with the tolerance of 1ms, as at the ends of the track
-            if 0.001 < delta:
-                captured_at = datetime.datetime.fromtimestamp(
-                    image_metadata.time, tz=datetime.timezone.utc
-                ).isoformat()
-                track_gap = describe_gap(before, after, before.time, after.time)
-                raise exceptions.MapillaryOutsideGPXTrackError(
-                    f"The GPX track has {track_gap}, which the image time {captured_at} falls in",
-                    image_time=build_capture_time(image_metadata.time),
-                    gpx_start_time=build_capture_time(sorted_points[0].time),
-                    gpx_end_time=build_capture_time(sorted_points[-1].time),
-                )
+        check_within_track(image_metadata.time, sorted_points, gaps)
 
         interpolated = geo.interpolate(sorted_points, image_metadata.time)
 

mapillary_tools/geotag/geotag_images_from_nmea_file.py

@@ -22,6 +22,15 @@
 # GL (GLONASS), GA (Galileo), ...
 _SENTENCE_RE = re.compile(r"\$[A-Z]{2}(RMC|GGA),")
 
+# The checksum that ends a sentence: *HH
+_CHECKSUM_RE = re.compile(r"\*[0-9A-Fa-f]{2}\s*$")
+
+# The time of a sentence (hhmmss, and a fraction of a second), and the date of
+# an RMC sentence (ddmmyy). pynmea2 reads a field cut short as another time or
+# date: an RMC sentence cut inside its date "2212" as 2012-02-02
+_TIME_RE = re.compile(r"\d{6}(\.\d+)?")
+_DATE_RE = re.compile(r"\d{6}")
+
 
 class GeotagImagesFromNMEAFile(GeotagImagesFromGPX):
     def __init__(
@@ -92,9 +101,10 @@ def get_lat_lon_time_from_nmea(nmea_file: Path) -> list[geo.Point]:
     for the same time: only the first is read, so that no two points share a
     time.
 
-    A malformed sentence, such as one whose checksum does not match, is
-    skipped with a warning. A file of which no fix can be read and that has
-    malformed sentences raises ValueError, as one that is not text does.
+    A malformed sentence, such as one whose checksum does not match, or one
+    cut short, is skipped with a warning. A file of which no fix can be read
+    and that has malformed sentences raises ValueError, as one that is not
+    text does.
     """
     # NMEA sentences are ASCII. Decode them as UTF-8 on every platform, so
     # that a file that is not text fails to read, not only where UTF-8 is the
@@ -103,15 +113,25 @@ def get_lat_lon_time_from_nmea(nmea_file: Path) -> list[geo.Point]:
         lines = f.readlines()
         lines = [line.rstrip("\n\r") for line in lines]
 
+    # (line number, line) of each RMC and GGA sentence
+    sentences = [
+        (number, line)
+        for number, line in enumerate(lines, start=1)
+        if _SENTENCE_RE.search(line)
+    ]
+    # Receivers end each sentence with its checksum, or none of them. One
+    # without, where the others have theirs, was cut short, such as the last
+    # one of a log when the recording stopped
+    checksummed = any(_CHECKSUM_RE.search(line) for _, line in sentences)
+
     messages = []
     # (line number, line, error) of each malformed sentence
     malformed: list[tuple[int, str, ValueError]] = []
-    for number, line in enumerate(lines, start=1):
-        if _SENTENCE_RE.search(line):
-            try:
-                messages.append(_parse_sentence(line))
-            except ValueError as ex:
-                malformed.append((number, line, ex))
+    for number, line in sentences:
+        try:
+            messages.append(_parse_sentence(line, checksummed=checksummed))
+        except ValueError as ex:
+            malformed.append((number, line, ex))
 
     points = _read_fixes(messages, nmea_file)
 
@@ -185,11 +205,13 @@ def _read_fixes(
     return points
 
 
-def _parse_sentence(line: str) -> pynmea2.NMEASentence:
+def _parse_sentence(line: str, checksummed: bool = False) -> pynmea2.NMEASentence:
     """
     Parse an NMEA sentence, raising ValueError if it is malformed: if it does
-    not parse or its checksum does not match, or if a field that a fix is
-    read from does not parse, which pynmea2 leaves a string
+    not parse or its checksum does not match, if it has no checksum but should
+    (**checksummed**), if its time or the date of an RMC sentence is not
+    hhmmss[.ss] or ddmmyy, or if a field that a fix is read from does not
+    parse, which pynmea2 leaves a string
 
     >>> _parse_sentence("$GPGGA,0246,3735.30670,N,12220.25810,W,1,08,1.0,0.0,M,0.0,M,,*64")
     Traceback (most recent call last):
@@ -199,13 +221,33 @@ def _parse_sentence(line: str) -> pynmea2.NMEASentence:
     Traceback (most recent call last):
     ...
     ValueError: checksum does not match: 00 != 4F
+    >>> _parse_sentence("$GPRMC,024636.00,A,3735.30670,N,12220.25810,W,0.0,0.0,2212", checksummed=True)
+    Traceback (most recent call last):
+    ...
+    ValueError: no checksum
+    >>> _parse_sentence("$GPRMC,024636.00,A,3735.30670,N,12220.25810,W,0.0,0.0,2212")
+    Traceback (most recent call last):
+    ...
+    ValueError: malformed datestamp: '2212'
     """
+    if checksummed and not _CHECKSUM_RE.search(line):
+        raise ValueError("no checksum")
+
     try:
         message = pynmea2.parse(line)
     except pynmea2.ParseError as ex:
         # Its argument is (reason, sentence)
         raise ValueError(ex.args[0][0]) from ex
 
+    formats = {"timestamp": _TIME_RE}
+    if isinstance(message, pynmea2.RMC):
+        formats["datestamp"] = _DATE_RE
+    for name, pattern in formats.items():
+        value = _raw_field(message, name)
+        # An empty field is no time or date, as before a fix
+        if value and not pattern.fullmatch(value):
+            raise ValueError(f"malformed {name}: {value!r}")
+
     fields: dict[str, type] = {}
     if isinstance(message, pynmea2.RMC):
         fields = {"timestamp": datetime.time, "datestamp": datetime.date}
@@ -224,6 +266,12 @@ def _parse_sentence(line: str) -> pynmea2.NMEASentence:
     return message
 
 
+def _raw_field(message: pynmea2.NMEASentence, name: str) -> str:
+    """The text of the field **name** of a sentence, empty if it is cut off"""
+    idx = message.name_to_idx[name]
+    return message.data[idx] if idx < len(message.data) else ""
+
+
 def _rmc_datetime(message: pynmea2.RMC) -> datetime.datetime:
     return datetime.datetime.combine(message.datestamp, message.timestamp)