Skip to content

samples: nrf_cloud: review docs for cell_location #26088

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.

Sign up for GitHub

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

Jump to bottom
Open
maxd-nordic wants to merge 1 commit into
base: main
Choose a base branch
from
Open

samples: nrf_cloud: review docs for cell_location #26088

maxd-nordic wants to merge 1 commit into from

Conversation

@maxd-nordic
Copy link
Contributor

@maxd-nordic maxd-nordic commented Dec 4, 2025

Fix issues found in the documentation of the cell location samples. Add examples of connection errors.

@maxd-nordic maxd-nordic requested a review from a team as a code owner December 4, 2025 12:54
Copilot AI review requested due to automatic review settings December 4, 2025 12:54
@maxd-nordic maxd-nordic requested a review from a team as a code owner December 4, 2025 12:54
@maxd-nordic maxd-nordic added this to the 3.2.0 milestone Dec 4, 2025
@NordicBuilder NordicBuilder added doc-required PR must not be merged without tech writer approval. changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Dec 4, 2025
@maxd-nordic maxd-nordic added backport v3.2-branch and removed doc-required PR must not be merged without tech writer approval. changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Dec 4, 2025
Copilot AI reviewed Dec 4, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates documentation for three nRF Cloud cell location samples (REST, MQTT, and CoAP) to improve clarity and add troubleshooting information. The changes correct outdated comments and simplify the overview sections while adding practical examples of common connection errors.

Key changes:

  • Updated code comments to clarify JWT authentication requirements
  • Simplified overview sections to reference the location library directly
  • Added troubleshooting examples for missing credentials and registration issues

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
samples/cellular/nrf_cloud_rest_cell_location/src/main.c Updated comment to clarify JWT authentication time requirement
samples/cellular/nrf_cloud_rest_cell_location/README.rst Simplified overview and added credential-related error examples
samples/cellular/nrf_cloud_mqtt_cell_location/src/main.c Updated comment about time requirement
samples/cellular/nrf_cloud_mqtt_cell_location/README.rst Simplified overview and added MQTT connection error examples
samples/cellular/nrf_cloud_coap_cell_location/src/main.c Updated comment to clarify JWT authentication time requirement
samples/cellular/nrf_cloud_coap_cell_location/README.rst Simplified overview and added CoAP-specific error examples

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@NordicBuilder NordicBuilder requested review from a team December 4, 2025 12:54
@maxd-nordic maxd-nordic requested a review from trantanen December 4, 2025 12:56
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Dec 4, 2025 •
edited
Loading

CI Information

To view the history of this post, click the 'edited' button above
Build number: 8

Inputs:

Sources:

sdk-nrf: PR head: b647c4fbc4bd956eea9bf92836d440244edef5ed

more details

sdk-nrf:

PR head: b647c4fbc4bd956eea9bf92836d440244edef5ed
merge base: d0f84cb57d7e07a99e2cd487eeb8c0503dd579de
target head (main): 059095979f2ce8c57fa9213f2e63126475ca3857
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (6) 
samples
│  ├── cellular
│  │  ├── nrf_cloud_coap_cell_location
│  │  │  ├── README.rst
│  │  │  ├── src
│  │  │  │  │ main.c
│  │  ├── nrf_cloud_mqtt_cell_location
│  │  │  ├── README.rst
│  │  │  ├── src
│  │  │  │  │ main.c
│  │  ├── nrf_cloud_rest_cell_location
│  │  │  ├── README.rst
│  │  │  ├── src
│  │  │  │  │ main.c

Outputs:

Toolchain

Version: 43683a87ea
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:43683a87ea_5ea73affbf

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 11
  • ✅ Integration tests
    • ✅ test-fw-nrfconnect-nrf-iot_cloud
Disabled integration tests
    • test-fw-nrfconnect-nrf_lrcs_mosh
    • test-fw-nrfconnect-nrf_lrcs_positioning
    • desktop52_verification
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-chip
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-ps-main
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread-main
    • test-low-level
    • test-sdk-audio
    • test-sdk-dfu
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

