Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
xiaoxiang781216 merged PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068 -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068#issuecomment-2939372969 @xiaoxiang781216 @acassis Is this waiting on anything from me? -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068#issuecomment-2916062753 @michallenc Thank you for your input - I always appreciate constructive criticism and advice that improve my coding skills :+1: -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2111671109
##
boot/nxboot/nxboot_main.c:
##
@@ -28,28 +28,233 @@
#include
#include
+#include
+#include
#include
#include
+/
+ * Private Types
+ /
+
+typedef struct progress_msgs_s
+{
+ enum progress_msg_e idx; /* Index to the message */
+ const char *msg; /* Corresponsing text message */
+} progress_msgs_t;
+
+/
+ * Public Data
+ /
+
+bool progress_dots_started = false;
Review Comment:
done
##
boot/nxboot/nxboot_main.c:
##
@@ -28,28 +28,233 @@
#include
#include
+#include
+#include
#include
#include
+/
+ * Private Types
+ /
+
+typedef struct progress_msgs_s
+{
+ enum progress_msg_e idx; /* Index to the message */
+ const char *msg; /* Corresponsing text message */
+} progress_msgs_t;
+
+/
+ * Public Data
+ /
+
+bool progress_dots_started = false;
+#ifdef CONFIG_NXBOOT_PRINTF_PROGRESS_PERCENT
+bool progress_percent_started = false;
Review Comment:
done
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2111669740
##
boot/nxboot/nxboot_main.c:
##
@@ -28,28 +28,233 @@
#include
#include
+#include
+#include
#include
#include
+/
+ * Private Types
+ /
+
+typedef struct progress_msgs_s
+{
+ enum progress_msg_e idx; /* Index to the message */
+ const char *msg; /* Corresponsing text message */
+} progress_msgs_t;
+
+/
+ * Public Data
+ /
+
+bool progress_dots_started = false;
+#ifdef CONFIG_NXBOOT_PRINTF_PROGRESS_PERCENT
+bool progress_percent_started = false;
+#endif
+
+/
+ * Private Data
+ /
+
+#ifdef CONFIG_NXBOOT_PRINTF_PROGRESS
+
+# ifdef CONFIG_NXBOOT_PRINTF_PROGRESS_PERCENT
+static const char backtab[] =
+{
+ ASCII_BS, ASCII_BS, ASCII_BS, ASCII_BS, '\0',
+};
+# endif
+
+static const progress_msgs_t progress_msgs[] =
Review Comment:
done :-)
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2106280699
##
boot/nxboot/nxboot_main.c:
##
@@ -28,28 +28,233 @@
#include
#include
+#include
+#include
#include
#include
+/
+ * Private Types
+ /
+
+typedef struct progress_msgs_s
+{
+ enum progress_msg_e idx; /* Index to the message */
+ const char *msg; /* Corresponsing text message */
+} progress_msgs_t;
+
+/
+ * Public Data
+ /
+
+bool progress_dots_started = false;
+#ifdef CONFIG_NXBOOT_PRINTF_PROGRESS_PERCENT
+bool progress_percent_started = false;
+#endif
+
+/
+ * Private Data
+ /
+
+#ifdef CONFIG_NXBOOT_PRINTF_PROGRESS
+
+# ifdef CONFIG_NXBOOT_PRINTF_PROGRESS_PERCENT
+static const char backtab[] =
+{
+ ASCII_BS, ASCII_BS, ASCII_BS, ASCII_BS, '\0',
+};
+# endif
+
+static const progress_msgs_t progress_msgs[] =
Review Comment:
Yes, like that suggestion. Will rework and test.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2106280437
##
boot/nxboot/nxboot_main.c:
##
@@ -28,28 +28,233 @@
#include
#include
+#include
+#include
#include
#include
+/
+ * Private Types
+ /
+
+typedef struct progress_msgs_s
+{
+ enum progress_msg_e idx; /* Index to the message */
+ const char *msg; /* Corresponsing text message */
+} progress_msgs_t;
+
+/
+ * Public Data
+ /
+
+bool progress_dots_started = false;
Review Comment:
Not "can be" but "should be". Good spot. I'll redeclare it, and also
`progress_percent_started`, as static. They should also have a g_ prefix to be
pedantic.
##
boot/nxboot/nxboot_main.c:
##
@@ -28,28 +28,233 @@
#include
#include
+#include
+#include
#include
#include
+/
+ * Private Types
+ /
+
+typedef struct progress_msgs_s
+{
+ enum progress_msg_e idx; /* Index to the message */
+ const char *msg; /* Corresponsing text message */
+} progress_msgs_t;
+
+/
+ * Public Data
+ /
+
+bool progress_dots_started = false;
+#ifdef CONFIG_NXBOOT_PRINTF_PROGRESS_PERCENT
+bool progress_percent_started = false;
Review Comment:
Yep - as above.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
michallenc commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2106272165
##
boot/nxboot/nxboot_main.c:
##
@@ -28,28 +28,233 @@
#include
#include
+#include
+#include
#include
#include
+/
+ * Private Types
+ /
+
+typedef struct progress_msgs_s
+{
+ enum progress_msg_e idx; /* Index to the message */
+ const char *msg; /* Corresponsing text message */
+} progress_msgs_t;
+
+/
+ * Public Data
+ /
+
+bool progress_dots_started = false;
Review Comment:
This can be static?
##
boot/nxboot/nxboot_main.c:
##
@@ -28,28 +28,233 @@
#include
#include
+#include
+#include
#include
#include
+/
+ * Private Types
+ /
+
+typedef struct progress_msgs_s
+{
+ enum progress_msg_e idx; /* Index to the message */
+ const char *msg; /* Corresponsing text message */
+} progress_msgs_t;
+
+/
+ * Public Data
+ /
+
+bool progress_dots_started = false;
+#ifdef CONFIG_NXBOOT_PRINTF_PROGRESS_PERCENT
+bool progress_percent_started = false;
+#endif
+
+/
+ * Private Data
+ /
+
+#ifdef CONFIG_NXBOOT_PRINTF_PROGRESS
+
+# ifdef CONFIG_NXBOOT_PRINTF_PROGRESS_PERCENT
+static const char backtab[] =
+{
+ ASCII_BS, ASCII_BS, ASCII_BS, ASCII_BS, '\0',
+};
+# endif
+
+static const progress_msgs_t progress_msgs[] =
Review Comment:
I would do
```
static const char *progress_msgs[] = {
[startup_msg] = "*** nxboot ***",
[power_reset] = "Power Reset detected, check images only",
...
}
```
This way it will basically be a lookup table and we can drop `get_msg` call
and access it directly (and `struct progress_msgs_s` should not be needed
either).
##
boot/nxboot/nxboot_main.c:
##
@@ -28,28 +28,233 @@
#include
#include
+#include
+#include
#include
#include
+/
+ * Private Types
+ /
+
+typedef struct progress_msgs_s
+{
+ enum progress_msg_e idx; /* Index to the message */
+ const char *msg; /* Corresponsing text message */
+} progress_msgs_t;
+
+/
+ * Public Data
+ /
+
+bool progress_dots_started = false;
+#ifdef CONFIG_NXBOOT_PRINTF_PROGRESS_PERCENT
+bool progress_percent_started = false;
Review Comment:
ditto
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068#issuecomment-2907971136 > @Cynerd @michallenc I have pushed a new commit with the suggested changes - and an enhancement to show percentage progress not just dots > > I have deliberately not squashed the commits so I can back the changes out if they're not what was wanted! > > Would appreciate your feedback and I will amend/fix, then squash/commit and change back to ready for review. Now squashed and await any further reviews! -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2106264435
##
boot/nxboot/include/nxboot.h:
##
@@ -131,10 +132,41 @@ struct nxboot_state
enum nxboot_update_type next_boot; /* nxboot_update_type with next
operation */
};
+enum progress_type_e
+{
+ nxboot_info = 0, /* Prefixes arg. string with "INFO:" */
+ nxboot_error, /* Prefixes arg. string with "ERR:" */
+ nxboot_progress_start, /* Prints arg. string with no newline to allow .
sequence to follow */
+ nxboot_progress_dot, /* Prints of a "." to the . progress sequence */
+ nxboot_progress_end, /* Flags end of a "..." progrees sequence and prints
newline */
+};
+
+enum progress_msg_e
+{
+ startup_msg = 0,
+ power_reset,
+ soft_reset,
+ found_bootable_image,
+ no_bootable_image,
+ boardioc_image_boot_fail,
+ ramcopy_started,
+ recovery_revert,
+ recovery_create,
+ update_from_update,
+ validate_primary,
+ validate_recovery,
+ validate_update,
+ recovery_created,
+ recovery_invalid,
+ update_failed,
+};
+
/
* Public Function Prototypes
/
+void nxboot_progress(enum progress_type_e type, ...);
Review Comment:
I have added documentation AND progress as a %.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2106263246
##
boot/nxboot/nxboot_main.c:
##
@@ -32,24 +32,146 @@
#include
#include
+/
+ * Public Data
+ /
+
+bool progress_dots_started = false;
+
+/
+ * Private Data
+ /
+
+#ifdef CONFIG_NXBOOT_PRINTF_PROGRESS
+static const char *g_progress_txt[] =
+{
+ /* Text that will be printed ...from...enum progress_msg_e */
+
+ "*** nxboot ***", /* startup_msg */
+ "Power Reset detected, check images only", /* power_reset */
+ "Soft reset detected, check for update", /* soft_reset */
+ "Found bootable image, boot from primary", /* found_bootable_image */
+ "No bootable image found", /* no_bootable_image */
+ "Board failed to boot bootable image", /* boardioc_image_boot_fail */
+ "Copying bootable image to RAM", /* ramcopy_started */
+ "Reverting image to recovery", /* recovery_revert */
+ "Creating recovery image", /* recovery_create */
+ "Updating from update image", /* update_from_update */
+ "Validating primary image",/* validate_primary */
+ "Validating recovery image", /* validate_recovery */
+ "Validating update image", /* validate_update */
+ "Recovery image created", /* recovery_created */
+ "Recovery image invalid, update stopped", /* recovery_invalid */
+ "Update failed", /* update_failed */
Review Comment:
reworked
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068#issuecomment-2858273438 Found a couple message enum inconsistencies so I'm sorting those out and put back to draft while I do so -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068#issuecomment-2856004589 Keeps failing CI...but can't see if it's something I've done/not done? -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2075523110 ## boot/nxboot/Kconfig: ## @@ -64,6 +64,17 @@ config NXBOOT_BOOTLOADER if NXBOOT_BOOTLOADER +config NXBOOT_COPY_TO_RAM + bool "Copy bootable image to RAM before calling board boot-image function" + default n + +config NXBOOT_RAMSTART + hex "Start address in RAM that the application is to be loaded" + default 0x0 + depends on NXBOOT_COPY_TO_RAM + ---help--- + This will be board specific. Review Comment: > did you forget to force-push? Ha - was ticking them off here...perhaps got ahead of myself marking them resolved (which perhaps I should have left to the reviewers?). Just doing the squash and force push. Watch this space :-) -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
jerpelea commented on code in PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2075517506 ## boot/nxboot/Kconfig: ## @@ -64,6 +64,17 @@ config NXBOOT_BOOTLOADER if NXBOOT_BOOTLOADER +config NXBOOT_COPY_TO_RAM + bool "Copy bootable image to RAM before calling board boot-image function" + default n + +config NXBOOT_RAMSTART + hex "Start address in RAM that the application is to be loaded" + default 0x0 + depends on NXBOOT_COPY_TO_RAM + ---help--- + This will be board specific. Review Comment: did you forget to force-push? -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2075513462 ## boot/nxboot/Kconfig: ## @@ -64,6 +64,17 @@ config NXBOOT_BOOTLOADER if NXBOOT_BOOTLOADER +config NXBOOT_COPY_TO_RAM + bool "Copy bootable image to RAM before calling board boot-image function" + default n Review Comment: Added -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2075512816 ## boot/nxboot/Kconfig: ## @@ -92,6 +103,10 @@ config NXBOOT_PREVENT_DOWNGRADE WARNING: NXboot currently implements preferences only for MAJOR.MINOR.PATCH and ignores prerelease. +config NXBOOT_PRINTF_PROGRESS + bool "Enable progress messages to be sent to STDOUT" + default n Review Comment: Added ## boot/nxboot/Kconfig: ## @@ -64,6 +64,17 @@ config NXBOOT_BOOTLOADER if NXBOOT_BOOTLOADER +config NXBOOT_COPY_TO_RAM + bool "Copy bootable image to RAM before calling board boot-image function" + default n + +config NXBOOT_RAMSTART + hex "Start address in RAM that the application is to be loaded" + default 0x0 + depends on NXBOOT_COPY_TO_RAM + ---help--- + This will be board specific. Review Comment: Added -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
michallenc commented on code in PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2075502357 ## boot/nxboot/Kconfig: ## @@ -92,6 +103,10 @@ config NXBOOT_PREVENT_DOWNGRADE WARNING: NXboot currently implements preferences only for MAJOR.MINOR.PATCH and ignores prerelease. +config NXBOOT_PRINTF_PROGRESS + bool "Enable progress messages to be sent to STDOUT" + default n Review Comment: I don't have a strong opinion on this. I personally prefer less verbose output, but it may be useful to users not familiar with the bootloader. So maybe we could enable it by default. I would definitely add `---help---` with the description how much size does it take. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2075464067 ## boot/nxboot/Kconfig: ## @@ -64,6 +64,17 @@ config NXBOOT_BOOTLOADER if NXBOOT_BOOTLOADER +config NXBOOT_COPY_TO_RAM + bool "Copy bootable image to RAM before calling board boot-image function" + default n + +config NXBOOT_RAMSTART + hex "Start address in RAM that the application is to be loaded" + default 0x0 + depends on NXBOOT_COPY_TO_RAM + ---help--- + This will be board specific. Review Comment: I'll do my best :-) ## boot/nxboot/Kconfig: ## @@ -64,6 +64,17 @@ config NXBOOT_BOOTLOADER if NXBOOT_BOOTLOADER +config NXBOOT_COPY_TO_RAM + bool "Copy bootable image to RAM before calling board boot-image function" + default n Review Comment: Sure! -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2075463497 ## boot/nxboot/Kconfig: ## @@ -92,6 +103,10 @@ config NXBOOT_PREVENT_DOWNGRADE WARNING: NXboot currently implements preferences only for MAJOR.MINOR.PATCH and ignores prerelease. +config NXBOOT_PRINTF_PROGRESS + bool "Enable progress messages to be sent to STDOUT" + default n Review Comment: > I think a progress percentage (or even a progress bar), is something expected to exist by default (y), maybe the option should be here to disable it. I was trying to make sure the behaviour remained the same for existing builds with no additional KiB used unless you want and choose this option? -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
acassis commented on code in PR #3068: URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2075411412 ## boot/nxboot/Kconfig: ## @@ -92,6 +103,10 @@ config NXBOOT_PREVENT_DOWNGRADE WARNING: NXboot currently implements preferences only for MAJOR.MINOR.PATCH and ignores prerelease. +config NXBOOT_PRINTF_PROGRESS + bool "Enable progress messages to be sent to STDOUT" + default n Review Comment: I think a progress percentage (or even a progress bar), is something expected to exist by default (y), maybe the option should be here to disable it. ## boot/nxboot/Kconfig: ## @@ -64,6 +64,17 @@ config NXBOOT_BOOTLOADER if NXBOOT_BOOTLOADER +config NXBOOT_COPY_TO_RAM + bool "Copy bootable image to RAM before calling board boot-image function" + default n + +config NXBOOT_RAMSTART + hex "Start address in RAM that the application is to be loaded" + default 0x0 + depends on NXBOOT_COPY_TO_RAM + ---help--- + This will be board specific. Review Comment: Please improve this help, i.e. is there some recommend position in the heap? Maybe including suggestions how to reserve the space in the linker script to avoid other applications to use, etc ## boot/nxboot/Kconfig: ## @@ -64,6 +64,17 @@ config NXBOOT_BOOTLOADER if NXBOOT_BOOTLOADER +config NXBOOT_COPY_TO_RAM + bool "Copy bootable image to RAM before calling board boot-image function" + default n Review Comment: Please include a ---help--- explaining when and why to use it, etc -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2073755822
##
boot/nxboot/loader/boot.c:
##
@@ -403,6 +440,42 @@ static int perform_update(struct nxboot_state *state, bool
check_only)
return OK;
}
+#ifdef CONFIG_NXBOOT_COPY_TO_RAM
+int nxboot_ramcopy(void)
+{
+ int primary;
+ struct nxboot_img_header header;
+ ssize_t bytes;
+ static uint8_t *buf;
+ size_t size;
+
+ primary = flash_partition_open(CONFIG_NXBOOT_PRIMARY_SLOT_PATH);
+ if (primary < 0)
+{
+ return ERROR;
+}
+
+ get_image_header(primary, &header);
+ buf = malloc(header.size);
+ if (!buf)
+{
+ return ERROR;
+}
+
+ size = header.size - CONFIG_NXBOOT_HEADER_SIZE;
Review Comment:
done
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2073755151
##
boot/nxboot/loader/boot.c:
##
@@ -403,6 +440,42 @@ static int perform_update(struct nxboot_state *state, bool
check_only)
return OK;
}
+#ifdef CONFIG_NXBOOT_COPY_TO_RAM
+int nxboot_ramcopy(void)
+{
+ int primary;
+ struct nxboot_img_header header;
+ ssize_t bytes;
+ static uint8_t *buf;
+ size_t size;
+
+ primary = flash_partition_open(CONFIG_NXBOOT_PRIMARY_SLOT_PATH);
+ if (primary < 0)
+{
+ return ERROR;
+}
+
+ get_image_header(primary, &header);
+ buf = malloc(header.size);
+ if (!buf)
+{
+ return ERROR;
Review Comment:
done
##
boot/nxboot/loader/boot.c:
##
@@ -403,6 +440,42 @@ static int perform_update(struct nxboot_state *state, bool
check_only)
return OK;
}
+#ifdef CONFIG_NXBOOT_COPY_TO_RAM
+int nxboot_ramcopy(void)
+{
+ int primary;
+ struct nxboot_img_header header;
+ ssize_t bytes;
+ static uint8_t *buf;
+ size_t size;
+
+ primary = flash_partition_open(CONFIG_NXBOOT_PRIMARY_SLOT_PATH);
+ if (primary < 0)
+{
+ return ERROR;
+}
+
+ get_image_header(primary, &header);
+ buf = malloc(header.size);
+ if (!buf)
+{
+ return ERROR;
+}
+
+ size = header.size - CONFIG_NXBOOT_HEADER_SIZE;
+ bytes = pread(primary, buf, size, CONFIG_NXBOOT_HEADER_SIZE);
+ if (bytes != size)
+{
+ return ERROR;
+}
+
+ memcpy((uint32_t *)CONFIG_NXBOOT_RAMSTART, buf, size);
+ flash_partition_close(primary);
+
Review Comment:
done
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2073276383
##
boot/nxboot/loader/boot.c:
##
@@ -403,6 +440,42 @@ static int perform_update(struct nxboot_state *state, bool
check_only)
return OK;
}
+#ifdef CONFIG_NXBOOT_COPY_TO_RAM
+int nxboot_ramcopy(void)
+{
+ int primary;
+ struct nxboot_img_header header;
+ ssize_t bytes;
+ static uint8_t *buf;
+ size_t size;
+
+ primary = flash_partition_open(CONFIG_NXBOOT_PRIMARY_SLOT_PATH);
+ if (primary < 0)
+{
+ return ERROR;
+}
+
+ get_image_header(primary, &header);
+ buf = malloc(header.size);
+ if (!buf)
+{
+ return ERROR;
+}
+
+ size = header.size - CONFIG_NXBOOT_HEADER_SIZE;
Review Comment:
> I will have to see why my loaded app ran OK missing its last 512 bytes!
It's just debug data.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2073149974
##
boot/nxboot/loader/boot.c:
##
@@ -403,6 +440,42 @@ static int perform_update(struct nxboot_state *state, bool
check_only)
return OK;
}
+#ifdef CONFIG_NXBOOT_COPY_TO_RAM
+int nxboot_ramcopy(void)
+{
+ int primary;
+ struct nxboot_img_header header;
+ ssize_t bytes;
+ static uint8_t *buf;
+ size_t size;
+
+ primary = flash_partition_open(CONFIG_NXBOOT_PRIMARY_SLOT_PATH);
+ if (primary < 0)
+{
+ return ERROR;
+}
+
+ get_image_header(primary, &header);
+ buf = malloc(header.size);
+ if (!buf)
+{
+ return ERROR;
Review Comment:
Agreed
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2073149974
##
boot/nxboot/loader/boot.c:
##
@@ -403,6 +440,42 @@ static int perform_update(struct nxboot_state *state, bool
check_only)
return OK;
}
+#ifdef CONFIG_NXBOOT_COPY_TO_RAM
+int nxboot_ramcopy(void)
+{
+ int primary;
+ struct nxboot_img_header header;
+ ssize_t bytes;
+ static uint8_t *buf;
+ size_t size;
+
+ primary = flash_partition_open(CONFIG_NXBOOT_PRIMARY_SLOT_PATH);
+ if (primary < 0)
+{
+ return ERROR;
+}
+
+ get_image_header(primary, &header);
+ buf = malloc(header.size);
+ if (!buf)
+{
+ return ERROR;
Review Comment:
As above, but agreed and will do it for thoroughness and to cover all use
cases :-)
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2073148927
##
boot/nxboot/loader/boot.c:
##
@@ -403,6 +440,42 @@ static int perform_update(struct nxboot_state *state, bool
check_only)
return OK;
}
+#ifdef CONFIG_NXBOOT_COPY_TO_RAM
+int nxboot_ramcopy(void)
+{
+ int primary;
+ struct nxboot_img_header header;
+ ssize_t bytes;
+ static uint8_t *buf;
+ size_t size;
+
+ primary = flash_partition_open(CONFIG_NXBOOT_PRIMARY_SLOT_PATH);
+ if (primary < 0)
+{
+ return ERROR;
+}
+
+ get_image_header(primary, &header);
+ buf = malloc(header.size);
+ if (!buf)
+{
+ return ERROR;
+}
+
+ size = header.size - CONFIG_NXBOOT_HEADER_SIZE;
+ bytes = pread(primary, buf, size, CONFIG_NXBOOT_HEADER_SIZE);
+ if (bytes != size)
+{
+ return ERROR;
+}
+
+ memcpy((uint32_t *)CONFIG_NXBOOT_RAMSTART, buf, size);
+ flash_partition_close(primary);
+
Review Comment:
@michallenc Would be the thorough approach for sure. In will be "zapped"
when the loaded image is run but, to be fair, that is probably only in my case
where it loads a complete binary included full initialization etc. I will add
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
TimJTi commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2073146547
##
boot/nxboot/loader/boot.c:
##
@@ -403,6 +440,42 @@ static int perform_update(struct nxboot_state *state, bool
check_only)
return OK;
}
+#ifdef CONFIG_NXBOOT_COPY_TO_RAM
+int nxboot_ramcopy(void)
+{
+ int primary;
+ struct nxboot_img_header header;
+ ssize_t bytes;
+ static uint8_t *buf;
+ size_t size;
+
+ primary = flash_partition_open(CONFIG_NXBOOT_PRIMARY_SLOT_PATH);
+ if (primary < 0)
+{
+ return ERROR;
+}
+
+ get_image_header(primary, &header);
+ buf = malloc(header.size);
+ if (!buf)
+{
+ return ERROR;
+}
+
+ size = header.size - CONFIG_NXBOOT_HEADER_SIZE;
Review Comment:
@michallenc - good catch, thanks. I will have to see why my loaded app ran
OK missing its last 512 bytes!
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] boot/nxboot: Enhancements to add progress messages and copy-to-RAM [nuttx-apps]
michallenc commented on code in PR #3068:
URL: https://github.com/apache/nuttx-apps/pull/3068#discussion_r2072977903
##
boot/nxboot/loader/boot.c:
##
@@ -403,6 +440,42 @@ static int perform_update(struct nxboot_state *state, bool
check_only)
return OK;
}
+#ifdef CONFIG_NXBOOT_COPY_TO_RAM
+int nxboot_ramcopy(void)
+{
+ int primary;
+ struct nxboot_img_header header;
+ ssize_t bytes;
+ static uint8_t *buf;
+ size_t size;
+
+ primary = flash_partition_open(CONFIG_NXBOOT_PRIMARY_SLOT_PATH);
+ if (primary < 0)
+{
+ return ERROR;
+}
+
+ get_image_header(primary, &header);
+ buf = malloc(header.size);
+ if (!buf)
+{
+ return ERROR;
+}
+
+ size = header.size - CONFIG_NXBOOT_HEADER_SIZE;
Review Comment:
`header.size` is already the size of the image excluding the header,
therefore subtracting `CONFIG_NXBOOT_HEADER_SIZE` shouldn't be needed.
##
boot/nxboot/loader/boot.c:
##
@@ -403,6 +440,42 @@ static int perform_update(struct nxboot_state *state, bool
check_only)
return OK;
}
+#ifdef CONFIG_NXBOOT_COPY_TO_RAM
+int nxboot_ramcopy(void)
+{
+ int primary;
+ struct nxboot_img_header header;
+ ssize_t bytes;
+ static uint8_t *buf;
+ size_t size;
+
+ primary = flash_partition_open(CONFIG_NXBOOT_PRIMARY_SLOT_PATH);
+ if (primary < 0)
+{
+ return ERROR;
+}
+
+ get_image_header(primary, &header);
+ buf = malloc(header.size);
+ if (!buf)
+{
+ return ERROR;
Review Comment:
Close primary on error. Same after `pread`.
##
boot/nxboot/loader/boot.c:
##
@@ -403,6 +440,42 @@ static int perform_update(struct nxboot_state *state, bool
check_only)
return OK;
}
+#ifdef CONFIG_NXBOOT_COPY_TO_RAM
+int nxboot_ramcopy(void)
+{
+ int primary;
+ struct nxboot_img_header header;
+ ssize_t bytes;
+ static uint8_t *buf;
+ size_t size;
+
+ primary = flash_partition_open(CONFIG_NXBOOT_PRIMARY_SLOT_PATH);
+ if (primary < 0)
+{
+ return ERROR;
+}
+
+ get_image_header(primary, &header);
+ buf = malloc(header.size);
+ if (!buf)
+{
+ return ERROR;
+}
+
+ size = header.size - CONFIG_NXBOOT_HEADER_SIZE;
+ bytes = pread(primary, buf, size, CONFIG_NXBOOT_HEADER_SIZE);
+ if (bytes != size)
+{
+ return ERROR;
+}
+
+ memcpy((uint32_t *)CONFIG_NXBOOT_RAMSTART, buf, size);
+ flash_partition_close(primary);
+
Review Comment:
`free(buf)`
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
