Skip to content

app: nrfcloud: Add GCI support to cellpos#270

Open
trantanen wants to merge 2 commits into
nrfconnect:mainfrom
trantanen:nrfcloud_cellpos_gci
Open

app: nrfcloud: Add GCI support to cellpos#270
trantanen wants to merge 2 commits into
nrfconnect:mainfrom
trantanen:nrfcloud_cellpos_gci

Conversation

@trantanen
Copy link
Copy Markdown
Collaborator

@trantanen trantanen commented Apr 29, 2026

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 and use of 1-3 AT%NCELLMEAS operations from location library.

Use of memory seen from numbers before:

           FLASH:      363716 B     413546 B     87.95%
             RAM:      175600 B       206 KB     83.24%

After:

           FLASH:      367148 B     413546 B     88.78%
             RAM:      175616 B       206 KB     83.25%

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:

  • Squash commits. They exist to show reviewers a bit on what has been done.
  • Verify A-GNSS functionality
  • Unit tests

@trantanen trantanen requested review from a team and tokangas April 29, 2026 08:58
@trantanen trantanen force-pushed the nrfcloud_cellpos_gci branch 2 times, most recently from fae69a2 to 242ede0 Compare May 5, 2026 07:13
@trantanen trantanen force-pushed the nrfcloud_cellpos_gci branch from 242ede0 to 9a28acf Compare May 12, 2026 05:59
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 12, 2026

You can find the documentation preview for this PR here.

@trantanen trantanen force-pushed the nrfcloud_cellpos_gci branch 4 times, most recently from bd39dd1 to fd3f8cc Compare May 12, 2026 15:47
@trantanen trantanen marked this pull request as ready for review May 12, 2026 15:48
@trantanen
Copy link
Copy Markdown
Collaborator Author

This is ready for review. I'll try to add unit tests also into the PR.
A lot of the parsing functions have been copied from lte_lc and I'm not sure how much we should fine-tune them.
The logical change of using AT%NCELLMEAS from SM instead of host triggering it is something to be noted.
I'd like to get this merged in a week or so to get testing for this over the following weeks in CI.

Comment thread app/src/sm_at_gnss.c
Comment thread app/src/sm_at_nrfcloud.h Outdated
Comment thread app/src/sm_at_nrfcloud.c
Comment thread app/src/sm_at_nrfcloud.c
Comment thread app/src/sm_at_nrfcloud.c
Comment thread app/src/sm_at_nrfcloud.c
Comment thread app/src/sm_at_nrfcloud.c Outdated
Comment thread app/src/sm_at_nrfcloud.c Outdated
Comment thread app/src/sm_at_nrfcloud.c
Comment thread app/src/sm_at_nrfcloud.c Outdated
Comment thread app/src/sm_at_nrfcloud.c Outdated
Copy link
Copy Markdown

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 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 before string_to_int(&tmp_str[3], ...) is called. Because the parser uses memcpy, 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 %NCELLMEAS format, the first neighbor field is at index AT_NCELLMEAS_PRE_NCELLS_PARAMS_COUNT (the previous parser used offset 11). Initializing curr_index to 11 and then pre-incrementing in parse_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.

Comment thread app/src/sm_at_gnss.c Outdated
Comment thread app/src/sm_at_nrfcloud.c Outdated
Comment thread app/src/sm_at_nrfcloud.c Outdated
Comment thread app/src/sm_at_nrfcloud.c
Comment thread app/src/sm_at_nrfcloud.c
Comment thread app/src/sm_at_nrfcloud.h Outdated
Comment thread app/src/sm_at_nrfcloud.c Outdated
Comment thread app/src/sm_at_nrfcloud.c Outdated
Comment thread app/src/sm_at_nrfcloud.c Outdated
Comment thread app/src/sm_at_nrfcloud.h Outdated
Comment thread app/src/sm_at_gnss.c Outdated
Comment thread app/src/sm_at_nrfcloud.c Outdated
Comment thread app/src/sm_at_nrfcloud.c Outdated
Comment thread app/src/sm_at_nrfcloud.c Outdated
@trantanen trantanen force-pushed the nrfcloud_cellpos_gci branch from be5482c to fabb777 Compare May 25, 2026 13:02
Copy link
Copy Markdown
Contributor

@MarkusLassila MarkusLassila left a comment

Choose a reason for hiding this comment

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

This is getting difficult to review.

Approving as this needs CI-time and testing.

Comment thread app/src/sm_at_nrfcloud.c Outdated
Copy link
Copy Markdown

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 8 out of 8 changed files in this pull request and generated 7 comments.

Comment thread app/src/sm_at_nrfcloud.c Outdated
Comment thread app/src/sm_at_nrfcloud.c Outdated
Comment thread app/src/sm_at_nrfcloud.c Outdated
Comment thread app/src/sm_at_nrfcloud.h Outdated
Comment thread app/src/sm_at_gnss.c Outdated
Comment thread doc/app/at_nrfcloud.rst Outdated
Comment thread app/src/sm_at_nrfcloud.c Outdated
@trantanen trantanen force-pushed the nrfcloud_cellpos_gci branch 3 times, most recently from 1db1d18 to e03545b Compare June 1, 2026 19:53
@trantanen trantanen requested a review from MarkusLassila June 2, 2026 09:44
Copy link
Copy Markdown
Contributor

@MarkusLassila MarkusLassila left a comment

Choose a reason for hiding this comment

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

The code changes are OK.

Rebase, fix the problems in compliance and the flash running out.

@trantanen trantanen force-pushed the nrfcloud_cellpos_gci branch 2 times, most recently from 636bb10 to 827382a Compare June 3, 2026 12:03
@trantanen trantanen requested a review from divipillai June 3, 2026 12:27
Comment thread doc/app/at_nrfcloud.rst Outdated
Comment thread doc/app/at_nrfcloud.rst Outdated
Comment thread doc/releases/migration_notes_v2.0.0.rst Outdated
Other changes
*************

* ``AT#XNRFCLOUDPOS``:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shouldn't these changes only be in the 2.0 migration guide?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No. Those who transition from NCS SLM 3.1.1 to SM v2.0.0 needs to do same changes.

@trantanen trantanen force-pushed the nrfcloud_cellpos_gci branch from 6e2e3db to 0d7b765 Compare June 3, 2026 12:58
trantanen added 2 commits June 3, 2026 16:11
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>
@trantanen trantanen force-pushed the nrfcloud_cellpos_gci branch from 0d7b765 to 1dac63b Compare June 3, 2026 13:11
@trantanen trantanen requested a review from divipillai June 3, 2026 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants