Fix MMSI/position decode convention per spec; flag uncertain positions instead of clamping/discarding; deduplicate methods

tests/test_dsc.py

@@ -527,22 +527,20 @@ class TestDSCDecoder:
 
     def test_decode_mmsi_valid(self, decoder):
         """Test MMSI decoding from symbols."""
-        # Each symbol is 2 BCD digits
+        # 5 symbols, transmitted C5C4C3C2C1, are 10 digits X1...X10.
-        # To encode MMSI 232123456, we need:
+        # Per ITU-R M.493-16: "digit X10 is always the digit 0" - X10 is
-        # 02-32-12-34-56 -> symbols [2, 32, 12, 34, 56]
+        # the LAST digit, so the real 9-digit MMSI is digits 1-9.
-        symbols = [2, 32, 12, 34, 56]
+        # Uses a real MMSI seen throughout this decoder's over-the-air testing.
+        symbols = [33, 82, 16, 79, 40]
         result = decoder._decode_mmsi(symbols)
-        assert result == "232123456"
+        assert result == "338216794"
-
     def test_decode_mmsi_with_leading_zeros(self, decoder):
-        """Test MMSI decoding handles leading zeros."""
+        """Test MMSI decoding handles leading zeros (coast station)."""
-        # Coast station: 002320001
+        # Coast station MMSI 002275100 (CROSS Gris Nez, a real coast
-        # Padded to 10 digits: 0002320001
+        # station seen in real DSC traffic during this decoder's testing)
-        # BCD pairs: 00-02-32-00-01 -> [0, 2, 32, 0, 1]
+        symbols = [0, 22, 75, 10, 0]
-        symbols = [0, 2, 32, 0, 1]
         result = decoder._decode_mmsi(symbols)
-        assert result == "002320001"
+        assert result == "002275100"
-
     def test_decode_mmsi_short_symbols(self, decoder):
         """Test MMSI decoding returns None for short symbol list."""
         result = decoder._decode_mmsi([1, 2, 3])
@@ -550,57 +548,59 @@ class TestDSCDecoder:
 
     def test_decode_mmsi_invalid_symbols(self, decoder):
         """Test MMSI decoding returns None for out-of-range symbols."""
-        # Symbols > 99 should cause decode to fail
         symbols = [100, 32, 12, 34, 56]
         result = decoder._decode_mmsi(symbols)
         assert result is None
-
     def test_decode_position_northeast(self, decoder):
         """Test position decoding for NE quadrant."""
-        # Quadrant 10 = NE (lat+, lon+)
+        # Quadrant 0 = NE per ITU-R M.493 Annex 1 Sec 8.1.2 (single digit,
-        # Position: 51°30'N, 0°10'E
+        # not a 2-digit symbol value). Position: 51 deg 30'N, 000 deg 10'E
-        # lon_deg = symbols[3]*100 + symbols[4] = 0, lon_min = symbols[5] = 10
+        symbols = [5, 13, 0, 0, 10]
-        symbols = [10, 51, 30, 0, 0, 10, 0, 0, 0, 0]
         result = decoder._decode_position(symbols)
-
         assert result is not None
         assert result["lat"] == pytest.approx(51.5, rel=0.01)
         assert result["lon"] == pytest.approx(0.1667, rel=0.01)
-
     def test_decode_position_northwest(self, decoder):
         """Test position decoding for NW quadrant."""
-        # Quadrant 11 = NW (lat+, lon-)
+        # Quadrant 1 = NW. Position: 40 deg 42'N, 074 deg 00'W (NYC area)
-        # Position: 40°42'N, 74°00'W (NYC area)
+        symbols = [14, 4, 20, 74, 0]
-        symbols = [11, 40, 42, 0, 74, 0, 0, 0, 0, 0]
         result = decoder._decode_position(symbols)
-
         assert result is not None
         assert result["lat"] > 0  # North
         assert result["lon"] < 0  # West
-
     def test_decode_position_southeast(self, decoder):
         """Test position decoding for SE quadrant."""
