[examples/pigweed-app] Added README file to the nRF Connect example. #3598
Conversation
pullapprove
bot
requested review from
andy31415,
BroderickCarlin,
bzbarsky-apple,
chrisdecenzo,
hawk248,
jelderton,
mspang,
rwalker-apple and
saurabhst
kkasperczyk-no
force-pushed
the
pigweed_app_doc
branch
from
f3f9f81 to
be72244
Compare
| $ mkdir ~/nrfconnect | ||
| $ mkdir ~/connectedhomeip | ||
| $ docker pull nordicsemi/nrfconnect-chip | ||
| $ docker run --rm -it -v ~/nrfconnect:/var/ncs -v ~/connectedhomeip:/var/chip -v /dev/bus/usb:/dev/bus/usb --device-cgroup-rule "c 189:* rmw" nordicsemi/nrfconnect-chip |
There was a problem hiding this comment.
This doesn't seem to work:
% docker run --rm -it -v ~/nrfconnect:/var/ncs -v ~/connectedhomeip:/var/chip -v /dev/bus/usb:/dev/bus/usb --device-cgroup-rule "c 189:* rmw" nordicsemi/nrfconnect-chip
Welcome to development environment for nRF Connect SDK and CHIP applications
- /var/ncs is empty
to enable build of nRF Connect SDK applications run "setup" command which will fetch
required nRF Connect SDK sources
build@4014282afacf:/$ setup
/var/ncs repository is empty. Do you wish to check out nRF Connect SDK sources [master]? [Y/N] y
=== Initializing in /var/ncs
--- Cloning manifest repository from https://github.com/nrfconnect/sdk-nrf, rev. master
fatal: cannot mkdir /var/ncs/.west/manifest-tmp: Permission denied
FATAL ERROR: command exited with status 128: git init /var/ncs/.west/manifest-tmp
It probably works only if your uid outside docker is 1000.
There was a problem hiding this comment.
I think VSCode container also assumes that your uid is 1000, right? It's kind of tricky to run a Docker container using the caller's uid since the Docker image must contain appropriate entries in /etc/passwd... The instruction for building without Docker is right below, but Docker is very helpful for people who don't want to spend time for installing nRF Connect dependencies. I may think about some workaround for your case - perhaps running the image as root and switching to the original user id in the entrypoint script would make it work for any user.
There was a problem hiding this comment.
I fixed that problem and updated the other nrfconnect READMEs: #3629. @kkasperczyk-no, could you apply the change to the pigweed-app as well?
There was a problem hiding this comment.
VSCode container does not assume your id is 1000.
There was a problem hiding this comment.
Hmm. .devcontainer/Dockerfile creates a user with the following data:
ARG USERNAME=vscode
ARG USER_UID=1000
ARG USER_GID=$USER_UID
ENV LANG en_US.utf8
and then:
"remoteUser": "vscode", in devcontainer.json. But I haven't tested how it behaves when started as a different user.
There was a problem hiding this comment.
ARG meaning arguments not constants.
There was a problem hiding this comment.
Ah, so VSCode rebuilds the image for each user. I get it now.
| other operations described in this document like flashing or debugging can also | ||
| be done in the container. | ||
|
|
||
| ### Using native shell |
There was a problem hiding this comment.
This version works better, can we document the checkout steps in "Supported nRF Connect SDK versions" better?
i.e, provide the steps
mkdir ~/nrfconnect
cd ~/nrfconnect
west init -m https://github.com/nrfconnect/sdk-nrf --mr 83764f01f04f58c80bb1ae1d3e0a640496f79b49
west update
How do you change the --mr hash used by west update ?
There was a problem hiding this comment.
I described it, as you requested. Actually this is a duplication of information located under the link attached in the instruction, but anyway I hope that now it will be easier for someone to set it up.
kkasperczyk-no
force-pushed
the
pigweed_app_doc
branch
from
be72244 to
df21b8f
Compare
|
I think you should also mention |
kkasperczyk-no
force-pushed
the
pigweed_app_doc
branch
from
ac4064e to
838bc76
Compare
Right, done. |
There is lack of information how to build pigweed-app example on the nRF Connect platform. * Added README file containing information how to build and use example.
kkasperczyk-no
force-pushed
the
pigweed_app_doc
branch
from
a9e8b11 to
faa9a4d
Compare
| $ mkdir ~/connectedhomeip | ||
| $ docker pull nordicsemi/nrfconnect-chip | ||
| $ docker run --rm -it -e RUNAS=$(id -u) -v ~/nrfconnect:/var/ncs -v ~/connectedhomeip:/var/chip \ | ||
| -v /dev/bus/usb:/dev/bus/usb --device-cgroup-rule "c 189:* rmw" nordicsemi/nrfconnect-chip |
There was a problem hiding this comment.
Why nordicsemi/nrfconnect-chip instead of project-chip/chip-build-nrf-platform ?
It seems like the instructions should be in sync with our GitHub CI.
There was a problem hiding this comment.
@Damian-Nordic could you comment on this, as I have never met with project-chip/chip-build-nrf-platform image?
There was a problem hiding this comment.
Sure. We introduced nordicsemi/nrfconnect-chip for several reasons:
chip-build-nrf-platformcomes with embedded nRF Connect sources to which you don't have access outside the container whilenordicsemi/nrfconnect-chipfetches SDK to a mounted host directory. I find it quite useful to be able to configure IDE to see headers from the SDK.chip-build-nrf-platformdoesn't support building as non-root usernordicsemi/nrfconnect-chipshortens development time for us as we don't need to wait for a) PR with Dockerfile changes to be merged b) the new version to be pushed to the docker hub c) do dependent changes in README. We can do a) & b) instantly and focus on c).nordicsemi/nrfconnect-chipis much smaller in size and faster to setup.
I admit 1) and 2) can be addressed by adding support for nRF Connect SDK to the VS Code container (which will be added by the end of this week), so we may rethink using nordicsemi/nrfconnect-chip once that is done although the VS Code container is quite large, time-consuming to initialize and may seem like a bit overkill for someone willing to try a single example. 3) and 4) were important for us, especially in the initial phase of CHIP development, but if you think CHIP shouldn't rely on external images, we won't insist to use it.
There was a problem hiding this comment.
It seems to me that the discussion turned to more general topics, which are not strictly related to this PR, so what do you think about creating some issue and continuing it there, so this one could not be blocked?
There was a problem hiding this comment.
Sure. We introduced
nordicsemi/nrfconnect-chipfor several reasons:
chip-build-nrf-platformcomes with embedded nRF Connect sources to which you don't have access outside the container whilenordicsemi/nrfconnect-chipfetches SDK to a mounted host directory. I find it quite useful to be able to configure IDE to see headers from the SDK.
This is fixable, and should be fixed. We shouldn't have to rebuild docker images to bump the revision.
chip-build-nrf-platformdoesn't support building as non-root user
Also fixable, and should be fixed.
nordicsemi/nrfconnect-chipshortens development time for us as we don't need to wait for a) PR with Dockerfile changes to be merged b) the new version to be pushed to the docker hub c) do dependent changes in README. We can do a) & b) instantly and focus on c).
This problem seems symmetric. Now we have the same problem, but instead CHIP's documented flows blocks on Nordic updating its containers. I think CHIP should own the images, so that our CI & documentation stay in sync.
I do however agree our current docker process needs improvement; ideally GitHub automation would handle building and pushing images from dockerfile changes. That's the way forward in my opinion.
nordicsemi/nrfconnect-chipis much smaller in size and faster to setup.
Is this just a result of 1) ?
I admit 1) and 2) can be addressed by adding support for nRF Connect SDK to the VS Code container (which will be added by the end of this week), so we may rethink using
nordicsemi/nrfconnect-chiponce that is done although the VS Code container is quite large, time-consuming to initialize and may seem like a bit overkill for someone willing to try a single example. 3) and 4) were important for us, especially in the initial phase of CHIP development, but if you think CHIP shouldn't rely on external images, we won't insist to use it.
I see the argument that your image works better right now, but I'd ask you to contribute those improvements.
There was a problem hiding this comment.
It seems to me that the discussion turned to more general topics, which are not strictly related to this PR, so what do you think about creating some issue and continuing it there, so this one could not be blocked?
Sounds good.
Problem
There is lack of information how to build pigweed-app example on the nRF Connect platform.
Summary of Changes