From 6f3f92d3d4d8ad2d432691842caacd8b71e33f3d Mon Sep 17 00:00:00 2001 From: Robert Walton Date: Mon, 24 May 2021 11:47:49 +0100 Subject: [PATCH 1/2] config: Remove `Memory` object 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. --- .../build/_internal/config/config.py | 21 +------- .../build/_internal/config/source.py | 53 +------------------ .../_internal/templates/mbed_config.tmpl | 4 -- tests/build/_internal/config/test_config.py | 14 +---- tests/build/_internal/config/test_source.py | 49 +---------------- 5 files changed, 5 insertions(+), 136 deletions(-) diff --git a/src/mbed_tools/build/_internal/config/config.py b/src/mbed_tools/build/_internal/config/config.py index bb493f2f..7f96862b 100644 --- a/src/mbed_tools/build/_internal/config/config.py +++ b/src/mbed_tools/build/_internal/config/config.py @@ -8,7 +8,7 @@ from collections import UserDict from typing import Any, Iterable, Hashable, Callable, List -from mbed_tools.build._internal.config.source import Memory, Override, ConfigSetting +from mbed_tools.build._internal.config.source import Override, ConfigSetting logger = logging.getLogger(__name__) @@ -18,15 +18,13 @@ class Config(UserDict): This object understands how to populate the different 'config sections' which all have different rules for how the settings are collected. - Applies overrides, appends macros, updates memories, and updates config settings. + Applies overrides, appends macros, and updates config settings. """ def __setitem__(self, key: Hashable, item: Any) -> None: """Set an item based on its key.""" if key == CONFIG_SECTION: self._update_config_section(item) - elif key == MEMORIES_SECTION: - self._update_memories_section(item) elif key == OVERRIDES_SECTION: self._handle_overrides(item) elif key == MACROS_SECTION: @@ -69,20 +67,6 @@ def _update_config_section(self, config_settings: List[ConfigSetting]) -> None: self.data[CONFIG_SECTION] = self.data.get(CONFIG_SECTION, []) + config_settings - def _update_memories_section(self, memories: List[Memory]) -> None: - defined_memories = self.data.get(MEMORIES_SECTION, []) - for memory in memories: - logger.debug(f"Adding memory settings `{memory.name}: start={memory.start} size={memory.size}`") - prev_defined = next((mem for mem in defined_memories if mem.name == memory.name), None) - if prev_defined is None: - defined_memories.append(memory) - else: - logger.warning( - f"You are attempting to redefine `{memory.name}` from {prev_defined.namespace}.\n" - f"The values from `{memory.namespace}` will be ignored" - ) - self.data[MEMORIES_SECTION] = defined_memories - def _find_first_config_setting(self, predicate: Callable) -> Any: """Find first config setting based on `predicate`. @@ -105,7 +89,6 @@ def _find_first_config_setting(self, predicate: Callable) -> Any: CONFIG_SECTION = "config" MACROS_SECTION = "macros" -MEMORIES_SECTION = "memories" OVERRIDES_SECTION = "overrides" diff --git a/src/mbed_tools/build/_internal/config/source.py b/src/mbed_tools/build/_internal/config/source.py index 59d01df8..54008bce 100644 --- a/src/mbed_tools/build/_internal/config/source.py +++ b/src/mbed_tools/build/_internal/config/source.py @@ -28,7 +28,7 @@ def prepare( ) -> dict: """Prepare a config source for entry into the Config object. - Extracts memory, config and override settings from the source. Flattens these nested dictionaries out into + Extracts config and override settings from the source. Flattens these nested dictionaries out into lists of objects which are namespaced in the way the Mbed config system expects. Args: @@ -46,11 +46,6 @@ def prepare( for key in data: data[key] = _sanitise_value(data[key]) - memories = _extract_memories(namespace, data) - - if memories: - data["memories"] = memories - if "config" in data: data["config"] = _extract_config_settings(namespace, data["config"]) @@ -83,31 +78,6 @@ def __post_init__(self) -> None: self.value = _sanitise_value(self.value) -@dataclass -class Memory: - """Representation of a defined RAM/ROM region.""" - - name: str - namespace: str - start: str - size: str - - def __post_init__(self) -> None: - """Convert start and size to hex format strings.""" - try: - self.start = hex(int(self.start, 0)) - except ValueError: - raise ValueError( - f"Value of MBED_{self.name}_START in {self.namespace}, {self.start} is invalid: must be an integer" - ) - try: - self.size = hex(int(self.size, 0)) - except ValueError: - raise ValueError( - f"Value of MBED_{self.name}_SIZE in {self.namespace}, {self.size} is invalid: must be an integer" - ) - - @dataclass class Override: """Representation of a config override. @@ -158,27 +128,6 @@ def _extract_config_settings(namespace: str, config_data: dict) -> List[ConfigSe return settings -def _extract_memories(namespace: str, data: dict) -> List[Memory]: - memories = [] - for mem in ["rom", "ram"]: - start_attr = f"mbed_{mem}_start" - size_attr = f"mbed_{mem}_size" - start = data.get(start_attr) - size = data.get(size_attr) - - if size is not None and start is not None: - logger.debug(f"Extracting MBED_{mem.upper()} definitions in {namespace}: _START={start}, _SIZE={size}.") - - memory = Memory(mem.upper(), namespace, start, size) - memories.append(memory) - elif start is not None or size is not None: - raise ValueError( - f"{size_attr.upper()} and {start_attr.upper()} must be defined together. Only " - f"{'START' if start is not None else 'SIZE'} is defined in the lib {namespace}." - ) - return memories - - def _extract_target_overrides( namespace: str, override_data: dict, allowed_target_labels: Iterable[str] ) -> List[Override]: diff --git a/src/mbed_tools/build/_internal/templates/mbed_config.tmpl b/src/mbed_tools/build/_internal/templates/mbed_config.tmpl index 7fadeb14..8fb21191 100644 --- a/src/mbed_tools/build/_internal/templates/mbed_config.tmpl +++ b/src/mbed_tools/build/_internal/templates/mbed_config.tmpl @@ -75,10 +75,6 @@ set(MBED_CONFIG_DEFINITIONS "-D{{setting_name}}={{value}}" {% endif -%} {%- endfor -%} -{% for memory in memories %} - "-DMBED_{{memory.name}}_START={{memory.start}}" - "-DMBED_{{memory.name}}_SIZE={{memory.size}}" -{%- endfor -%} {% for macro in macros %} "{{macro|replace("\"", "\\\"")}}" {%- endfor %} diff --git a/tests/build/_internal/config/test_config.py b/tests/build/_internal/config/test_config.py index c7e2e352..980ed4d2 100644 --- a/tests/build/_internal/config/test_config.py +++ b/tests/build/_internal/config/test_config.py @@ -2,11 +2,10 @@ # Copyright (c) 2020-2021 Arm Limited and Contributors. All rights reserved. # SPDX-License-Identifier: Apache-2.0 # -import logging import pytest from mbed_tools.build._internal.config.config import Config -from mbed_tools.build._internal.config.source import prepare, ConfigSetting, Memory, Override +from mbed_tools.build._internal.config.source import prepare, ConfigSetting, Override class TestConfig: @@ -25,17 +24,6 @@ def test_raises_when_trying_to_add_duplicate_config_setting(self): with pytest.raises(ValueError, match="lib.param already defined"): conf.update(prepare({"config": {"param": {"value": 0}}}, source_name="lib")) - def test_logs_ignore_mbed_ram_repeated(self, caplog): - caplog.set_level(logging.DEBUG) - input_dict = {"mbed_ram_size": "0x80000", "mbed_ram_start": "0x24000000"} - input_dict2 = {"mbed_ram_size": "0x78000", "mbed_ram_start": "0x24200000"} - - conf = Config(prepare(input_dict, source_name="lib1")) - conf.update(prepare(input_dict2, source_name="lib2")) - - assert "values from `lib2` will be ignored" in caplog.text - assert conf["memories"] == [Memory("RAM", "lib1", "0x24000000", "0x80000")] - def test_target_overrides_handled(self): conf = Config( { diff --git a/tests/build/_internal/config/test_source.py b/tests/build/_internal/config/test_source.py index b7f4a2a7..962315af 100644 --- a/tests/build/_internal/config/test_source.py +++ b/tests/build/_internal/config/test_source.py @@ -2,10 +2,8 @@ # Copyright (c) 2020-2021 Arm Limited and Contributors. All rights reserved. # SPDX-License-Identifier: Apache-2.0 # -import pytest - from mbed_tools.build._internal.config import source -from mbed_tools.build._internal.config.source import Memory, Override +from mbed_tools.build._internal.config.source import Override class TestPrepareSource: @@ -120,48 +118,3 @@ def test_converts_config_setting_value_lists_to_sets(self): assert conf["config"][0].value == {"ETHERNET", "WIFI"} assert conf["sectors"] == {0, 2048} assert conf["header_info"] == {0, 2048, "bobbins", "magic"} - - def test_memory_attr_extracted(self): - lib = { - "mbed_ram_size": "0x80000", - "mbed_ram_start": "0x24000000", - "mbed_rom_size": "0x200000", - "mbed_rom_start": "0x08000000", - } - - conf = source.prepare(lib, "lib") - - assert Memory("RAM", "lib", "0x24000000", "0x80000") in conf["memories"] - assert Memory("ROM", "lib", "0x8000000", "0x200000") in conf["memories"] - - def test_memory_attr_converted_as_hex(self): - input_dict = {"mbed_ram_size": "1024", "mbed_ram_start": "0x24000000"} - - conf = source.prepare(input_dict, source_name="lib") - - memory, *_ = conf["memories"] - assert memory.size == "0x400" - - def test_raises_memory_size_not_integer(self): - input_dict = {"mbed_ram_size": "NOT INT", "mbed_ram_start": "0x24000000"} - - with pytest.raises(ValueError, match="_SIZE in lib, NOT INT is invalid: must be an integer"): - source.prepare(input_dict, "lib") - - def test_raises_memory_start_not_integer(self): - input_dict = {"mbed_ram_size": "0x80000", "mbed_ram_start": "NOT INT"} - - with pytest.raises(ValueError, match="_START in lib, NOT INT is invalid: must be an integer"): - source.prepare(input_dict, "lib") - - def test_raises_memory_size_defined_not_start(self): - input_dict = {"mbed_ram_size": "0x80000"} - - with pytest.raises(ValueError, match="Only SIZE is defined"): - source.prepare(input_dict) - - def test_raises_memory_start_defined_not_size(self): - input_dict = {"mbed_ram_start": "0x24000000"} - - with pytest.raises(ValueError, match="Only START is defined"): - source.prepare(input_dict) From 8f1df1bb6034947b91c43aba57feb71b6e9aedf3 Mon Sep 17 00:00:00 2001 From: Robert Walton Date: Mon, 24 May 2021 12:27:21 +0100 Subject: [PATCH 2/2] config: Add memory region parameters to mbed_config.cmake Add memory region defines to mbed_config.tmpl so they end up in mbed_config.cmake. Fixes #283 --- news/20210524113403.bugfix | 1 + src/mbed_tools/build/_internal/cmake_file.py | 8 ++++++++ .../build/_internal/templates/mbed_config.tmpl | 12 ++++++++++++ tests/build/test_generate_config.py | 8 ++++++++ 4 files changed, 29 insertions(+) create mode 100644 news/20210524113403.bugfix diff --git a/news/20210524113403.bugfix b/news/20210524113403.bugfix new file mode 100644 index 00000000..2f178f56 --- /dev/null +++ b/news/20210524113403.bugfix @@ -0,0 +1 @@ +Fix issue with memory region overrides being ignored. diff --git a/src/mbed_tools/build/_internal/cmake_file.py b/src/mbed_tools/build/_internal/cmake_file.py index 09d507c4..d6b550bf 100644 --- a/src/mbed_tools/build/_internal/cmake_file.py +++ b/src/mbed_tools/build/_internal/cmake_file.py @@ -5,6 +5,8 @@ """Module in charge of CMake file generation.""" import pathlib +from typing import Any + import jinja2 from mbed_tools.build._internal.config.config import Config @@ -25,7 +27,13 @@ def render_mbed_config_cmake_template(config: Config, toolchain_name: str, targe The rendered mbed_config template. """ env = jinja2.Environment(loader=jinja2.PackageLoader("mbed_tools.build", str(TEMPLATES_DIRECTORY)),) + env.filters["to_hex"] = to_hex template = env.get_template(TEMPLATE_NAME) config["supported_c_libs"] = [x for x in config["supported_c_libs"][toolchain_name.lower()]] context = {"target_name": target_name, "toolchain_name": toolchain_name, **config} return template.render(context) + + +def to_hex(s: Any) -> str: + """Filter to convert integers to hex.""" + return hex(int(s, 0)) diff --git a/src/mbed_tools/build/_internal/templates/mbed_config.tmpl b/src/mbed_tools/build/_internal/templates/mbed_config.tmpl index 8fb21191..89308ac5 100644 --- a/src/mbed_tools/build/_internal/templates/mbed_config.tmpl +++ b/src/mbed_tools/build/_internal/templates/mbed_config.tmpl @@ -54,6 +54,18 @@ set(MBED_TARGET_DEFINITIONS{% for component in components %} {% for form_factor in supported_form_factors %} TARGET_FF_{{form_factor}} {%- endfor %} +{% if mbed_rom_start is defined %} + MBED_ROM_START={{ mbed_rom_start | to_hex }} +{%- endif %} +{% if mbed_rom_size is defined %} + MBED_ROM_SIZE={{ mbed_rom_size | to_hex }} +{%- endif %} +{% if mbed_ram_start is defined %} + MBED_RAM_START={{ mbed_ram_start | to_hex }} +{%- endif %} +{% if mbed_ram_size is defined %} + MBED_RAM_SIZE={{ mbed_ram_size | to_hex }} +{%- endif %} TARGET_LIKE_MBED __MBED__=1 ) diff --git a/tests/build/test_generate_config.py b/tests/build/test_generate_config.py index b18bb2b1..6605f5b8 100644 --- a/tests/build/test_generate_config.py +++ b/tests/build/test_generate_config.py @@ -48,6 +48,10 @@ "supported_toolchains": ["ARM", "GCC_ARM", "IAR"], "trustzone": False, "OUTPUT_EXT": "hex", + "mbed_ram_start": "0", + "mbed_ram_size": "0", + "mbed_rom_start": "0", + "mbed_rom_size": "0", } @@ -289,6 +293,10 @@ def test_overrides_target_config_param_from_app(matching_target_and_filter, prog ("target.macros", ["DEFINE"], "DEFINE"), ("target.device_has", ["NOTHING"], "DEVICE_NOTHING"), ("target.features", ["ELECTRICITY"], "FEATURE_ELECTRICITY"), + ("target.mbed_rom_start", "99", "MBED_ROM_START=0x63"), + ("target.mbed_rom_size", "1010", "MBED_ROM_SIZE=0x3f2"), + ("target.mbed_ram_start", "99", "MBED_RAM_START=0x63"), + ("target.mbed_ram_size", "1010", "MBED_RAM_SIZE=0x3f2"), ("OUTPUT_EXT", "hex", 'MBED_OUTPUT_EXT "hex"'), ], )