-        # Quadrant 0 = SE (lat-, lon+)
+        # Quadrant 2 = SE. Position: 33 deg 51'S, 151 deg 12'E (Sydney area)
-        symbols = [0, 33, 51, 1, 51, 12, 0, 0, 0, 0]
+        symbols = [23, 35, 11, 51, 12]
         result = decoder._decode_position(symbols)
-
         assert result is not None
         assert result["lat"] < 0  # South
         assert result["lon"] > 0  # East
-
     def test_decode_position_short_symbols(self, decoder):
         """Test position decoding handles short symbol list."""
         result = decoder._decode_position([10, 51, 30])
         assert result is None
 
     def test_decode_position_invalid_values(self, decoder):
-        """Test position decoding handles invalid values gracefully."""
+        """Test position decoding flags (not silently discards or
-        # Latitude > 90 should be treated as 0
+        clamps) physically-impossible values.
-        symbols = [10, 95, 30, 0, 10, 0, 0, 0, 0, 0]
+
+        Design: a partial/uncertain position can still be valuable
+        (e.g. for search and rescue, where an approximate position
+        beats none at all) - so out-of-range components are NOT
+        discarded, but the result is explicitly flagged via
+        position_uncertain/position_issue so downstream consumers know
+        not to trust it blindly. Constructed so degrees genuinely
+        decode to 95 (impossible) - see position_issue for exact
+        derivation.
+        """
+        symbols = [9, 50, 30, 0, 10]
         result = decoder._decode_position(symbols)
         assert result is not None
-        assert result["lat"] == pytest.approx(0.5, rel=0.01)  # 0 deg + 30 min
+        assert result["position_uncertain"] is True
-
+        assert "latitude degrees" in result["position_issue"]
+        # the longitude portion is unaffected and still usable
+        assert result["lon"] == pytest.approx(0.1667, rel=0.01)
     def test_bits_to_symbol(self, decoder):
         """Test bit to symbol conversion."""
         # Symbol value is first 7 bits (LSB first)
@@ -622,8 +622,14 @@ class TestDSCDecoder:
         assert decoder._detect_dot_pattern() is True
 
     def test_detect_dot_pattern_insufficient(self, decoder):
-        """Test dot pattern not detected with insufficient alternations."""
+        """Test dot pattern not detected with insufficient bits buffered.
-        decoder.bit_buffer = [1, 0] * 40  # Only 80 bits, below 200 threshold
+
+        VHF DSC uses a 20-bit dot pattern (checked within a 24-bit
+        window) - not the old HF/MF 200-bit/100-alternation threshold.
+        16 bits is below the minimum window needed to even attempt
+        detection.
+        """
+        decoder.bit_buffer = [1, 0] * 8  # 16 bits, below the 24-bit window
         assert decoder._detect_dot_pattern() is False
 
     def test_detect_dot_pattern_not_alternating(self, decoder):
@@ -631,20 +637,26 @@ class TestDSCDecoder:
         decoder.bit_buffer = [1, 1, 1, 1, 0, 0, 0, 0] * 5
         assert decoder._detect_dot_pattern() is False
 
