doc: gsg cleanups #23977

mbolivar-nordic · 2020-04-01T03:05:30Z

Fix up some error handling for the new zephyr-export extension used in the getting started guide, and do some other cleanups in the GSG.

doc/getting_started/index.rst

nashif · 2020-04-01T14:06:23Z

for the gsg, this is no cleanup, this is almost a rewrite :)

mbolivar-nordic · 2020-04-01T16:24:51Z

for the gsg, this is no cleanup, this is almost a rewrite :)

That's not totally unfair. In my defense, I will say that there were items in #19123 and #23054 that I had either suggested or intimated should be done "later". Later is now :).

tejlmand

Thanks for this.

In general I think the cleanup looks good, but still has some suggestions.

scripts/west_commands/export.py

doc/getting_started/index.rst

tejlmand · 2020-04-03T12:19:06Z

doc/getting_started/index.rst

Personally, I prefer to have my tools / programs outside any repository.

Especially as tools might be up-/downgrade independently of the repo.

Or I could have multiple up-/downstream workspaces, but just a single toolchain.

So I would suggest to still advice users with a different path.

Which of course also means the comment goes for remaining part of this commit.

I am doing this on purpose for the getting started guide precisely to make the Zephyr environment more self-contained, so we appear to have different preferences in this regard. The question is really what will be easier for more people, I suppose.

could you take a look at:
zephyrproject-rtos/sdk-ng#199
#24143

those will remove the need for the environment variables, and in such way make it easier to get started.
And not polluting the Zephyr workspace with a toolchain folder.

could you take a look at:
zephyrproject-rtos/sdk-ng#199
#24143

OK, I like that PR very much. Let's wait until it gets reviewed by @nashif and @galak to decide how to proceed here.

I completely agree with @tejlmand here. I think we should not encourage users to install tools, any tools, inside a west workspace because of the following reasons:

A west workspace is akin to a Git repo in many ways, and in fact I am still considering adding some sort of west reset --hard in the future because there are many valid use cases for this, such as cleaning up after modules have been renamed

You might have multiple workspaces and you'd likely want to share the SDK among those

I suggest using either /opt/ or ~/bin/` instead

I suggest using either /opt/ or ~/bin/` instead

I think ~/bin is a bad option, as the Zephyr SDK is not a single executable file, but several folders with executables, libraries, etc.

Currently, the suggested default paths to search for the SDK are:

$HOME/zephyr-sdk* $HOME/.local/zephyr-sdk* $HOME/.local/opt/zephyr-sdk* /opt/zephyr-sdk* /usr/zephyr-sdk* /usr/local/zephyr-sdk*

so I suggest we pick one among those in the example given in gsg.

Feel free to comment on default paths for SDK in this PR:
zephyrproject-rtos/sdk-ng#199

I meant ~/bin/zephyr-sdk @tejlmand

tejlmand · 2020-04-03T12:25:48Z

doc/getting_started/index.rst

Changing the changes in later commit in same PR.
So unfriendly of you 😉

Suggested change

#. Export a :ref:`Zephyr CMake package <cmake_pkg>`. This allows CMake to

load boilerplate code required for building Zephyr applications.

#. Export a :ref:`Zephyr CMake package <cmake_pkg>`. This allows CMake to

automatically load boilerplate code required for building Zephyr applications.

Sorry, I will try to be more friendly in the future. Comments now addressed.

mbolivar-nordic · 2020-04-14T16:37:51Z

Last force-push is just a rebase. Will address comments after to make it easier for reviewers.

Edit: comments on this PR now addressed in a separate push.

carlescufi

I finally had a chance to render this locally and review it. In general this is a great improvement, thank you @mbolivar-nordic. I only have a few suggestions and one section that I'd like changed.

carlescufi · 2020-04-17T16:38:20Z

doc/getting_started/index.rst

While the rest of the changes in this section are fine, I'd keep the link for the rest of Linux distros here. Makes it easier for all those on non-Ubuntu distros to find what they need quickly from the beginning of the GSG.

OK, I'll remove it from the first ubuntu tab instead, then. The same link appears twice in consecutive paragraphs, so I was trying to remove one of them. I agree keeping it here makes more sense.

carlescufi · 2020-04-17T16:55:59Z

doc/getting_started/index.rst

I completely agree with @tejlmand here. I think we should not encourage users to install tools, any tools, inside a west workspace because of the following reasons:

A west workspace is akin to a Git repo in many ways, and in fact I am still considering adding some sort of west reset --hard in the future because there are many valid use cases for this, such as cleaning up after modules have been renamed

You might have multiple workspaces and you'd likely want to share the SDK among those

carlescufi · 2020-04-17T17:10:30Z

doc/getting_started/index.rst

Suggested change

They rely on `Chocolatey`_. If Chocolatey isn't an option, you can

The instructions rely on `Chocolatey`_. If Chocolatey isn't an option, you can

carlescufi · 2020-04-17T17:14:12Z

