lib: lte_link_control: Protect ncellmeas with semaphore

We need to protect execution of AT%NCELLMEAS with a semaphore.
Otherwise a client that calls new lte_lc_neighbor_cell_measurement()
right after receiving LTE_LC_EVT_NEIGHBOR_CELL_MEAS event,
may get -EINPROGRESS error code even though AT%NCELLMEAS
notification had been received.

Signed-off-by: Tommi Rantanen <tommi.rantanen@nordicsemi.no>

lib/location/scan_cellular.c

@@ -139,16 +139,13 @@ int scan_cellular_start(uint8_t cell_count)
  */
  scan_cellular_cell_count = scan_cellular_cell_count - scan_cellular_info.ncells_count;
 
- if (scan_cellular_cell_count > 0) {
+ LOG_DBG("scan_cellular_start: scan_cellular_cell_count=%d", scan_cellular_cell_count);
+
+ if (scan_cellular_cell_count > 1) {
 
  ncellmeas_params.search_type = LTE_LC_NEIGHBOR_SEARCH_TYPE_GCI_EXTENDED_LIGHT;
  ncellmeas_params.gci_count = scan_cellular_cell_count;
 
- /* GCI count given to modem cannot be one */
- if (ncellmeas_params.gci_count == 1) {
- ncellmeas_params.gci_count++;
- }
-
  err = lte_lc_neighbor_cell_measurement(&ncellmeas_params);
  if (err) {
  LOG_WRN("Failed to initiate GCI cell measurements: %d", err);

lib/lte_link_control/lte_lc.c

@@ -67,7 +67,8 @@ static char rai_param[2] = CONFIG_LTE_RAI_REQ_VALUE;
 
 /* Requested NCELLMEAS params */
 static struct lte_lc_ncellmeas_params ncellmeas_params;
-static bool ncellmeas_ongoing;
+/* Sempahore value 1 means ncellmeas is not ongoing, and 0 means it's ongoing. */
+K_SEM_DEFINE(ncellmeas_idle_sem, 1, 1);
 
 static const enum lte_lc_system_mode sys_mode_preferred = SYS_MODE_PREFERRED;
 
@@ -394,7 +395,7 @@ static void at_handler_ncellmeas(const char *response)
 
  __ASSERT_NO_MSG(response != NULL);
 
- if (event_handler_list_is_empty() || !ncellmeas_ongoing) {
+ if (event_handler_list_is_empty() || k_sem_count_get(&ncellmeas_idle_sem) > 0) {
  /* No need to parse the response if there is no handler
  * to receive the parsed data or
  * if a measurement is not going/started by using
@@ -445,7 +446,7 @@ static void at_handler_ncellmeas(const char *response)
  k_free(neighbor_cells);
  }
 exit:
- ncellmeas_ongoing = false;
+ k_sem_give(&ncellmeas_idle_sem);
 }
 
 static void at_handler_xmodemsleep(const char *response)
@@ -1461,7 +1462,8 @@ int lte_lc_neighbor_cell_measurement(struct lte_lc_ncellmeas_params *params)
  LTE_LC_NEIGHBOR_SEARCH_TYPE_GCI_EXTENDED_COMPLETE),
  "Invalid argument, API does not accept enum values directly anymore");
 
- if (ncellmeas_ongoing) {
+ if (k_sem_take(&ncellmeas_idle_sem, K_SECONDS(1)) != 0) {
+ LOG_WRN("Neighbor cell measurement already in progress");
  return -EINPROGRESS;
  }
 
@@ -1493,9 +1495,9 @@ int lte_lc_neighbor_cell_measurement(struct lte_lc_ncellmeas_params *params)
 
  if (err) {
  err = -EFAULT;
+ k_sem_give(&ncellmeas_idle_sem);
  } else {
  ncellmeas_params = used_params;
- ncellmeas_ongoing = true;
  }
 
  return err;
@@ -1508,7 +1510,8 @@ int lte_lc_neighbor_cell_measurement_cancel(void)
  if (err) {
  err = -EFAULT;
  }
- ncellmeas_ongoing = false;
+
+ k_sem_give(&ncellmeas_idle_sem);
 
  return err;
 }

tests/lib/location/src/location_test.c

@@ -49,8 +49,8 @@ static const char ncellmeas_resp[] =
  "150344527,2300,8,60,29,0,2400,11,55,26,184\r\n";
 
 static const char ncellmeas_gci_resp[] =
- "%NCELLMEAS:0,\"00023906\",\"24405\",\"5A00\",64,1830518,6400,116,53,16,1840265,1,0,"
- "\"001E3904\",\"24405\",\"5A00\",65535,0,6400,42,48,7,1840265,0,0\r\n";
+ "%NCELLMEAS:0,\"00011B07\",\"26295\",\"00B7\",10512,9034,2300,7,63,31,150344527,1,0,"
+ "\"00011B08\",\"26295\",\"00B7\",65535,0,2300,9,62,30,150345527,0,0\r\n";
 
 char http_resp[512];
 
@@ -545,14 +545,14 @@ void test_location_request_default(void)
  * Otherwise, lte_lc would ignore NCELLMEAS notification because no NCELLMEAS on going
  * from lte_lc point of view.
  */
- k_sleep(K_MSEC(1000));
+ k_sleep(K_MSEC(100));
 
  /* Trigger NCELLMEAS response which further triggers the rest of the location calculation */
  at_monitor_dispatch(ncellmeas_resp);
 
  cellular_rest_req_resp_handle();
 
- k_sleep(K_MSEC(1000));
+ k_sleep(K_MSEC(100));
  at_monitor_dispatch(ncellmeas_gci_resp);
 }
 

tests/lib/lte_lc_api/src/lte_lc_api_test.c

@@ -975,6 +975,22 @@ void test_lte_lc_cereg(void)
  at_monitor_dispatch(at_notif);
 }
 
+/* Test failing neighbor cell measurement first to see that the semaphore is given properly */
+void test_lte_lc_neighbor_cell_measurement_fail(void)
+{
+ int ret;
+
+ struct lte_lc_ncellmeas_params params = {
+ .search_type = LTE_LC_NEIGHBOR_SEARCH_TYPE_EXTENDED_COMPLETE,
+ .gci_count = 0,
+ };
+
+ __cmock_nrf_modem_at_printf_ExpectAndReturn("AT%%NCELLMEAS=2", -NRF_ENOMEM);
+
+ ret = lte_lc_neighbor_cell_measurement(&params);
+ TEST_ASSERT_EQUAL(-EFAULT, ret);
+}
+
 void test_lte_lc_neighbor_cell_measurement_neighbors(void)
 {
  int ret;