-    def test_bounded_phasing_strip(self, decoder):
+    def test_no_anchor_returns_none(self, decoder):
-        """Test that >7 phasing symbols causes decode to return None."""
+        """Test that a symbol stream with no valid, repeating format
-        # Build message bits: 10 phasing symbols (120) + format + data
+        specifier never produces a decode.
-        # Each symbol is 10 bits. Phasing symbol 120 = 0b1111000 LSB first
+
-        # 120 in 7 bits LSB-first: 0,0,0,1,1,1,1 + 3 check bits
+        Replaces the old test_bounded_phasing_strip, which tested the
-        # 120 = 0b1111000 -> LSB first: 0,0,0,1,1,1,1 -> ones=4 (even) -> check [0,0,0]
+        pre-fix-9 flat-parser's phasing-symbol-counting logic (bounding
-        phasing_bits = [0, 0, 0, 1, 1, 1, 1, 0, 0, 0]  # symbol 120
+        a linear strip to <=7 symbols). That mechanism no longer exists
-        # 10 phasing symbols (>7 max)
+        - fix 9 replaced it with anchor detection (the first position
-        decoder.message_bits = phasing_bits * 10
+        where a real format-specifier value repeats 2 symbol-positions
-        # Add some non-phasing symbols after (enough for a message)
+        later). This test exercises the equivalent safety property
-        # Symbol 112 (INDIVIDUAL) = 0b1110000 LSB-first: 0,0,0,0,1,1,1 -> ones=3 (odd) -> need odd check
+        (malformed/non-message data never produces a bogus decode)
-        # For simplicity, just add enough bits for the decoder to attempt
+        against the actual current mechanism: symbol 126 is a real,
-        for _ in range(20):
+        valid ten-unit-code value but is never a format specifier, so
-            decoder.message_bits.extend([0, 0, 0, 0, 1, 1, 1, 1, 0, 0])
+        no anchor can ever be found no matter how much data accumulates.
+        """
+        # Symbol 126 = info bits [0,1,1,1,1,1,1] (LSB first) + check
+        # bits [0,0,1] (MSB first, encoding 1 B-element) - a genuinely
+        # valid, check-bit-correct symbol, just never a format specifier.
+        symbol_126_bits = [0, 1, 1, 1, 1, 1, 1, 0, 0, 1]
+        decoder.message_bits = symbol_126_bits * 60  # 60 symbols, plenty
         result = decoder._try_decode_message()
         assert result is None
 

utils/dsc/decoder.py

@@ -527,49 +527,6 @@ class DSCDecoder:
         if eos is None:
             return None  # message not complete yet
 
