rbaron / b-parasite

🌱💧 An open source DIY soil moisture sensor
1.85k stars 143 forks source link

Wiki instructions for docker build are outdated #142

Closed jinnatar closed 1 year ago

jinnatar commented 1 year ago

Would've made a PR but the wiki editing seems to be locked down. Here's a patch instead:

diff --git a/How-to-Build-the-Firmware-Samples.md b/How-to-Build-the-Firmware-Samples.md
index 648fa14..9e70284 100644
--- a/How-to-Build-the-Firmware-Samples.md
+++ b/How-to-Build-the-Firmware-Samples.md
@@ -10,7 +10,7 @@ We support [different nRF52 SoCs in different hardware versions](https://github.
 # With Docker
 To build the `ble` sample for the `nRF52840`, revision `1.2.0`, `cd` into `code/nrf-connect/` and run:
 ```
-$ docker run --rm -v ${PWD}:/code -w /code/samples/ble nordicplayground/nrfconnect-sdk:v2.2-branch west build --build-dir ./build --pristine --board bparasite_nrf52840@1.2.0
+docker run --rm -it -v ${PWD}/code:/code -w /code/nrf-connect/samples/ble nordicplayground/nrfconnect-sdk:v2.2-branch west build --build-dir ./build --pristine --board bparasite_nrf52840@1.2.0
 ```
 The output will be in `code/nrf-connect/samples/ble/build/zephyr/zephyr.hex`.
rbaron commented 1 year ago

Hi @Artanicus,

I think the issue is that the existing command must be run in the code/nrf-connect/ directory (as written above there [...]cdintocode/nrf-connect/and run:). With the change, the command must be run in the root of the repo, and that makes the line above it incorrect. The other change (-it) I believe has no effect, since we don't need an interactive shell.

Please let me know if I understood it correctly.

jinnatar commented 1 year ago

You're absolutely right, I just was so used to running builds from the root of a project and didn't notice the guidance above the code snippet!

rbaron commented 1 year ago

Great idea. I updated the instructions and made it more copy-and-paste friendly. Tks!

drspangle commented 6 months ago

I hate to reraise a dead issue, but the current build instructions don't work and I actually used this suggested change as inspiration to fix them. When I simply copy and pasted the commands the docker image that was pulled didn't seem to have zephyr set up properly at all, and the west tool didn't have a build command. So instead I ran an interactive bash shell (rather than running the actual command):

# Make sure you start in the correct directory as the documentation currently states.
cd code/nrf-connect/

# Rather than running everything in one command, we're going to need to start an interactive shell.
docker run -it --rm -v ${PWD}:/code -w /code/samples/ble nordicplayground/nrfconnect-sdk:v2.5-branch bash

# Within the nordicplayground/nrfconnect-sdk:v2.5-branch container's shell, execute the following...
# This will take a while... 
# The nordicplayground/nrfconnect-sdk:v2.5-branch container seemingly doesn't have many of the deps set up already.
west init && west update

# Now build the binary for the b-parasite.
# Remember to put your hardware revision at the end of this command, I happen to be using 1.2.0.
west build --build-dir ./build --pristine --board bparasite_nrf52840@1.2.0

When I did it this way it output the file as expected. It seemed the west init was necessary (and required an interactive shell) to get everything set up. I had expected the container image to have this already set up, but it clearly wasn't.

rbaron commented 6 months ago

@drspangle indeed it seems like the Docker image changed how it handles freestanding Zephyr applications.

From their docs (under "freestanding application"), I think we don't have to run west init nor west update, but we can do it like this:

From the repo's root:

docker run --rm -v ${PWD}:/repo nordicplayground/nrfconnect-sdk:v2.5-branch west build --pristine --board bparasite_nrf52840@2.0.0 --build-dir /repo/code/nrf-connect/samples/ble/build /repo/code/nrf-connect/samples/ble

This is basically what I set up in the GitHub action that runs on every commit.

Could you try this one and let me know if it works for you too? I will then update the wiki.

drspangle commented 6 months ago

@rbaron Confirming that the new command works. I purged my local docker image cache and cloned the repo from scratch, then executed the command. It took only a minute. Definitely saves a huge amount of time pulling down all the deps into the docker container. Great!