From d80904e285abaaf2c1e55290e6ceb574000c53a6 Mon Sep 17 00:00:00 2001 From: Sander Roosendaal Date: Tue, 13 Oct 2026 17:18:45 +0200 Subject: [PATCH] Python: never write the FIT invalid value as a measurement. encode clips a value that would encode to the base type's invalid value to the next valid one, and decode returns None for the invalid value. Values beyond the type limit still raise. Co-Authored-By: Claude Sonnet 5.5 --- python/src/rowing_data/codec.py | 11 +++++--- python/src/rowing_data/fields.py | 9 +++++++ python/tests/test_codec.py | 18 ++++++++++++- python/tests/test_reader.py | 45 ++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 4 deletions(-) diff --git a/python/src/rowing_data/codec.py b/python/src/rowing_data/codec.py index 47250a4..55407a3 100644 --- a/python/src/rowing_data/codec.py +++ b/python/src/rowing_data/codec.py @@ -4,12 +4,14 @@ ``physical = raw / scale`` and ``raw = round(physical * scale)``. Absent values are ``None`` and MUST be omitted from the file — never encoded -as a placeholder such as -1 or 999. +as a placeholder such as -1 or 999. Each base type's FIT invalid value (spec +§7.2) means "no data": ``encode`` never produces it for a measurement, and +``decode`` returns ``None`` for it. """ from __future__ import annotations -from .fields import BASE_TYPE_RANGE, FieldDef +from .fields import BASE_TYPE_INVALID, BASE_TYPE_RANGE, FieldDef class CodecError(ValueError): @@ -27,12 +29,15 @@ def encode(field: FieldDef, physical: float | int | None) -> int | None: f"{field.name}: encoded value {raw} does not fit {field.base_type} " f"range [{lo}, {hi}] (physical={physical!r}, scale={field.scale})" ) + invalid = BASE_TYPE_INVALID[field.base_type] + if raw == invalid: + return invalid - 1 return raw def decode(field: FieldDef, raw: int | None) -> float | int | None: """Convert a FIT raw integer to a physical value, or ``None`` if absent.""" - if raw is None: + if raw is None or raw == BASE_TYPE_INVALID[field.base_type]: return None if field.scale == 1: return int(raw) diff --git a/python/src/rowing_data/fields.py b/python/src/rowing_data/fields.py index 955da4d..1d0e763 100644 --- a/python/src/rowing_data/fields.py +++ b/python/src/rowing_data/fields.py @@ -40,6 +40,15 @@ class BaseType(StrEnum): BaseType.SINT32: (-2_147_483_648, 2_147_483_647), } +# FIT invalid value per base type: it means "no data" and is never a measurement. +BASE_TYPE_INVALID: dict[BaseType, int] = { + BaseType.UINT8: 255, + BaseType.UINT16: 65535, + BaseType.UINT32: 4_294_967_295, + BaseType.SINT16: 32767, + BaseType.SINT32: 2_147_483_647, +} + @dataclass(frozen=True, slots=True) class FieldDef: diff --git a/python/tests/test_codec.py b/python/tests/test_codec.py index ff3ba0d..9697fb5 100644 --- a/python/tests/test_codec.py +++ b/python/tests/test_codec.py @@ -75,6 +75,22 @@ def test_encode_overflow_raises() -> None: def test_uint16_upper_bound_accepted() -> None: work = field_by_id(19) - assert encode(work, 65535) == 65535 + assert encode(work, 65534) == 65534 with pytest.raises(CodecError): encode(work, 65536) + + +@pytest.mark.parametrize( + ("name", "invalid"), + [ + ("StrokeWork", 65535), # UINT16 + ("WorkoutState", 255), # UINT8 + ], +) +def test_invalid_value_is_never_written_as_a_measurement( + name: str, invalid: int +) -> None: + field = field_by_name(name) + assert encode(field, invalid / field.scale) == invalid - 1 + assert decode(field, invalid) is None + assert decode(field, invalid - 1) is not None diff --git a/python/tests/test_reader.py b/python/tests/test_reader.py index 8507ca7..93cdc3e 100644 --- a/python/tests/test_reader.py +++ b/python/tests/test_reader.py @@ -240,6 +240,51 @@ def test_unknown_stroke_state_is_unknown(tmp_path: Path) -> None: assert any(issue.code == "stroke_state" for issue in loaded.read_issues) +def test_invalid_value_is_read_as_missing(tmp_path: Path) -> None: + path = tmp_path / "invalid.fit" + builder = FitFileBuilder(auto_define=True, min_string_size=64) + file_id = FileIdMessage() + file_id.type = FileType.ACTIVITY + file_id.manufacturer = Manufacturer.DEVELOPMENT + file_id.time_created = _ts_ms() + builder.add(file_id) + dev_id = DeveloperDataIdMessage() + dev_id.application_id = APPLICATION_ID + dev_id.developer_data_index = 0 + builder.add(dev_id) + desc = FieldDescriptionMessage() + desc.developer_data_index = 0 + desc.field_definition_number = 19 + desc.fit_base_type_id = BaseType.UINT16.value + desc.field_name = "StrokeWork" + desc.scale = 1 + desc.offset = 0 + desc.units = "" + builder.add(desc) + work = DeveloperField( + developer_data_index=0, + field_id=19, + size=2, + name="StrokeWork", + base_type=BaseType.UINT16, + scale=1, + offset=0, + units="", + ) + work.set_value(0, 65535) + session = SessionMessage() + session.start_time = _ts_ms() + session.timestamp = _ts_ms() + builder.add(session) + rec = RecordMessage(developer_fields=[work]) + rec.timestamp = _ts_ms() + builder.add(rec) + builder.build().to_file(str(path)) + + loaded = read_fit(path) + assert loaded.records[0].stroke_work_j is None + + def test_invalid_recording_strategy_is_unknown_with_warning(tmp_path: Path) -> None: path = tmp_path / "bad_strategy.fit" builder = FitFileBuilder(auto_define=True, min_string_size=64)