-        def decode_mmsi(syms):
-            if len(syms) != 5 or any(s < 0 or s > 99 for s in syms):
-                return None
-            digits = "".join(f"{s:02d}" for s in syms)
-            return digits[:9]
-
-        def decode_position(syms):
-            """
-            Decode 5 coordinate symbols into a position dict, per ITU-R
-            M.493 Annex 1 Sec 8.1.2/8.2.3 (Table A1-2), confirmed
-            consistent across M.493-8 through -15. 10 digits total:
-            digit 1 = quadrant (0=NE, 1=NW, 2=SE, 3=SW), digits 2-5 =
-            latitude (2 degrees + 2 minutes), digits 6-10 = longitude
-            (3 degrees + 2 minutes). All-9s (9999999999) is the spec-
-            defined "position not available" sentinel.
-
-            Returns a dict with "lat"/"lon" as signed decimal degrees
-            (matching the pre-existing message["position"] shape this
-            decoder's downstream consumers - parser.py/routes/dsc.py -
-            expect) plus human-readable degree/minute text fields.
-            Returns None if the position is unavailable or malformed.
-            """
-            if len(syms) != 5 or any(s < 0 or s > 99 for s in syms):
-                return None
-            digits = "".join(f"{s:02d}" for s in syms)
-            if digits == "9999999999":
-                return None
-            quadrant = int(digits[0])
-            lat_deg = int(digits[1:3])
-            lat_min = int(digits[3:5])
-            lon_deg = int(digits[5:8])
-            lon_min = int(digits[8:10])
-            lat_sign = 1 if quadrant in (0, 1) else -1  # NE, NW -> North
-            lon_sign = 1 if quadrant in (0, 2) else -1  # NE, SE -> East
-            lat = lat_sign * (lat_deg + lat_min / 60.0)
-            lon = lon_sign * (lon_deg + lon_min / 60.0)
-            return {
-                "lat": round(lat, 6),
-                "lon": round(lon, 6),
-                "lat_text": f"{lat_deg:02d}°{lat_min:02d}'{'N' if lat_sign > 0 else 'S'}",
-                "lon_text": f"{lon_deg:03d}°{lon_min:02d}'{'E' if lon_sign > 0 else 'W'}",
-            }
-
         message = {
             "type": "dsc",
             "format": format_code,
@@ -581,11 +538,11 @@ class DSCDecoder:
         if is_distress_ack:
             message["format_text"] = "DISTRESS_ACK_OR_SELF_CANCEL"
         if "address" in fields:
-            message["dest_mmsi"] = decode_mmsi(fields["address"])
+            message["dest_mmsi"] = self._decode_mmsi(fields["address"])
         if "self_id" in fields:
-            message["source_mmsi"] = decode_mmsi(fields["self_id"])
+            message["source_mmsi"] = self._decode_mmsi(fields["self_id"])
         if "ship_in_distress_id" in fields:
-            ship_mmsi = decode_mmsi(fields["ship_in_distress_id"])
+            ship_mmsi = self._decode_mmsi(fields["ship_in_distress_id"])
             message["ship_in_distress_mmsi"] = ship_mmsi
             if "source_mmsi" not in message:
                 # for a self-cancel, transmitting station IS the ship in
@@ -603,7 +560,7 @@ class DSCDecoder:
                 message["nature_text"] = NATURE_TEXT.get(nature_val, f"UNKNOWN-{nature_val}")
             if "coordinates" in fields:
                 message["coordinates_raw"] = fields["coordinates"]
-                message["position"] = decode_position(fields["coordinates"])
+                message["position"] = self._decode_position(fields["coordinates"])
             if "time" in fields:
                 message["time_raw"] = fields["time"]
         if "telecommand" in fields:
@@ -718,143 +675,95 @@ class DSCDecoder:
         return (f"format={format_code}, all fields complete, but no valid EOS "
                 f"found in remaining {len(dedup) - idx} symbols")
 
-    def _diagnose_decode_failure(self) -> str:
+    def _decode_mmsi(self, symbols: list[int]) -> str | None:
         """
-        Explain exactly why the current message_bits buffer failed to
+        Decode 5 symbols into a 9-digit MMSI string.
-        decode, for logging at timeout. Mirrors _try_decode_message's
+
-        logic but reports where it stopped instead of returning None
+        Per ITU-R M.493-16: identities are transmitted as five
-        silently - normal accumulation still returns None quietly (this
+        characters C5C4C3C2C1, comprising ten digits X1...X10, "whereas
-        is only called once, at the moment of timeout, not on every
+        digit X10 is always the digit 0" (unless M.1080 extended
-        partial-accumulation call).
+        addressing is in use, not handled here). X10 is the LAST digit
+        of the sequence, not the first - so the real 9-digit MMSI is
+        digits 1-9, dropping the trailing padding digit. Confirmed
+        against real over-the-air captures: this produces MMSIs with
+        recognizable, valid Maritime Identification Digits (e.g. "338"
+        = USA) consistently across independent real recordings.
+
+        Args:
+            symbols: 5 raw symbol values (0-99 each)
+
+        Returns:
+            9-digit MMSI string, or None if malformed input.
         """
-        FORMAT_SPECIFIERS = {102, 112, 114, 116, 120, 123}
+        if len(symbols) != 5 or any(s < 0 or s > 99 for s in symbols):
-        FIELD_LAYOUTS = {
+            return None
-            120: [("address", 5), ("category", 1), ("self_id", 5),
+        digits = "".join(f"{s:02d}" for s in symbols)
-                  ("telecommand", 2), ("frequency_1", 3), ("frequency_2", 3)],
+        return digits[:9]
-            123: [("address", 5), ("category", 1), ("self_id", 5),
+
-                  ("telecommand", 2), ("frequency_1", 3)],
+    def _decode_position(self, symbols: list[int]) -> dict | None:
-            116: [("category", 1), ("self_id", 5), ("telecommand", 2),
+        """
-                  ("frequency_1", 3)],
+        Decode 5 coordinate symbols into a position dict, per ITU-R
-            114: [("address", 5), ("category", 1), ("self_id", 5),
+        M.493 Annex 1 Sec 8.1.2/8.2.3 (Table A1-2), confirmed
-                  ("telecommand", 2), ("frequency_1", 3), ("frequency_2", 3)],
+        consistent across M.493-8 through -15. 10 digits total:
-            102: [("address", 5), ("category", 1), ("self_id", 5),
+        digit 1 = quadrant (0=NE, 1=NW, 2=SE, 3=SW), digits 2-5 =
-                  ("telecommand", 2), ("frequency_1", 3), ("frequency_2", 3)],
+        latitude (2 degrees + 2 minutes), digits 6-10 = longitude
-            112: [("self_id", 5), ("nature", 1), ("coordinates", 5),
+        (3 degrees + 2 minutes). All-9s (9999999999) is the spec-
-                  ("time", 2), ("telecommand_msg4", 1)],
+        defined "position not available" sentinel.
+
+        Physically-impossible values (degrees/minutes outside valid
+        range - e.g. from a residual bit error the diversity-combining
+        fallback in _process_bit didn't catch) are NOT silently
+        discarded or clamped: the computed position is still returned
+        (a partial position can still be useful - e.g. for search and
+        rescue, where even an approximate position beats none at all),
+        but flagged via "position_uncertain"/"position_issue" so
+        downstream consumers can decide how much to trust it rather
+        than being silently given a wrong-but-plausible-looking value.
+
+        Returns a dict with "lat"/"lon" as signed decimal degrees
+        (matching the pre-existing message["position"] shape this
+        decoder's downstream consumers - parser.py/routes/dsc.py -
+        expect) plus human-readable degree/minute text fields and the
+        uncertainty flag described above. Returns None only if the
+        position is unavailable (all-9s sentinel) or the input itself
+        is malformed (wrong length/out-of-range symbols).
+
+        Args:
+            symbols: 5 raw symbol values (0-99 each)
+        """
+        if len(symbols) != 5 or any(s < 0 or s > 99 for s in symbols):
+            return None
+        digits = "".join(f"{s:02d}" for s in symbols)
+        if digits == "9999999999":
+            return None
+        quadrant = int(digits[0])
+        lat_deg = int(digits[1:3])
+        lat_min = int(digits[3:5])
+        lon_deg = int(digits[5:8])
+        lon_min = int(digits[8:10])
+
+        issues = []
+        if lat_deg > 90:
+            issues.append(f"latitude degrees ({lat_deg}) exceeds valid range (0-90)")
+        if lon_deg > 180:
+            issues.append(f"longitude degrees ({lon_deg}) exceeds valid range (0-180)")
+        if lat_min > 59:
+            issues.append(f"latitude minutes ({lat_min}) exceeds valid range (0-59)")
+        if lon_min > 59:
+            issues.append(f"longitude minutes ({lon_min}) exceeds valid range (0-59)")
+
+        lat_sign = 1 if quadrant in (0, 1) else -1  # NE, NW -> North
+        lon_sign = 1 if quadrant in (0, 2) else -1  # NE, SE -> East
+        lat = lat_sign * (lat_deg + lat_min / 60.0)
+        lon = lon_sign * (lon_deg + lon_min / 60.0)
+        return {
+            "lat": round(lat, 6),
+            "lon": round(lon, 6),
+            "lat_text": f"{lat_deg:02d}°{lat_min:02d}'{'N' if lat_sign > 0 else 'S'}",
+            "lon_text": f"{lon_deg:03d}°{lon_min:02d}'{'E' if lon_sign > 0 else 'W'}",
+            "position_uncertain": bool(issues),
+            "position_issue": "; ".join(issues) if issues else None,
         }
-        VALID_EOS = {117, 122, 127}
-
-        num_symbols = len(self.message_bits) // 10
-        symbols = [self._bits_to_symbol(self.message_bits[i * 10:i * 10 + 10]) for i in range(num_symbols)]
-        invalid_count = sum(1 for s in symbols if s == -1)
-
-        anchor = None
-        for i in range(len(symbols) - 2):
-            if symbols[i] in FORMAT_SPECIFIERS and symbols[i] == symbols[i + 2]:
-                anchor = i
-                break
-        if anchor is None:
-            return (f"no anchor found ({num_symbols} symbols, {invalid_count} invalid, "
-                    f"first 15 symbols={symbols[:15]})")
-
-        dedup = symbols[anchor::2]
-        dedup_b = symbols[anchor + 1::2]
-
-        def value_at(k):
-            primary = dedup[k] if k < len(dedup) else -1
-            if primary != -1:
-                return primary
-            fb = k + 2
-            return dedup_b[fb] if 0 <= fb < len(dedup_b) else -1
-
-        format_code = value_at(0)
-        layout = FIELD_LAYOUTS.get(format_code)
-        if layout is None:
-            return f"anchor found at {anchor}, but format specifier {format_code} not recognized"
-
-        idx = 2
-        for name, count in layout:
-            if idx + count > len(dedup):
-                have = len(dedup) - idx
-                return (f"format={format_code}, stalled waiting for field "
-                        f"'{name}' (need {count} symbols, have {max(have, 0)})")
-            idx += count
-
-        for k in range(idx, len(dedup)):
-            if value_at(k) in VALID_EOS:
-                return "all fields complete, EOS found - should have decoded (unexpected)"
-
-        return (f"format={format_code}, all fields complete, but no valid EOS "
-                f"found in remaining {len(dedup) - idx} symbols")
-
-    def _diagnose_decode_failure(self) -> str:
-        """
-        Explain exactly why the current message_bits buffer failed to
-        decode, for logging at timeout. Mirrors _try_decode_message's
-        logic but reports where it stopped instead of returning None
-        silently - normal accumulation still returns None quietly (this
-        is only called once, at the moment of timeout, not on every
-        partial-accumulation call).
-        """
-        FORMAT_SPECIFIERS = {102, 112, 114, 116, 120, 123}
-        FIELD_LAYOUTS = {
-            120: [("address", 5), ("category", 1), ("self_id", 5),
-                  ("telecommand", 2), ("frequency_1", 3), ("frequency_2", 3)],
-            123: [("address", 5), ("category", 1), ("self_id", 5),
-                  ("telecommand", 2), ("frequency_1", 3)],
-            116: [("category", 1), ("self_id", 5), ("telecommand", 2),
-                  ("frequency_1", 3)],
-            114: [("address", 5), ("category", 1), ("self_id", 5),
-                  ("telecommand", 2), ("frequency_1", 3), ("frequency_2", 3)],
-            102: [("address", 5), ("category", 1), ("self_id", 5),
-                  ("telecommand", 2), ("frequency_1", 3), ("frequency_2", 3)],
-            112: [("self_id", 5), ("nature", 1), ("coordinates", 5),
-                  ("time", 2), ("telecommand_msg4", 1)],
-        }
-        VALID_EOS = {117, 122, 127}
-
-        num_symbols = len(self.message_bits) // 10
-        symbols = [self._bits_to_symbol(self.message_bits[i * 10:i * 10 + 10]) for i in range(num_symbols)]
-        invalid_count = sum(1 for s in symbols if s == -1)
-
-        anchor = None
-        for i in range(len(symbols) - 2):
-            if symbols[i] in FORMAT_SPECIFIERS and symbols[i] == symbols[i + 2]:
-                anchor = i
-                break
-        if anchor is None:
-            return (f"no anchor found ({num_symbols} symbols, {invalid_count} invalid, "
-                    f"first 15 symbols={symbols[:15]})")
-
-        dedup = symbols[anchor::2]
-        dedup_b = symbols[anchor + 1::2]
-
-        def value_at(k):
-            primary = dedup[k] if k < len(dedup) else -1
-            if primary != -1:
-                return primary
-            fb = k + 2
-            return dedup_b[fb] if 0 <= fb < len(dedup_b) else -1
-
-        format_code = value_at(0)
-        layout = FIELD_LAYOUTS.get(format_code)
-        if layout is None:
-            return f"anchor found at {anchor}, but format specifier {format_code} not recognized"
-
-        idx = 2
-        for name, count in layout:
-            if idx + count > len(dedup):
-                have = len(dedup) - idx
-                return (f"format={format_code}, stalled waiting for field "
-                        f"'{name}' (need {count} symbols, have {max(have, 0)})")
-            idx += count
-
-        for k in range(idx, len(dedup)):
-            if value_at(k) in VALID_EOS:
-                return "all fields complete, EOS found - should have decoded (unexpected)"
-
-        return (f"format={format_code}, all fields complete, but no valid EOS "
-                f"found in remaining {len(dedup) - idx} symbols")
 
     def _bits_to_symbol(self, bits: list[int]) -> int:
         """