From a262ded8c91705f5cf011813f96726704e7b6ecc Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 16 Apr 2021 16:39:52 +0200 Subject: [PATCH 1/5] Experimental: optimized touch + wait --- arduino/serialutils/serialutils.go | 75 ++++++++++++++++++++++++++++++ commands/upload/upload.go | 70 ++++++++++++++-------------- 2 files changed, 110 insertions(+), 35 deletions(-) diff --git a/arduino/serialutils/serialutils.go b/arduino/serialutils/serialutils.go index e45fe130ce6..6e9235ed19c 100644 --- a/arduino/serialutils/serialutils.go +++ b/arduino/serialutils/serialutils.go @@ -16,6 +16,7 @@ package serialutils import ( + "fmt" "time" "github.com/pkg/errors" @@ -127,3 +128,77 @@ func WaitForNewSerialPort() (string, error) { return "", nil } + +// TouchAndWait FIXMEDOCS +func TouchAndWait(portToTouch string, wait bool) (string, error) { + getPortMap := func() (map[string]bool, error) { + ports, err := serial.GetPortsList() + if err != nil { + return nil, errors.WithMessage(err, "listing serial ports") + } + res := map[string]bool{} + for _, port := range ports { + res[port] = true + } + return res, nil + } + + last, err := getPortMap() + fmt.Println("LAST:", last) + if err != nil { + return "", err + } + + if portToTouch != "" { + fmt.Println("TOUCH:", portToTouch) + TouchSerialPortAt1200bps(portToTouch) + } + + if !wait { + return "", nil + } + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) { + now, err := getPortMap() + fmt.Println("NOW:", now) + if err != nil { + return "", err + } + hasNewPorts := false + for p := range now { + if !last[p] { + hasNewPorts = true + break + } + } + + if hasNewPorts { + fmt.Println("HAS NEW PORTS!") + // on OS X, if the port is opened too quickly after it is detected, + // a "Resource busy" error occurs, add a delay to workaround. + // This apply to other platforms as well. + time.Sleep(time.Second) + + // Some boards have a glitch in the bootloader: some user experienced + // the USB serial port appearing and disappearing rapidly before + // settling. + // This check ensure that the port is stable after one second. + check, err := getPortMap() + fmt.Println("CHECK:", check) + if err != nil { + return "", err + } + for p := range check { + if !last[p] { + fmt.Println("FOUND:", p) + return p, nil // Found it! + } + } + } + + last = now + time.Sleep(250 * time.Millisecond) + } + + return "", nil +} diff --git a/commands/upload/upload.go b/commands/upload/upload.go index fe51d396c14..0b3540e2193 100644 --- a/commands/upload/upload.go +++ b/commands/upload/upload.go @@ -36,7 +36,6 @@ import ( properties "github.com/arduino/go-properties-orderedmap" "github.com/pkg/errors" "github.com/sirupsen/logrus" - "go.bug.st/serial" ) // Upload FIXMEDOC @@ -293,44 +292,45 @@ func runProgramAction(pm *packagemanager.PackageManager, // to set the board in bootloader mode actualPort := port if programmer == nil && !burnBootloader { - // Perform reset via 1200bps touch if requested - if uploadProperties.GetBoolean("upload.use_1200bps_touch") { - if port == "" { - outStream.Write([]byte(fmt.Sprintln("Skipping 1200-bps touch reset: no serial port selected!"))) - } else { - ports, err := serial.GetPortsList() - if err != nil { - return fmt.Errorf("cannot get serial port list: %s", err) - } - for _, p := range ports { - if p == port { - if verbose { - outStream.Write([]byte(fmt.Sprintf("Performing 1200-bps touch reset on serial port %s", p))) - outStream.Write([]byte(fmt.Sprintln())) - } - logrus.Infof("Touching port %s at 1200bps", port) - if err := serialutils.TouchSerialPortAt1200bps(p); err != nil { - outStream.Write([]byte(fmt.Sprintf("Cannot perform port reset: %s", err))) - outStream.Write([]byte(fmt.Sprintln())) - } - break - } - } - } + // Perform reset via 1200bps touch if requested and wait for upload port if requested. + touch := uploadProperties.GetBoolean("upload.use_1200bps_touch") + wait := uploadProperties.GetBoolean("upload.wait_for_upload_port") + if touch && port == "" { + outStream.Write([]byte(fmt.Sprintln("Skipping 1200-bps touch reset: no serial port selected!"))) } - // Wait for upload port if requested - if uploadProperties.GetBoolean("upload.wait_for_upload_port") { - if verbose { - outStream.Write([]byte(fmt.Sprintln("Waiting for upload port..."))) - } - - actualPort, err = serialutils.WaitForNewSerialPortOrDefaultTo(actualPort) - if err != nil { - return errors.WithMessage(err, "detecting serial port") + if newPort, err := serialutils.TouchAndWait(port, wait); err != nil { + } else { + if newPort != "" { + actualPort = newPort } } - } + // ports, err := serial.GetPortsList() + // for _, p := range ports { + // if p == port { + // if verbose { + // outStream.Write([]byte(fmt.Sprintf("Performing 1200-bps touch reset on serial port %s", p))) + // outStream.Write([]byte(fmt.Sprintln())) + // } + // logrus.Infof("Touching port %s at 1200bps", port) + // if err := serialutils.TouchSerialPortAt1200bps(p); err != nil { + // outStream.Write([]byte(fmt.Sprintf("Cannot perform port reset: %s", err))) + // outStream.Write([]byte(fmt.Sprintln())) + // } + // break + // } + // } + } + + // Wait for upload port if requested + // if verbose { + // outStream.Write([]byte(fmt.Sprintln("Waiting for upload port..."))) + // } + + // actualPort, err = serialutils.WaitForNewSerialPortOrDefaultTo(actualPort) + // if err != nil { + // return errors.WithMessage(err, "detecting serial port") + // } if port != "" { // Set serial port property From 5ee5a80935f028b2ac695dbce12260dcf32349f9 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 19 Apr 2021 11:25:23 +0200 Subject: [PATCH 2/5] Refactored upload output and debug info and removed unused functions --- arduino/serialutils/serialutils.go | 156 ++++++++++++----------------- commands/upload/upload.go | 48 +++++---- 2 files changed, 94 insertions(+), 110 deletions(-) diff --git a/arduino/serialutils/serialutils.go b/arduino/serialutils/serialutils.go index 6e9235ed19c..4a9119db0b0 100644 --- a/arduino/serialutils/serialutils.go +++ b/arduino/serialutils/serialutils.go @@ -23,27 +23,6 @@ import ( "go.bug.st/serial" ) -// Reset a board using the 1200 bps port-touch. If wait is true, it will wait -// for a new port to appear (which could change sometimes) and returns that. -// The error is set if the port listing fails. -func Reset(port string, wait bool) (string, error) { - // Touch port at 1200bps - if err := TouchSerialPortAt1200bps(port); err != nil { - return "", errors.New("1200bps Touch") - } - - if wait { - // Wait for port to disappear and reappear - if p, err := WaitForNewSerialPortOrDefaultTo(port); err == nil { - port = p - } else { - return "", errors.WithMessage(err, "detecting upload port") - } - } - - return port, nil -} - // TouchSerialPortAt1200bps open and close the serial port at 1200 bps. This // is used on many Arduino boards as a signal to put the board in "bootloader" // mode. @@ -72,98 +51,76 @@ func TouchSerialPortAt1200bps(port string) error { return nil } -// WaitForNewSerialPortOrDefaultTo is meant to be called just after a reset. It watches the ports connected -// to the machine until a port appears. The new appeared port is returned or, if the operation -// timeouts, the default port provided as parameter is returned. -func WaitForNewSerialPortOrDefaultTo(defaultPort string) (string, error) { - if p, err := WaitForNewSerialPort(); err != nil { - return "", errors.WithMessage(err, "detecting upload port") - } else if p != "" { - // on OS X, if the port is opened too quickly after it is detected, - // a "Resource busy" error occurs, add a delay to workaround. - // This apply to other platforms as well. - time.Sleep(500 * time.Millisecond) - - return p, nil - } - return defaultPort, nil -} - -// WaitForNewSerialPort is meant to be called just after a reset. It watches the ports connected -// to the machine until a port appears. The new appeared port is returned. -func WaitForNewSerialPort() (string, error) { - getPortMap := func() (map[string]bool, error) { - ports, err := serial.GetPortsList() - if err != nil { - return nil, errors.WithMessage(err, "listing serial ports") - } - res := map[string]bool{} - for _, port := range ports { - res[port] = true - } - return res, nil - } - - last, err := getPortMap() +func getPortMap() (map[string]bool, error) { + ports, err := serial.GetPortsList() if err != nil { - return "", err + return nil, errors.WithMessage(err, "listing serial ports") } - - deadline := time.Now().Add(10 * time.Second) - for time.Now().Before(deadline) { - now, err := getPortMap() - if err != nil { - return "", err - } - - for p := range now { - if !last[p] { - return p, nil // Found it! - } - } - - last = now - time.Sleep(250 * time.Millisecond) + res := map[string]bool{} + for _, port := range ports { + res[port] = true } - - return "", nil + return res, nil } -// TouchAndWait FIXMEDOCS -func TouchAndWait(portToTouch string, wait bool) (string, error) { - getPortMap := func() (map[string]bool, error) { - ports, err := serial.GetPortsList() - if err != nil { - return nil, errors.WithMessage(err, "listing serial ports") - } - res := map[string]bool{} - for _, port := range ports { - res[port] = true - } - return res, nil - } +// ResetProgressCallbacks is a struct that defines a bunch of function callback +// to observe the Reset function progress. +type ResetProgressCallbacks struct { + // TouchingPort is called to signal the 1200-bps touch of the reported port + TouchingPort func(port string) + // WaitingForNewSerial is called to signal that we are waiting for a new port + WaitingForNewSerial func() + // BootloaderPortFound is called to signal that the wait is completed and to + // report the port found, or the empty string if no ports have been found and + // the wait has timed-out. + BootloaderPortFound func(port string) + // Debug reports messages useful for debugging purposes. In normal conditions + // these messages should not be displayed to the user. + Debug func(msg string) +} +// Reset a board using the 1200 bps port-touch. If wait is true, it will wait +// for a new port to appear (which could change sometimes) and returns that +// one, otherwise the empty string is returned if the new port can not be +// detected or if the wait parameter is false. +// The error is set if the port listing fails. +func Reset(portToTouch string, wait bool, cb *ResetProgressCallbacks) (string, error) { last, err := getPortMap() - fmt.Println("LAST:", last) + if cb != nil && cb.Debug != nil { + cb.Debug(fmt.Sprintf("LAST: %v", last)) + } if err != nil { return "", err } if portToTouch != "" { - fmt.Println("TOUCH:", portToTouch) - TouchSerialPortAt1200bps(portToTouch) + if cb != nil && cb.Debug != nil { + cb.Debug(fmt.Sprintf("TOUCH: %v", portToTouch)) + } + if cb != nil && cb.TouchingPort != nil { + cb.TouchingPort(portToTouch) + } + if err := TouchSerialPortAt1200bps(portToTouch); err != nil { + fmt.Println("TOUCH: error during reset:", err) + } } if !wait { return "", nil } + if cb != nil && cb.WaitingForNewSerial != nil { + cb.WaitingForNewSerial() + } + deadline := time.Now().Add(10 * time.Second) for time.Now().Before(deadline) { now, err := getPortMap() - fmt.Println("NOW:", now) if err != nil { return "", err } + if cb != nil && cb.Debug != nil { + cb.Debug(fmt.Sprintf("WAIT: %v", now)) + } hasNewPorts := false for p := range now { if !last[p] { @@ -173,7 +130,10 @@ func TouchAndWait(portToTouch string, wait bool) (string, error) { } if hasNewPorts { - fmt.Println("HAS NEW PORTS!") + if cb != nil && cb.Debug != nil { + cb.Debug("New ports found!") + } + // on OS X, if the port is opened too quickly after it is detected, // a "Resource busy" error occurs, add a delay to workaround. // This apply to other platforms as well. @@ -184,21 +144,31 @@ func TouchAndWait(portToTouch string, wait bool) (string, error) { // settling. // This check ensure that the port is stable after one second. check, err := getPortMap() - fmt.Println("CHECK:", check) if err != nil { return "", err } + if cb != nil && cb.Debug != nil { + cb.Debug(fmt.Sprintf("CHECK: %v", check)) + } for p := range check { if !last[p] { - fmt.Println("FOUND:", p) + if cb != nil && cb.BootloaderPortFound != nil { + cb.BootloaderPortFound(p) + } return p, nil // Found it! } } + if cb != nil && cb.Debug != nil { + cb.Debug("Port check failed... still waiting") + } } last = now time.Sleep(250 * time.Millisecond) } + if cb != nil && cb.BootloaderPortFound != nil { + cb.BootloaderPortFound("") + } return "", nil } diff --git a/commands/upload/upload.go b/commands/upload/upload.go index 0b3540e2193..383c66ed823 100644 --- a/commands/upload/upload.go +++ b/commands/upload/upload.go @@ -299,7 +299,37 @@ func runProgramAction(pm *packagemanager.PackageManager, outStream.Write([]byte(fmt.Sprintln("Skipping 1200-bps touch reset: no serial port selected!"))) } - if newPort, err := serialutils.TouchAndWait(port, wait); err != nil { + var cb *serialutils.ResetProgressCallbacks + if verbose { + cb = &serialutils.ResetProgressCallbacks{ + TouchingPort: func(port string) { + logrus.WithField("phase", "board reset").Infof("Performing 1200-bps touch reset on serial port %s", port) + outStream.Write([]byte(fmt.Sprintf("Performing 1200-bps touch reset on serial port %s", port))) + outStream.Write([]byte(fmt.Sprintln())) + }, + WaitingForNewSerial: func() { + logrus.WithField("phase", "board reset").Info("Waiting for upload port...") + outStream.Write([]byte(fmt.Sprintln("Waiting for upload port..."))) + }, + BootloaderPortFound: func(port string) { + if port != "" { + logrus.WithField("phase", "board reset").Infof("Upload port found on %s", port) + outStream.Write([]byte(fmt.Sprintf("Upload port found on %s", port))) + outStream.Write([]byte(fmt.Sprintln())) + } else { + logrus.WithField("phase", "board reset").Infof("No upload port found, using %s as fallback", actualPort) + outStream.Write([]byte(fmt.Sprintf("No upload port found, using %s as fallback", actualPort))) + outStream.Write([]byte(fmt.Sprintln())) + } + }, + Debug: func(msg string) { + logrus.WithField("phase", "board reset").Debug(msg) + }, + } + } + if newPort, err := serialutils.Reset(port, wait, cb); err != nil { + outStream.Write([]byte(fmt.Sprintf("Cannot perform port reset: %s", err))) + outStream.Write([]byte(fmt.Sprintln())) } else { if newPort != "" { actualPort = newPort @@ -308,30 +338,14 @@ func runProgramAction(pm *packagemanager.PackageManager, // ports, err := serial.GetPortsList() // for _, p := range ports { // if p == port { - // if verbose { - // outStream.Write([]byte(fmt.Sprintf("Performing 1200-bps touch reset on serial port %s", p))) - // outStream.Write([]byte(fmt.Sprintln())) - // } // logrus.Infof("Touching port %s at 1200bps", port) // if err := serialutils.TouchSerialPortAt1200bps(p); err != nil { - // outStream.Write([]byte(fmt.Sprintf("Cannot perform port reset: %s", err))) - // outStream.Write([]byte(fmt.Sprintln())) // } // break // } // } } - // Wait for upload port if requested - // if verbose { - // outStream.Write([]byte(fmt.Sprintln("Waiting for upload port..."))) - // } - - // actualPort, err = serialutils.WaitForNewSerialPortOrDefaultTo(actualPort) - // if err != nil { - // return errors.WithMessage(err, "detecting serial port") - // } - if port != "" { // Set serial port property uploadProperties.Set("serial.port", actualPort) From 6ca680d235a377f1d82cde8ea5acc506e3b7ba7c Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 19 Apr 2021 11:30:35 +0200 Subject: [PATCH 3/5] Skip serial port touch if the port is not connected --- arduino/serialutils/serialutils.go | 2 +- commands/upload/upload.go | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/arduino/serialutils/serialutils.go b/arduino/serialutils/serialutils.go index 4a9119db0b0..33e231381bc 100644 --- a/arduino/serialutils/serialutils.go +++ b/arduino/serialutils/serialutils.go @@ -93,7 +93,7 @@ func Reset(portToTouch string, wait bool, cb *ResetProgressCallbacks) (string, e return "", err } - if portToTouch != "" { + if portToTouch != "" && last[portToTouch] { if cb != nil && cb.Debug != nil { cb.Debug(fmt.Sprintf("TOUCH: %v", portToTouch)) } diff --git a/commands/upload/upload.go b/commands/upload/upload.go index 383c66ed823..460aa701b2d 100644 --- a/commands/upload/upload.go +++ b/commands/upload/upload.go @@ -335,15 +335,6 @@ func runProgramAction(pm *packagemanager.PackageManager, actualPort = newPort } } - // ports, err := serial.GetPortsList() - // for _, p := range ports { - // if p == port { - // logrus.Infof("Touching port %s at 1200bps", port) - // if err := serialutils.TouchSerialPortAt1200bps(p); err != nil { - // } - // break - // } - // } } if port != "" { From 239a85afe5b3349db7e23397bd1bc36131aa61af Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 22 Apr 2021 15:59:45 +0200 Subject: [PATCH 4/5] Perform touch only if required --- arduino/serialutils/serialutils.go | 11 +++++++---- commands/upload/upload.go | 11 +++++++++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/arduino/serialutils/serialutils.go b/arduino/serialutils/serialutils.go index 33e231381bc..5e224824189 100644 --- a/arduino/serialutils/serialutils.go +++ b/arduino/serialutils/serialutils.go @@ -79,10 +79,13 @@ type ResetProgressCallbacks struct { Debug func(msg string) } -// Reset a board using the 1200 bps port-touch. If wait is true, it will wait -// for a new port to appear (which could change sometimes) and returns that -// one, otherwise the empty string is returned if the new port can not be -// detected or if the wait parameter is false. +// Reset a board using the 1200 bps port-touch and wait for new ports. +// Both reset and wait are optional: +// - if port is "" touch will be skipped +// - if wait is false waiting will be skipped +// If wait is true, this function will wait for a new port to appear and returns that +// one, otherwise the empty string is returned if the new port can not be detected or +// if the wait parameter is false. // The error is set if the port listing fails. func Reset(portToTouch string, wait bool, cb *ResetProgressCallbacks) (string, error) { last, err := getPortMap() diff --git a/commands/upload/upload.go b/commands/upload/upload.go index 460aa701b2d..06032a34b20 100644 --- a/commands/upload/upload.go +++ b/commands/upload/upload.go @@ -293,9 +293,16 @@ func runProgramAction(pm *packagemanager.PackageManager, actualPort := port if programmer == nil && !burnBootloader { // Perform reset via 1200bps touch if requested and wait for upload port if requested. + touch := uploadProperties.GetBoolean("upload.use_1200bps_touch") wait := uploadProperties.GetBoolean("upload.wait_for_upload_port") - if touch && port == "" { + portToTouch := "" + if touch { + portToTouch = port + } + + // if touch is requested but port is not specified, print a warning + if touch && portToTouch == "" { outStream.Write([]byte(fmt.Sprintln("Skipping 1200-bps touch reset: no serial port selected!"))) } @@ -327,7 +334,7 @@ func runProgramAction(pm *packagemanager.PackageManager, }, } } - if newPort, err := serialutils.Reset(port, wait, cb); err != nil { + if newPort, err := serialutils.Reset(portToTouch, wait, cb); err != nil { outStream.Write([]byte(fmt.Sprintf("Cannot perform port reset: %s", err))) outStream.Write([]byte(fmt.Sprintln())) } else { From 26de324650d045a7e7ed2a4aaafeb2259996b158 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 22 Apr 2021 16:01:05 +0200 Subject: [PATCH 5/5] Slightly change test to set serial port properties It should be an equivalent change, but it's better when reading code. --- commands/upload/upload.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/upload/upload.go b/commands/upload/upload.go index 06032a34b20..545651d4f0d 100644 --- a/commands/upload/upload.go +++ b/commands/upload/upload.go @@ -344,7 +344,7 @@ func runProgramAction(pm *packagemanager.PackageManager, } } - if port != "" { + if actualPort != "" { // Set serial port property uploadProperties.Set("serial.port", actualPort) if strings.HasPrefix(actualPort, "/dev/") {