fix(decoder): should reset crc16 when file's crc is 0 (#221)

decoder/decoder.go

@@ -209,14 +209,14 @@ func New(r io.Reader, opts ...Option) *Decoder {
 		developerDataIds:        make([]*mesgdef.DeveloperDataId, 0),
 		fieldDescriptions:       make([]*mesgdef.FieldDescription, 0),
 	}
-	d.initDecodeHeaderOnce()
+	d.initDecodeFileHeaderOnce()
 
 	return d
 }
 
-// initDecodeHeaderOnce initializes decodeHeaderOnce() to decode the header exactly once, if error occur
+// initDecodeFileHeaderOnce initializes decodeFileHeaderOnce() to decode the header exactly once, if error occur
 // during the first invocation, the error will be returned everytime decodeHeaderOnce() is invoked.
-func (d *Decoder) initDecodeHeaderOnce() {
+func (d *Decoder) initDecodeFileHeaderOnce() {
 	var once = sync.Once{}
 	d.decodeFileHeaderOnce = func() error {
 		once.Do(func() { d.err = d.decodeFileHeader() })
@@ -300,7 +300,7 @@ func (d *Decoder) CheckIntegrity() (seq int, err error) {
 			err = ErrCRCChecksumMismatch
 			break
 		}
-		d.initDecodeHeaderOnce()
+		d.initDecodeFileHeaderOnce()
 		d.crc16.Reset()
 		d.cur = 0
 		seq++
@@ -410,7 +410,7 @@ func (d *Decoder) reset() {
 	d.lastTimeOffset = 0
 	d.sequenceCompleted = false
 
-	d.initDecodeHeaderOnce() // reset to enable invocation.
+	d.initDecodeFileHeaderOnce() // reset to enable invocation.
 }
 
 // Reset resets the Decoder to read its input from r, clear any error and
@@ -515,6 +515,7 @@ func (d *Decoder) decodeFileHeader() error {
 	}
 
 	if d.fileHeader.CRC == 0x0000 || !d.options.shouldChecksum { // do not need to check header's crc integrity.
+		d.crc16.Reset()
 		return nil
 	}
 

decoder/decoder_test.go

@@ -223,7 +223,7 @@ var (
 	fnReaderErr = fnReader(func(b []byte) (n int, err error) { return 0, io.EOF })
 )
 
-func TestDecodeHeaderOnce(t *testing.T) {
+func TestDecodeFileHeaderOnce(t *testing.T) {
 	var r io.Reader = func() io.Reader {
 		fnInstances := []io.Reader{
 			fnReader(func(b []byte) (n int, err error) {
@@ -1045,14 +1045,15 @@ func TestDecode(t *testing.T) {
 	}
 }
 
-func TestDecodeHeader(t *testing.T) {
+func TestDecodeFileHeader(t *testing.T) {
 	fit, buf := createFitForTest()
 
 	tt := []struct {
-		name   string
-		r      io.Reader
-		header proto.FileHeader
-		err    error
+		name       string
+		r          io.Reader
+		header     proto.FileHeader
+		err        error
+		validateFn func(d *Decoder) error // multi-purpose extra validation func
 	}{
 		{
 			name: "decode header happy flow",
@@ -1177,6 +1178,12 @@ func TestDecodeHeader(t *testing.T) {
 				header.CRC = 0
 				return header
 			}(),
+			validateFn: func(d *Decoder) error {
+				if crc := d.crc16.Sum16(); crc != 0 {
+					return fmt.Errorf("expected zero, got: %d", crc)
+				}
+				return nil
+			},
 		},
 		{
 			name: "decode crc mismatch",
@@ -1214,6 +1221,12 @@ func TestDecodeHeader(t *testing.T) {
 			if diff := cmp.Diff(dec.fileHeader, tc.header); diff != "" {
 				t.Fatal(diff)
 			}
+			if tc.validateFn == nil {
+				return
+			}
+			if err := tc.validateFn(dec); err != nil {
+				t.Fatalf("expected validateFn is nil, got: %v", err)
+			}
 		})
 	}
 }

decoder/raw.go

@@ -59,16 +59,14 @@ type RawDecoder struct {
 	// This is exported to allow the unused space to be utilized in a tight RAM, for instance, an embedded device.
 	// Using Index >= len(b) is safe on each Decode's callback function call.
 	BytesArray [1 + (255 * 255 * 2)]byte
-
-	lenMesgs [proto.LocalMesgNumMask + 1]uint32
 }
 
 // NewRaw creates new RawDecoder which provides low-level building block to work with FIT bytes for the
 // maximum performance gain. RawDecoder will split bytes by its corresponding RawFlag (FileHeader,
 // MessageDefinition, MessageData and CRC) for scoping the operation.
 //
 // However, this is still considered unsafe operation since we work with bytes directly and the responsibility
-// for validation now placed on the user-space.  The only thing that this validates is the reader should be a FIT
+// for validation now placed on the user-space. The only thing that this validates is the reader should be a FIT
 // (FileHeader: has valid Size and bytes 8-12 is ".FIT").
 //
 // The idea is to allow us to use a minimal viable decoder for performance and memory-critical situations,
@@ -93,10 +91,10 @@ func NewRaw() *RawDecoder {
 // byte by byte reading and having frequent read on non-buffered reader might impact performance, especially
 // if it involves syscall such as reading a file.
 func (d *RawDecoder) Decode(r io.Reader, fn func(flag RawFlag, b []byte) error) (n int64, err error) {
-	defer d.reset() // Must reset before return, so we can invoke Decode again for the next reader.
-
 	var seq int
 	for {
+		lenMesgs := [proto.LocalMesgNumMask + 1]uint32{}
+
 		// 1. Decode File Header
 		nr, err := io.ReadFull(r, d.BytesArray[:1])
 		n += int64(nr)
@@ -185,7 +183,7 @@ func (d *RawDecoder) Decode(r io.Reader, fn func(flag RawFlag, b []byte) error)
 				}
 
 				localMesgNum := d.BytesArray[0] & proto.LocalMesgNumMask
-				d.lenMesgs[localMesgNum] = lenMesg
+				lenMesgs[localMesgNum] = lenMesg
 
 				if err := fn(RawFlagMesgDef, d.BytesArray[:lenMesgDef]); err != nil {
 					return n, err
@@ -196,7 +194,7 @@ func (d *RawDecoder) Decode(r io.Reader, fn func(flag RawFlag, b []byte) error)
 
 			// 2. b. Decode Message Data
 			localMesgNum := proto.LocalMesgNum(d.BytesArray[0])
-			lenMesg := d.lenMesgs[localMesgNum]
+			lenMesg := lenMesgs[localMesgNum]
 			if lenMesg == 0 {
 				return n, fmt.Errorf("localMesgNum: %d: %w", localMesgNum, ErrMesgDefMissing)
 			}
@@ -224,13 +222,5 @@ func (d *RawDecoder) Decode(r io.Reader, fn func(flag RawFlag, b []byte) error)
 		}
 
 		seq++
-
-		d.reset() // reset for next FIT sequence.
-	}
-}
-
-func (d *RawDecoder) reset() {
-	for i := range d.lenMesgs {
-		d.lenMesgs[i] = 0
 	}
 }

decoder/raw_test.go

@@ -474,7 +474,6 @@ func TestRawDecoderDecode(t *testing.T) {
 			}
 
 			result.Reset()
-			dec.reset()
 			hash16.Reset()
 		})
 	}