-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Update PGPS docs and kconfig #26426
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
Update PGPS docs and kconfig #26426
Conversation
There was a problem hiding this 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 the P-GPS (Predicted GPS) documentation and configuration to reflect current nRF Cloud capabilities, specifically removing the unsupported 120-minute prediction period option and adjusting documentation accordingly.
Key changes:
- Removes the 120-minute prediction period configuration option, leaving only 240-minute periods supported
- Updates maximum prediction count from 84 to 42 (one week instead of two weeks)
- Corrects a typo in the Kconfig help text
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| subsys/net/lib/nrf_cloud/common/src/nrf_cloud_pgps.c | Removed conditional compilation for 120-minute prediction period, hardcoding to 240 minutes |
| subsys/net/lib/nrf_cloud/Kconfig.nrf_cloud_pgps | Removed 120-minute prediction period choice, adjusted range limits for prediction counts, and fixed typo in help text |
| doc/nrf/libraries/networking/nrf_cloud_pgps.rst | Updated documentation to reflect one-week (instead of two-week) prediction support and removed references to 120-minute periods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f4bfc42 to
225b362
Compare
There was a problem hiding this 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 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CI InformationTo view the history of this post, click the 'edited' button above Inputs:Sources:sdk-nrf: PR head: 96f902b96357519c6a89657a7b2e5dc3a81968ef more detailssdk-nrf:
Github labels
List of changed files detected by CI (6)Outputs:ToolchainVersion: f911d4f4e7 Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
|
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-26426/nrf/app_dev/device_guides/nrf91/nrf91_features.html |
225b362 to
2385285
Compare
peknis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this need a changelog entry?
@peknis Yes, that's probably worth mentioning. I can add it. |
|
You would also need to update the nRF Cloud documentation, it mention the 2 weeks period: https://docs.nordicsemi.com/bundle/nrf-cloud/page/LocationServices/Features.html |
There was a problem hiding this 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 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst
Outdated
Show resolved
Hide resolved
@pascal-nordic That text seems to be in a different repo. So won't be part of this PR, but I found some other places in this repo that mentioned "two weeks" that I'll update. |
9b96c15 to
76d89e4
Compare
| Predicted GPS (P-GPS) is a form of assistance that reduces the :term:`Time to First Fix (TTFF)`, the time needed by a GNSS module to estimate its position. | ||
| It is provided through :term:`nRF Cloud` services. | ||
| In P-GPS, nRF Cloud provides data containing information about the estimated orbits (`Ephemerides <Ephemeris_>`_) of the 32 GPS satellites for up to two weeks. | ||
| In P-GPS, nRF Cloud provides data containing information about the estimated orbits (`Ephemerides <Ephemeris_>`_) of the 32 GPS satellites for up to one week. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| In P-GPS, nRF Cloud provides data containing information about the estimated orbits (`Ephemerides <Ephemeris_>`_) of the 32 GPS satellites for up to one week. | |
| In P-GPS, nRF Cloud provides data containing information about the estimated orbits (`Ephemerides <Ephemeris_>`_) of the 32 GPS satellites. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree we shouldn't mention the exact number of days/weeks here, but I think the suggested change is too vague, so it doesn't really separate P-GPS from A-GPS. I changed it to «multiple days into the future» like I've used several other places.
|
nRF Cloud Docs updated: https://github.com/nRFCloud/nrfcloud-docs/pull/459 |
The nRF Cloud REST API only supports 1 week of predictions now. Remove NRF_CLOUD_PGPS_PREDICTION_PERIOD choice because 240 is the only valid value in nRF Cloud. Update max value for NRF_CLOUD_PGPS_NUM_PREDICTIONS and NRF_CLOUD_PGPS_REPLACEMENT_THRESHOLD according to what is supported by nRF Cloud. Add changelog entry for nrf_cloud_pgps library changes. Signed-off-by: Gregers Gram Rygg <[email protected]>
76d89e4 to
96f902b
Compare
There was a problem hiding this 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 no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The documentation and kconfig options for PGPS needs to be updated to reflect whats currently supported by nRF Cloud.