-
Notifications
You must be signed in to change notification settings - Fork 176
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
Update serial communication and clean up duplicated & outdated info #1317
Conversation
Co-authored-by: Hugues Kamba <41612201+hugueskamba@users.noreply.github.com>
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.
@hugueskamba Thanks for the suggestions, I'll address them
Hi @iriark01, this PR cleans up outdated info and refactors duplications in multiple docs, would you please have a look? Thanks |
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.
I did not see the reference to minimal-printf in debug_with_printf.md. If it is not there, then we should add a link.
@@ -1,25 +1,21 @@ | |||
<h1 id="debug-ide-qs">Debugging the quick start</h1> | |||
<h1 id="debug-ide-qs">Debugging with the Online Compiler</h1> |
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.
There is a lot of similarities with cli debug. I think it would be better to combine the two pages.
What do you think @iriark01 ?
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.
I personally agree - the quick start pages are rather brief, maybe CLI and online compiler can go into different sections of one page
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.
People using one tool don't like having to read through instructions/info for another tool. And sometimes they don't even realise it's there so they assume it's missing.
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.
@iriark01 I understand if the instructions are quite different and long. In this case it is rather short and it would remove the duplication. I thought you combined instructions for blinky bare metal ; this would be another example of combined instructions.
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.
The current documentation layout on the website
Quick start
|- Introduction
|- Online with the online compiler
|- Importing and compiling the code
|- Debugging with the online compiler *
|- Offline with Mbed CLI
|- Setting up and compiling the code
|- Debugging with Mbed CLI *
|- Further reading
* what this PR touches
Maybe, we can rename both "Debugging with ..." to "Exporting to a desktop IDE", and factor out "Using printf" (and how to drag & drop an image) into a separate page "Debugging"? It would still look natural in my personal opinion.
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.
I combined bare metal and the full profile, but not into the same tool. The tools are seen as very separate beasts - Mbed Studio actually has its own site.
I would argue that the problem here is that there's no real value to having either of these pages in the quick start. Debugging and exporting are both covered in much greater detail elsewhere in the docs.
@LDong-Arm please can you use a branch on this repo rather than a fork? That will generate a preview of the docs site with your changes, which will make this discussion much easier.
It already contains
which explains minimal-printf. It's not introduced by this PR. |
Co-authored-by: Hugues Kamba <41612201+hugueskamba@users.noreply.github.com>
So is this ready for me, or still waiting on engineering? |
It's ready for you to review, thanks |
- Read input from the host PC keyboard. | ||
- Communicate with applications and programming languages running on the host PC that can communicate with a serial port. Examples are Perl, Python and Java. | ||
- Communicate with applications running on the host computer to exchange data. |
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.
It needs to be either "host computer" or "host PC" consistently. Although I might just go with "PC" or "computer" - does the word "host" really clarify anything?
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.
It's a leftover from the old documentation, I'll remove it
- [PuTTY](http://www.chiark.greenend.org.uk/~sgtatham/putty/). | ||
- Some Windows PCs come with **Hyperterminal** installed. | ||
Messages printed after this point will be displayed, restart the application using the board's reset button or press `Ctrl + B` on the serial terminal. | ||
The console prints "Hello World!" to the terminal after the reset. |
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.
We haven't used the word "console" until now. What does it refer to?
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.
Good catch, it should be "example"
|
||
1. **On Windows**: | ||
<span class="notes">**Note:** Unless otherwise specified, `printf` defaults to a baud rate of `9600` on Mbed OS. You can modify this value in the `mbed_app.json` file. To configure your terminal client to this baud rate, change the speed option when selecting the port. You can view the [configuration options page](../reference/configuration.html) to learn more about how to configure OS-level options.</span> |
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.
This is sort of related to my earlier question about baud rate
@iriark01 Thanks for the suggestions, I will move the work to a branch in this repo |
Co-authored-by: Irit Arkin <irit.arkin@arm.com>
Replaced by #1326 to allow previewing.
This PR is created to
mbed sterm
commandprintf()
(instead of serial classes) for debug printingThe following files are refactored:
mbed sterm
, ...) with updated information.mbed sterm
.mbed sterm
does not really belong there.printf
guide that does not duplicate info in serial communication.Note:
minimal-printf
is linked in debug_with_printf.md as before. Also, it's now the default printf implementation (ARMmbed/mbed-os#12233) so we shouldn't need to reiterate much in my opinion.