You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2021/06/10 09:07:35 UTC

[GitHub] [mynewt-mcumgr] de-nordic opened a new pull request #123: img_mgmt: Add interpretation of "image" parameter

de-nordic opened a new pull request #123:
URL: https://github.com/apache/mynewt-mcumgr/pull/123


   The commit extends img_mgmt_upload_req with image member that will
   be used to hold image number; it also adds code that picks the
   image number from update frame.
   
   Signed-off-by: Dominik Ermel <do...@nordicsemi.no>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mynewt-mcumgr] de-nordic commented on pull request #123: img_mgmt: Add interpretation of "image" parameter

Posted by GitBox <gi...@apache.org>.
de-nordic commented on pull request #123:
URL: https://github.com/apache/mynewt-mcumgr/pull/123#issuecomment-858453717


   Hello :) @mlaz @nvlsianpu @utzig


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mynewt-mcumgr] de-nordic commented on a change in pull request #123: img_mgmt: Add interpretation of "image" parameter

Posted by GitBox <gi...@apache.org>.
de-nordic commented on a change in pull request #123:
URL: https://github.com/apache/mynewt-mcumgr/pull/123#discussion_r649070524



##########
File path: cmd/img_mgmt/src/img_mgmt.c
##########
@@ -394,42 +394,49 @@ img_mgmt_upload(struct mgmt_ctxt *ctxt)
         .data_len = 0,
         .data_sha_len = 0,
         .upgrade = false,
+        .image = 0,
     };
 
     const struct cbor_attr_t off_attr[] = {
         [0] = {
+            .attribute = "image",
+            .type = CborAttrUnsignedIntegerType,
+            .addr.integer = &req.image,

Review comment:
       I have also changed the comment on the `image` member of the `img_mgmt_upload_req` as the "auto-select" has been system-specific.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mynewt-mcumgr] de-nordic commented on pull request #123: img_mgmt: Add interpretation of "image" parameter

Posted by GitBox <gi...@apache.org>.
de-nordic commented on pull request #123:
URL: https://github.com/apache/mynewt-mcumgr/pull/123#issuecomment-858452954


   Note: image 0 has been marked as auto-select; this is due to compatibility with current state as this is the number that is always being sent, if no override from `-n` parameter to `image update`, and currently the code would just auto-select destination for image.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mynewt-mcumgr] de-nordic commented on a change in pull request #123: img_mgmt: Add interpretation of "image" parameter

Posted by GitBox <gi...@apache.org>.
de-nordic commented on a change in pull request #123:
URL: https://github.com/apache/mynewt-mcumgr/pull/123#discussion_r649061301



##########
File path: cmd/img_mgmt/src/img_mgmt.c
##########
@@ -394,42 +394,49 @@ img_mgmt_upload(struct mgmt_ctxt *ctxt)
         .data_len = 0,
         .data_sha_len = 0,
         .upgrade = false,
+        .image = 0,
     };
 
     const struct cbor_attr_t off_attr[] = {
         [0] = {
+            .attribute = "image",
+            .type = CborAttrUnsignedIntegerType,
+            .addr.integer = &req.image,

Review comment:
       > (...) this line should read `.addr.uinteger = &req.image`, what do you think?
   
   Yes, you are right this should be fixed.
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mynewt-mcumgr] utzig merged pull request #123: img_mgmt: Add interpretation of "image" parameter

Posted by GitBox <gi...@apache.org>.
utzig merged pull request #123:
URL: https://github.com/apache/mynewt-mcumgr/pull/123


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mynewt-mcumgr] utzig commented on a change in pull request #123: img_mgmt: Add interpretation of "image" parameter

Posted by GitBox <gi...@apache.org>.
utzig commented on a change in pull request #123:
URL: https://github.com/apache/mynewt-mcumgr/pull/123#discussion_r649044768



##########
File path: cmd/img_mgmt/src/img_mgmt.c
##########
@@ -394,42 +394,49 @@ img_mgmt_upload(struct mgmt_ctxt *ctxt)
         .data_len = 0,
         .data_sha_len = 0,
         .upgrade = false,
+        .image = 0,
     };
 
     const struct cbor_attr_t off_attr[] = {
         [0] = {
+            .attribute = "image",
+            .type = CborAttrUnsignedIntegerType,
+            .addr.integer = &req.image,

Review comment:
       ```
   Error: repos/apache-mynewt-mcumgr/cmd/img_mgmt/src/img_mgmt.c: In function 'img_mgmt_upload':
   repos/apache-mynewt-mcumgr/cmd/img_mgmt/src/img_mgmt.c:404:29: error: pointer targets in initialization of 'long long int *' from 'long long unsigned int *' differ in signedness [-Werror=pointer-sign]
                .addr.integer = &req.image,
                                ^
   repos/apache-mynewt-mcumgr/cmd/img_mgmt/src/img_mgmt.c:404:29: note: (near initialization for 'off_attr[0].addr.integer')
   ```
   
   When building I get this error, since you chose the type to be `CborAttrUnsignedIntegerType`, this line should read `.addr.uinteger = &req.image`, what do you think?
   
   Otherwise the PR looks good.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org