Skip to content
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.

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

Fix issue with memory region overrides #284

Merged
merged 2 commits into from
May 26, 2021

Conversation

rwalton-arm
Copy link
Contributor

@rwalton-arm rwalton-arm commented May 24, 2021

Description

MBED_ROM/RAM_START/SIZE overrides were not added to
mbed_config.cmake.

The reason mbed_app.json overrides were "ignored" is
because the overrides were applied to the already collected string
parameters and weren't applied to the extracted Memory objects we
expose in the template.

We shouldn't need special handling for mbed_rom/ram_start or
mbed_rom/ram_size as they are already collected (if present in
targets.json). So remove the special handling for memory regions.
A side effect of this approach is that we remove some of the validation
checks for START and SIZE both being defined. If we feel this is a
necessary check we can add it back into the Config object.

Fixes #283

Test Coverage

  • This change is covered by existing or additional automated tests.
  • Manual testing has been performed (and evidence provided) as automated testing was not feasible.
  • Additional tests are not required for this change (e.g. documentation update).

We shouldn't need special handling for mbed_rom/ram_start or
mbed_rom/ram_size as they are already collected (if present in
targets.json). The reason mbed_app.json overrides were "ignored" is
because the overrides were applied to the already collected string
parameters and weren't applied to the extracted `Memory` objects we
expose in the template.
Add memory region defines to mbed_config.tmpl so they end up in
mbed_config.cmake.

Fixes ARMmbed#283
@codecov
Copy link

codecov bot commented May 24, 2021

Codecov Report

Merging #284 (8f1df1b) into master (71e9707) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
- Coverage   97.12%   97.07%   -0.05%     
==========================================
  Files          92       92              
  Lines        2813     2773      -40     
==========================================
- Hits         2732     2692      -40     
  Misses         81       81              
Impacted Files Coverage Δ
src/mbed_tools/build/_internal/config/source.py 100.00% <ø> (ø)
src/mbed_tools/build/_internal/cmake_file.py 100.00% <100.00%> (ø)
src/mbed_tools/build/_internal/config/config.py 100.00% <100.00%> (ø)

Copy link
Contributor

@Patater Patater left a comment

Choose a reason for hiding this comment

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

LGTM

@Patater
Copy link
Contributor

Patater commented May 26, 2021

Travis passed, but failed to report back to GitHub.

@Patater Patater merged commit ff2da40 into ARMmbed:master May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mbed_rom_size values from mbed_app.json are ignored
2 participants