Skip to content

Remove hiding of config errors related to ROM regions. #12007

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Remove hiding of config errors related to ROM regions. #12007

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 39 additions & 49 deletions tools/toolchains/mbed_toolchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -895,62 +895,52 @@ def add_regions(self):
"""Add regions to the build profile, if there are any.
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""
"""
if not getattr(self.target, "bootloader_supported", False):
return

Copy link
Contributor

@0xc0170 0xc0170 Dec 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit msg (explanation):

tools: regions only if bootloader is supported
    
    We removed catching and passing, we want to know for any misconfiguration if a bootloader
    is supported. Regions function should check if a bootloader is supported. In case not, just return.
    Otherwise we catch any error.
    
    This should help us to uncover missing regions or other config error (in case bootloader
    is enabled via bootloader_supported set to true).

Copy link
Contributor

@0xc0170 0xc0170 Dec 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not able to push to your branch . I tested this for WIZWIKI_W7500, it builds without an error. but target like TB_SENSE_1 fails with a proper error - bootloader can't be enabled due to configuration error.

I tried to catch proper config exception but it is not easy the way config is executed (not that clear what with/without bootloader should work), I found this check as the best one to do prior doing any region work (it is done in config anyway later but then we are deeply in already bootoader case).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IF we can test this commit, we should get reduce list of targets that are not supporting bootloader properly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@madchutney please review

if self.config.has_regions:
try:
regions = list(self.config.regions)
regions.sort(key=lambda x: x.start)
self.notify.info("Using ROM region%s %s in this build." % (
"s" if len(regions) > 1 else "",
", ".join(r.name for r in regions)
))
self._add_all_regions(regions, "MBED_APP")
except ConfigException:
pass
regions = list(self.config.regions)
regions.sort(key=lambda x: x.start)
self.notify.info("Using ROM region%s %s in this build." % (
"s" if len(regions) > 1 else "",
", ".join(r.name for r in regions)
))
self._add_all_regions(regions, "MBED_APP")

if self.config.has_ram_regions:
try:
regions = list(self.config.ram_regions)
self.notify.info("Using RAM region%s %s in this build." % (
"s" if len(regions) > 1 else "",
", ".join(r.name for r in regions)
))
self._add_all_regions(regions, None)
except ConfigException:
pass
regions = list(self.config.ram_regions)
self.notify.info("Using RAM region%s %s in this build." % (
"s" if len(regions) > 1 else "",
", ".join(r.name for r in regions)
))
self._add_all_regions(regions, None)

Region = namedtuple("Region", "name start size")

try:
# Add all available ROM regions to build profile
if not getattr(self.target, "static_memory_defines", False):
raise ConfigException()
rom_available_regions = self.config.get_all_active_memories(
ROM_ALL_MEMORIES
# Add all available ROM regions to build profile
if not getattr(self.target, "static_memory_defines", False):
raise ConfigException()
rom_available_regions = self.config.get_all_active_memories(
ROM_ALL_MEMORIES
)
for key, value in rom_available_regions.items():
rom_start, rom_size = value
self._add_defines_from_region(
Region("MBED_" + key, rom_start, rom_size),
True,
suffixes=["_START", "_SIZE"]
)
for key, value in rom_available_regions.items():
rom_start, rom_size = value
self._add_defines_from_region(
Region("MBED_" + key, rom_start, rom_size),
True,
suffixes=["_START", "_SIZE"]
)
except ConfigException:
pass
try:
# Add all available RAM regions to build profile
if not getattr(self.target, "static_memory_defines", False):
raise ConfigException()
ram_available_regions = self.config.get_all_active_memories(
RAM_ALL_MEMORIES

# Add all available RAM regions to build profile
if not getattr(self.target, "static_memory_defines", False):
raise ConfigException()
ram_available_regions = self.config.get_all_active_memories(
RAM_ALL_MEMORIES
)
for key, value in ram_available_regions.items():
ram_start, ram_size = value
self._add_defines_from_region(
Region("MBED_" + key, ram_start, ram_size),
True,
suffixes=["_START", "_SIZE"]
)
for key, value in ram_available_regions.items():
ram_start, ram_size = value
self._add_defines_from_region(
Region("MBED_" + key, ram_start, ram_size),
True,
suffixes=["_START", "_SIZE"]
)
except ConfigException:
pass


STACK_PARAM = "target.boot-stack-size"
TFM_LVL_PARAM = "tfm.level"
Expand Down