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

Add ram and rom size config to targets.json (Copy mem reg info from index.json) #15094

Closed
wants to merge 1 commit into from

Conversation

rajkan01
Copy link
Contributor

Summary of changes

ROM and RAM size is required in some of the greeentea tests to skip or include the test into the build, but when we copy the mbed_rom_size, mbed_ram_size configs from index.json to targets.json causes the build failure for non-bootloader supported targets as MBED CLI1 expects that if any of the mem (ROM, RAM) configs are present in targets.json should have bootloader support, hence introducing new rom-size and ram-size using config in targets.json to generate MBED_CONF_TARGET_ROM_SIZE and MBED_CONF_TARGET_RAM_SIZE macros by Mbed CLI1 and Mbed CLI2 which can be used in the greentea test.

Impact of changes

None.

Migration actions required

Update the greentea test, which depends on MBED_RAM_SIZE and MBED_ROM_SIZE macros to replace with MBED_CONF_TARGET_ROM_SIZE and MBED_CONF_TARGET_RAM_SIZE.

Documentation


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers


…rmation from index.json)

ROM and RAM size is required in some of the greeentea tests to skip or
include the test into the build, but when we copy the "mbed-rom-size",
"mbed-ram-size" configs from index.json to targets.json causes the build
failure for non-bootloader supported targets as MBED CLI1 expects that
if any of the mem (ROM, RAM) configs are present in targets.json should
have bootloader support, hence introducing new rom-size and ram-size
using "config" in targets.json to generate MBED_CONF_TARGET_ROM_SIZE
and MBED_CONF_TARGET_RAM_SIZE macros by Mbed CLI1 and Mbed CLI2 which
can be used in the greentea test.
@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Sep 20, 2021
@ciarmcom ciarmcom requested a review from a team September 20, 2021 16:00
@ciarmcom
Copy link
Member

@rajkan01, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

Copy link
Collaborator

@jeromecoutant jeromecoutant left a comment

Choose a reason for hiding this comment

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

Lots of targets are missing ...?

You could also define this new config at "target" level at the beginning of the file,
and then avoid all these hep duplication lines ?

This is replacing #14975 ?

@ciarmcom ciarmcom added the stale Stale Pull Request label Sep 24, 2021
@ciarmcom
Copy link
Member

This pull request has automatically been marked as stale because it has had no recent activity. @Patater, @LDong-Arm, @rwalton-arm, @ARMmbed/mbed-os-maintainers, please complete review of the changes to move the PR forward. Thank you for your contributions.

@ciarmcom ciarmcom added stale Stale Pull Request and removed stale Stale Pull Request labels Sep 24, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Sep 29, 2021

This is replacing #14975 ?

@rajkan01 How this one differs from 14975 ?

@0xc0170 0xc0170 removed the stale Stale Pull Request label Sep 29, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Feb 9, 2022

We are closing this pull request due to inactivity

@0xc0170 0xc0170 closed this Feb 9, 2022
@mergify mergify bot removed needs: review release-type: patch Indentifies a PR as containing just a patch labels Feb 9, 2022
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.

4 participants