diff --git a/command/ota/generate.go b/command/ota/generate.go index dba44309..653b9d2b 100644 --- a/command/ota/generate.go +++ b/command/ota/generate.go @@ -18,8 +18,8 @@ package ota import ( - "bytes" "errors" + "fmt" "io/ioutil" "os" @@ -51,17 +51,16 @@ func Generate(binFile string, outFile string, fqbn string) error { return err } - var w bytes.Buffer - otaWriter := inota.NewWriter(&w, arduinoVendorID, productID) - _, err = otaWriter.Write(data) + out, err := os.Create(outFile) if err != nil { return err } - otaWriter.Close() + defer out.Close() - err = ioutil.WriteFile(outFile, w.Bytes(), os.FileMode(0644)) + enc := inota.NewEncoder(out, arduinoVendorID, productID) + err = enc.Encode(data) if err != nil { - return err + return fmt.Errorf("failed to encode binary file: %w", err) } return nil diff --git a/command/ota/massupload.go b/command/ota/massupload.go index 8bf6bc93..956cf991 100644 --- a/command/ota/massupload.go +++ b/command/ota/massupload.go @@ -171,6 +171,7 @@ func run(uploader otaUploader, ids []string, otaFile string, expiration int) []R results = append(results, r) continue } + defer file.Close() jobs <- job{id: id, file: file} } close(jobs) diff --git a/command/ota/upload.go b/command/ota/upload.go index cf3fd288..898dc83f 100644 --- a/command/ota/upload.go +++ b/command/ota/upload.go @@ -71,6 +71,7 @@ func Upload(params *UploadParams, cred *config.Credentials) error { if err != nil { return fmt.Errorf("%s: %w", "cannot open ota file", err) } + defer file.Close() expiration := otaExpirationMins if params.Deferred { diff --git a/internal/ota/encoder.go b/internal/ota/encoder.go index a9ab891e..b921e968 100644 --- a/internal/ota/encoder.go +++ b/internal/ota/encoder.go @@ -18,60 +18,48 @@ package ota import ( - "bufio" "encoding/binary" + "fmt" "hash/crc32" "io" "strconv" "github.com/arduino/arduino-cloud-cli/internal/lzss" - "github.com/juju/errors" ) -// A writer is a buffered, flushable writer. -type writer interface { - io.Writer - Flush() error -} - -// encoder encodes a binary into an .ota file. -type encoder struct { - // w is the writer that compressed bytes are written to. - w writer +// Encoder writes a binary to an output stream in the ota format. +type Encoder struct { + // w is the stream where encoded bytes are written. + w io.Writer - // vendorID is the ID of the board vendor + // vendorID is the ID of the board vendor. vendorID string - // is the ID of the board vendor is the ID of the board model + // productID is the ID of the board model. productID string } -// NewWriter creates a new `WriteCloser` for the the given VID/PID. -func NewWriter(w io.Writer, vendorID, productID string) io.WriteCloser { - bw, ok := w.(writer) - if !ok { - bw = bufio.NewWriter(w) - } - return &encoder{ - w: bw, +// NewEncoder creates a new ota encoder. +func NewEncoder(w io.Writer, vendorID, productID string) *Encoder { + return &Encoder{ + w: w, vendorID: vendorID, productID: productID, } } -// Write writes a compressed representation of p to e's underlying writer. -func (e *encoder) Write(binaryData []byte) (int, error) { - //log.Println("original binaryData is", len(binaryData), "bytes length") - - // Magic number (VID/PID) +// Encode compresses data using a lzss algorithm, encodes the result +// in ota format and writes it to e's underlying writer. +func (e *Encoder) Encode(data []byte) error { + // Compute the magic number (VID/PID) magicNumber := make([]byte, 4) vid, err := strconv.ParseUint(e.vendorID, 16, 16) if err != nil { - return 0, errors.Annotate(err, "OTA encoder: failed to parse vendorID") + return fmt.Errorf("cannot parse vendorID: %w", err) } pid, err := strconv.ParseUint(e.productID, 16, 16) if err != nil { - return 0, errors.Annotate(err, "OTA encoder: failed to parse productID") + return fmt.Errorf("cannot parse productID: %w", err) } binary.LittleEndian.PutUint16(magicNumber[0:2], uint16(pid)) @@ -82,61 +70,43 @@ func (e *encoder) Write(binaryData []byte) (int, error) { Compression: true, } - // Compress the compiled binary - compressed := lzss.Encode(binaryData) - + compressed := lzss.Encode(data) // Prepend magic number and version field to payload - var binDataComplete []byte - binDataComplete = append(binDataComplete, magicNumber...) - binDataComplete = append(binDataComplete, version.AsBytes()...) - binDataComplete = append(binDataComplete, compressed...) - //log.Println("binDataComplete is", len(binDataComplete), "bytes length") + var outData []byte + outData = append(outData, magicNumber...) + outData = append(outData, version.Bytes()...) + outData = append(outData, compressed...) - headerSize, err := e.writeHeader(binDataComplete) + err = e.writeHeader(outData) if err != nil { - return headerSize, err + return fmt.Errorf("cannot write data header to output stream: %w", err) } - payloadSize, err := e.writePayload(binDataComplete) + _, err = e.w.Write(outData) if err != nil { - return payloadSize, err + return fmt.Errorf("cannot write encoded data to output stream: %w", err) } - return headerSize + payloadSize, nil -} - -// Close closes the encoder, flushing any pending output. It does not close or -// flush e's underlying writer. -func (e *encoder) Close() error { - return e.w.Flush() + return nil } -func (e *encoder) writeHeader(binDataComplete []byte) (int, error) { - +func (e *Encoder) writeHeader(data []byte) error { // Write the length of the content lengthAsBytes := make([]byte, 4) - binary.LittleEndian.PutUint32(lengthAsBytes, uint32(len(binDataComplete))) - - n, err := e.w.Write(lengthAsBytes) + binary.LittleEndian.PutUint32(lengthAsBytes, uint32(len(data))) + _, err := e.w.Write(lengthAsBytes) if err != nil { - return n, err + return err } - // Calculate the checksum for binDataComplete - crc := crc32.ChecksumIEEE(binDataComplete) - - // encode the checksum uint32 value as 4 bytes + // Write the checksum uint32 value as 4 bytes + crc := crc32.ChecksumIEEE(data) crcAsBytes := make([]byte, 4) binary.LittleEndian.PutUint32(crcAsBytes, crc) - - n, err = e.w.Write(crcAsBytes) + _, err = e.w.Write(crcAsBytes) if err != nil { - return n, err + return err } - return len(lengthAsBytes) + len(crcAsBytes), nil -} - -func (e *encoder) writePayload(data []byte) (int, error) { - return e.w.Write(data) + return nil } diff --git a/internal/ota/encoder_test.go b/internal/ota/encoder_test.go index b4af65b6..dbb88f1e 100644 --- a/internal/ota/encoder_test.go +++ b/internal/ota/encoder_test.go @@ -20,7 +20,7 @@ package ota import ( "bytes" "encoding/hex" - "log" + "io/ioutil" "fmt" "hash/crc32" @@ -37,8 +37,7 @@ func TestComputeCrc32Checksum(t *testing.T) { assert.Equal(t, crc, uint32(2090640218)) } -func TestEncoderWrite(t *testing.T) { - +func TestEncode(t *testing.T) { // Setup test data data, _ := hex.DecodeString("DEADBEEF") // uncompressed, or 'ef 6b 77 de f0' (compressed w/ LZSS) @@ -46,21 +45,17 @@ func TestEncoderWrite(t *testing.T) { vendorID := "2341" // Arduino productID := "8054" // MRK Wifi 1010 - otaWriter := NewWriter(&w, vendorID, productID) - defer otaWriter.Close() + enc := NewEncoder(&w, vendorID, productID) - n, err := otaWriter.Write(data) + err := enc.Encode(data) if err != nil { t.Error(err) - t.Fail() } - log.Println("written ota of", n, "bytes length") - otaWriter.Close() actual := w.Bytes() - // You can get the expected result creating an `.ota` file using Alex's tools: - // https://github.com/arduino-libraries/ArduinoIoTCloud/tree/master/extras/tools + // Expected result has been computed with the following tool: + // https://github.com/arduino-libraries/ArduinoIoTCloud/tree/master/extras/tools . expected, _ := hex.DecodeString("11000000a1744bd4548041230000000000000040ef6b77def0") res := bytes.Compare(expected, actual) @@ -72,3 +67,51 @@ func TestEncoderWrite(t *testing.T) { assert.Assert(t, res == 0) // 0 means equal } + +// Expected '.ota' files contained in testdata have been computed with the following tool: +// https://github.com/arduino-libraries/ArduinoIoTCloud/tree/master/extras/tools . +func TestEncodeFiles(t *testing.T) { + tests := []struct { + name string + infile string + outfile string + }{ + { + name: "blink", + infile: "testdata/blink.bin", + outfile: "testdata/blink.ota", + }, + { + name: "cloud sketch", + infile: "testdata/cloud.bin", + outfile: "testdata/cloud.ota", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + input, err := ioutil.ReadFile(tt.infile) + if err != nil { + t.Fatal("couldn't open test file") + } + + want, err := ioutil.ReadFile(tt.outfile) + if err != nil { + t.Fatal("couldn't open test file") + } + + var got bytes.Buffer + vendorID := "2341" // Arduino + productID := "8057" // Nano 33 IoT + otaenc := NewEncoder(&got, vendorID, productID) + err = otaenc.Encode(input) + if err != nil { + t.Error(err) + } + + if !bytes.Equal(want, got.Bytes()) { + t.Error("encoding failed") + } + }) + } +} diff --git a/internal/ota/testdata/blink.bin b/internal/ota/testdata/blink.bin new file mode 100755 index 00000000..b3731ec4 Binary files /dev/null and b/internal/ota/testdata/blink.bin differ diff --git a/internal/ota/testdata/blink.ota b/internal/ota/testdata/blink.ota new file mode 100644 index 00000000..c0006864 Binary files /dev/null and b/internal/ota/testdata/blink.ota differ diff --git a/internal/ota/testdata/cloud.bin b/internal/ota/testdata/cloud.bin new file mode 100755 index 00000000..0bf2398a Binary files /dev/null and b/internal/ota/testdata/cloud.bin differ diff --git a/internal/ota/testdata/cloud.ota b/internal/ota/testdata/cloud.ota new file mode 100644 index 00000000..867ec182 Binary files /dev/null and b/internal/ota/testdata/cloud.ota differ diff --git a/internal/ota/testdata/lorem.lzss b/internal/ota/testdata/lorem.lzss deleted file mode 100644 index 07d0070c..00000000 Binary files a/internal/ota/testdata/lorem.lzss and /dev/null differ diff --git a/internal/ota/testdata/lorem.txt b/internal/ota/testdata/lorem.txt deleted file mode 100644 index c942efc7..00000000 --- a/internal/ota/testdata/lorem.txt +++ /dev/null @@ -1,9 +0,0 @@ -Lorem ipsum dolor sit amet, consectetur adipiscing elit. Suspendisse ligula dui, imperdiet ut vulputate semper, sollicitudin ut eros. Aliquam erat volutpat. In hac habitasse platea dictumst. Nam non tortor sit amet mauris rutrum eleifend. Pellentesque vel justo nibh. Vivamus sem risus, pharetra eget egestas eget, venenatis a ipsum. Cras ultrices aliquam sagittis. Donec lacinia urna ac orci congue ut adipiscing dolor fringilla. Nullam nibh magna, bibendum vulputate ornare id, hendrerit et metus. Nullam dapibus neque quis mi laoreet molestie. Mauris et dui lacus, sit amet egestas purus. - -Donec accumsan elementum accumsan. Nullam gravida dictum diam non semper. Curabitur vel magna in velit accumsan pulvinar eget in lorem. Duis vitae ante velit, at hendrerit nibh. Pellentesque lacus urna, cursus ac semper sagittis, viverra at sem. Quisque ullamcorper odio dolor. In quis pretium lacus. Maecenas lacinia urna id massa congue blandit. Suspendisse dapibus eros sit amet neque fermentum imperdiet. Cras interdum pulvinar eleifend. Suspendisse molestie neque a risus imperdiet convallis. In interdum dignissim pharetra. Morbi lectus tortor, pulvinar quis eleifend in, placerat at risus. Sed aliquam diam at metus adipiscing blandit. - -Integer tristique metus vel ipsum pulvinar dignissim quis vel quam. Donec auctor aliquet bibendum. Morbi aliquet malesuada ultrices. Vivamus ac leo odio. Nam tristique eros non arcu porttitor non volutpat mauris tempus. Proin vestibulum suscipit pretium. Etiam elit tortor, dictum a gravida porta, congue id dolor. Duis eget est vitae elit facilisis blandit. Proin tincidunt felis et ipsum pharetra tempor. Fusce imperdiet vulputate magna, vel lacinia neque volutpat a. Vivamus a elit dolor. Aliquam sollicitudin dui et leo elementum mattis. Quisque suscipit, lorem id eleifend imperdiet, ipsum lorem pharetra purus, vel tempus lectus ligula id tortor. Morbi eget eros vel sapien scelerisque aliquam pellentesque sed turpis. Duis vel lorem non eros semper fringilla vitae vitae erat. - -Vivamus porttitor pulvinar tristique. Proin sed elit ipsum. Phasellus faucibus pulvinar dapibus. Praesent quis sem in purus ultrices imperdiet. Aenean ut nulla urna. In tristique tincidunt urna, nec adipiscing velit laoreet ut. Curabitur et ante sed libero tristique pellentesque. Quisque porttitor sodales ipsum ut rhoncus. Nunc vitae diam gravida orci aliquam cursus vitae ut sapien. Proin ullamcorper felis eu nulla dapibus nec faucibus odio hendrerit. Aenean lorem magna, fermentum in tristique sit amet, accumsan ut massa. Fusce tristique, lectus rhoncus commodo sagittis, ligula felis consequat arcu, id pretium enim dolor id mi. Donec facilisis pulvinar luctus. Pellentesque vitae condimentum risus. Nam quis elit a orci adipiscing bibendum. - -Ut quis felis lorem, dignissim varius turpis. Sed convallis dui semper mauris fermentum porta. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Aliquam erat volutpat. Proin lorem felis, scelerisque nec commodo non, porta eu metus. Nulla id augue a turpis mollis pellentesque. Aenean in lectus et leo tincidunt auctor eu interdum est. Nulla varius, lorem congue laoreet laoreet, felis quam ullamcorper ligula, non pellentesque dui ipsum quis odio. Etiam sit amet blandit leo. Aenean venenatis molestie eros, in fermentum ipsum dictum eget. Donec ultricies feugiat nisl, non molestie mi congue quis. Quisque mattis augue nec neque fringilla varius. Proin sollicitudin risus et elit pretium congue. Sed consequat eros sit amet felis pulvinar pulvinar. Morbi in turpis eu nulla cursus venenatis at ut urna. Donec vel lectus quis nisi aliquam varius. \ No newline at end of file diff --git a/internal/ota/version.go b/internal/ota/version.go index 18dd72e9..5a34e238 100644 --- a/internal/ota/version.go +++ b/internal/ota/version.go @@ -32,8 +32,8 @@ type Version struct { PayloadBuildNum uint32 } -// AsBytes builds a 8 byte length representation of the Version Struct for the OTA update. -func (v *Version) AsBytes() []byte { +// Bytes builds a 8 byte length representation of the Version Struct for the OTA update. +func (v *Version) Bytes() []byte { version := []byte{0, 0, 0, 0, 0, 0, 0, 0} // Set compression diff --git a/internal/ota/version_test.go b/internal/ota/version_test.go index d5b83d33..e5c900af 100644 --- a/internal/ota/version_test.go +++ b/internal/ota/version_test.go @@ -34,7 +34,7 @@ func TestVersionWithCompressionEnabled(t *testing.T) { } expected := []byte{0, 0, 0, 0, 0, 0, 0, 0x40} - actual := version.AsBytes() + actual := version.Bytes() // create a tabwriter for formatting the output w := new(tabwriter.Writer)