From d58886e646a0c5c5ab36316d4a4dc0292110bb76 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Sat, 6 Jun 2020 18:02:17 +0200 Subject: [PATCH 1/3] Fixed skethbook+bootloader hex merger. Previous merger doesn't take into account .bin bootloaders and it may have problems if .hex data is not sorted by address. --- go.mod | 3 +- go.sum | 4 + .../builder/merge_sketch_with_bootloader.go | 115 +++++++++++------- .../test/merge_sketch_with_bootloader_test.go | 110 +++++++++++++++-- 4 files changed, 172 insertions(+), 60 deletions(-) diff --git a/go.mod b/go.mod index fa398bf170e..c96a173dc12 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( bou.ke/monkey v1.0.1 github.com/GeertJohan/go.rice v1.0.0 github.com/arduino/board-discovery v0.0.0-20180823133458-1ba29327fb0c - github.com/arduino/go-paths-helper v1.0.1 + github.com/arduino/go-paths-helper v1.2.0 github.com/arduino/go-properties-orderedmap v1.0.0 github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b github.com/arduino/go-win32-utils v0.0.0-20180330194947-ed041402e83b @@ -25,6 +25,7 @@ require ( github.com/juju/loggo v0.0.0-20190526231331-6e530bcce5d8 // indirect github.com/juju/testing v0.0.0-20190429233213-dfc56b8c09fc // indirect github.com/leonelquinteros/gotext v1.4.0 + github.com/marcinbor85/gohex v0.0.0-20200531163658-baab2527a9a2 github.com/mattn/go-colorable v0.1.2 github.com/mattn/go-runewidth v0.0.2 // indirect github.com/miekg/dns v1.0.5 // indirect diff --git a/go.sum b/go.sum index 52b9bdc331c..a15036f81ac 100644 --- a/go.sum +++ b/go.sum @@ -16,6 +16,8 @@ github.com/arduino/board-discovery v0.0.0-20180823133458-1ba29327fb0c h1:agh2JT9 github.com/arduino/board-discovery v0.0.0-20180823133458-1ba29327fb0c/go.mod h1:HK7SpkEax/3P+0w78iRQx1sz1vCDYYw9RXwHjQTB5i8= github.com/arduino/go-paths-helper v1.0.1 h1:utYXLM2RfFlc9qp/MJTIYp3t6ux/xM6mWjeEb/WLK4Q= github.com/arduino/go-paths-helper v1.0.1/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck= +github.com/arduino/go-paths-helper v1.2.0 h1:qDW93PR5IZUN/jzO4rCtexiwF8P4OIcOmcSgAYLZfY4= +github.com/arduino/go-paths-helper v1.2.0/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck= github.com/arduino/go-properties-orderedmap v1.0.0 h1:caaM25TQZKkytoKQUsgqtOVbrM5i8Gb427JmW0KL05s= github.com/arduino/go-properties-orderedmap v1.0.0/go.mod h1:DKjD2VXY/NZmlingh4lSFMEYCVubfeArCsGPGDwb2yk= github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b h1:9hDi4F2st6dbLC3y4i02zFT5quS4X6iioWifGlVwfy4= @@ -125,6 +127,8 @@ github.com/magiconair/properties v1.8.0 h1:LLgXmsheXeRoUOBOjtwPQCWIYqM/LU1ayDtDe github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/magiconair/properties v1.8.1 h1:ZC2Vc7/ZFkGmsVC9KvOjumD+G5lXy2RtTKyzRKO2BQ4= github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= +github.com/marcinbor85/gohex v0.0.0-20200531163658-baab2527a9a2 h1:n7R8fUwWZUB2XtyzBNsYNNm9/XgOBj6pvLi7GLMCHtM= +github.com/marcinbor85/gohex v0.0.0-20200531163658-baab2527a9a2/go.mod h1:Pb6XcsXyropB9LNHhnqaknG/vEwYztLkQzVCHv8sQ3M= github.com/mattn/go-colorable v0.1.2 h1:/bC9yWikZXAL9uJdulbSfyVNIR3n3trXl+v8+1sx8mU= github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-isatty v0.0.8 h1:HLtExJ+uU2HOZ+wI0Tt5DtUDrx8yhUqDcp7fYERX4CE= diff --git a/legacy/builder/merge_sketch_with_bootloader.go b/legacy/builder/merge_sketch_with_bootloader.go index 742d8f57db6..cc869116192 100644 --- a/legacy/builder/merge_sketch_with_bootloader.go +++ b/legacy/builder/merge_sketch_with_bootloader.go @@ -16,12 +16,15 @@ package builder import ( + "math" "os" + "strconv" "strings" "github.com/arduino/arduino-cli/legacy/builder/constants" "github.com/arduino/arduino-cli/legacy/builder/types" "github.com/arduino/go-paths-helper" + "github.com/marcinbor85/gohex" "github.com/pkg/errors" ) @@ -66,68 +69,86 @@ func (s *MergeSketchWithBootloader) Run(ctx *types.Context) error { mergedSketchPath := builtSketchPath.Parent().Join(sketchFileName + ".with_bootloader.hex") - return merge(builtSketchPath, bootloaderPath, mergedSketchPath) -} - -func hexLineOnlyContainsFF(line string) bool { - //:206FE000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFB1 - if len(line) <= 11 { - return false + // Ignore merger errors for the first iteration + maximumBinSize := 16000000 + if uploadMaxSize, ok := ctx.BuildProperties.GetOk(constants.PROPERTY_UPLOAD_MAX_SIZE); ok { + maximumBinSize, _ = strconv.Atoi(uploadMaxSize) + maximumBinSize *= 2 } - byteArray := []byte(line) - for _, char := range byteArray[9:(len(byteArray) - 2)] { - if char != 'F' { - return false - } + err := merge(builtSketchPath, bootloaderPath, mergedSketchPath, maximumBinSize) + if err != nil { + logger.Fprintln(os.Stdout, constants.LOG_LEVEL_WARN, err.Error()) } - return true -} - -func extractActualBootloader(bootloader []string) []string { - var realBootloader []string + return nil +} - // skip until we find a line full of FFFFFF (except address and checksum) - for i, row := range bootloader { - if hexLineOnlyContainsFF(row) { - realBootloader = bootloader[i:len(bootloader)] - break - } +func merge(builtSketchPath, bootloaderPath, mergedSketchPath *paths.Path, maximumBinSize int) error { + if bootloaderPath.Ext() == ".bin" { + bootloaderPath = paths.New(strings.TrimSuffix(bootloaderPath.String(), ".bin") + ".hex") } - // drop all "empty" lines - for i, row := range realBootloader { - if !hexLineOnlyContainsFF(row) { - realBootloader = realBootloader[i:len(realBootloader)] - break + memBoot := gohex.NewMemory() + if bootFile, err := bootloaderPath.Open(); err == nil { + defer bootFile.Close() + if err := memBoot.ParseIntelHex(bootFile); err != nil { + return errors.New(bootFile.Name() + " " + err.Error()) } + } else { + return err } - if len(realBootloader) == 0 { - // we didn't find any line full of FFFF, thus it's a standalone bootloader - realBootloader = bootloader + memSketch := gohex.NewMemory() + if buildFile, err := builtSketchPath.Open(); err == nil { + defer buildFile.Close() + if err := memSketch.ParseIntelHex(buildFile); err != nil { + return errors.New(buildFile.Name() + " " + err.Error()) + } + } else { + return err } - return realBootloader -} + memMerged := gohex.NewMemory() + initialAddress := uint32(math.MaxUint32) + lastAddress := uint32(0) -func merge(builtSketchPath, bootloaderPath, mergedSketchPath *paths.Path) error { - sketch, err := builtSketchPath.ReadFileAsLines() - if err != nil { - return errors.WithStack(err) + for _, segment := range memBoot.GetDataSegments() { + if err := memMerged.AddBinary(segment.Address, segment.Data); err != nil { + continue + } + if segment.Address < initialAddress { + initialAddress = segment.Address + } + if segment.Address+uint32(len(segment.Data)) > lastAddress { + lastAddress = segment.Address + uint32(len(segment.Data)) + } } - sketch = sketch[:len(sketch)-2] - - bootloader, err := bootloaderPath.ReadFileAsLines() - if err != nil { - return errors.WithStack(err) + for _, segment := range memSketch.GetDataSegments() { + if err := memMerged.AddBinary(segment.Address, segment.Data); err != nil { + continue + } + if segment.Address < initialAddress { + initialAddress = segment.Address + } + if segment.Address+uint32(len(segment.Data)) > lastAddress { + lastAddress = segment.Address + uint32(len(segment.Data)) + } } - realBootloader := extractActualBootloader(bootloader) - - for _, row := range realBootloader { - sketch = append(sketch, row) + if mergeFile, err := mergedSketchPath.Create(); err == nil { + defer mergeFile.Close() + memMerged.DumpIntelHex(mergeFile, 16) + } else { + return err } - return mergedSketchPath.WriteFile([]byte(strings.Join(sketch, "\n"))) + // size := lastAddress - initialAddress + // if size > uint32(maximumBinSize) { + // return nil + // } + // mergedSketchPathBin := paths.New(strings.TrimSuffix(mergedSketchPath.String(), ".hex") + ".bin") + // data := memMerged.ToBinary(initialAddress, size, 0xFF) + // return mergedSketchPathBin.WriteFile(data) + + return nil } diff --git a/legacy/builder/test/merge_sketch_with_bootloader_test.go b/legacy/builder/test/merge_sketch_with_bootloader_test.go index 556d4d2c4d5..68441b5c1c6 100644 --- a/legacy/builder/test/merge_sketch_with_bootloader_test.go +++ b/legacy/builder/test/merge_sketch_with_bootloader_test.go @@ -16,6 +16,7 @@ package test import ( + "fmt" "path/filepath" "strings" "testing" @@ -46,8 +47,36 @@ func TestMergeSketchWithBootloader(t *testing.T) { err := buildPath.Join("sketch").MkdirAll() NoError(t, err) - fakeSketchHex := "row 1\n" + - "row 2\n" + fakeSketchHex := `:100000000C9434000C9446000C9446000C9446006A +:100010000C9446000C9446000C9446000C94460048 +:100020000C9446000C9446000C9446000C94460038 +:100030000C9446000C9446000C9446000C94460028 +:100040000C9448000C9446000C9446000C94460016 +:100050000C9446000C9446000C9446000C94460008 +:100060000C9446000C94460011241FBECFEFD8E03C +:10007000DEBFCDBF21E0A0E0B1E001C01D92A930FC +:10008000B207E1F70E9492000C94DC000C9400008F +:100090001F920F920FB60F9211242F933F938F93BD +:1000A0009F93AF93BF938091050190910601A0911A +:1000B0000701B09108013091040123E0230F2D378F +:1000C00020F40196A11DB11D05C026E8230F02965C +:1000D000A11DB11D20930401809305019093060199 +:1000E000A0930701B0930801809100019091010154 +:1000F000A0910201B09103010196A11DB11D809351 +:10010000000190930101A0930201B0930301BF91FC +:10011000AF919F918F913F912F910F900FBE0F90B4 +:100120001F901895789484B5826084BD84B58160F1 +:1001300084BD85B5826085BD85B5816085BD8091B2 +:100140006E00816080936E0010928100809181002A +:100150008260809381008091810081608093810022 +:10016000809180008160809380008091B1008460E4 +:100170008093B1008091B00081608093B000809145 +:100180007A00846080937A0080917A008260809304 +:100190007A0080917A00816080937A0080917A0061 +:1001A000806880937A001092C100C0E0D0E0209770 +:0C01B000F1F30E940000FBCFF894FFCF99 +:00000001FF +` err = buildPath.Join("sketch", "sketch.ino.hex").WriteFile([]byte(fakeSketchHex)) NoError(t, err) @@ -65,8 +94,8 @@ func TestMergeSketchWithBootloader(t *testing.T) { NoError(t, err) mergedSketchHex := string(bytes) - require.True(t, strings.HasPrefix(mergedSketchHex, "row 1\n:107E0000112484B714BE81FFF0D085E080938100F7\n")) - require.True(t, strings.HasSuffix(mergedSketchHex, ":0400000300007E007B\n:00000001FF\n")) + require.Contains(t, mergedSketchHex, ":100000000C9434000C9446000C9446000C9446006A\n") + require.True(t, strings.HasSuffix(mergedSketchHex, ":00000001FF\n")) } func TestMergeSketchWithBootloaderSketchInBuildPath(t *testing.T) { @@ -88,8 +117,36 @@ func TestMergeSketchWithBootloaderSketchInBuildPath(t *testing.T) { err := buildPath.Join("sketch").MkdirAll() NoError(t, err) - fakeSketchHex := "row 1\n" + - "row 2\n" + fakeSketchHex := `:100000000C9434000C9446000C9446000C9446006A +:100010000C9446000C9446000C9446000C94460048 +:100020000C9446000C9446000C9446000C94460038 +:100030000C9446000C9446000C9446000C94460028 +:100040000C9448000C9446000C9446000C94460016 +:100050000C9446000C9446000C9446000C94460008 +:100060000C9446000C94460011241FBECFEFD8E03C +:10007000DEBFCDBF21E0A0E0B1E001C01D92A930FC +:10008000B207E1F70E9492000C94DC000C9400008F +:100090001F920F920FB60F9211242F933F938F93BD +:1000A0009F93AF93BF938091050190910601A0911A +:1000B0000701B09108013091040123E0230F2D378F +:1000C00020F40196A11DB11D05C026E8230F02965C +:1000D000A11DB11D20930401809305019093060199 +:1000E000A0930701B0930801809100019091010154 +:1000F000A0910201B09103010196A11DB11D809351 +:10010000000190930101A0930201B0930301BF91FC +:10011000AF919F918F913F912F910F900FBE0F90B4 +:100120001F901895789484B5826084BD84B58160F1 +:1001300084BD85B5826085BD85B5816085BD8091B2 +:100140006E00816080936E0010928100809181002A +:100150008260809381008091810081608093810022 +:10016000809180008160809380008091B1008460E4 +:100170008093B1008091B00081608093B000809145 +:100180007A00846080937A0080917A008260809304 +:100190007A0080917A00816080937A0080917A0061 +:1001A000806880937A001092C100C0E0D0E0209770 +:0C01B000F1F30E940000FBCFF894FFCF99 +:00000001FF +` err = buildPath.Join("sketch.ino.hex").WriteFile([]byte(fakeSketchHex)) NoError(t, err) @@ -107,8 +164,9 @@ func TestMergeSketchWithBootloaderSketchInBuildPath(t *testing.T) { NoError(t, err) mergedSketchHex := string(bytes) - require.True(t, strings.HasPrefix(mergedSketchHex, "row 1\n:107E0000112484B714BE81FFF0D085E080938100F7\n")) - require.True(t, strings.HasSuffix(mergedSketchHex, ":0400000300007E007B\n:00000001FF\n")) + fmt.Println(string(mergedSketchHex)) + require.Contains(t, mergedSketchHex, ":100000000C9434000C9446000C9446000C9446006A\n") + require.True(t, strings.HasSuffix(mergedSketchHex, ":00000001FF\n")) } func TestMergeSketchWithBootloaderWhenNoBootloaderAvailable(t *testing.T) { @@ -168,8 +226,36 @@ func TestMergeSketchWithBootloaderPathIsParameterized(t *testing.T) { err := buildPath.Join("sketch").MkdirAll() NoError(t, err) - fakeSketchHex := "row 1\n" + - "row 2\n" + fakeSketchHex := `:100000000C9434000C9446000C9446000C9446006A +:100010000C9446000C9446000C9446000C94460048 +:100020000C9446000C9446000C9446000C94460038 +:100030000C9446000C9446000C9446000C94460028 +:100040000C9448000C9446000C9446000C94460016 +:100050000C9446000C9446000C9446000C94460008 +:100060000C9446000C94460011241FBECFEFD8E03C +:10007000DEBFCDBF21E0A0E0B1E001C01D92A930FC +:10008000B207E1F70E9492000C94DC000C9400008F +:100090001F920F920FB60F9211242F933F938F93BD +:1000A0009F93AF93BF938091050190910601A0911A +:1000B0000701B09108013091040123E0230F2D378F +:1000C00020F40196A11DB11D05C026E8230F02965C +:1000D000A11DB11D20930401809305019093060199 +:1000E000A0930701B0930801809100019091010154 +:1000F000A0910201B09103010196A11DB11D809351 +:10010000000190930101A0930201B0930301BF91FC +:10011000AF919F918F913F912F910F900FBE0F90B4 +:100120001F901895789484B5826084BD84B58160F1 +:1001300084BD85B5826085BD85B5816085BD8091B2 +:100140006E00816080936E0010928100809181002A +:100150008260809381008091810081608093810022 +:10016000809180008160809380008091B1008460E4 +:100170008093B1008091B00081608093B000809145 +:100180007A00846080937A0080917A008260809304 +:100190007A0080917A00816080937A0080917A0061 +:1001A000806880937A001092C100C0E0D0E0209770 +:0C01B000F1F30E940000FBCFF894FFCF99 +:00000001FF +` err = buildPath.Join("sketch", "sketch.ino.hex").WriteFile([]byte(fakeSketchHex)) NoError(t, err) @@ -187,6 +273,6 @@ func TestMergeSketchWithBootloaderPathIsParameterized(t *testing.T) { NoError(t, err) mergedSketchHex := string(bytes) - require.True(t, strings.HasPrefix(mergedSketchHex, "row 1\n:020000023000CC")) - require.True(t, strings.HasSuffix(mergedSketchHex, ":040000033000E000E9\n:00000001FF\n")) + require.Contains(t, mergedSketchHex, ":100000000C9434000C9446000C9446000C9446006A\n") + require.True(t, strings.HasSuffix(mergedSketchHex, ":00000001FF\n")) } From 9b2842cc61fc943ba8d7484b6dbbef7dbb471f43 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 8 Jun 2020 16:54:07 +0200 Subject: [PATCH 2/3] output a merged sketch+bootloader bin file too --- legacy/builder/merge_sketch_with_bootloader.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/legacy/builder/merge_sketch_with_bootloader.go b/legacy/builder/merge_sketch_with_bootloader.go index cc869116192..a7c971b4296 100644 --- a/legacy/builder/merge_sketch_with_bootloader.go +++ b/legacy/builder/merge_sketch_with_bootloader.go @@ -142,13 +142,13 @@ func merge(builtSketchPath, bootloaderPath, mergedSketchPath *paths.Path, maximu return err } - // size := lastAddress - initialAddress - // if size > uint32(maximumBinSize) { - // return nil - // } - // mergedSketchPathBin := paths.New(strings.TrimSuffix(mergedSketchPath.String(), ".hex") + ".bin") - // data := memMerged.ToBinary(initialAddress, size, 0xFF) - // return mergedSketchPathBin.WriteFile(data) - - return nil + // Write out a .bin if the addresses doesn't go too far away from origin + // (and consequently produce a very large bin) + size := lastAddress - initialAddress + if size > uint32(maximumBinSize) { + return nil + } + mergedSketchPathBin := paths.New(strings.TrimSuffix(mergedSketchPath.String(), ".hex") + ".bin") + data := memMerged.ToBinary(initialAddress, size, 0xFF) + return mergedSketchPathBin.WriteFile(data) } From 2da824d7b2b6cc2119ce03af514018d2be7cc5b5 Mon Sep 17 00:00:00 2001 From: Martino Facchin Date: Thu, 11 Jun 2020 17:32:55 +0200 Subject: [PATCH 3/3] Replace logger with utils.LogIfVerbose --- legacy/builder/merge_sketch_with_bootloader.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/legacy/builder/merge_sketch_with_bootloader.go b/legacy/builder/merge_sketch_with_bootloader.go index a7c971b4296..87b4f954fd2 100644 --- a/legacy/builder/merge_sketch_with_bootloader.go +++ b/legacy/builder/merge_sketch_with_bootloader.go @@ -17,12 +17,12 @@ package builder import ( "math" - "os" "strconv" "strings" "github.com/arduino/arduino-cli/legacy/builder/constants" "github.com/arduino/arduino-cli/legacy/builder/types" + "github.com/arduino/arduino-cli/legacy/builder/utils" "github.com/arduino/go-paths-helper" "github.com/marcinbor85/gohex" "github.com/pkg/errors" @@ -39,7 +39,6 @@ func (s *MergeSketchWithBootloader) Run(ctx *types.Context) error { buildPath := ctx.BuildPath sketch := ctx.Sketch sketchFileName := sketch.MainFile.Name.Base() - logger := ctx.GetLogger() sketchInBuildPath := buildPath.Join(sketchFileName + ".hex") sketchInSubfolder := buildPath.Join(constants.FOLDER_SKETCH, sketchFileName+".hex") @@ -63,7 +62,7 @@ func (s *MergeSketchWithBootloader) Run(ctx *types.Context) error { bootloaderPath := buildProperties.GetPath(constants.BUILD_PROPERTIES_RUNTIME_PLATFORM_PATH).Join(constants.FOLDER_BOOTLOADERS, bootloader) if bootloaderPath.NotExist() { - logger.Fprintln(os.Stdout, constants.LOG_LEVEL_WARN, constants.MSG_BOOTLOADER_FILE_MISSING, bootloaderPath) + utils.LogIfVerbose(constants.LOG_LEVEL_WARN, constants.MSG_BOOTLOADER_FILE_MISSING, bootloaderPath) return nil } @@ -77,7 +76,7 @@ func (s *MergeSketchWithBootloader) Run(ctx *types.Context) error { } err := merge(builtSketchPath, bootloaderPath, mergedSketchPath, maximumBinSize) if err != nil { - logger.Fprintln(os.Stdout, constants.LOG_LEVEL_WARN, err.Error()) + utils.LogIfVerbose(constants.LOG_LEVEL_INFO, err.Error()) } return nil