app: nrfcloud: Add GCI support to cellpos#270
Conversation
fae69a2 to
242ede0
Compare
242ede0 to
9a28acf
Compare
|
You can find the documentation preview for this PR here. |
bd39dd1 to
fd3f8cc
Compare
|
This is ready for review. I'll try to add unit tests also into the PR. |
There was a problem hiding this comment.
Pull request overview
This PR adds internal AT%NCELLMEAS-based cellular positioning for AT#XNRFCLOUDPOS, including GCI parsing, and updates nRF Cloud/GNSS integration plus related documentation.
Changes:
- Reworks nRF Cloud location requests from
<cell_pos>modes to a<cell_count>-based measurement flow. - Adds NCELLMEAS/GCI parsing and asynchronous location request handling.
- Updates A-GNSS cell-info collection and command documentation/migration notes.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
doc/releases/migration_notes_v2.0.0.rst |
Documents the AT#XNRFCLOUDPOS parameter migration. |
doc/releases/migration_notes_ncs_slm_v3.1.x.rst |
Adds migration guidance for the new nRF Cloud positioning behavior. |
doc/app/at_nrfcloud.rst |
Updates command reference syntax and notes for <cell_count> and internal NCELLMEAS use. |
app/src/sm_at_nrfcloud.h |
Replaces single-cell info API with the new NCELLMEAS helper declaration. |
app/src/sm_at_nrfcloud.c |
Implements NCELLMEAS/GCI parsing and location request orchestration. |
app/src/sm_at_gnss.c |
Switches A-GNSS network info collection to the new NCELLMEAS helper. |
Comments suppressed due to low confidence (2)
app/src/sm_at_nrfcloud.c:919
- This PLMN string is also read with
at_parser_string_get()but is not NUL-terminated beforestring_to_int(&tmp_str[3], ...)is called. Because the parser usesmemcpy, the MNC parse can read beyond the PLMN field and produce spurious failures or incorrect MCC/MNC values for GCI cells.
curr_index++;
err = at_parser_string_get(&parser, curr_index, tmp_str, &len);
if (err) {
LOG_ERR("Could not parse plmn, error: %d", err);
goto clean_exit;
}
/* Read MNC and store as integer. The MNC starts as the fourth character
* in the string, following three characters long MCC.
*/
err = string_to_int(&tmp_str[3], 10, &parsed_cell.mnc);
if (err) {
app/src/sm_at_nrfcloud.c:1055
- For the normal
%NCELLMEASformat, the first neighbor field is at indexAT_NCELLMEAS_PRE_NCELLS_PARAMS_COUNT(the previous parser used offset 11). Initializingcurr_indexto 11 and then pre-incrementing inparse_ncellmeas_neighbors()starts at index 12, so neighbor fields are shifted and parsed into the wrong struct members.
int err, status, tmp;
struct at_parser parser;
size_t count = 0;
int curr_index = AT_NCELLMEAS_PRE_NCELLS_PARAMS_COUNT;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
be5482c to
fabb777
Compare
MarkusLassila
left a comment
There was a problem hiding this comment.
This is getting difficult to review.
Approving as this needs CI-time and testing.
1db1d18 to
e03545b
Compare
MarkusLassila
left a comment
There was a problem hiding this comment.
The code changes are OK.
Rebase, fix the problems in compliance and the flash running out.
636bb10 to
827382a
Compare
| Other changes | ||
| ************* | ||
|
|
||
| * ``AT#XNRFCLOUDPOS``: |
There was a problem hiding this comment.
Shouldn't these changes only be in the 2.0 migration guide?
There was a problem hiding this comment.
No. Those who transition from NCS SLM 3.1.1 to SM v2.0.0 needs to do same changes.
6e2e3db to
0d7b765
Compare
Implement cell positioning similarly to Location library meaning AT%NCELLMEAS will be done 1-3 times depending on how many cells are requested and found in each attempt. Implementation done by copying GCI parsing from lte_lc. Signed-off-by: Tommi Rantanen <tommi.rantanen@nordicsemi.no>
Add new unit tests for nRF Cloud related functionality. Signed-off-by: Tommi Rantanen <tommi.rantanen@nordicsemi.no>
0d7b765 to
1dac63b
Compare
Implement cell positioning similarly to Location library meaning
AT%NCELLMEASwill be done 1-3 times depending on how many cells are requested and found in each attempt. Implementation done by copying GCI parsing from lte_lc and use of 1-3AT%NCELLMEASoperations from location library.Use of memory seen from numbers before:
After:
This change removes use of modem_info from
sm_at_nrfcloud.c. It is still needed for nrf_provisioning and nrf_cloud_agnss.Jira: SM-223
TODO: