fix: update rule regarding field num timestamp (#298)

profile/filedef/activity_test.go

@@ -138,12 +138,15 @@ func newActivityMessagesWithExpectedOrder(now time.Time) (mesgs []proto.Message,
 		21: factory.CreateMesgOnly(mesgnum.BarometerData).WithFields(
 			factory.CreateField(mesgnum.BarometerData, fieldnum.BarometerDataTimestamp).WithValue(datetime.ToUint32(incrementSecond(&now))),
 		),
-		// Special case these messages are not counted:
+		// Special case:
 		// 1. CoursePoint's Timestamp Num is 1
 		// 2. Set's Timestamp Num is 254
 		22: factory.CreateMesgOnly(mesgnum.CoursePoint).WithFields(
 			factory.CreateField(mesgnum.CoursePoint, fieldnum.CoursePointTimestamp).WithValue(datetime.ToUint32(incrementSecond(&now))),
 		),
+		23: factory.CreateMesgOnly(mesgnum.Set).WithFields(
+			factory.CreateField(mesgnum.Set, fieldnum.SetTimestamp).WithValue(datetime.ToUint32(incrementSecond(&now))),
+		),
 	}
 
 	ordered = []proto.Message{
@@ -158,18 +161,19 @@ func newActivityMessagesWithExpectedOrder(now time.Time) (mesgs []proto.Message,
 		8:  mesgs[17],
 		9:  mesgs[18],
 		10: mesgs[20],
-		11: mesgs[22],
-		12: mesgs[5],
-		13: mesgs[6],
-		14: mesgs[7],
-		15: mesgs[8],
-		16: mesgs[9],
-		17: mesgs[10],
-		18: mesgs[11],
-		19: mesgs[12],
-		20: mesgs[13],
-		21: mesgs[19],
-		22: mesgs[21],
+		11: mesgs[5],
+		12: mesgs[6],
+		13: mesgs[7],
+		14: mesgs[8],
+		15: mesgs[9],
+		16: mesgs[10],
+		17: mesgs[11],
+		18: mesgs[12],
+		19: mesgs[13],
+		20: mesgs[19],
+		21: mesgs[21],
+		22: mesgs[22],
+		23: mesgs[23],
 	}
 	return
 }

profile/filedef/filedef.go

@@ -6,6 +6,8 @@ package filedef
 
 import (
 	"github.com/muktihari/fit/profile/mesgdef"
+	"github.com/muktihari/fit/profile/untyped/fieldnum"
+	"github.com/muktihari/fit/profile/untyped/mesgnum"
 	"github.com/muktihari/fit/proto"
 	"golang.org/x/exp/slices"
 )
@@ -23,24 +25,50 @@ type File interface {
 //   - Any message without timestamp field will be placed to the beginning of the slice
 //     to enable these messages to be retrieved early such as UserProfile.
 //   - Any message with invalid timestamp will be places at the end of the slices.
+//
+// Special Case:
+//
+// All timestamp fields should have num 253, except:
+//   - Course Point's Timestamp num: 1
+//   - Set's Timestamp num: 254
+//
+// We will sort these timestamps accordingly since the messages' order matters.
+//
+// For details, see [github.com/muktihari/fit/proto.FieldNumTimestamp] doc.
 func SortMessagesByTimestamp(messages []proto.Message) {
 	slices.SortStableFunc(messages, func(m1, m2 proto.Message) int {
-		v1 := m1.FieldByNum(proto.FieldNumTimestamp)
-		v2 := m2.FieldByNum(proto.FieldNumTimestamp)
+		var f1, f2 *proto.Field
+		switch m1.Num {
+		case mesgnum.CoursePoint:
+			f1 = m1.FieldByNum(fieldnum.CoursePointTimestamp)
+		case mesgnum.Set:
+			f1 = m1.FieldByNum(fieldnum.SetTimestamp)
+		default:
+			f1 = m1.FieldByNum(proto.FieldNumTimestamp)
+		}
+
+		switch m2.Num {
+		case mesgnum.CoursePoint:
+			f2 = m2.FieldByNum(fieldnum.CoursePointTimestamp)
+		case mesgnum.Set:
+			f2 = m2.FieldByNum(fieldnum.SetTimestamp)
+		default:
+			f2 = m2.FieldByNum(proto.FieldNumTimestamp)
+		}
 
-		// Place message which does not have a timestamp at the beginning of the slice.
-		if v1 == nil && v2 == nil {
+		// Place messages which does not have a timestamp at the beginning of the slice.
+		if f1 == nil && f2 == nil {
 			return 0
-		} else if v1 == nil {
+		} else if f1 == nil {
 			return -1
-		} else if v2 == nil {
+		} else if f2 == nil {
 			return 1
 		}
 
 		// Sort timestamps regardless of whether any of the values are invalid.
 		// Any invalid value will be placed at the end of the slice.
-		t1 := v1.Value.Uint32()
-		t2 := v2.Value.Uint32()
+		t1 := f1.Value.Uint32()
+		t2 := f2.Value.Uint32()
 		if t1 < t2 {
 			return -1
 		}

profile/filedef/filedef_test.go

@@ -34,6 +34,9 @@ func TestToMesgs(t *testing.T) {
 func TestSortMessagesByTimestamp(t *testing.T) {
 	now := time.Now()
 
+	// Special case:
+	// 1. CoursePoint's Timestamp Num is 1
+	// 2. Set's Timestamp Num is 254
 	messages := []proto.Message{
 		0: factory.CreateMesgOnly(mesgnum.FileId).WithFields(
 			factory.CreateField(mesgnum.FileId, fieldnum.FileIdManufacturer).WithValue(typedef.ManufacturerDevelopment.Uint16()),
@@ -56,9 +59,15 @@ func TestSortMessagesByTimestamp(t *testing.T) {
 		6: factory.CreateMesgOnly(mesgnum.UserProfile).WithFields(
 			factory.CreateField(mesgnum.UserProfile, fieldnum.UserProfileFriendlyName).WithValue("muktihari"),
 		),
-		7: factory.CreateMesgOnly(mesgnum.Record).WithFields(
+		7: factory.CreateMesgOnly(mesgnum.Set).WithFields(
+			factory.CreateField(mesgnum.Set, fieldnum.SetTimestamp).WithValue(datetime.ToUint32(now.Add(4 * time.Second))),
+		),
+		8: factory.CreateMesgOnly(mesgnum.Record).WithFields(
 			factory.CreateField(mesgnum.Record, fieldnum.RecordTimestamp).WithValue(datetime.ToUint32(now.Add(2 * time.Second))),
 		),
+		9: factory.CreateMesgOnly(mesgnum.CoursePoint).WithFields(
+			factory.CreateField(mesgnum.CoursePoint, fieldnum.CoursePointTimestamp).WithValue(datetime.ToUint32(now.Add(3 * time.Second))),
+		),
 	}
 
 	expected := []proto.Message{
@@ -69,6 +78,8 @@ func TestSortMessagesByTimestamp(t *testing.T) {
 		messages[2],
 		messages[4],
 		messages[5],
+		messages[8],
+		messages[9],
 		messages[7],
 	}
 

proto/proto.go

@@ -31,7 +31,18 @@ const ( // header is 1 byte ->	 0bxxxxxxxx
 	DefaultFileHeaderSize byte   = 14     // The preferred size is 14
 	DataTypeFIT           string = ".FIT" // FIT is a constant string ".FIT"
 
-	FieldNumTimestamp = 253 // Field Num for timestamp across all defined messages in the profile.
+	// Field Num for timestamp across all defined messages in the profile.
+	//
+	// All timestamp fields should have num 253, except for these two messages that have different num
+	// due to the messages may have been added before the rule was put in place:
+	//   - Course Point's Timestamp num: 1
+	//   - Set's Timestamp num: 254
+	//
+	// However, we don't see Official SDK implementations making exception regarding this, such as when decoding
+	// and encoding compressed timestamp, only messages that have timestamp num 253 are eligible for that feature.
+	//
+	// Ref: https://forums.garmin.com/developer/fit-sdk/f/discussion/311106/why-has-course_point-message-a-timestamp-field-1-and-not-253
+	FieldNumTimestamp = 253
 )
 
 // LocalMesgNum extracts LocalMesgNum from message header.