doc/getting_started/index.rst

I suggest using either /opt/ or ~/bin/` instead

carlescufi · 2020-04-17T17:15:25Z

doc/getting_started/index.rst

Suggested change

does not meet Blinky's :ref:`blinky-sample-requirements`, :ref:`hello_world`

does not meet Blinky's :ref:`blinky-sample-requirements`, then :ref:`hello_world`

This is so that the 2 hyperlinks do not "visually merge"

carlescufi · 2020-04-17T17:16:59Z

doc/getting_started/index.rst

I suggest also linking to the debugging section here, which is likely one of the things users want to try first.

mbolivar-nordic · 2020-04-17T18:07:53Z

I suggest using either /opt/ or ~/bin/ instead

I will just drop the patch affecting the Zephyr SDK entirely if that is OK, since @tejlmand already has a plan for that. This will minimize user-visible churn during the release cycle.

mbolivar-nordic · 2020-04-17T18:43:53Z

@carlescufi rebased and comments addressed. I added a couple of patches to the main two samples referenced from the guide

tejlmand · 2020-04-17T18:48:52Z

I suggest using either /opt/ or ~/bin/ instead

I will just drop the patch affecting the Zephyr SDK entirely if that is OK, since @tejlmand already has a plan for that. This will minimize user-visible churn during the release cycle.

That is fine with me.
And I think it makes it easier to get this in, and I can rebase #24143 and then adjust the Zephyr SDK accordingly.
Makes it clearer what changes are introduced for which reason.

tejlmand

Great cleanup.
Found two small places I think can improve even more, feel free to take them in or ignore them.

tejlmand · 2020-04-18T08:14:52Z

doc/getting_started/index.rst

When looking at the rendered doc, I think there should be just a single line explaining why users should do those commands, so maybe add a little:

Suggested change

Click the operating system you are using.

Click the operating system you are using and follow the guide to ensure it is up-to-date.

I think this is implied by the title "Select and Update OS"

yeah, but when I first read it, I just read the content and my eyes didn't notice the title.
Not sure if i'm the only one 😆

I guess adding more text isn't likely to make eyes glaze less, though. That's exactly why I'm trying to strip this down and remove any text I can -- people generally ignore text and just start copy pasting command lines into their terminals as fast as they can, in my experience.

doc/getting_started/index.rst

mbolivar-nordic · 2020-04-20T16:35:44Z

Rebased, hopefully CI is happy now

carlescufi

This LGTM now, thank you @mbolivar-nordic, this will be clearer and more accessible for upcoming new users

mbolivar-nordic · 2020-04-27T16:27:47Z

Thanks! Rebased, hopefully good to go after CI re-runs.

tejlmand · 2020-04-30T12:43:14Z

If we get this in, then I can rebase: #24143
and adjust the getting started changes in that PR accordingly.

nashif · 2020-04-30T12:55:50Z

doc/getting_started/index.rst

reason for removing qemu and dtc?

That was a leftover from removing optional dependencies. Fixed, thanks.

nashif · 2020-04-30T12:58:36Z

doc/getting_started/index.rst

Are we really installing Zephyr? This is misleading, we are just getting the code, Install is used for applications, we do not want to make it sound as if Zephyr was an installable application, because it is not.

we do not want to make it sound as if Zephyr was an installable application, because it is not.

addressed

Strip out text that isn't needed to try to minimize the GSG's length. Fix up some grammar nits. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

Various commands are getting put into their own sections when they are really just steps along the way towards getting zephyr and installing Python dependencies. Group them together in a section by that name, moving the west install step there. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

The devicetree link is rendering as "Devicetree Guide" instead of "devicetree". Fix that. My guess is that most users won't care about the details of the requirements, and rather just want to know if their board is supported or not. So move the error you'll see on unsupported boards before the details about how to add support (which involve a pretty significant learning curve). Also mention overlays as a way to get this working. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

Since this is such a basic sample, it's worth nitpicking the wording a bit: - Add a link to the supported boards, since many first time users will not know where it is and may start near here. - Remove the single and multi threaded note. There's no information on how to build it in either of these two modes or what the default is, so it feels like a distraction for such a basic sample. - Give a clue about how to build for another board. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

nashif · 2020-05-06T02:03:08Z

probably not fault of this PR, I just tried following the new gsg on new ubuntu 2020.04 and python3-dev is required by some python package we install (psutil)...
Without the pkg the install fails...

erwango · 2020-12-17T08:00:32Z

doc/getting_started/index.rst

-
- .. code-block:: console
-
- choco install git python ninja dtc-msys2 gperf


@mbolivar-nordic I'm sure this is documented somewhere (as otherwise we'd have new users complaining all day long), but I wonder how windows users are supposed to get dtc if not documented here.

@mbolivar-nordic I'm sure this is documented somewhere (as otherwise we'd have new users complaining all day long), but I wonder how windows users are supposed to get dtc if not documented here.

It's not required; 23df708

I'm sure this is documented somewhere

https://docs.zephyrproject.org/latest/guides/dts/intro.html#scripts-and-tools

Ho, right!
I was confused about dtc status, I knew it was still in use (at least on my side).
That's clear now. Txs @mbolivar-nordic

mbolivar-nordic requested review from carlescufi and tejlmand April 1, 2020 03:05

mbolivar-nordic requested a review from nashif as a code owner April 1, 2020 03:05

zephyrbot added the area: Documentation label Apr 1, 2020

mbolivar-nordic force-pushed the gsg-cleanups branch 2 times, most recently from c3f588e to 7633b38 Compare April 1, 2020 03:40

mbolivar-nordic mentioned this pull request Apr 1, 2020

Install Python deps in a virtual environment #23979

Closed

carlescufi reviewed Apr 1, 2020

View reviewed changes

doc/getting_started/index.rst Outdated Show resolved Hide resolved

tejlmand requested changes Apr 3, 2020

View reviewed changes

mbolivar-nordic mentioned this pull request Apr 9, 2020

scripts: fix west zephyr-export error handling #24248

Merged

mbolivar-nordic force-pushed the gsg-cleanups branch 2 times, most recently from 62a9fb9 to 19efd2f Compare April 14, 2020 16:37

mbolivar-nordic force-pushed the gsg-cleanups branch from 19efd2f to 6ffbb2d Compare April 14, 2020 16:48

carlescufi requested changes Apr 17, 2020

View reviewed changes

mbolivar-nordic force-pushed the gsg-cleanups branch from 6ffbb2d to ce983ad Compare April 17, 2020 18:43

zephyrbot added the area: Samples Samples label Apr 17, 2020

tejlmand approved these changes Apr 18, 2020

View reviewed changes

mbolivar-nordic force-pushed the gsg-cleanups branch from ce983ad to 5dfc3ff Compare April 20, 2020 16:35

mbolivar-nordic force-pushed the gsg-cleanups branch from 5dfc3ff to 0c2c22c Compare April 23, 2020 22:25

carlescufi approved these changes Apr 27, 2020

View reviewed changes

mbolivar-nordic force-pushed the gsg-cleanups branch from 0c2c22c to bdc627c Compare April 27, 2020 16:27

mbolivar-nordic force-pushed the gsg-cleanups branch from bdc627c to 68cadd3 Compare April 27, 2020 17:04

nashif requested changes Apr 30, 2020

View reviewed changes

mbolivar-nordic force-pushed the gsg-cleanups branch 2 times, most recently from d42d345 to 2f30206 Compare May 5, 2020 16:37

mbolivar-nordic added 4 commits May 5, 2020 09:41

doc: gsg: simplifications and fixups

986770a

Strip out text that isn't needed to try to minimize the GSG's length. Fix up some grammar nits. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

mbolivar-nordic force-pushed the gsg-cleanups branch from 2f30206 to 05461fe Compare May 5, 2020 16:42

nashif approved these changes May 5, 2020

View reviewed changes

carlescufi merged commit a6b8644 into zephyrproject-rtos:master May 6, 2020

tejlmand mentioned this pull request May 6, 2020

Zephyr sdk improvement #24143

Merged

erwango reviewed Dec 17, 2020

View reviewed changes

- #. Export a :ref:`Zephyr CMake package <cmake_pkg>`. This allows CMake to
- load boilerplate code required for building Zephyr applications.
+ #. Export a :ref:`Zephyr CMake package <cmake_pkg>`. This allows CMake to
+ automatically load boilerplate code required for building Zephyr applications.

	They rely on `Chocolatey`_. If Chocolatey isn't an option, you can
	The instructions rely on `Chocolatey`_. If Chocolatey isn't an option, you can

	does not meet Blinky's :ref:`blinky-sample-requirements`, :ref:`hello_world`
	does not meet Blinky's :ref:`blinky-sample-requirements`, then :ref:`hello_world`

	Click the operating system you are using.
	Click the operating system you are using and follow the guide to ensure it is up-to-date.


		.. code-block:: console

		choco install git python ninja dtc-msys2 gperf

doc: gsg cleanups #23977

doc: gsg cleanups #23977

Uh oh!

Conversation

mbolivar-nordic commented Apr 1, 2020

Uh oh!

nashif commented Apr 1, 2020

mbolivar-nordic commented Apr 1, 2020

tejlmand left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbolivar-nordic commented Apr 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

carlescufi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbolivar-nordic commented Apr 17, 2020

mbolivar-nordic commented Apr 17, 2020

tejlmand commented Apr 17, 2020

tejlmand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

mbolivar-nordic commented Apr 20, 2020

carlescufi left a comment

Choose a reason for hiding this comment

mbolivar-nordic commented Apr 27, 2020

tejlmand commented Apr 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nashif commented May 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Labels

6 participants

mbolivar-nordic commented Apr 14, 2020 •

edited

Loading