diff --git a/decoder/decoder.go b/decoder/decoder.go index d7f5064d..274391f6 100644 --- a/decoder/decoder.go +++ b/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 } diff --git a/decoder/decoder_test.go b/decoder/decoder_test.go index a8cd1ca1..09feece9 100644 --- a/decoder/decoder_test.go +++ b/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) + } }) } } diff --git a/decoder/raw.go b/decoder/raw.go index f409e0c4..511c9174 100644 --- a/decoder/raw.go +++ b/decoder/raw.go @@ -59,8 +59,6 @@ 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 @@ -68,7 +66,7 @@ type RawDecoder struct { // 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 } } diff --git a/decoder/raw_test.go b/decoder/raw_test.go index 83095a14..74e47b72 100644 --- a/decoder/raw_test.go +++ b/decoder/raw_test.go @@ -474,7 +474,6 @@ func TestRawDecoderDecode(t *testing.T) { } result.Reset() - dec.reset() hash16.Reset() }) }