You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@mynewt.apache.org by "wiktorkwiatkowski (via GitHub)" <gi...@apache.org> on 2025/10/07 10:02:56 UTC

[PR] newt/builder: read BOOTUTIL_MAX_IMG_SECTORS from syscfg for boot trailer [mynewt-newt]

wiktorkwiatkowski opened a new pull request, #605:
URL: https://github.com/apache/mynewt-newt/pull/605

   Boot trailer size was hardcoded to 128 sectors, ignoring the value set in syscfg. This caused mismatches between trailer size and the actual image size when `BOOTUTIL_MAX_IMG_SECTORS` was set to value other than 128, potentially making images not fit in the slot. Now the trailer size is calculated based on the syscfg value.


-- 
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: notifications-unsubscribe@mynewt.apache.org

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


Re: [PR] newt/builder: read BOOTUTIL_MAX_IMG_SECTORS from syscfg for boot trailer [mynewt-newt]

Posted by "m-gorecki (via GitHub)" <gi...@apache.org>.
m-gorecki merged PR #605:
URL: https://github.com/apache/mynewt-newt/pull/605


-- 
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: notifications-unsubscribe@mynewt.apache.org

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


Re: [PR] newt/builder: read BOOTUTIL_MAX_IMG_SECTORS from syscfg for boot trailer [mynewt-newt]

Posted by "m-gorecki (via GitHub)" <gi...@apache.org>.
m-gorecki commented on code in PR #605:
URL: https://github.com/apache/mynewt-newt/pull/605#discussion_r2410384375


##########
newt/builder/targetbuild.go:
##########
@@ -797,6 +798,26 @@ func (t *TargetBuilder) bootTrailerSize() int {
 		}
 	}
 
+	sectors, ok := t.res.Cfg.Settings["BOOTUTIL_MAX_IMG_SECTORS"]
+	if !ok {
+		util.StatusMessage(util.VERBOSITY_DEFAULT,
+			"* Warning: target does not define BOOTUTIL_MAX_IMG_SECTORS "+
+				"setting; assuming a value of 128.\n")
+		maxImgSectors = 128
+	} else {
+		val, err := util.AtoiNoOct(sectors.Value)
+		if err != nil {
+			util.StatusMessage(util.VERBOSITY_DEFAULT,
+				"* Warning: target specifies invalid non-integer "+
+					"BOOTUTIL_MAX_IMG_SECTORS setting; assuming a "+
+					"value of 128.\n")
+			maxImgSectors = 128
+		} else {
+			maxImgSectors = val
+		}
+	}

Review Comment:
   This does the same as previous block of code for MCU_FLASH_MIN_WRITE_SIZE. You could add private helper function which will validate the setting and return it's value (or default value if proper setting is not present). Helper would be used like this:
   ```
   	minWriteSz := t.getSettingIntValue(t.res.Cfg.Settings, "MCU_FLASH_MIN_WRITE_SIZE", 1)
   	maxImgSectors := t.getIntSettingIntValue(t.res.Cfg.Settings, "BOOTUTIL_MAX_IMG_SECTORS", 128)
   ```
   where 1 and 128 are configurable default values returned in case of wrong/lack of syscfg.



-- 
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: notifications-unsubscribe@mynewt.apache.org

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


Re: [PR] newt/builder: read BOOTUTIL_MAX_IMG_SECTORS from syscfg for boot trailer [mynewt-newt]

Posted by "m-gorecki (via GitHub)" <gi...@apache.org>.
m-gorecki commented on code in PR #605:
URL: https://github.com/apache/mynewt-newt/pull/605#discussion_r2410384375


##########
newt/builder/targetbuild.go:
##########
@@ -797,6 +798,26 @@ func (t *TargetBuilder) bootTrailerSize() int {
 		}
 	}
 
+	sectors, ok := t.res.Cfg.Settings["BOOTUTIL_MAX_IMG_SECTORS"]
+	if !ok {
+		util.StatusMessage(util.VERBOSITY_DEFAULT,
+			"* Warning: target does not define BOOTUTIL_MAX_IMG_SECTORS "+
+				"setting; assuming a value of 128.\n")
+		maxImgSectors = 128
+	} else {
+		val, err := util.AtoiNoOct(sectors.Value)
+		if err != nil {
+			util.StatusMessage(util.VERBOSITY_DEFAULT,
+				"* Warning: target specifies invalid non-integer "+
+					"BOOTUTIL_MAX_IMG_SECTORS setting; assuming a "+
+					"value of 128.\n")
+			maxImgSectors = 128
+		} else {
+			maxImgSectors = val
+		}
+	}

Review Comment:
   This does the same as previous block of code for MCU_FLASH_MIN_WRITE_SIZE. You could add private helper function which will validate the setting and return it's value (or default value if proper setting is not present). Helper would be used like this:
   ```
   	minWriteSz := t.getSettingIntValue(t.res.Cfg.Settings, "MCU_FLASH_MIN_WRITE_SIZE", 1)
   	maxImgSectors := t.getSettingIntValue(t.res.Cfg.Settings, "BOOTUTIL_MAX_IMG_SECTORS", 128)
   ```
   where 1 and 128 are configurable default values returned in case of wrong/lack of syscfg.



-- 
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: notifications-unsubscribe@mynewt.apache.org

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