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

Enable post build bootloader merging in uvision #10021

Merged
merged 11 commits into from Apr 12, 2019

Conversation

Projects
None yet
7 participants
@bridadan
Copy link
Contributor

commented Mar 8, 2019

Description

This PR enables the following:

  • Header generation and bootloader merging after building with Uvision
  • Flashing of the complete merged image when debugging
  • Debugging with the proper symbols loaded

This could probably use some more testing on my end. I recently had to a big rebase to get this up to speed with master and the application I was using to test is not compatible with the master branch.

Fairly confident this is now in working order after doing quite a bit of testing offline. There are some other issues with the uvision exporter at the moment. Some of them are being addressed in #10045 (which this PR depends on). I've raised #10045 separately because those fixes are higher priority than the feature added in this PR.

Pull request type

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

Reviewers

@theotherjimmy @SenRamakri @dlfryar

Release Notes

Offline projects that use the managed bootloader mode (as is the case for all Pelion Device Management projects using the update capability) should now be able to export and debug properly in uVision. A post build script is now enabled which takes care of the header generation, binary merging, and loading of the correct symbols for your application. This only allows debugging the application, not the bootloader.

Note: This feature is only enabled when exporting offline with Mbed CLI. This is because the post build script has a dependency on the Mbed OS tools and their Python dependencies. This means projects exported from the Online Compiler will not be able to use this capability.

@ciarmcom ciarmcom requested review from dlfryar, SenRamakri, theotherjimmy and ARMmbed/mbed-os-maintainers Mar 9, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Mar 9, 2019

@theotherjimmy

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2019

@bridadan This also requires the tools directory in the exported project.

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2019

@theotherjimmy, very true. Is this currently omitted when exporting Mbed OS 5 projects from the online compiler? I realize the tools directory would not be present in Mbed OS 2 projects, but those projects can't/shouldn't be using managed bootloader?

@theotherjimmy

This comment has been minimized.

Copy link
Contributor

commented Mar 11, 2019

@bridadan Yes, there is an .mbedignore file in the root of the tools dir.

@theotherjimmy

This comment has been minimized.

Copy link
Contributor

commented Mar 11, 2019

@bridadan mbed2 docs don't say anything about managed bootloader mode, but nothing is stopping them from using it. I don't think we need to worry about that though.

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2019

@bridadan Yes, there is an .mbedignore file in the root of the tools dir.

Crud you're right. Well I'm open to suggestions! It may be that this is only possible to achieve offline.

@bridadan bridadan force-pushed the bridadan:uvision_postbuild_regions branch to 0d77cb2 Mar 12, 2019

@bridadan bridadan marked this pull request as ready for review Mar 12, 2019

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

@theotherjimmy oh how about I make the post build script a generated file?

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

Actually that definitely wouldn't work as is, the post build script has dependencies in the rest of the config system. The way I see it, my options are:

  1. Remove the dependencies on the rest of the tools and treat the post build script as a generated file. This actually doesn't sound too bad looking at regions.py, but it would take longer than I'd like and I'd have duplicate all that functionality in this script.
  2. Add the ability to add directories to exported projects (but not through the Resource class)
  3. Disable the post build script functionality when exporting to a zipped file. I believe the online compiler always uses this path when exporting. This would still enable the feature for offline exports, which I know a few people who would still find this very useful.

Any thoughts on the above options?

@theotherjimmy

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2019

@bridadan I'm down for 3. 1 would need to require that the script is actually not duplicated. I'm not sure that 2 is a good idea. Recent refactors have taken the direction of migrating files into the Resources style of working, so adding yet another way to do the same thing feels like adding tech debt to me.

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

Ok cool, I'll go with 3 then.

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

Note to self: update the release notes in the PR description

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

@bridadan This needs a rebase.

@bridadan bridadan force-pushed the bridadan:uvision_postbuild_regions branch from 0d77cb2 to 63155db Apr 8, 2019

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

Ok @theotherjimmy, I've implemented the option we discussed above (choice 3). I've also updated the release notes to reflect these new changes.

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

I should note that in order for the entire uvision exporter to be working 100% fixes from #9966 and #9967 are still needed.

@bridadan bridadan removed the request for review from dlfryar Apr 9, 2019

bridadan added some commits Mar 8, 2019

merge_region_list now takes in just restrict_size instead of config.
merge_region_list was changed to do some extra checks regarding the
different regions. It only was checking the "restrict_size" parameter
and not the whole config option. So this reduces the argument down to
just this value. This makes it easier to serialize the data needed for
post build steps after being built in an exported project.

bridadan added some commits Mar 8, 2019

Add debug and flash init scripts for uvision.
These files are used when the post build script is enabled to support
projects that are using managed bootloader mode.
Only enable uvision postbuild when in a non-zipped exported project.
Projects that are zipped are typically from the online compiler or they
are meant to be used in a separate environment. Since the postbuild
script requires the Mbed OS tools to present in the project, we will
disable the postbuild script when the project is exported to a zipped
project.

@bridadan bridadan force-pushed the bridadan:uvision_postbuild_regions branch from 63155db to fb6fcc5 Apr 9, 2019

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Apr 10, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

Ci started

@mbed-ci

This comment has been minimized.

Copy link

commented Apr 10, 2019

Test run: SUCCESS

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

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Apr 10, 2019

@cmonr cmonr merged commit 3bda0ef into ARMmbed:master Apr 12, 2019

26 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(-36 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 9925 cycles (+121 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B (+0.00%)
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details

@cmonr cmonr removed the ready for merge label Apr 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.