ARMmbed / mbed-tools

⚠️ Beta Status: New command line tooling for Mbed OS
Apache License 2.0
45 stars 30 forks source link

build: Automatically determine image format to flash #290

Closed LDong-Arm closed 3 years ago

LDong-Arm commented 3 years ago

Description

The default image format is bin, but targets can override it with "OUTPUT_EXT": "hex" in targets.json. This is already taken care of in the CMake configuration generated by mbed-tools, but user still need to pass --hex-image if they want to run mbed-tools compile with --flash to flash a hex image.

This commit makes the image selection automatic based on OUTPUT_EXT.

Note: --hex-file is now redundant and thus removed. It does not make sense to have it anymore without also offering --bin-file for completeness.

The test cases for bin and hex image flashing have been updated accordingly. Coverage for OUTPUT_EXT has been improved, with both mbed_config.cmake and image format to flash checked.

Test Coverage

The test case for hex image flashing has been updated accordingly. Also manually tested on NRF52_DK (hex image) and DISCO_L475VG_IOT01A (bin image).

LDong-Arm commented 3 years ago

@ARMmbed/mbed-os-core

codecov[bot] commented 3 years ago

Codecov Report

Merging #290 (049a07b) into master (dd71209) will decrease coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #290      +/-   ##
==========================================
- Coverage   97.08%   97.08%   -0.01%     
==========================================
  Files          92       92              
  Lines        2781     2779       -2     
==========================================
- Hits         2700     2698       -2     
  Misses         81       81              
Impacted Files Coverage Δ
src/mbed_tools/build/config.py 100.00% <100.00%> (ø)
src/mbed_tools/cli/build.py 100.00% <100.00%> (ø)
src/mbed_tools/cli/configure.py 100.00% <100.00%> (ø)
LDong-Arm commented 3 years ago

Now tests and code coverage should be good, the PR is ready for review.

rwalton-arm commented 3 years ago

LGTM, just needs to be rebased.

LDong-Arm commented 3 years ago

@rwalton-arm Thanks, rebased.