-
Notifications
You must be signed in to change notification settings - Fork 34
add DISCO_L496AG + STModCellular BG96 support #113
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
Conversation
Thanks for your changes. |
@soleilplanet does this not depend on ARMmbed/mbed-os#12411 ? |
@teetak01 yes, this depends on ARMmbed/mbed-os#12411, just forgot to put this information. Sorry about that. |
Thanks for clarifying. I have already mentioned earlier for the testing team about this issue with the test-timeout. |
@teetak01 I've added an additional line and it is now not dependent on the Mbed-OS PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rerun the the tests which the config change to see that the override works as expected.
@teetak01 here are the logs with the new commit. |
Can you fix the configuration? I am not sure that will even compile? Having two identical entries in .json should not even pass Mbed CLI verification, or if it does, one of those parameters would get silently ignored. |
@teetak01 it would compile and here's the log with a dump of the mbed_app.json: Combining those two lines in one will not be able to build; you will see 'unresolved symbol to pal_plat_XXXXX()' in pal_plat_internalFlash.cpp during compilation and it will just fail. That's why I filed the PR in Mbed-OS at the first place. by the way, I'm using all latest tools as I created a new VM for this test. |
@teetak01 I found that the build error I mentioned above is not observed after I upgraded python to 2.7.17 (was 2.7.12) with single line of 'components_add' |
Yeah good. I was not able to reproduce the issue either. |
You could also rebase the PR. |
test logs with combined components_add: |
build-only |
updated test log with new commit: |
build-only |
@soleilplanet this would still require:
|
fd940b7
to
f80f135
Compare
@teetak01 I just squashed and rebased from master, but I'm not sure if that's the result you'd like to see. Would you please check? |
f80f135
to
8e648f6
Compare
build-only |
Looks good, thanks @soleilplanet |
Summary of changes
[X] I confirm this contribution is my own and I agree to license it with Apache 2.0.
[X] I confirm the moderators may change the PR before merging it in.
For new board enablements only:
[X] I confirm the board is Mbed Enabled and passes the Mbed Enabled test set.
[X] I confirm the contribution has been tested properly and the tests results for TESTS are attached.
[X] I confirm
mbed-os.lib
andmbed-cloud-client.lib
hashes or the content in foldersmbed-os
andmbed-cloud-client
were not modified in order to pass the tests.Add support for DISCO_L496AG + STModCellular BG96.
Integration tests all passed, Pelion E2E tests all passed except for update test due to python package issue. Update campaign succeeded with manual 'mbed dm' commands.
disco_l496ag_external_kvstore_pelion_e2e_test.log
disco_l496ag_external_kvstore_integration_test.log