peknis
peknis requested changes Dec 4, 2025
View reviewed changes
Copy link
Contributor

@peknis peknis left a comment

Choose a reason for hiding this comment

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

Make sure this can still go to 3.2.0. Talk to Shantha.

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

You can find the documentation preview for this PR here.

Preview links for modified nRF Connect SDK documents:

https://ncsdoc.z6.web.core.windows.net/PR-26088/nrf/samples/cellular/nrf_cloud_coap_cell_location/README.html
https://ncsdoc.z6.web.core.windows.net/PR-26088/nrf/samples/cellular/nrf_cloud_mqtt_cell_location/README.html
https://ncsdoc.z6.web.core.windows.net/PR-26088/nrf/samples/cellular/nrf_cloud_rest_cell_location/README.html

@FrancescoSer FrancescoSer self-requested a review December 5, 2025 11:08
@maxd-nordic maxd-nordic force-pushed the nrfcloud-cell-location-doc-review branch from b241937 to a3f8454 Compare December 8, 2025 09:15
@NordicBuilder NordicBuilder added doc-required PR must not be merged without tech writer approval. changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Dec 8, 2025
@maxd-nordic maxd-nordic requested a review from peknis December 8, 2025 09:16
peknis
peknis requested changes Dec 8, 2025
View reviewed changes
Copilot AI review requested due to automatic review settings December 8, 2025 09:40
Copilot AI reviewed Dec 8, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@maxd-nordic maxd-nordic force-pushed the nrfcloud-cell-location-doc-review branch from 7ad916a to c8724c8 Compare December 8, 2025 09:40
Copilot AI review requested due to automatic review settings December 8, 2025 09:41
@maxd-nordic maxd-nordic force-pushed the nrfcloud-cell-location-doc-review branch from c8724c8 to 9d97909 Compare December 8, 2025 09:41
@maxd-nordic maxd-nordic requested a review from peknis December 8, 2025 09:41
Copilot AI reviewed Dec 8, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

peknis
peknis approved these changes Dec 8, 2025
View reviewed changes
Copy link
Contributor

@peknis peknis left a comment

Choose a reason for hiding this comment

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

Approved with a minor change. Also fix the one Copilot mentions.

Copilot AI review requested due to automatic review settings December 8, 2025 15:16
Copilot AI reviewed Dec 8, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@maxd-nordic
samples: nrf_cloud: review docs for cell_location
b647c4f 
Fix issues found in the documentation of the cell location samples.
Add examples of connection errors.

Signed-off-by: Maximilian Deubel <[email protected]>
@maxd-nordic maxd-nordic force-pushed the nrfcloud-cell-location-doc-review branch from dcae612 to b647c4f Compare December 8, 2025 15:18
trantanen
trantanen reviewed Dec 8, 2025
View reviewed changes
samples/cellular/nrf_cloud_mqtt_cell_location/README.rst
Otherwise, the request is single-cell.

In either mode, the sample sends a new location request if a change in cell ID is detected.
After the sample initializes and connects to the network, it sends a cell location request to nRF Cloud.
Copy link
Contributor

@trantanen trantanen Dec 8, 2025

Choose a reason for hiding this comment

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

multi-cell is still mentioned in the log on line 111. This comes from the code and I guess in the cloud response. This is fine because it's in cloud response so it's not concluded by the device based on NCELLMEAS results etc. anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@trantanen trantanen trantanen approved these changes

@peknis peknis peknis approved these changes

@pascal-nordic pascal-nordic pascal-nordic approved these changes

@FrancescoSer FrancescoSer Awaiting requested review from FrancescoSer

Assignees

No one assigned

Labels

backport v3.2-branch changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. doc-required PR must not be merged without tech writer approval.

Projects

None yet

Milestone

3.2.0

Development

Successfully merging this pull request may close these issues.

5 participants

@maxd-nordic @NordicBuilder @trantanen @peknis @pascal-nordic