Skip to content

Calculate FlashIAPBlockDevice start address and size automatically for test only #9268

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

Merged
merged 1 commit into from
Jan 8, 2019

Conversation

yossi2le
Copy link
Contributor

@yossi2le yossi2le commented Jan 6, 2019

Description

This PR fixes an issue in which boards with FlashIAP block device enabled fails the FlashIAP block device tests exists under the component directory. That's because they have no start address and size configured in the mbed_lib.json file.
In order to simplify the test for targets with no definitions in the mbed_lib.json, the test will calculate the start address as the first sector after the application ends and up to the max size available.

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@ciarmcom
Copy link
Member

ciarmcom commented Jan 6, 2019

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

@ciarmcom ciarmcom requested review from a team January 6, 2019 14:00
@jeromecoutant
Copy link
Collaborator

Is this fixing #9255 ?

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 7, 2019

This PR fixes an issue in which boards with FlashIAP block device enabled fails the FlashIAP block device tests exists under the component directory. That's because they have no start address and size configured in the mbed_lib.json file.
In order to simplify the test for targets with no definitions in the mbed_lib.json, the test will calculate the start address as the first sector after the application ends and up to the max size available.

Can you add this to the commit msg (at least some info, it's one liner at the moment).

mbed::FlashIAP flash_driver;

ret = flash_driver.init();
TEST_ASSERT_EQUAL(0, ret);
Copy link
Contributor

Choose a reason for hiding this comment

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

how does this work? assert equal and the condition below returns to the next test case (does it reach line 1273?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the remark. The code will get to 1273 if the ret value is 0. The only problem is that the if after the ASSERT is redundant. I have removed it and updated the commit message.

@jeromecoutant
Copy link
Collaborator

Is this fixing #9255 ?

It seems OK :-)

target platform_name test suite result elapsed_time (sec) copy_method
DISCO_L475VG_IOT01A-ARM DISCO_L475VG_IOT01A components-storage-blockdevice-component_flashiap-tests-filesystem-fopen OK 28.11 default

@yossi2le
Copy link
Contributor Author

yossi2le commented Jan 7, 2019

Yes, this is fixing #9255. I missed the fact that you open a bug for that.

… enabled fails the FlashIAP block device tests exists under the component directory. That's because they have no start address and size configured in the mbed_lib.json file. In order to simplify the test for targets with no definitions in the mbed_lib.json, the test will calculate the start address as the first sector after the application ends and up to the max size available.
@yossi2le yossi2le force-pushed the yossi_fix_flashiap_test branch from f35f9a0 to bdeb545 Compare January 7, 2019 13:26
@0xc0170
Copy link
Contributor

0xc0170 commented Jan 7, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Jan 7, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

@0xc0170 0xc0170 merged commit 1c3b6f4 into ARMmbed:master Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants