location: Refactor cloud request data handling

Introduce a unified `location_cloud_request_data` struct for improved
data transfer between modules.

Changes:
- Add `location_helper.[ch]` with `location_cloud_request_data_copy()`
        to convert location library data to module format
- Update `location.c` and `location.h` to use new struct and helper
- Refactor `cloud_location.c` to reconstruct cellular and Wi-Fi data
        from the new struct, improving modularity and clarity
- Increase thread stack sizes for cloud, location, and storage modules
- Add Kconfig options for max Wi-Fi APs and neighbor cells
- Update CMakeLists.txt to include new helper source

This fixes an issue where the cloud location request
would point to memory allocated in the location library and
not use actual data copies. This broke storage of location
because all the entries would point to the same memory location.

Signed-off-by: Simen S. Røstad <simen.rostad@nordicsemi.no>

Author: simensrostad

app/src/main.c

@@ -477,8 +477,6 @@ static void sampling_begin_common(struct main_state *state_object)
 
 		return;
 	}
-
-	LOG_ERR("LED pattern published");
 #endif /* CONFIG_APP_LED */
 
 	state_object->sample_start_time = k_uptime_seconds();

app/src/modules/cloud/Kconfig.cloud

@@ -82,7 +82,7 @@ config APP_CLOUD_BACKOFF_MAX_SECONDS
 
 config APP_CLOUD_THREAD_STACK_SIZE
 	int "Thread stack size"
-	default 6144
+	default 6272
 
 config APP_CLOUD_MESSAGE_QUEUE_SIZE
 	int "Message queue size"

app/src/modules/cloud/cloud_location.c

@@ -21,6 +21,149 @@ LOG_MODULE_DECLARE(cloud, CONFIG_APP_CLOUD_LOG_LEVEL);
 
 #define AGNSS_MAX_DATA_SIZE 3800
 
+/**
+ * @brief Reconstruct cellular network information from location module format
+ *
+ * This function converts cellular network data from the location module's format
+ * (location_cloud_request_data) to the LTE link controller's format (lte_lc_cells_info).
+ * It copies the current cell information, neighbor cells, and GCI (Global Cell Identity)
+ * cells into the provided destination structures.
+ *
+ * The function performs validation to ensure sufficient buffer sizes are provided for
+ * both neighbor cells and GCI cells arrays before performing the copy operation.
+ *
+ * @param[out] dest Pointer to destination lte_lc_cells_info structure to be populated
+ * @param[out] neighbor_cells Array to store reconstructed neighbor cell information
+ * @param[in] neighbor_cells_count Size of the neighbor_cells array
+ * @param[out] gci_cells Array to store reconstructed GCI cell information
+ * @param[in] gci_cells_count Size of the gci_cells array
+ * @param[in] src Pointer to source location_cloud_request_data structure containing
+ *                the cellular data to be converted
+ *
+ * @retval 0 Success
+ * @retval -EINVAL Invalid NULL parameter provided
+ * @retval -ENOMEM Insufficient buffer size for neighbor_cells or gci_cells arrays
+ */
+#if defined(CONFIG_LOCATION_METHOD_CELLULAR)
+static int reconstruct_cellular_data(struct lte_lc_cells_info *dest,
+				     struct lte_lc_ncell *neighbor_cells,
+				     size_t neighbor_cells_count,
+				     struct lte_lc_cell *gci_cells,
+				     size_t gci_cells_count,
+				     const struct location_cloud_request_data *src)
+{
+	if (!dest || !src || !neighbor_cells || !gci_cells) {
+		LOG_ERR("Invalid NULL parameter(s) provided");
+		return -EINVAL;
+	}
+
+	if (neighbor_cells_count < src->ncells_count) {
+		LOG_ERR("Insufficient neighbor_cells_count: %zu, required: %d",
+			neighbor_cells_count, src->ncells_count);
+		return -ENOMEM;
+	}
+
+	if (gci_cells_count < src->gci_cells_count) {
+		LOG_ERR("Insufficient gci_cells_count: %zu, required: %d",
+			gci_cells_count, src->gci_cells_count);
+		return -ENOMEM;
+	}
+
+	/* Copy current cell information */
+	dest->current_cell.mcc = src->current_cell.mcc;
+	dest->current_cell.mnc = src->current_cell.mnc;
+	dest->current_cell.id = src->current_cell.id;
+	dest->current_cell.tac = src->current_cell.tac;
+	dest->current_cell.timing_advance = src->current_cell.timing_advance;
+	dest->current_cell.earfcn = src->current_cell.earfcn;
+	dest->current_cell.rsrp = src->current_cell.rsrp;
+	dest->current_cell.rsrq = src->current_cell.rsrq;
+
+	dest->ncells_count = src->ncells_count;
+	dest->gci_cells_count = src->gci_cells_count;
+
+	/* Copy neighbor cells */
+	for (uint8_t i = 0; i < src->ncells_count; i++) {
+		neighbor_cells[i].earfcn = src->neighbor_cells[i].earfcn;
+		neighbor_cells[i].time_diff = src->neighbor_cells[i].time_diff;
+		neighbor_cells[i].phys_cell_id = src->neighbor_cells[i].phys_cell_id;
+		neighbor_cells[i].rsrp = src->neighbor_cells[i].rsrp;
+		neighbor_cells[i].rsrq = src->neighbor_cells[i].rsrq;
+	}
+
+	dest->neighbor_cells = neighbor_cells;
+
+	/* Copy GCI cells */
+	for (uint8_t i = 0; i < src->gci_cells_count; i++) {
+		gci_cells[i].id = src->gci_cells[i].id;
+		gci_cells[i].mcc = src->gci_cells[i].mcc;
+		gci_cells[i].mnc = src->gci_cells[i].mnc;
+		gci_cells[i].tac = src->gci_cells[i].tac;
+		gci_cells[i].timing_advance = src->gci_cells[i].timing_advance;
+		gci_cells[i].earfcn = src->gci_cells[i].earfcn;
+		gci_cells[i].rsrp = src->gci_cells[i].rsrp;
+		gci_cells[i].rsrq = src->gci_cells[i].rsrq;
+	}
+
+	dest->gci_cells = gci_cells;
+
+	return 0;
+}
+#endif	/* CONFIG_LOCATION_METHOD_CELLULAR */
+
+#if defined(CONFIG_LOCATION_METHOD_WIFI)
+/**
+ * @brief Reconstruct wifi_scan_info structure from location module format
+ *
+ * This function converts location module's WiFi access point data format into
+ * the wifi_scan_info structure format. It copies RSSI values, MAC addresses,
+ * and MAC address lengths for each access point.
+ *
+ * @param[out] dest Pointer to destination wifi_scan_info structure to populate
+ * @param[out] ap_info Array to store wifi_scan_result entries
+ * @param[in] ap_info_count Size of the ap_info array (number of entries)
+ * @param[in] src Pointer to source location_cloud_request_data structure
+ *
+ * @retval 0 On success
+ * @retval -EINVAL If any pointer parameter is NULL
+ * @retval -ENOMEM If ap_info_count is insufficient to hold all WiFi APs from src
+ *
+ * @note This function is only available when CONFIG_LOCATION_METHOD_WIFI is defined
+ * @note The dest->ap_info will point to the provided ap_info array
+ * @note WiFi MAC addresses are expected to be WIFI_MAC_ADDR_LEN bytes long
+ */
+static int reconstruct_wifi_data(struct wifi_scan_info *dest,
+				  struct wifi_scan_result *ap_info,
+				  size_t ap_info_count,
+				  const struct location_cloud_request_data *src)
+{
+	if (!dest || !src || !ap_info) {
+		LOG_ERR("Invalid NULL parameter(s) provided");
+		return -EINVAL;
+	}
+
+	if ((ap_info_count < src->wifi_cnt)) {
+		LOG_ERR("Insufficient ap_info_count: %zu, required: %d",
+			ap_info_count, src->wifi_cnt);
+		return -ENOMEM;
+	}
+
+	/* Copy WiFi AP data */
+	for (uint16_t i = 0; i < ap_info_count; i++) {
+		ap_info[i].rssi = src->wifi_aps[i].rssi;
+		memcpy(ap_info[i].mac,
+		       src->wifi_aps[i].mac,
+		       WIFI_MAC_ADDR_LEN);
+		ap_info[i].mac_length = src->wifi_aps[i].mac_length;
+	}
+
+	dest->ap_info = ap_info;
+	dest->cnt = ap_info_count;
+
+	return 0;
+}
+#endif /* CONFIG_LOCATION_METHOD_WIFI */
+
 static void send_request_failed(void)
 {
 	int err;
@@ -34,7 +177,7 @@ static void send_request_failed(void)
 }
 
 /* Handle cloud location requests from the location module */
-static void handle_cloud_location_request(const struct location_data_cloud *request)
+static void handle_cloud_location_request(const struct location_cloud_request_data *request)
 {
 	int err;
 	struct nrf_cloud_location_config loc_config = {
@@ -48,32 +191,48 @@ static void handle_cloud_location_request(const struct location_data_cloud *requ
 	LOG_DBG("Handling cloud location request");
 
 #if defined(CONFIG_LOCATION_METHOD_CELLULAR)
-	if (request->cell_data != NULL) {
-		/* Cast away const: nRF Cloud API limitation - struct fields are non-const
-		 * but the data is not modified by the API
-		 */
-		loc_req.cell_info = (struct lte_lc_cells_info *)request->cell_data; /* NOSONAR */
+	struct lte_lc_cells_info cell_info = { 0 };
+	struct lte_lc_ncell neighbor_cells[CONFIG_LTE_NEIGHBOR_CELLS_MAX];
+	struct lte_lc_cell gci_cells[CONFIG_LTE_NEIGHBOR_CELLS_MAX];
+
+	if ((request->current_cell.id != LTE_LC_CELL_EUTRAN_ID_INVALID) &&
+	    (request->ncells_count > 0)) {
+		err = reconstruct_cellular_data(&cell_info, neighbor_cells,
+						ARRAY_SIZE(neighbor_cells),
+						gci_cells,
+						ARRAY_SIZE(gci_cells),
+						request);
+		if (err) {
+			LOG_ERR("Failed to reconstruct cellular data, error: %d", err);
+			SEND_FATAL_ERROR();
+			return;
+		}
 
-		LOG_DBG("Cellular data present: current cell ID: %d, neighbor cells: %d, "
-			"GCI cells count: %d",
-			request->cell_data->current_cell.id,
-			request->cell_data->ncells_count,
-			request->cell_data->gci_cells_count);
+		loc_req.cell_info = &cell_info;
 	}
-#endif
+#endif /* CONFIG_LOCATION_METHOD_CELLULAR */
 
 #if defined(CONFIG_LOCATION_METHOD_WIFI)
-	if (request->wifi_data != NULL) {
-		/* Cast away const: nRF Cloud API limitation - struct fields are non-const
-		 * but the data is not modified by the API
-		 */
-		loc_req.wifi_info = (struct wifi_scan_info *)request->wifi_data; /* NOSONAR */
+	struct wifi_scan_result ap_info[CONFIG_LOCATION_METHOD_WIFI_SCANNING_RESULTS_MAX_CNT];
+	struct wifi_scan_info wifi_info = { 0 };
+
+	if (request->wifi_cnt > 0) {
+		err = reconstruct_wifi_data(&wifi_info, ap_info, ARRAY_SIZE(ap_info), request);
+		if (err) {
+			LOG_ERR("Failed to reconstruct Wi-Fi data, error: %d", err);
+			SEND_FATAL_ERROR();
+			return;
+		}
 
-		LOG_DBG("Wi-Fi data present: %d APs", request->wifi_data->cnt);
+		loc_req.wifi_info = &wifi_info;
+	}
+#endif /* CONFIG_LOCATION_METHOD_WIFI */
+
+	if (!loc_req.cell_info && !loc_req.wifi_info) {
+		LOG_ERR("No cellular or Wi-Fi data provided for location request");
+		return;
 	}
-#endif
 
-	/* Send location request to nRF Cloud */
 	err = nrf_cloud_coap_location_get(&loc_req, &result);
 	if (err == COAP_RESPONSE_CODE_NOT_FOUND) {
 		LOG_WRN("nRF Cloud CoAP location coordinates not found, error: %d", err);

app/src/modules/location/CMakeLists.txt

@@ -4,5 +4,8 @@
 # SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
 #
 
-target_sources(app PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/location.c)
+target_sources(app PRIVATE
+	${CMAKE_CURRENT_SOURCE_DIR}/location.c
+	${CMAKE_CURRENT_SOURCE_DIR}/location_helper.c
+)
 target_include_directories(app PRIVATE .)

app/src/modules/location/Kconfig.location

@@ -12,7 +12,7 @@ if APP_LOCATION
 
 config APP_LOCATION_THREAD_STACK_SIZE
 	int "Thread stack size"
-	default 2048
+	default 4096
 
 config APP_LOCATION_WATCHDOG_TIMEOUT_SECONDS
 	int "Watchdog timeout"
@@ -34,6 +34,18 @@ config APP_LOCATION_MSG_PROCESSING_TIMEOUT_SECONDS
 	  Maximum time allowed for processing a single message in the module's state machine.
 	  The value must be smaller than CONFIG_APP_LOCATION_WATCHDOG_TIMEOUT_SECONDS.
 
+config APP_LOCATION_WIFI_APS_MAX
+	int "Maximum number of Wi-Fi APs"
+	default 10
+	help
+	  Maximum number of Wi-Fi access points to store in a location request.
+
+config APP_LOCATION_NEIGHBOR_CELLS_MAX
+	int "Maximum number of neighbor cells"
+	default 10
+	help
+	  Maximum number of neighbor cells to store in a location request.
+
 module = APP_LOCATION
 module-str = Location
 source "subsys/logging/Kconfig.template.log_config"

app/src/modules/location/location.c

@@ -18,13 +18,22 @@
 #include "app_common.h"
 #include "modem/lte_lc.h"
 #include "location.h"
+#include "location_helper.h"
 
 LOG_MODULE_REGISTER(location_module, CONFIG_APP_LOCATION_LOG_LEVEL);
 
 BUILD_ASSERT(CONFIG_APP_LOCATION_WATCHDOG_TIMEOUT_SECONDS >
 	     CONFIG_APP_LOCATION_MSG_PROCESSING_TIMEOUT_SECONDS,
 	     "Watchdog timeout must be greater than maximum message processing time");
 
+#if defined(CONFIG_LOCATION_METHOD_CELLULAR)
+BUILD_ASSERT(CONFIG_APP_LOCATION_NEIGHBOR_CELLS_MAX >= CONFIG_LTE_NEIGHBOR_CELLS_MAX);
+#endif /* CONFIG_LOCATION_METHOD_CELLULAR */
+
+#if defined(CONFIG_LOCATION_METHOD_WIFI)
+BUILD_ASSERT(CONFIG_APP_LOCATION_WIFI_APS_MAX >= CONFIG_LOCATION_METHOD_WIFI_SCANNING_RESULTS_MAX_CNT);
+#endif /* CONFIG_LOCATION_METHOD_WIFI */
+
 /* Define channels provided by this module */
 ZBUS_CHAN_DEFINE(LOCATION_CHAN,
 		 struct location_msg,
@@ -116,9 +125,15 @@ static void cloud_request_send(const struct location_data_cloud *cloud_request)
 	int err;
 	struct location_msg location_msg = {
 		.type = LOCATION_CLOUD_REQUEST,
-		.cloud_request = *cloud_request
 	};
 
+	err = location_cloud_request_data_copy(&location_msg.cloud_request, cloud_request);
+	if (err) {
+		LOG_ERR("location_cloud_request_data_copy, error: %d", err);
+		SEND_FATAL_ERROR();
+		return;
+	}
+
 	err = zbus_chan_pub(&LOCATION_CHAN, &location_msg, K_SECONDS(1));
 	if (err) {
 		LOG_ERR("zbus_chan_pub, error: %d", err);

app/src/modules/location/location.h

@@ -79,14 +79,56 @@ enum location_msg_type {
 	LOCATION_SEARCH_CANCEL,
 };
 
+struct location_wifi_ap_info {
+	int8_t rssi;
+	uint8_t mac[WIFI_MAC_ADDR_LEN];
+	uint8_t mac_length;
+};
+
+struct location_cell_info {
+	uint32_t id;             /* Cell ID (ECI) */
+	int mcc;                 /* Mobile Country Code */
+	int mnc;                 /* Mobile Network Code */
+	uint32_t tac;            /* Tracking area Code */
+
+	uint16_t timing_advance; /* Timing advance */
+	uint32_t earfcn;
+	int16_t rsrp;
+	int16_t rsrq;
+};
+
+struct location_neighbor_cell_info {
+	uint32_t earfcn;       /* E-UTRA Absolute Radio Frequency Channel Number */
+	int time_diff;         /* Time difference for neighbor cells */
+	uint16_t phys_cell_id; /* Physical Cell ID (PCI) */
+	int16_t rsrp;          /* Reference Signal Received Power */
+	int16_t rsrq;          /* Reference Signal Received Quality */
+};
+
+struct location_cloud_request_data {
+	/* Current cell information, valid if current_cell.id != LTE_LC_CELL_EUTRAN_ID_INVALID */
+	struct location_cell_info current_cell;
+
+	/* Neighboring and GCI cell information, valid if count > 0 */
+	uint8_t ncells_count;
+	struct location_neighbor_cell_info neighbor_cells[CONFIG_APP_LOCATION_NEIGHBOR_CELLS_MAX];
+
+	uint8_t gci_cells_count;
+	struct location_cell_info gci_cells[CONFIG_APP_LOCATION_NEIGHBOR_CELLS_MAX];
+
+	/* Wi-Fi data, valid if wifi_cnt > 0 */
+	uint16_t wifi_cnt;
+	struct location_wifi_ap_info wifi_aps[CONFIG_APP_LOCATION_WIFI_APS_MAX];
+};
+
 /* Structure to pass location data through zbus */
 struct location_msg {
 	enum location_msg_type type;
 	union {
 		/** Contains cloud location request data with cellular and/or Wi-Fi information.
 		 *  cloud_request is valid for LOCATION_CLOUD_REQUEST events.
 		 */
-		struct location_data_cloud cloud_request;
+		struct location_cloud_request_data cloud_request;
 
 		/** Contains A-GNSS assistance data request parameters.
 		 *  agnss_request is valid for LOCATION_AGNSS_REQUEST events.