From dfcf13a97b561da3c54258a227336ca158c722c4 Mon Sep 17 00:00:00 2001 From: kingult <82867213+kingult@users.noreply.github.com> Date: Wed, 6 May 2026 16:03:03 -0700 Subject: [PATCH] fix: lat/lon double-write in buildUpdateContactPathFrame The function emitted two consecutive 8-byte position blocks instead of one, producing a frame 8 bytes longer than the documented layout. When a caller passed lastModified, the firmware parsed the duplicated second lat as the timestamp, giving wildly wrong "last seen" values on imported contacts. Delete the unconditional first block; keep the conditional block that correctly skips the optional tail when neither location nor lastModified is set, zero-fills position slots when only lastModified is present, and appends the optional timestamp. Adds test/connector/build_update_contact_path_frame_test.dart with five cases covering all four optional-tail combinations plus the fixed-point lat/lon encoding. Fixes #427 --- lib/connector/meshcore_protocol.dart | 21 +--- .../build_update_contact_path_frame_test.dart | 105 ++++++++++++++++++ 2 files changed, 110 insertions(+), 16 deletions(-) create mode 100644 test/connector/build_update_contact_path_frame_test.dart diff --git a/lib/connector/meshcore_protocol.dart b/lib/connector/meshcore_protocol.dart index c212acc3..f55e9687 100644 --- a/lib/connector/meshcore_protocol.dart +++ b/lib/connector/meshcore_protocol.dart @@ -720,27 +720,16 @@ Uint8List buildUpdateContactPathFrame( final timestamp = DateTime.now().millisecondsSinceEpoch ~/ 1000; writer.writeUInt32LE(timestamp); - if ((lat == null || lon == null) && lastModified != null) { - // If lat/lon not provided, write zeros - writer.writeInt32LE(0); - writer.writeInt32LE(0); - } else { - // Latitude and Longitude are expected in degrees, convert to int by multiplying by 1e6 - // Latitude - final latitude = lat ?? 0.0; - writer.writeInt32LE((latitude * 1e6).round()); - - // Longitude - final longitude = lon ?? 0.0; - writer.writeInt32LE((longitude * 1e6).round()); - } - + // Optional [Lat x4, Lon x4][timestamp x4] tail per the doc comment above. + // Emit 8 bytes of position (zero-filled when only lastModified is provided) + // followed by an optional 4-byte timestamp. Earlier code emitted the + // position block twice, which corrupted the tail and caused the firmware + // to parse the second lat as the timestamp. See #427. final hasLocation = lat != null && lon != null; if (hasLocation || lastModified != null) { writer.writeInt32LE(hasLocation ? (lat * 1e6).round() : 0); writer.writeInt32LE(hasLocation ? (lon * 1e6).round() : 0); if (lastModified != null) { - // Last modified final lastModifiedTimestamp = lastModified.millisecondsSinceEpoch ~/ 1000; writer.writeUInt32LE(lastModifiedTimestamp); } diff --git a/test/connector/build_update_contact_path_frame_test.dart b/test/connector/build_update_contact_path_frame_test.dart new file mode 100644 index 00000000..da88f9aa --- /dev/null +++ b/test/connector/build_update_contact_path_frame_test.dart @@ -0,0 +1,105 @@ +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:meshcore_open/connector/meshcore_protocol.dart'; + +void main() { + // Frame layout per the doc comment on buildUpdateContactPathFrame: + // [cmd][pub_key x32][type][flags][path_len][path x64][name x32] + // [timestamp x4][Lat? x4, Lon? x4][timestamp? x4] + // + // Base (mandatory) bytes: + // 1 cmd + 32 pubKey + 1 type + 1 flags + 1 pathLen + 64 path + // + 32 name + 4 timestamp = 136 bytes + const int baseFrameLength = 136; + + final pubKey = Uint8List.fromList(List.generate(32, (i) => i)); + final path = Uint8List.fromList([0xAA, 0xBB]); + + group('buildUpdateContactPathFrame', () { + test('omits lat/lon and timestamp tail when neither is provided', () { + final frame = buildUpdateContactPathFrame( + pubKey, + path, + path.length, + name: 'Alice', + ); + + // Should be exactly the base frame, no optional tail. + expect(frame.length, baseFrameLength); + }); + + test('appends only an 8-byte lat/lon tail when location is provided', () { + final frame = buildUpdateContactPathFrame( + pubKey, + path, + path.length, + lat: 49.123456, + lon: -123.123456, + ); + + expect(frame.length, baseFrameLength + 8); + }); + + test( + 'appends 8 bytes lat/lon + 4 bytes timestamp when both are provided', + () { + final frame = buildUpdateContactPathFrame( + pubKey, + path, + path.length, + lat: 49.0, + lon: -123.0, + lastModified: DateTime.utc(2026, 1, 2, 3, 4, 5), + ); + + expect(frame.length, baseFrameLength + 8 + 4); + }, + ); + + test('zero-fills the lat/lon slots and appends timestamp when only ' + 'lastModified is provided', () { + final frame = buildUpdateContactPathFrame( + pubKey, + path, + path.length, + lastModified: DateTime.utc(2026, 1, 2, 3, 4, 5), + ); + + // 8 zero bytes for lat/lon + 4 bytes timestamp + expect(frame.length, baseFrameLength + 8 + 4); + + // Verify the lat/lon slot is actually zero — guards against a + // regression where the function writes garbage into those bytes. + final tailStart = baseFrameLength; + for (var i = tailStart; i < tailStart + 8; i++) { + expect(frame[i], 0, reason: 'byte $i in lat/lon slot must be 0'); + } + }); + + test('encodes positive lat/lon as little-endian fixed-point (×1e6)', () { + final frame = buildUpdateContactPathFrame( + pubKey, + path, + path.length, + lat: 49.123456, + lon: -123.123456, + ); + + // Latitude is the first 4 bytes of the optional tail. + final latBytes = ByteData.sublistView( + frame, + baseFrameLength, + baseFrameLength + 4, + ); + final lonBytes = ByteData.sublistView( + frame, + baseFrameLength + 4, + baseFrameLength + 8, + ); + + expect(latBytes.getInt32(0, Endian.little), (49.123456 * 1e6).round()); + expect(lonBytes.getInt32(0, Endian.little), (-123.123456 * 1e6).round()); + }); + }); +}