Skip to content
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

Adjust build instructions #2203

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

zyphlar
Copy link

@zyphlar zyphlar commented Dec 18, 2024

cd build before cmake seems to cause issues, as does a cmake command with insufficient details. Finally, many end users and developers will want to OTA flash their firmware, so handy DFU instructions in the correct folder might be appreciated.

`cd build` before `cmake` seems to cause issues, as does a cmake command with insufficient details. Finally, many end users and developers will want to OTA flash their firmware, so handy DFU instructions in the correct folder might be appreciated.
@mark9064 mark9064 added maintenance Background work documentation Improvements or additions to documentation and removed maintenance Background work labels Dec 20, 2024
Copy link
Member

@mark9064 mark9064 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

So you might already know but there is #2157 open which suggests some similar changes cmake wise. Any advantage to doing cmake with -B?

make -j4 pinetime-app
```

However if you want to flash the firmware OTA using an app like Gadgetbridge, using *pinetime-mcuboot-app* will create the `pinetime-mcuboot-app-dfu-$VERSION.zip` file you want in the `build/src` folder.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great addition. Maybe modify the above L84 to specify "PineTime devkit"? Currently these instructions kind of assume a devkit, whereas I think a lot of people work on sealed now as the bootloader is very stable

```
cmake -DARM_NONE_EABI_TOOLCHAIN_PATH=... -DNRF5_SDK_PATH=...
cmake -S "." -B "build" -DCMAKE_BUILD_TYPE=Release -DARM_NONE_EABI_TOOLCHAIN_PATH=... -DNRF5_SDK_PATH=.. -DBUILD_DFU=1 -DBUILD_RESOURCES=1 -DNRF_TARGET=nrf52
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-DNRF_TARGET=nrf52 isn't needed, it's already specified in the cmake file. Release is also the default type, so I don't think that needs to be specified either (it's listed above, and I think people who need debug will know when they need it, you have to be pretty involved at that point)

As for DFU/resources, how about giving two example commands, one which builds neither (as current) and one which builds both, I think that way users can easily get the idea of what the options do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants