-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
docs: Rework of getting started guide #46949
docs: Rework of getting started guide #46949
Conversation
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.
Overall, I find the changes good, I just have a couple of comments
.. note:: | ||
|
||
Although the Zephyr SDK is recommended for new users, alternative | ||
toolchains are supported. Users may check the list of | ||
:ref:`alternate toolchains <toolchains>` to see if an existing toolchain | ||
they have installed is supported by Zephyr. |
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.
While I see no problem on mentioning this here, I'd not make it a note. I think that new users will have a better/more uniform experience if they use the SDK because it also includes some host tools.
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.
See the discussion below- this is dropped from the changes in the latest push.
The ``-p auto`` option automatically cleans byproducts from a previous build | ||
if necessary, which is useful if you try building another sample. | ||
The ``-p always`` option may also be used to force west to create a clean build. |
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 should probably mention -p
(always pristine) in the getting started, it's probably more deterministic (?)
pristine builds:
A "pristine" build directory is empty. The -p option controls
whether the build directory is made pristine before the build
is done. A bare '--pristine' with no value is the same as
--pristine=always. Setting --pristine=auto uses heuristics to
guess if a pristine build may be necessary.
-p [{auto,always,never}], --pristine [{auto,always,never}]
pristine build folder setting
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've moved the default instructions to use -p always
, and made a note about -p auto
so that users are aware of it.
Although the Zephyr SDK is recommended for new users, alternative | ||
toolchains are supported. Users may check the list of | ||
:ref:`alternate toolchains <toolchains>` to see if an existing toolchain | ||
they have installed is supported by Zephyr. |
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 am not sure why this needs to be mentioned in the Getting Started Guide. Using a third-party toolchain is not really something for those who want to get started with Zephyr in the quickest and the most hassle-free way. Providing beginners with too many options at first may only confuse them and send them down the wrong path.
to see if an existing toolchain they have installed is supported by Zephyr.
they can likely use the GNU ARM embedded toolchain they already have installed and save on setup time and disk space
This will cause more problems for the people who are new to Zephyr because:
- Third-party toolchains do not include any host tools, and people need to either install every host tool manually (not really something for the beginners) or download the Zephyr SDK anyway to get them.
- There exist subtle yet very impactful differences between the Zephyr SDK and the third-party GNU toolchains (e.g. GNU Arm Embedded), which can lead to very confusing and hard to troubleshoot issues (see GNU Arm Embedded toolchain deprecation #43843; all third-party GNU toolchains should be deprecated eventually).
- Third-party toolchains do not have the same level of support and userbase as the Zephyr SDK; so, you are likely to encounter more issues in the long term and any questions you ask about them will likely go unanswered. Moreover, if you have any problems using a third-party GNU toolchain like the GNU Arm Embedded (and you almost certainly will), your question will very likely be answered with "please try with the Zephyr SDK and let me know how it goes," in which case you might as well just install the Zephyr SDK in the first place to save the hassle.
For the reasons stated above, using a third-party toolchain is not something for the people who are getting started with Zephyr. IMHO, this should be dropped. Also note that the instructions for using an alternate toolchain is already mentioned in the "Beyond Getting Started Guide," which is sufficient in my 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.
Using a third-party toolchain is not really something for those who want to get started with Zephyr in the quickest and the most hassle-free way.
In my experience helping developers get started, it was. After reading through the discussions you mentioned, I see the reasoning behind potentially deprecating GNU ARM embedded in the future, but I think if that is a goal we have perhaps the solution here is not to add this note to the getting started guide, but to discuss why those developers struggled with Zephyr SDK (and what we could do to improve it)
From their feedback (as well as my personal experience), I gathered the following:
- The SDK is large- the disk space required to support toolchains for every architecture is significant. Maybe we should direct users to download the minimal SDK bundle instead, and provide guidance on choosing their architecture.
- The Windows SDK is built against python 3.8- since the chocolatey installer uses python 3.10 by default, SDK components like gdb that require python will fail to start until python 3.8 is installed. This is an issue I've seen on other systems as well (I recall running into it on Arch Linux), and maybe could be resolved by changing the chocolatey install instructions to use a specific python version.
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.
to discuss why those developers struggled with Zephyr SDK (and what we could do to improve it)
Sure
Maybe we should direct users to download the minimal SDK bundle instead, and provide guidance on choosing their architecture.
That sounds reasonable. I thought of doing that initially, but I was afraid the "provid[-ing] guidance on choosing their architecture" part could further confuse the people who are just getting started. If well stated enough, however, it could definitely improve developer experience.
SDK components like gdb that require python will fail to start until python 3.8 is installed.
GNU Arm Embedded is no exception to this either. For this reason, we provide a separate GDB variant called "gdb-no-py" that builds without Python scripting support, and the Zephyr build system automatically defaults to using it if the required version of libpython is not locally available (by the way, from SDK 0.15.0 and on, the default "gdb" will become Python-less and a separate Python-capable variant called "gdb-py" will be provided).
maybe could be resolved by changing the chocolatey install instructions to use a specific python version.
That sounds reasonable. The same can be done for macOS/Homebrew (python@3.8
).
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.
That sounds reasonable. I thought of doing that initially, but I was afraid the "provid[-ing] guidance on choosing their architecture" part could further confuse the people who are just getting started. If well stated enough, however, it could definitely improve developer experience.
I'll update this PR with directions on doing just that. I'd welcome feedback on the change.
That sounds reasonable. The same can be done for macOS/Homebrew (python@3.8).
If we do this, I assume we would just update the required version of python when we updated getting started directions to use a new version of the SDK, correct?
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.
If we do this, I assume we would just update the required version of python when we updated getting started directions to use a new version of the SDK, correct?
Correct.
I'm unsure how often the python dependency of GDB will change.
So far, it has not changed for almost 2 years; but, we will likely update to 3.10 soon since Ubuntu 22.04 has been released.
(based on the change to using no-py in the next SDK, it sounds like this may not be an issue long term)
Sure, from the SDK 0.15.0 release and on, the default gdb
executable (e.g. arm-zephyr-eabi-gdb
) will no longer link against the libpython, and a separate Python-capable gdb-py
(e.g. arm-zephyr-eabi-gdb-py
) variant will be provided. This will make IDE integration, which may look for *-gdb
by default, much easier.
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.
@stephanosio I've changed the SDK setup step to direct users to install the minimal SDK, and select the toolchain they need with the -t
switch. Please let me know your thoughts on this approach. I've also updated the dependency instructions to install python 3.8 on Windows and Mac OS
@@ -119,7 +119,7 @@ The current minimum required version for the main dependencies are: | |||
|
|||
.. code-block:: bash | |||
|
|||
brew install cmake ninja gperf python3 ccache qemu dtc wget | |||
brew install cmake ninja gperf python@3.8 ccache qemu dtc wget |
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.
A note here: I unfortunately don't have an Apple platform to test the Mac OS SDK setup on, and would like to ensure these commands work. Would appreciate if someone developing on a Mac could follow the SDK setup and dependency install setup steps on a Mac to verify everything still works correctly
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 have tested this on a Mac and it works.
#. Determine what architectures you intended to use with Zephyr: | ||
|
||
The SDK supports all architectures Zephyr targets, but users may also | ||
choose to install only a single architecture's toolchain if they do not | ||
intend to use Zephyr across multiple architectures. Below is a list of | ||
the toolchains supported by the SDK: | ||
|
||
* aarch64-zephyr-elf | ||
* arc64-zephyr-elf | ||
* arc-zephyr-elf | ||
* arm-zephyr-eabi | ||
* mips-zephyr-elf | ||
* nios2-zephyr-elf | ||
* riscv64-zephyr-elf | ||
* sparc-zephyr-elf | ||
* x86_64-zephyr-elf | ||
* xtensa-espressif_esp32_zephyr-elf | ||
* xtensa-espressif_esp32s2_zephyr-elf | ||
* xtensa-intel_apl_adsp_zephyr-elf | ||
* xtensa-intel_bdw_adsp_zephyr-elf | ||
* xtensa-intel_byt_adsp_zephyr-elf | ||
* xtensa-intel_s1000_zephyr-elf | ||
* xtensa-nxp_imx_adsp_zephyr-elf | ||
* xtensa-nxp_imx8m_adsp_zephyr-elf | ||
* xtensa-sample_controller_zephyr-elf | ||
|
||
For example, a user who intends to only use Zephyr with ARM | ||
architectures could install the SDK with the following command: | ||
|
||
.. code-block:: bash | ||
|
||
./setup.sh -h -c -t arm-zephyr-eabi | ||
|
||
If you are unsure which architecture you need, or would like to use | ||
multiple, substitute ``-t all`` for ``-t {toolchain_name}`` below. |
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 think this is too much information for a GSG. I think that a better solution would probably be to create an online toolchain installer that asks which archs you want, if you want host tools, etc. then downloads just what is needed @stephanosio would that make sense?
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.
an online toolchain installer that asks which archs you want, if you want host tools, etc. then downloads just what is needed
That is exactly what the minimal distribution bundle mentioned in #46949 (comment) is.
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.
Just discovered this, minimal
sounds like the "minimal to operate" not an online installer. Does it have the capability to list available archs? I was thinking on something like the Qt online installer, but CLI only.
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.
Yes, it supports both interactive (it asks if you want to install arch A, B, C, and so on) and non-interactive (command-line param controlled) mode.
You can try it out by downloading one here:
https://github.com/zephyrproject-rtos/sdk-ng/releases/tag/v0.14.2
[1] Minimal bundle does not contain any toolchains and allows users to choose the toolchains to download and install.
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.
Would we prefer to instead use the prompting feature available in the toolchain installer? It is easy to use, my only concern is that users might skip installing something they need (CMake package registration, for instance), and using flags to control the installation helps reduce the chance of this happening, IMO
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 problem with this has more to do with, for instance, "I want to build esp32c3_devkitm
, which toolchain do I need? Is it xtensa-espressif_esp32_zephyr-elf
or riscv64-zephyr-elf
?" (FYI, it is the latter).
For some, it may be quite obvious and straight-forward (e.g. qemu_riscv64
requires riscv64-zephyr-elf
); for others, not so much:
qemu_xtensa
requiresxtensa-sample_controller_zephyr-elf
(and not any otherxtensa-
toolchains)fvp_baser_aemv8r
requiresaarch64-zephyr-elf
and notarm-zephyr-eabi
xenvm
requiresaarch64-zephyr-elf
and the list goes on.
For the seasoned Zephyr developers and users, it is easy enough; not so much for the people who are just getting started with Zephyr and have no idea what "boards" and "SoC" really mean and/or how they are organised.
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.
For the seasoned Zephyr developers and users, it is easy enough; not so much for the people who are just getting started with Zephyr and have no idea what "boards" and "SoC" really mean and/or how they are organised.
For someone new to embedded development, I agree that we should not guide them down this path. Maybe the best way to do that is recommending that developers install all toolchains by default.
However, I would expect any experienced embedded developer who is getting started with Zephyr to know the toolchain they need. I personally feel the getting started guide should at least inform developers that the minimal toolchain option exists.
Maybe the best way to approach this is to provide guidance to run the toolchain installer with -t all
, and mention that it can be run interactively to only install specific toolchains. What are your thoughts on that approach?
High level feedback from me:
I wholeheartedly agree with this. I tried this myself in #23979 and then basically rage-quit when people kept putting up objections. I think it's way past time that we do this and I would honestly want to try to force the issue through the TSC if individual developers keep trying to stand in the way of what is just basic Python hygiene at this point.
No comment
Nice find
No objections, but this is something the SDK maintainer will have to keep sync'ed up as time evolves and versions change; would be nice to see an ACK for that.
No objections
No objections |
@mbolivar-nordic Glad to see you agree with the general direction here. I'd like to ensure this PR doesn't go stale, and that we at least generate some meaningful discussion from it. @stephanosio, while I personally feel that the minimal SDK installation directions add value, I don't feel that change is very high priority. If I drop the SDK installation changes from this PR, would the changes be acceptable to you? |
I have no objection to any other changes in this PR, or even the change adding the instructions for the minimal SDK install in itself -- I think it is quite useful when given as extra information (maybe in a section under the Zephyr SDK doc, with a link to it in the GSG), just not as the default method of installation in the way it is currently stated. |
Please remember that the tendency for the GSG is to expand over time, and we must resist this temptation. The steps in the GSG should be as minimal as we can make them to get new users running hello_world or blinky. If we give in to temptation, we end up with a giant GSG that contains a bunch of "or you could do it this way, or this way, or this way, ...". Users hate this. |
Given what @mbolivar-nordic pointed out about GSG expansion, I've dropped the minimal SDK installation instructions from this PR. The PR now only contains the changes regarding virtual environments, python 3.8, and the |
choco install ninja gperf python git dtc-msys2 wget unzip | ||
choco install python --version=3.8.0 | ||
choco install ninja gperf git dtc-msys2 wget unzip |
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.
whitespace issue (mixed TAB and SPACEs)
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. Should be fixed now
@@ -65,7 +65,7 @@ The current minimum required version for the main dependencies are: | |||
- 3.20.0 | |||
|
|||
* - `Python <https://www.python.org/>`_ | |||
- 3.6 | |||
- 3.8 |
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.
Sorry, but I disagree with this. Zephyr doesn't require Python 3.6, except for Python support in GDB I assume. But that's not enough to force 3.8 on everyone. That said, I am OK with upgrading the minimum required Python from 3.6 to 3.8 or higher, but let's discuss this in a separate PR where we see the benefits of going as high as possible in one swoop.
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.
Fine dropping this. Reason for the change was the fact that our default GDB used to require python, but pending confirmation that is no longer the case, I agree this change should be dropped.
@@ -119,7 +119,7 @@ The current minimum required version for the main dependencies are: | |||
|
|||
.. code-block:: bash | |||
|
|||
brew install cmake ninja gperf python3 ccache qemu dtc wget libmagic | |||
brew install cmake ninja gperf python@3.8 ccache qemu dtc wget |
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, in my opinion, wrong. Requiring every Zephyr user to install an outdated version of Python just because a tiny fraction of them will want to debug with Python support is just not right. Instead I think we should document this in the Zephyr SDK page in the doc.
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.
that's indeed a good point, I actually use Python 3.10 but I also have 3.8 installed for GDB and everything works fine (using FC36). And there's the no-py
version, right? Maybe add a note about this requirement if using gdb.
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 a good point. The initial goal here was to ensure that a user getting started with Zephyr would be able to debug using GDB, as GDB in our toolchain required python support. However, I am looking at the latest 0.15.0 SDK build, and it looks like GDB now defaults to not using python, and only requires it when running the gdb-py
version of the executable. (@stephanosio could you confirm this?) If that is the case, I am completely fine dropping this, I just want to ensure the first thing users encounter when running west debug
isn't a python dependency error
@@ -163,7 +163,8 @@ The current minimum required version for the main dependencies are: | |||
.. code-block:: console | |||
|
|||
choco install cmake --installargs 'ADD_CMAKE_TO_PATH=System' | |||
choco install ninja gperf python git dtc-msys2 wget unzip | |||
choco install python --version=3.8.0 |
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.
Same as above
Change default python setup suggested in getting started guide to use virtual environments. This aligns with the documentation itself, which suggests that users use virtual environments with Zephyr. The Windows python setup is also updated with the correct command to create a virtual environment, as "python3" is not a defined executable in a windows commandline after installing python with chocolatey Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>
Add note about west boards command to getting started documentation, to make it easier for users to determine the name of their board when used with west. Also change "-p auto" to "-p always" in the build step, since this is more deterministic Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>
4c85ee3
@carlescufi I have dropped the python 3.8 requirement change from the documentation, given that @stephanosio said on here on discord that the 0.15.0 release of the SDK makes python-less gdb the default |
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.
apart from how some examples are using pip
while others are using pip3
for the same host operating system (does not have anything to do with the changes in this PR), looks good.
This PR is an attempt to clarify some issues I have encountered while helping developers new to Zephyr get started with it. Since this a relatively large change, I've listed out each change I've made to the getting started guide below, along with my reasoning. Changes are divided up into commits based on which section of the guide they affect.
Feedback on these proposed changes is welcome. My goal here is to make it possible for a new Zephyr user to read the getting started guide, while following the recommended path for each setup step, and end up with a working Zephyr environment every time without needing to ask for assistance.
Changed the ordering of the tabs in the "Get Zephyr and install Python dependencies" section to direct the user to install Zephyr dependencies in a virtual environment
pip3 install -U west
command (while running on Windows). Additionally, the PATH modification step given for Ubuntu is specific to bash, and may not work with an alternative shell.Changed the python virtual environment initialization command for Windows
python3
alias, sopython -m venv zephyrproject\.venv
is the correct command, notpython3 -m venv zephyrproject\.venv
Added a note about the
west boards
command to the "Build the Blinky Sample" sectionfrdm_k64f
.west build
will produce a useful error message when an invalid board name is used, I feel that developers may have an easier time getting started if they don't have to guess what name to use for their board, and can use the west command to find it.Changed the
-p auto
directive to-p always
in the "Build the Blinky Sample" sectionwest build
, the-p auto
option isn't always perfect at detecting when a clean build is required. I made this change to help users when troubleshooting, so they can force a clean build and remove any invalid CMake configuration their last build attempt saved.