Skip to content

Enhance targets.json with components #9721

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

Closed

Conversation

theamirocohen
Copy link
Contributor

Description

The enhancement is further specified here.
Due to the enhancement some boards failed the general block device tests for flashiap component,
The fails were due to boards containing inconsistent sector sizes.
The tests were modified but should be improved to address the problem.

Pull request type

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

Reviewers

@ARMmbed/mbed-os-storage

@cmonr cmonr requested a review from a team February 14, 2019 15:24
"MBEDTLS_PSA_CRYPTO_C"
],
"components_add": ["SD", "FLASHIAP"],
"macros_add": ["MBEDTLS_PSA_CRYPTO_C"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why changing styling for this line? should be separate commit if needed

@@ -195,6 +195,11 @@ void basic_erase_program_read_test(BlockDevice *block_device, bd_size_t block_si
// Find a random block
bd_addr_t block = (rand() * block_size) % (block_device->size());

// Flashiap boards with inconsistent sector size will to align with random start addresses
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this related to json update? Separate commit?

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

The tests were modified but should be improved to address the problem.

Where is this issue tracked?

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 18, 2019

CI started

This PR is waiting for final approval from @ARMmbed/mbed-os-storage team

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 18, 2019

What's the release target ? Is this fix for 5.11.5?

@theamirocohen
Copy link
Contributor Author

The tests were modified but should be improved to address the problem.

Where is this issue tracked?

https://jira.arm.com/browse/IOTSTOR-789 & https://jira.arm.com/browse/IOTSTOR-790

@theamirocohen
Copy link
Contributor Author

What's the release target ? Is this fix for 5.11.5?

It can be for 5.12 (because we're adding targets)

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 18, 2019

It's adding support for components to targets, set for 5.11.5

@mbed-ci
Copy link

mbed-ci commented Feb 18, 2019

Test run: FAILED

Summary: 3 of 8 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-ARM
  • jenkins-ci/mbed-os-ci_build-IAR
  • jenkins-ci/mbed-os-ci_build-GCC_ARM

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 18, 2019

Please review build failures

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 18, 2019

CI restarted

@@ -2560,7 +2562,7 @@
}
},
"DISCO_F413ZH": {
"components_add": ["QSPIF"],
"components_add": ["QSPIF", "SD", "FLASHIAP"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think adding SD is correct
(on DISCO_F413ZH, SD connector is using SDIO not SPI)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @jeromecoutant,
So how can I approach this? should I delete it?

@@ -3791,7 +3795,7 @@
"STM32F746NG",
"STM_EMAC"
],
"components_add": ["QSPIF"],
"components_add": ["QSPIF", "SD", "FLASHIAP"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

SD seems not correct ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @jeromecoutant,
How do you know it is not correct?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Because board doesn't have any embedded SD connector over SPI

@mbed-ci
Copy link

mbed-ci commented Feb 18, 2019

Test run: FAILED

Summary: 1 of 12 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 20, 2019

@theamirocohen Can you review test failures?

@cmonr
Copy link
Contributor

cmonr commented Feb 20, 2019

@theamirocohen All greentea failures are around NUCLEO_F746ZG. Please review.

@cmonr
Copy link
Contributor

cmonr commented Feb 25, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Feb 26, 2019

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 3
Build artifacts

@cmonr
Copy link
Contributor

cmonr commented Feb 26, 2019

@ARMmbed/mbed-os-storage Anyone care to give the final OK?

Amir Cohen added 2 commits February 26, 2019 09:26
Adding storage components to boards
Due to targets enhancement some boards failed the general block device tests for flashiap component,
The fails were due to boards containing inconsistent sector sizes.
The tests were modified but should be improved to address the problem.
Rand() function issues were fixed.
@0xc0170
Copy link
Contributor

0xc0170 commented Feb 26, 2019

This fix needed for IAR8, changed destination branch.

Update: we will take this via rollup PR to the feature branch, will update here once done

@deepikabhavnani
Copy link

Roll up PR is created for this change to perform CI testing. #9858

@cmonr
Copy link
Contributor

cmonr commented Feb 26, 2019

Leaving a note. This should automatically close once #9808 is merged.

@cmonr
Copy link
Contributor

cmonr commented Feb 26, 2019

Closing since rollup PR, and feature-iar8 are now in master.

PR was not automatically closed because PR's commits were cherrypicked instead of merged (b8fed58 c07ba7c)

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.

7 participants