Skip to content

Commit

Permalink
chore: clean up decoder code (#491)
Browse files Browse the repository at this point in the history
  • Loading branch information
@muktihari
muktihari authored Oct 9, 2024
1 parent 6bf188f commit f20393c
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 34 deletions.
66 changes: 33 additions & 33 deletions decoder/decoder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ func (d *Decoder) releaseTemporaryObjects() {
d.localMessageDefinitions = [proto.LocalMesgNumMask + 1]*proto.MessageDefinition{}
d.fieldsArray = [255]proto.Field{}
d.developerFieldsArray = [255]proto.DeveloperField{}
d.fileId = nil
d.messages = nil
d.developerDataIndexes = d.developerDataIndexes[:0]
for i := range d.fieldDescriptions {
Expand Down Expand Up @@ -699,7 +700,7 @@ func (d *Decoder) decodeMessageData(header byte) (err error) {
return nil
}

func (d *Decoder) decodeFields(mesgDef *proto.MessageDefinition, mesg *proto.Message) error {
func (d *Decoder) decodeFields(mesgDef *proto.MessageDefinition, mesg *proto.Message) (err error) {
for i := range mesgDef.FieldDefinitions {
fieldDef := &mesgDef.FieldDefinitions[i]

Expand All @@ -717,9 +718,9 @@ func (d *Decoder) decodeFields(mesgDef *proto.MessageDefinition, mesg *proto.Mes
}

var (
baseType = field.BaseType
profilType = field.Type
array = field.Array
baseType = field.BaseType
profileType = field.Type
array = field.Array
)

// Gracefully handle poorly encoded FIT file.
Expand All @@ -728,31 +729,29 @@ func (d *Decoder) decodeFields(mesgDef *proto.MessageDefinition, mesg *proto.Mes
continue
} else if fieldDef.Size < baseType.Size() {
baseType = basetype.Byte
profilType = profile.Byte
profileType = profile.Byte
array = fieldDef.Size > baseType.Size() && fieldDef.Size&baseType.Size() == 0
d.logField(mesg, fieldDef, "Size is less than expected. Fallback: decode as byte(s) and convert the value")
} else if fieldDef.Size > baseType.Size() && !field.Array && baseType != basetype.String {
d.logField(mesg, fieldDef, "field.Array is false. Fallback: retrieve first array's value only")
}

val, err := d.readValue(fieldDef.Size, mesgDef.Architecture, baseType, profilType, array, overrideStringArray)
field.Value, err = d.readValue(fieldDef.Size, mesgDef.Architecture, baseType, profileType, array, overrideStringArray)
if err != nil {
return err
}

if baseType != field.BaseType { // Convert value
val = convertBytesToValue(val, field.BaseType)
field.Value = convertBytesToValue(field.Value, field.BaseType)
}

if field.Num == proto.FieldNumTimestamp && val.Type() == proto.TypeUint32 {
timestamp := val.Uint32()
if field.Num == proto.FieldNumTimestamp && field.Value.Type() == proto.TypeUint32 {
timestamp := field.Value.Uint32()
d.timestamp = timestamp
d.lastTimeOffset = byte(timestamp & proto.CompressedTimeMask)
}

field.Value = val

if d.options.shouldExpandComponent && field.Accumulate {
if field.Accumulate && d.options.shouldExpandComponent {
d.collectAccumulableValues(mesg.Num, field.Num, field.Value)
}

Expand Down Expand Up @@ -951,8 +950,11 @@ func (d *Decoder) decodeDeveloperFields(mesgDef *proto.MessageDefinition, mesg *
if !ok {
// NOTE: Currently, we allow missing DeveloperDataId message,
// we only use FieldDescription messages to decode developer data.
d.log("mesg.Num: %d, developerFields[%d].Num: %d: missing developer data id with developer data index '%d'",
mesg.Num, i, devFieldDef.Num, devFieldDef.DeveloperDataIndex)
if d.options.logWriter != nil {
fmt.Fprintf(d.options.logWriter,
"mesg.Num: %d, developerFields[%d].Num: %d: missing developer data id with developer data index '%d'",
mesg.Num, i, devFieldDef.Num, devFieldDef.DeveloperDataIndex)
}
}

// Find the FieldDescription that refers to this DeveloperField.
Expand All @@ -971,9 +973,11 @@ func (d *Decoder) decodeDeveloperFields(mesgDef *proto.MessageDefinition, mesg *
}

if fieldDesc == nil {
d.log("mesg.Num: %d, developerFields[%d].Num: %d: Can't interpret developer field, "+
"no field description mesg found. Just read acquired bytes (%d) and move forward. [byte pos: %d]\n",
mesg.Num, i, devFieldDef.Num, devFieldDef.Size, d.n)
if d.options.logWriter != nil {
fmt.Fprintf(d.options.logWriter, "mesg.Num: %d, developerFields[%d].Num: %d: Can't interpret developer field, "+
"no field description mesg found. Just read acquired bytes (%d) and move forward. [byte pos: %d]\n",
mesg.Num, i, devFieldDef.Num, devFieldDef.Size, d.n)
}
if _, err := d.readN(int(devFieldDef.Size)); err != nil {
return fmt.Errorf("no field description found, unable to read acquired bytes: %w", err)
}
Expand All @@ -986,11 +990,6 @@ func (d *Decoder) decodeDeveloperFields(mesgDef *proto.MessageDefinition, mesg *
}

var isArray bool
typeSize := fieldDesc.FitBaseTypeId.Size()
if devFieldDef.Size > typeSize && devFieldDef.Size%typeSize == 0 {
isArray = true
}

baseType := fieldDesc.FitBaseTypeId
profileType := profile.ProfileTypeFromBaseType(baseType)

Expand All @@ -1001,11 +1000,14 @@ func (d *Decoder) decodeDeveloperFields(mesgDef *proto.MessageDefinition, mesg *
} else if devFieldDef.Size < fieldDesc.FitBaseTypeId.Size() {
baseType = basetype.Byte
profileType = profile.Byte
isArray = devFieldDef.Size > baseType.Size() && devFieldDef.Size&baseType.Size() == 0
d.logDeveloperField(mesg, devFieldDef, fieldDesc.FitBaseTypeId,
"Size is less than expected. Fallback: decode as byte(s) and convert the value")
}

if devFieldDef.Size > baseType.Size() && devFieldDef.Size%baseType.Size() == 0 {
isArray = true
}

// NOTE: It seems there is no standard on utilizing Array field to handle []string in developer fields.
// Discussion: https://forums.garmin.com/developer/fit-sdk/f/discussion/355554/how-to-determine-developer-field-s-value-type-is-a-string-or-string
overrideStringArray := fieldDesc.FitBaseTypeId == basetype.String
Expand Down Expand Up @@ -1068,24 +1070,22 @@ func (d *Decoder) readValue(size byte, arch byte, baseType basetype.BaseType, pr
return proto.UnmarshalValue(b, arch, baseType, profileType, isArray)
}

// log logs only if logWriter is not nil.
func (d *Decoder) log(format string, args ...any) {
if d.options.logWriter == nil {
return
}
fmt.Fprintf(d.options.logWriter, format, args...)
}

const logFieldTemplate = "mesg.Num: %q, %s.Num: %d, size: %d, type: %q (size: %d). %s. [bytes pos: %d]\n"

// logField logs field related issues only if logWriter is not nil.
func (d *Decoder) logField(m *proto.Message, fd *proto.FieldDefinition, msg string) {
d.log(logFieldTemplate, m.Num, "field", fd.Num, fd.Size, fd.BaseType, fd.BaseType.Size(), msg, d.n)
if d.options.logWriter == nil {
return
}
fmt.Fprintf(d.options.logWriter, logFieldTemplate, m.Num, "field", fd.Num, fd.Size, fd.BaseType, fd.BaseType.Size(), msg, d.n)
}

// logDeveloperField logs developerField related issues only if logWriter is not nil.
func (d *Decoder) logDeveloperField(m *proto.Message, dfd *proto.DeveloperFieldDefinition, bt basetype.BaseType, msg string) {
d.log(logFieldTemplate, m.Num, "developerField", dfd.Num, dfd.Size, bt, bt.Size(), msg, d.n)
if d.options.logWriter == nil {
return
}
fmt.Fprintf(d.options.logWriter, logFieldTemplate, m.Num, "developerField", dfd.Num, dfd.Size, bt, bt.Size(), msg, d.n)
}

// DecodeWithContext is similar to Decode but with respect to context propagation.
Expand Down
36 changes: 35 additions & 1 deletion decoder/decoder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2532,7 +2532,7 @@ func TestDecodeDeveloperFields(t *testing.T) {

for i, tc := range tt {
t.Run(fmt.Sprintf("[%d] %s", i, tc.name), func(t *testing.T) {
dec := New(tc.r)
dec := New(tc.r, WithLogWriter(io.Discard))
dec.developerDataIndexes = append(dec.developerDataIndexes, tc.developerDataIndexes...)
dec.fieldDescriptions = append(dec.fieldDescriptions, tc.fieldDescription)
err := dec.decodeDeveloperFields(tc.mesgDef, tc.mesg)
Expand Down Expand Up @@ -2889,6 +2889,40 @@ func TestReset(t *testing.T) {
}
}

func TestLogs(t *testing.T) {
mesg := proto.Message{Num: mesgnum.Record}
fieldDef := proto.FieldDefinition{Num: fieldnum.RecordTimestamp, Size: 4, BaseType: basetype.Uint32}
devFieldDef := proto.DeveloperFieldDefinition{Num: fieldnum.RecordTimestamp, Size: 4, DeveloperDataIndex: 0}

t.Run("logField: nil log writter", func(t *testing.T) {
dec := New(nil)
dec.logField(&mesg, &fieldDef, "msg")
})

t.Run("logField: with log writter", func(t *testing.T) {
buf := bytes.NewBuffer(nil)
dec := New(nil, WithLogWriter(buf))
dec.logField(&mesg, &fieldDef, "msg")
if buf.Len() == 0 {
t.Fatalf("expected non zero, got zero buf.Len()")
}
})

t.Run("logDeveloperField: nil log writter", func(t *testing.T) {
dec := New(nil)
dec.logDeveloperField(&mesg, &devFieldDef, basetype.Uint32, "msg")
})

t.Run("logDeveloperField: with log writter", func(t *testing.T) {
buf := bytes.NewBuffer(nil)
dec := New(nil, WithLogWriter(buf))
dec.logDeveloperField(&mesg, &devFieldDef, basetype.Uint32, "msg")
if buf.Len() == 0 {
t.Fatalf("expected non zero, got zero buf.Len()")
}
})
}

func TestConvertUint32ToValue(t *testing.T) {
tt := []struct {
value uint32
Expand Down

0 comments on commit f20393c

Please sign in to comment.