From f147a23f9cfe73f353b5090a390b8a11c45fdc7d Mon Sep 17 00:00:00 2001 From: Sander Roosendaal Date: Wed, 7 Oct 2026 09:27:38 +0200 Subject: [PATCH] Python: read AverageBoatSpeed at scale 100, and honour a file scale. Same units with a different declared scale decode with that scale. A missing scale on this field is the old scale 255, which FIT hides because 255 is the UINT8 invalid value. Co-authored-by: Cursor --- python/README.md | 5 ++ python/src/rowing_data/fields.py | 2 +- python/src/rowing_data/reader.py | 60 +++++++++++++++--- python/tests/test_codec.py | 2 +- python/tests/test_fields.py | 2 +- python/tests/test_reader.py | 105 +++++++++++++++++++++++++++++++ 6 files changed, 165 insertions(+), 11 deletions(-) diff --git a/python/README.md b/python/README.md index 2d4bfaa..a50e81f 100644 --- a/python/README.md +++ b/python/README.md @@ -25,6 +25,11 @@ so this library writes and reads ID 93. - In-stroke axis metadata 90–92, curve arrays, curve summaries, `.instroke.json` - WorkoutState enumeration (the draft has no value table; ID 9 is an opaque UINT8) +`AverageBoatSpeed` (ID 8) uses scale 100 (0.01 m/s). When a file declares the +same units and a different scale, the reader uses the scale in the file. Scale +is a UINT8 and 255 is its invalid value, so an older scale of 255 does not +surface; that missing scale is still read as 255 m/s steps. + **Pre-v1.2 files:** DriveLength used scale 100 / metres. This reader decodes with **v0.1 units (mm)** and records a `field_scale` warning. It does not silently convert old files into millimetres. diff --git a/python/src/rowing_data/fields.py b/python/src/rowing_data/fields.py index f8d2720..bd2685f 100644 --- a/python/src/rowing_data/fields.py +++ b/python/src/rowing_data/fields.py @@ -128,7 +128,7 @@ def _native( ), _dev(6, "AverageDriveForceN", MessageType.RECORD, BaseType.UINT16, 10, "N"), _dev(7, "PeakDriveForceN", MessageType.RECORD, BaseType.UINT16, 10, "N"), - _dev(8, "AverageBoatSpeed", MessageType.RECORD, BaseType.UINT16, 255, "m/s"), + _dev(8, "AverageBoatSpeed", MessageType.RECORD, BaseType.UINT16, 100, "m/s"), _dev( 9, "WorkoutState", diff --git a/python/src/rowing_data/reader.py b/python/src/rowing_data/reader.py index bec34d1..c5e0ea1 100644 --- a/python/src/rowing_data/reader.py +++ b/python/src/rowing_data/reader.py @@ -2,6 +2,7 @@ from __future__ import annotations +from dataclasses import replace from datetime import UTC, datetime from pathlib import Path @@ -96,9 +97,10 @@ def _our_developer_fields( ) -> tuple[dict[str, FieldDef], list[Issue]]: """Map names to registry FieldDefs for the standard application UUID only. - Decode always uses Draft v0.1 scales/units. A file that advertises a - different scale (for example pre-v1.2 DriveLength in metres) is not - converted; a warning is recorded instead. + When the file's units match the registry and only the scale differs, decode + with the scale declared in the file (AverageBoatSpeed 255 and 100 are both + m/s). When the units differ, as with pre-v1.2 DriveLength in metres, keep + the registry units and record a warning. Do not convert those values. """ by_name: dict[str, FieldDef] = {} issues: list[Issue] = [] @@ -116,11 +118,32 @@ def _our_developer_fields( registry = field_by_id(int(field_id)) except KeyError: continue - file_scale = message.get_value("scale") + file_scale = _raw_scale(message) + file_units = message.get_value("units") + # Scale is a UINT8. 255 is its invalid value, so parsers drop it. + # That byte was the old AverageBoatSpeed scale. Units are still m/s. if ( - file_scale not in (None, 0) - and float(file_scale) != float(registry.scale) + file_scale is None + and registry.field_id == 8 + and _units_match(file_units, registry.units) ): + file_scale = 255 + decode_as = registry + scale_differs = file_scale not in (None, 0) and float(file_scale) != float( + registry.scale + ) + if scale_differs and _units_match(file_units, registry.units): + decode_as = replace(registry, scale=float(file_scale)) + issues.append( + Issue( + "warning", + "field_scale", + f"{registry.name} (ID {registry.field_id}) has file scale " + f"{file_scale}; decoded with that scale " + f"(registry scale is {registry.scale})", + ) + ) + elif scale_differs: issues.append( Issue( "warning", @@ -130,12 +153,33 @@ def _our_developer_fields( "decoded with v0.1 units, not converted", ) ) - by_name[registry.name] = registry + by_name[decode_as.name] = decode_as if name: - by_name[str(name)] = registry + by_name[str(name)] = decode_as return by_name, issues +def _raw_scale(message) -> object: + """Scale byte from the field description, including 255. + + 255 is the UINT8 invalid value, so ``get_value`` drops a scale of 255. + AverageBoatSpeed used that scale before it moved to 100, and the byte is + still meaningful. + """ + field = message.get("scale") + if field is None or field.raw_value is None: + return None + if isinstance(field.raw_value, list | tuple): + return None + return field.raw_value + + +def _units_match(file_units: object, registry_units: str) -> bool: + if not isinstance(file_units, str): + return False + return file_units.strip() == registry_units + + def _recording_strategy( message, our_fields: dict[str, FieldDef] ) -> tuple[RecordingStrategy, Issue | None]: diff --git a/python/tests/test_codec.py b/python/tests/test_codec.py index 230315c..3248878 100644 --- a/python/tests/test_codec.py +++ b/python/tests/test_codec.py @@ -15,7 +15,7 @@ (1, 450, 450), # StrokeDriveTime ms (6, 412.3, 4123), # AverageDriveForceN, scale 10 (7, 0, 0), # measured zero is a real value - (8, 4.0, 1020), # AverageBoatSpeed, scale 255 + (8, 4.0, 400), # AverageBoatSpeed, scale 100 (19, 250, 250), # StrokeWork J (93, 28.50, 2850), # StrokeRate, scale 100 (10, 1, 1), # RecordingStrategy diff --git a/python/tests/test_fields.py b/python/tests/test_fields.py index 658f8b0..5dab025 100644 --- a/python/tests/test_fields.py +++ b/python/tests/test_fields.py @@ -27,7 +27,7 @@ (5, "PeakDriveForceLbs", MessageType.RECORD, BaseType.UINT16, 10, "lbs", True), (6, "AverageDriveForceN", MessageType.RECORD, BaseType.UINT16, 10, "N", False), (7, "PeakDriveForceN", MessageType.RECORD, BaseType.UINT16, 10, "N", False), - (8, "AverageBoatSpeed", MessageType.RECORD, BaseType.UINT16, 255, "m/s", False), + (8, "AverageBoatSpeed", MessageType.RECORD, BaseType.UINT16, 100, "m/s", False), (9, "WorkoutState", MessageType.RECORD, BaseType.UINT8, 1, "", False), (10, "RecordingStrategy", MessageType.SESSION, BaseType.UINT8, 1, "", False), (19, "StrokeWork", MessageType.RECORD, BaseType.UINT16, 1, "J", False), diff --git a/python/tests/test_reader.py b/python/tests/test_reader.py index f795459..330153e 100644 --- a/python/tests/test_reader.py +++ b/python/tests/test_reader.py @@ -132,6 +132,111 @@ def test_mismatched_file_scale_warns_and_uses_v01_units(tmp_path: Path) -> None: assert any(issue.code == "field_scale" for issue in validate(loaded)) +def test_same_unit_file_scale_is_used_for_boat_speed(tmp_path: Path) -> None: + """Older AverageBoatSpeed files use scale 255. Units are still m/s.""" + path = tmp_path / "boat_speed_255.fit" + field = field_by_id(8) + 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 = field.field_id + desc.fit_base_type_id = BaseType.UINT16.value + desc.field_name = field.name + desc.scale = 255 + desc.offset = 0 + desc.units = "m/s" + builder.add(desc) + + session = SessionMessage() + session.start_time = _ts_ms() + session.timestamp = _ts_ms() + builder.add(session) + + speed = DeveloperField( + developer_data_index=0, + field_id=8, + size=2, + name=field.name, + base_type=BaseType.UINT16, + scale=255, + offset=0, + units="m/s", + ) + speed.set_value(0, 4.0) # raw 1020 at scale 255 + rec = RecordMessage(developer_fields=[speed]) + rec.timestamp = _ts_ms() + builder.add(rec) + builder.build().to_file(str(path)) + + loaded = read_fit(path) + assert loaded.records[0].average_boat_speed_mps == 4.0 + assert any(issue.code == "field_scale" for issue in loaded.read_issues) + + +def test_declared_boat_speed_scale_is_used(tmp_path: Path) -> None: + """A readable scale other than the registry scale still yields m/s.""" + path = tmp_path / "boat_speed_10.fit" + field = field_by_id(8) + 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 = field.field_id + desc.fit_base_type_id = BaseType.UINT16.value + desc.field_name = field.name + desc.scale = 10 + desc.offset = 0 + desc.units = "m/s" + builder.add(desc) + + session = SessionMessage() + session.start_time = _ts_ms() + session.timestamp = _ts_ms() + builder.add(session) + + speed = DeveloperField( + developer_data_index=0, + field_id=8, + size=2, + name=field.name, + base_type=BaseType.UINT16, + scale=10, + offset=0, + units="m/s", + ) + speed.set_value(0, 4.0) # raw 40 at scale 10; registry scale 100 would yield 0.4 + rec = RecordMessage(developer_fields=[speed]) + rec.timestamp = _ts_ms() + builder.add(rec) + builder.build().to_file(str(path)) + + loaded = read_fit(path) + assert loaded.records[0].average_boat_speed_mps == 4.0 + + 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)