nmxact: Don't skip seq numbers in `image upload`
nmxact generates several image upload requests for each one that it
actually sends. It does this to determine the ideal chunk size.
Each of these requests was consuming a sequence number. As a result,
the sequence number would increase by three for each request that was
actually transmitted.
This commit changes the image upload command such that the sequence
number is only increased when a request is actually sent.
diff --git a/nmxact/nmp/image.go b/nmxact/nmp/image.go
index efc32d8..10b70b7 100644
--- a/nmxact/nmp/image.go
+++ b/nmxact/nmp/image.go
@@ -45,6 +45,12 @@
return r
}
+func NewImageUploadReqWithSeq(seq uint8) *ImageUploadReq {
+ r := &ImageUploadReq{}
+ fillNmpReqWithSeq(r, NMP_OP_WRITE, NMP_GROUP_IMAGE, NMP_ID_IMAGE_UPLOAD, seq)
+ return r
+}
+
func (r *ImageUploadReq) Msg() *NmpMsg { return MsgFromReq(r) }
func NewImageUploadRsp() *ImageUploadRsp {
diff --git a/nmxact/nmp/nmp.go b/nmxact/nmp/nmp.go
index 2953949..acf7a40 100644
--- a/nmxact/nmp/nmp.go
+++ b/nmxact/nmp/nmp.go
@@ -150,15 +150,19 @@
return data, nil
}
-func fillNmpReq(req NmpReq, op uint8, group uint16, id uint8) {
+func fillNmpReqWithSeq(req NmpReq, op uint8, group uint16, id uint8, seq uint8) {
hdr := NmpHdr{
Op: op,
Flags: 0,
Len: 0,
Group: group,
- Seq: nmxutil.NextNmpSeq(),
+ Seq: seq,
Id: id,
}
req.SetHdr(&hdr)
}
+
+func fillNmpReq(req NmpReq, op uint8, group uint16, id uint8) {
+ fillNmpReqWithSeq(req, op, group, id, nmxutil.NextNmpSeq())
+}
diff --git a/nmxact/xact/image.go b/nmxact/xact/image.go
index 07330ae..8ad616f 100644
--- a/nmxact/xact/image.go
+++ b/nmxact/xact/image.go
@@ -27,6 +27,7 @@
"mynewt.apache.org/newtmgr/nmxact/mgmt"
"mynewt.apache.org/newtmgr/nmxact/nmp"
+ "mynewt.apache.org/newtmgr/nmxact/nmxutil"
"mynewt.apache.org/newtmgr/nmxact/sesn"
)
@@ -69,9 +70,9 @@
}
func buildImageUploadReq(imageSz int, hash []byte, upgrade bool, chunk []byte,
- off int, imageNum int) *nmp.ImageUploadReq {
+ off int, imageNum int, seq uint8) *nmp.ImageUploadReq {
- r := nmp.NewImageUploadReq()
+ r := nmp.NewImageUploadReqWithSeq(seq)
if off == 0 {
r.Len = uint32(imageSz)
@@ -93,26 +94,20 @@
}
func encodeUploadReq(s sesn.Sesn, hash []byte, upgrade bool, data []byte,
- off int, chunklen int, imageNum int) ([]byte, error) {
+ off int, chunklen int, imageNum int, seq uint8) ([]byte, error) {
r := buildImageUploadReq(len(data), hash, upgrade, data[off:off+chunklen],
- off, imageNum)
+ off, imageNum, seq)
enc, err := mgmt.EncodeMgmt(s, r.Msg())
if err != nil {
return nil, err
}
- // If encoded length is larger than MTU, we need to make chunk shorter
- if len(enc) > s.MtuOut() {
- overflow := len(enc) - s.MtuOut()
- chunklen -= overflow
- }
-
return enc, nil
}
func findChunkLen(s sesn.Sesn, hash []byte, upgrade bool, data []byte,
- off int, imageNum int) (int, error) {
+ off int, imageNum int, seq uint8) (int, error) {
// Let's start by encoding max allowed chunk len and we will see how many
// bytes we need to cut
@@ -120,7 +115,7 @@
// Keep reducing the chunk size until the request fits the MTU.
for {
- enc, err := encodeUploadReq(s, hash, upgrade, data, off, chunklen, imageNum)
+ enc, err := encodeUploadReq(s, hash, upgrade, data, off, chunklen, imageNum, seq)
if err != nil {
return 0, err
}
@@ -147,8 +142,10 @@
hash = sha[:]
}
+ seq := nmxutil.NextNmpSeq()
+
// Find chunk length
- chunklen, err := findChunkLen(s, hash, upgrade, data, off, imageNum)
+ chunklen, err := findChunkLen(s, hash, upgrade, data, off, imageNum, seq)
if err != nil {
return nil, err
}
@@ -157,7 +154,7 @@
// fit we'll recalculate without hash
if off == 0 && chunklen < IMAGE_UPLOAD_MIN_1ST_CHUNK {
hash = nil
- chunklen, err = findChunkLen(s, hash, upgrade, data, off, imageNum)
+ chunklen, err = findChunkLen(s, hash, upgrade, data, off, imageNum, seq)
if err != nil {
return nil, err
}
@@ -172,7 +169,7 @@
}
r := buildImageUploadReq(len(data), hash, upgrade,
- data[off:off+chunklen], off, imageNum)
+ data[off:off+chunklen], off, imageNum, seq)
// Request above should encode just fine since we calculate proper chunk
// length but (at least for now) let's double check it
