(mynewt-newtmgr) 05/12: nmxact/xact: Default image upload window size

[ja...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sjanc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-newtmgr.git

commit eb4fb87e9bc21383c95f8c94e16ccbd4cbba48de
Author: Szymon Janc <sz...@codecoup.pl>
AuthorDate: Thu Oct 22 01:00:17 2026 +0200

    nmxact/xact: Default image upload window size
    
    NewImageUploadCmd() and NewImageUpgradeCmd() leave MaxWinSz at zero,
    which ImageUploadCmd.Run() uses as the capacity of the response and
    error channels.  With sessions which deliver the response from within
    TxRxMgmtAsync() (serial, naked BLE), the send on the unbuffered channel
    blocks before any receiver exists and the upload deadlocks on the
    first chunk.  The CLI is not affected as it always sets MaxWinSz, but
    library users relying on the constructors hang.
    
    Treat a MaxWinSz of zero or less as IMAGE_UPLOAD_DEF_MAX_WS, the same
    default the CLI uses.
    
    Assisted-by: Claude:claude-opus-5-5
---
 nmxact/xact/image.go             |  7 +++++++
 nmxact/xact/image_upload_test.go | 26 ++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/nmxact/xact/image.go b/nmxact/xact/image.go
index 7548074..f164ef6 100644
--- a/nmxact/xact/image.go
+++ b/nmxact/xact/image.go
@@ -340,6 +340,13 @@ func (t *ImageUploadIntTracker) HandleError(off int, err error) bool {
 }
 
 func (c *ImageUploadCmd) Run(s sesn.Sesn) (Result, error) {
+	// The window size is also the capacity of the response channels.
+	// Unbuffered channels deadlock with sessions which deliver the
+	// response before TxRxMgmtAsync() returns.
+	if c.MaxWinSz <= 0 {
+		c.MaxWinSz = IMAGE_UPLOAD_DEF_MAX_WS
+	}
+
 	res := newImageUploadResult()
 
 	// Signalled (without blocking) whenever a response or error has been
diff --git a/nmxact/xact/image_upload_test.go b/nmxact/xact/image_upload_test.go
index 39e76fc..9918602 100644
--- a/nmxact/xact/image_upload_test.go
+++ b/nmxact/xact/image_upload_test.go
@@ -598,3 +598,29 @@ func TestImageUpgradeRunDeviceError(t *testing.T) {
 		t.Errorf("status %d", st)
 	}
 }
+
+// Commands created by the constructors must work without setting
+// MaxWinSz explicitly.
+func TestImageUploadRunDefaultWindow(t *testing.T) {
+	data := testImage(3000)
+
+	dev := &fakeImgDev{}
+	s := newFakeSesn(dev.handle)
+	uc := NewImageUploadCmd()
+	uc.Data = data
+	if _, err := runCmd(t, uc, s); err != nil {
+		t.Fatal(err)
+	}
+	checkUploaded(t, dev, data)
+
+	udev := &imgUpgradeDev{}
+	s = newFakeSesn(udev.handle)
+	udev.s = s
+	gc := NewImageUpgradeCmd()
+	gc.Data = data
+	gc.ProgressCb = func(*ImageUploadCmd, *nmp.ImageUploadRsp) {}
+	if _, err := runCmd(t, gc, s); err != nil {
+		t.Fatal(err)
+	}
+	checkUploaded(t, &udev.fakeImgDev, data)
+}