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 2020/03/17 06:52:35 UTC
[GitHub] [mynewt-mcumgr] de-nordic opened a new pull request #72: fs_mgmt:
Fix file download for Zephyr
de-nordic opened a new pull request #72: fs_mgmt: Fix file download for Zephyr
URL: https://github.com/apache/mynewt-mcumgr/pull/72
File download worked only for files small enough to fit into small mcumgr
buffer, dedicated for CBOR encoding. Because file read buffer size is
independent from the size of CBOR encoding buffer and part of CBOR buffer
is used outside of control of download backend, it could happen that the
chunk read could not fit into actual free space left within CBOR buffer,
automatically failing the download.
Signed-off-by: Dominik Ermel <do...@nordicsemi.no>
This is alternative, less code invasive, solution to https://github.com/apache/mynewt-mcumgr/pull/65 which does compile time, instead of run time, compilation.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [mynewt-mcumgr] sjanc merged pull request #72: fs_mgmt: Fix file
download for Zephyr
Posted by GitBox <gi...@apache.org>.
sjanc merged pull request #72: fs_mgmt: Fix file download for Zephyr
URL: https://github.com/apache/mynewt-mcumgr/pull/72
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [mynewt-mcumgr] de-nordic commented on issue #72: fs_mgmt: Fix file
download for Zephyr
Posted by GitBox <gi...@apache.org>.
de-nordic commented on issue #72: fs_mgmt: Fix file download for Zephyr
URL: https://github.com/apache/mynewt-mcumgr/pull/72#issuecomment-600045211
@ccollins476ad , @carlescufi , @nvlsianpu please review.
The zephyr changes in Kconfig are reviewed here: https://github.com/zephyrproject-rtos/zephyr/pull/23515
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [mynewt-mcumgr] de-nordic commented on issue #72: fs_mgmt: Fix file
download for Zephyr
Posted by GitBox <gi...@apache.org>.
de-nordic commented on issue #72: fs_mgmt: Fix file download for Zephyr
URL: https://github.com/apache/mynewt-mcumgr/pull/72#issuecomment-600455674
>
>
> Looks good to me.
>
> I assume `CONFIG_FS_MGMT_MAX_OFFSET_LEN` is defined in a zephyr repo?
It is implicitly defined by Kconfig menu, basing on selected maximal file size that will be downloadable.
>
> Also, just a nit pick - I noticed a few typos in the large comment at the top: MCUGMR_BUF_SZIE, cbod.
Oops. Will fix it.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [mynewt-mcumgr] nvlsianpu commented on a change in pull request
#72: fs_mgmt: Fix file download for Zephyr
Posted by GitBox <gi...@apache.org>.
nvlsianpu commented on a change in pull request #72: fs_mgmt: Fix file download for Zephyr
URL: https://github.com/apache/mynewt-mcumgr/pull/72#discussion_r394293364
##########
File path: cmd/fs_mgmt/include/fs_mgmt/fs_mgmt_config.h
##########
@@ -30,7 +30,44 @@
#elif defined __ZEPHYR__
-#define FS_MGMT_DL_CHUNK_SIZE CONFIG_FS_MGMT_DL_CHUNK_SIZE
+#define MCUMGR_BUF_SIZE CONFIG_MCUMGR_BUF_SIZE
+/* File chunk needs to fit into MCUMGR_BUF_SIZE with all required headers
+ * and other data fields; following information takes space off the
+ * MCUMGR_BUF_SIZE, N is CONFIG_FS_MGMT_MAX_OFFSET_LEN
+ * MGMT_HDR_SIZE - header that is placed in front of buffer and not
+ * visible for cbod encoder (see smp_handle_single_req);
+ * 9 + 1 -- bytes taken by definition of CBOR undefined length map and map
+ * terminator (break) character;
+ * 1 + strlen("off") + [1, N] -- CBOR encoded pair of "off" marker and
+ * offset of the chunk within the file;
+ * 1 + strlen("data") + [1, N] -- CBOR encoded "data" marker; this marker
+ * will be followed by file chunk of size FS_MGMT_DL_CHUNK_SIZE
+ * 1 + strlen("rc") + 1 -- status code of operation;
+ * 1 + strlen("len") + [1, N] -- CBOR encoded "len" marker and complete
+ * length of a file; this is only sent once when "off" is 0;
+ *
+ * FS_MGMT_DL_CHUNK_SIZE is calculated with most pessimistic estimations,
+ * that is with headers fields taking most space, i.e. N bytes.
+ */
+#define CBOR_AND_OTHER_HDR \
Review comment:
+1 for comprehensive description.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [mynewt-mcumgr] carlescufi commented on issue #72: fs_mgmt: Fix
file download for Zephyr
Posted by GitBox <gi...@apache.org>.
carlescufi commented on issue #72: fs_mgmt: Fix file download for Zephyr
URL: https://github.com/apache/mynewt-mcumgr/pull/72#issuecomment-602717705
@ccollins476ad, @mlaz or @vrahane could you merge this please?
----------------------------------------------------------------
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
With regards,
Apache Git Services