Feedback on porting guide #857
Conversation
|
Adding Don, as it'll be easier if everything's in the same PR rather than clashing ones. |
| @@ -31,7 +31,7 @@ The following items might help you test SPI, I2C and Pins: | |||
|
|
|||
| Please install the following: | |||
|
|
|||
| - [Python 2.7](https://www.python.org/downloads/release/python-2715/). | |||
| - [Python](https://www.python.org/downloads). | |||
There was a problem hiding this comment.
Context: tools should work with Python 3 now as well and Python 2.7.x will be unsupported in 2020
|
|
||
| - A storage device (SD or external flash). | ||
| - A micro SD card for the CI test shield.<!--are these the same thing?--> | ||
|
|
||
| - A micro USB cable to connect the evaluation board to your development PC. | ||
| - A USB cable to connect the evaluation board to your development PC. |
There was a problem hiding this comment.
Context: may not necessarily be a micro usb cable. Just a bit too specific
| vi mbed-os.lib | ||
| mbed deploy | ||
| mbed compile --target <new_target> --toolchain GCC_ARM | ||
| mbed compile -m <new_target> -t gcc_arm |
There was a problem hiding this comment.
Context: Trying to keep the arguments to Mbed CLI consistent with the other guides.
| @@ -211,6 +227,7 @@ mbed compile --target <target_name> --toolchain IAR | |||
| ``` | |||
|
|
|||
| Tweak your code until your build succeeds. | |||
| <!-- Little bit hand-wavy here. We should give common failure modes (and solutions) here, an email for folks to contact us (if we want to), or just leave this out --> | |||
There was a problem hiding this comment.
I just merged a PR from @SenRamakri that deleted this line.
|
@aashishc1988 Could you please review and coordinate with @bridadan ? |
There was a problem hiding this comment.
I feel that this doc really need some re-organisation. It's not clear to me here:
- What's the goal
- What are the high-level steps
- Are we trying to cover getting something to sort of work, or reliably? Some sections of the doc mention running tests, other mention trying out examples instead
- How do we go faster if we're deriving from an existing platform?
- What are the success criteria? Definition of Done?
As a side-note some of these instructions are quite rigid in terms of which tools to use (pyOCD, Eclipse, etc) although the porter might prefer to use other tools.
| @@ -32,7 +32,7 @@ To run the Mbed OS built-in tests, you need to have ported and verified at least | |||
|
|
|||
There was a problem hiding this comment.
Above: these shouldn't be called components; maybe HAL modules or platform prerequisites
| @@ -32,7 +32,7 @@ To run the Mbed OS built-in tests, you need to have ported and verified at least | |||
|
|
|||
| - DAPLink. | |||
There was a problem hiding this comment.
AFAIK, DAPLink itself is not required as we also support STLink etc?
There was a problem hiding this comment.
DAPLink or compatible interface firmware.
| @@ -32,7 +32,7 @@ To run the Mbed OS built-in tests, you need to have ported and verified at least | |||
|
|
|||
| - DAPLink. | |||
|
|
|||
| <span class="notes">If DAPLink is still under development, please [use manual tests](#manual-testing).</span> | |||
| <span class="notes">If DAPLink is still under development, please [use manual tests](#manual-testing)<!-- This link doesn't work -->.</span> | |||
There was a problem hiding this comment.
What feature in DAPLink do I need? What are "manual tests" (we need to flash the board manually but that's not clear here)
There was a problem hiding this comment.
For testing, you need mass storage device and serial.
If your hardware/software isn't far enough to support automated testing, you'll use manual testing. In that case, you'll use the IDE (use the debugger to flash).
| @@ -48,10 +48,11 @@ The board under test needs to be supported in mbedls for automated tests to work | |||
| If the official mbedls pip package hasn't been released yet, you need to direct pip to use your local directory (which includes the code changes to support the new board): | |||
There was a problem hiding this comment.
--> If an updated mbedls pip package...
| @@ -61,17 +62,20 @@ If the official mbedls pip package hasn't been released yet, you need to direct | |||
| } | |||
| ``` | |||
|
|
|||
| Where `"33000000e062afa300000000000000000000000097969902"` is the correct target id. | |||
|
|
|||
| ### Compiling and running tests | |||
|
|
|||
| 1. Compile your tests: | |||
There was a problem hiding this comment.
These tests are not yours :)
| @@ -134,17 +138,16 @@ You may want to run manual tests, for example if DAPLink is still under developm | |||
| 1. Find the test directory: | |||
There was a problem hiding this comment.
Again, what does manual testing mean
| @@ -158,8 +161,11 @@ You may want to run manual tests, for example if DAPLink is still under developm | |||
| ``` | |||
| mbed export -i gcc_arm -m <new_target> | |||
| ``` | |||
|
|
|||
| <!-- gcc_arm is not a valid exporter. A list of valid exporters are here: https://github.com/ARMmbed/mbed-os/blob/master/tools/export/__init__.py#L36-L60 --> | |||
| <!-- If this is actually meant to use Eclipse, a valid eclipse exporter needs to be used (and ensure that the exporter is working before committing it to the docs) --> | |||
| 1. Open the project with pyOCD (using the same configuration you used [when you initially set up pyOCD]( Creating GDB pyOCD debug configuration). | |||
There was a problem hiding this comment.
What if my target is not supported by pyOCD?
There was a problem hiding this comment.
Configure your IDE and/or debugger to program your device [only for manual tests/examples]
Note: Update throughout doc to show pyOCD is not required
| ``` | ||
| mbed test --compile -m <new_target> -t gcc_arm -c | ||
| mbed test -m <new_target> -t gcc_arm --compile -c | ||
| ``` | ||
|
|
||
| You'll see some build errors. These errors should reduce and eventually disappear as more HAL components are ported. |
There was a problem hiding this comment.
This sends the wrong message. Should be rephrased as: If you see some build errors, it means that some HAL modules required to run the tests are missing and need porting.
| ``` | ||
|
|
||
| In this example: | ||
|
|
||
| - The `common_tickers` test image is at `mbed-os-example-blinky/BUILD/tests/<new_target>/gcc_arm/mbed-os/TESTS/mbed_hal/common_tickers`. | ||
| - The `common_tickers_freq` test image is at `mbed-os-example-blinky/BUILD/tests/<new_target>/gcc_arm/mbed-os/TESTS/mbed_hal/common_tickers_freq.` | ||
|
|
||
| 1. You need to flash the test image to the board. You can use either DAPLink or Eclipe IDE. You may also be able to use IAR and Keil (if they already support the new target). | ||
| 1. You need to flash the test image to the board. You can use either pyOCD or the Eclipe IDE. You may also be able to use IAR and Keil (if they already support the new target). |
There was a problem hiding this comment.
Again, we probably support other methods? And Eclipse is an IDE so don't get why it's an option. On the other hand, Open-OCD, J-Link software, or even uVision and EW Workbench (as they provide this functionality under the hood) are options.
There was a problem hiding this comment.
"You need to flash the test image to the board."
Consider moving paragraph toward top of guide, as flashing test image is important to concept.
|
|
||
| The easiest method is using the pyOCD flash tool: | ||
|
|
||
| ``` | ||
| pyocd-flashtool BUILD/mbed-os-example-blinky.bin or | ||
| pyocd-flashtool BUILD/mbed-os-example-blinky.hex | ||
| pyocd-flashtool BUILD/mbed-os-example-blinky.bin # Use the .hex file if appropriate |
There was a problem hiding this comment.
... if you're using pyOCD
|
@bridadan @linlingao Do y'all have time this afternoon to sit with me to resolve the comments from @donatieng ? |
|
@donatieng I've added clarification that this document shows how to port a new MCU target with pyOCD and DAPLink. Porting instructions for existing targets and other methods can come in another document after 5.11. |
|
@donatieng and @bridadan I think it's best to close this PR and start a fresh review - everything here is out of date. |
|
@iriark01 @AnotherButler I can resolve the conflicts with a rebase if you like. Once I do that would you like me to go over the changes one last time? |
|
Yes please |
1 similar comment
|
Yes please |
|
Cool, give me a ping when it's ready for re-review! |
Add note about using debuggers.
Address comments to fix phrasing and for product positioning.
Update wording for precise language to avoid confusion.
Add content about how to know you're finished, remove pyOCD as a requirement and fix phrasing for consistent language.
Update wording to show you don't have to use pyOCD and DAPLink.
Fix phrasing.
Fix file to remove incorrect dependencies.
Add DAPLink content back in.
Clarify that this documentation uses pyOCD and DAPLink.
bridadan
force-pushed
the
porting_guide_feedback
branch
from
23b3a84 to
72d6840
Compare
|
Ok rebased! |
Replace link to personal repo with transcluded example in public repo.
Address comment.
Revert changes incorrectly changed during rebase.
Fix commas.
Address remaining comments.
|
ping @donatieng Ready for rereview |
Add comments from engineering.
Add changes from engineering.
Add comments from engineering.
Add changes from engineering.
|
@donatieng and @AnotherButler I'm merging this so I can make the 5.11 branch for testing; please finish your edits on 5.11 this week and then, at your leisure, copy them to development. |
I've included quite a bit of comments and corrections here. This PR isn't intended to be merged as is, its more like a starting point of feedback that can be referenced.
I only covered the "Porting process" section in the porting guide. The guide is quite extensive and has lots of potential!