moved cell selection/search complete event signalling out of phy_controller

lib/include/srslte/common/fsm.h

@@ -239,7 +239,7 @@ struct apply_first_guard_pass<FSM, type_list<> > {
   template <typename SrcState, typename Event>
   static bool trigger(FSM* f, SrcState& s, const Event& ev)
   {
-    f->log_fsm_activity("FSM \"%s\": Unhandled event caught: \"%s\"\n",
+    f->get_log()->debug("FSM \"%s\": Unhandled event caught: \"%s\"\n",
                         get_type_name<typename FSM::derived_t>().c_str(),
                         get_type_name<Event>().c_str());
     return false;
@@ -492,6 +492,8 @@ public:
 
   srslte::log_ref get_log() const { return log_h; }
 
+  bool is_trigger_locked() const { return trigger_locked; }
+
   //! Log FSM activity method, e.g. state transitions
   template <typename... Args>
   void log_fsm_activity(const char* format, Args&&... args)

srsue/hdr/stack/rrc/phy_controller.h

@@ -49,7 +49,6 @@ public:
   using cell_search_cmd = srslte::event_callback<cell_srch_res>;
   struct in_sync_ev {};
   struct out_sync_ev {};
-  struct timeout_ev {};
 
   explicit phy_controller(phy_interface_rrc_lte* phy_, stack_interface_rrc* stack_);
 
@@ -58,18 +57,13 @@ public:
   bool start_cell_search(const srslte::event_callback<cell_srch_res>& on_complete);
   bool cell_search_completed(cell_search_ret_t cs_ret, phy_cell_t found_cell);
   bool cell_selection_completed(bool outcome);
-  void in_sync() { trigger(in_sync_ev{}); }
+  void in_sync();
   void out_sync() { trigger(out_sync_ev{}); }
 
   // state getters
   bool cell_is_camping() { return phy->cell_is_camping(); }
   bool is_in_sync() const { return is_in_state<in_sync_st>(); }
 
-private:
-  phy_interface_rrc_lte* phy   = nullptr;
-  stack_interface_rrc*   stack = nullptr;
-
-protected:
   // states
   struct unknown_st {};
   struct in_sync_st {};
@@ -86,10 +80,9 @@ protected:
     void enter(phy_controller* f, const cell_sel_cmd& ev);
     void exit(phy_controller* f);
 
-    srslte::timer_handler::unique_timer  wait_in_sync_timer;
+    srslte::timer_handler::unique_timer wait_in_sync_timer;
-    phy_cell_t                           target_cell = {};
+    phy_cell_t                          target_cell = {};
-    cell_sel_res                         result      = {};
+    cell_sel_res                        result      = {};
-    srslte::event_callback<cell_sel_res> csel_callback;
 
   protected:
     // guard functions
@@ -109,16 +102,23 @@ protected:
     row< wait_result,     unknown_st,     cell_sel_res                                           >,
     // +----------------+---------------+--------------+------------------+----------------------+
     row< wait_in_sync,    in_sync_st,     in_sync_ev,    &c::set_success                         >,
-    row< wait_in_sync,    out_sync_st,    timeout_ev                                             >
+    row< wait_in_sync,    unknown_st,     timeout_ev                                             >
     // +----------------+---------------+--------------+------------------+----------------------+
     >;
     // clang-format on
   };
   struct searching_cell {
     void enter(phy_controller* f, const cell_search_cmd& cmd);
-
-    std::vector<srslte::event_callback<cell_srch_res> > csearch_callbacks;
   };
+
+private:
+  phy_interface_rrc_lte* phy   = nullptr;
+  stack_interface_rrc*   stack = nullptr;
+
+  std::vector<srslte::event_callback<cell_srch_res> > csearch_callbacks;
+  srslte::event_callback<cell_sel_res>                csel_callback;
+
+protected:
   state_list<unknown_st, in_sync_st, out_sync_st, searching_cell, selecting_cell> states{this,
                                                                                          unknown_st{},
                                                                                          in_sync_st{},
@@ -148,8 +148,8 @@ protected:
   row< out_sync_st,     searching_cell,   cell_search_cmd >,
   row< out_sync_st,     in_sync_st,       in_sync_ev >,
   // +----------------+-----------------+------------------+------------------------------+---------------------+
-  row< searching_cell,  unknown_st,       cell_srch_res,     &c::handle_cell_search_res >,
+  row< searching_cell,  unknown_st,       cell_srch_res,     &c::handle_cell_search_res                         >,
-  upd< searching_cell,                    cell_search_cmd,   &c::share_cell_search_res >
+  upd< searching_cell,                    cell_search_cmd,   &c::share_cell_search_res                          >
   // +----------------+-----------------+------------------+------------------------------+---------------------+
   >;
   // clang-format on

srsue/src/stack/rrc/phy_controller.cc

@@ -43,13 +43,29 @@ phy_controller::phy_controller(srsue::phy_interface_rrc_lte* phy_, srsue::stack_
 bool phy_controller::start_cell_select(const phy_cell_t&                           phy_cell,
                                        const srslte::event_callback<cell_sel_res>& on_complete)
 {
-  if (not trigger(cell_sel_cmd{phy_cell, on_complete})) {
+  trigger(cell_sel_cmd{phy_cell, on_complete});
-    log_h->warning("Failed to launch cell selection\n");
+  if (not is_in_state<selecting_cell>()) {
+    log_h->warning("Failed to launch cell selection. Current state: %s\n", current_state_name().c_str());
     return false;
   }
   return true;
 }
 
+bool phy_controller::cell_selection_completed(bool outcome)
+{
+  return trigger(outcome);
+}
+
+void phy_controller::in_sync()
+{
+  bool is_selecting_cell = is_in_state<selecting_cell>();
+  trigger(in_sync_ev{});
+  if (is_selecting_cell and not is_in_state<selecting_cell>()) {
+    // Signal result back to FSM that called cell selection
+    csel_callback(get_state<selecting_cell>()->result);
+  }
+}
+
 phy_controller::selecting_cell::selecting_cell(phy_controller* parent_) : nested_fsm_t(parent_)
 {
   wait_in_sync_timer = parent_fsm()->stack->get_unique_timer();
@@ -57,9 +73,9 @@ phy_controller::selecting_cell::selecting_cell(phy_controller* parent_) : nested
 
 void phy_controller::selecting_cell::enter(phy_controller* f, const cell_sel_cmd& ev)
 {
-  target_cell   = ev.phy_cell;
+  target_cell      = ev.phy_cell;
-  csel_callback = ev.callback;
+  f->csel_callback = ev.callback;
-  result        = false;
+  result           = false;
 
   f->log_h->info("Starting \"%s\" for pci=%d, earfcn=%d\n",
                  srslte::get_type_name(*this).c_str(),
@@ -77,14 +93,16 @@ void phy_controller::selecting_cell::exit(phy_controller* f)
   } else {
     log_h->warning("Failed to select cell %s\n", to_string(target_cell).c_str());
   }
-
-  // Signal result back to FSM that called cell selection
-  csel_callback(result);
 }
 
 void phy_controller::selecting_cell::wait_in_sync::enter(selecting_cell* f, const cell_sel_res& ev)
 {
-  f->wait_in_sync_timer.set(wait_sync_timeout_ms, [f](uint32_t tid) { f->trigger(timeout_ev{}); });
+  f->wait_in_sync_timer.set(wait_sync_timeout_ms, [f](uint32_t tid) {
+    f->parent_fsm()->trigger(timeout_ev{});
+    if (not f->parent_fsm()->is_in_state<selecting_cell>()) {
+      f->parent_fsm()->csel_callback(false);
+    }
+  });
   f->wait_in_sync_timer.run();
 }
 
@@ -95,7 +113,8 @@ void phy_controller::selecting_cell::wait_in_sync::enter(selecting_cell* f, cons
 //! Searches for a cell in the current frequency and retrieves SIB1 if not retrieved yet
 bool phy_controller::start_cell_search(const srslte::event_callback<cell_srch_res>& on_complete)
 {
-  if (not trigger(on_complete)) {
+  trigger(on_complete);
+  if (not is_in_state<searching_cell>()) {
     log_h->warning("Failed to launch cell search\n");
     return false;
   }
@@ -104,18 +123,22 @@ bool phy_controller::start_cell_search(const srslte::event_callback<cell_srch_re
 
 bool phy_controller::cell_search_completed(cell_search_ret_t cs_ret, phy_cell_t found_cell)
 {
-  return trigger(cell_srch_res{cs_ret, found_cell});
+  cell_srch_res res{cs_ret, found_cell};
-}
+  if (trigger(res)) {
-
+    // Signal callers the result of cell search
-bool phy_controller::cell_selection_completed(bool outcome)
+    for (auto& f : csearch_callbacks) {
-{
+      f(res);
-  return trigger(outcome);
+    }
+    csearch_callbacks.clear();
+    return true;
+  }
+  return false;
 }
 
 void phy_controller::searching_cell::enter(phy_controller* f, const cell_search_cmd& cmd)
 {
   f->log_h->info("Initiated Cell search\n");
-  csearch_callbacks.emplace_back(cmd);
+  f->csearch_callbacks.emplace_back(cmd);
   f->stack->start_cell_search();
 }
 
@@ -133,17 +156,14 @@ void phy_controller::handle_cell_search_res(searching_cell& s, const cell_srch_r
       // TODO: check what errors can happen (currently not handled in our code)
   }
 
-  // Signal back completion
+  // Forward cell search result to be handled in next state
-  for (auto& f : s.csearch_callbacks) {
+  //  trigger(result);
-    f(result);
-  }
-  s.csearch_callbacks.clear();
 }
 
 void phy_controller::share_cell_search_res(searching_cell& s, const cell_search_cmd& callback)
 {
   log_h->info("Cell Search already running. Re-utilizing result.\n");
-  s.csearch_callbacks.emplace_back(callback);
+  csearch_callbacks.emplace_back(callback);
 }
 
 } // namespace srsue

srsue/src/stack/rrc/rrc_procedures.cc

@@ -46,8 +46,9 @@ proc_outcome_t rrc::cell_search_proc::init()
 {
   Info("Starting...\n");
   state = state_t::phy_cell_search;
-  if (not rrc_ptr->phy_ctrl->start_cell_search(
+  rrc_ptr->phy_ctrl->start_cell_search(
-          [this](const phy_controller::cell_srch_res& res) { rrc_ptr->cell_searcher.trigger(res); })) {
+      [this](const phy_controller::cell_srch_res& res) { rrc_ptr->cell_searcher.trigger(res); });
+  if (not rrc_ptr->phy_ctrl->is_in_state<phy_controller::searching_cell>()) {
     Warning("Failed to initiate Cell Search.\n");
     return proc_outcome_t::error;
   }
@@ -546,7 +547,8 @@ proc_outcome_t rrc::cell_selection_proc::start_cell_selection()
 
       state            = search_state_t::cell_selection;
       auto on_complete = [this](const bool& result) { rrc_ptr->cell_selector.trigger(cell_select_event_t{result}); };
-      if (not rrc_ptr->phy_ctrl->start_cell_select(rrc_ptr->meas_cells.serving_cell().phy_cell, on_complete)) {
+      rrc_ptr->phy_ctrl->start_cell_select(rrc_ptr->meas_cells.serving_cell().phy_cell, on_complete);
+      if (not rrc_ptr->phy_ctrl->is_in_state<phy_controller::selecting_cell>()) {
         Error("Failed to launch PHY Cell Selection\n");
         return proc_outcome_t::error;
       }
@@ -1487,7 +1489,8 @@ srslte::proc_outcome_t rrc::ho_proc::step()
     cell_t* target_cell = 
         rrc_ptr->meas_cells.get_neighbour_cell_handle(target_earfcn, recfg_r8.mob_ctrl_info.target_pci);
     auto    on_complete = [this](const bool& result) { rrc_ptr->ho_handler.trigger(cell_select_event_t{result}); };
-    if (not rrc_ptr->phy_ctrl->start_cell_select(target_cell->phy_cell, on_complete)) {
+    rrc_ptr->phy_ctrl->start_cell_select(rrc_ptr->meas_cells.serving_cell().phy_cell, on_complete);
+    if (not rrc_ptr->phy_ctrl->is_in_state<phy_controller::selecting_cell>()) {
       Error("Failed to launch the selection of target cell %s\n", target_cell->to_string().c_str());
       return proc_outcome_t::error;
     }

srsue/test/upper/rrc_phy_ctrl_test.cc

@@ -46,10 +46,16 @@ int test_phy_ctrl_fsm()
   // TEST: Correct initiation of Cell Search state
   bool                          csearch_res_present = false;
   phy_controller::cell_srch_res csearch_res         = {};
-  auto cell_sel_callback = [&csearch_res_present, &csearch_res](const phy_controller::cell_srch_res& result) {
+  auto                          cell_sel_callback =
-    csearch_res_present = true;
+      [&csearch_res_present, &csearch_res, &phy_ctrl](const phy_controller::cell_srch_res& result) {
-    csearch_res         = result;
+        csearch_res_present = true;
-  };
+        csearch_res         = result;
+        if (phy_ctrl.current_state_name() == "searching_cell" or phy_ctrl.is_trigger_locked()) {
+          phy_ctrl.get_log()->error(
+              "When caller is signalled with cell search result, cell search state cannot be active\n");
+          exit(1);
+        }
+      };
   TESTASSERT(phy_ctrl.start_cell_search(cell_sel_callback));
   TESTASSERT(not phy_ctrl.is_in_sync());
 
@@ -76,7 +82,14 @@ int test_phy_ctrl_fsm()
 
   // TEST: Correct initiation of Cell Select state
   int  cell_select_success = -1;
-  auto csel_callback       = [&cell_select_success](const bool& res) { cell_select_success = res ? 1 : 0; };
+  auto csel_callback       = [&cell_select_success, &phy_ctrl](const bool& res) {
+    cell_select_success = res ? 1 : 0;
+    if (phy_ctrl.current_state_name() == "selecting_cell" or phy_ctrl.is_trigger_locked()) {
+      phy_ctrl.get_log()->error(
+          "When caller is signalled with cell selection result, cell selection state cannot be active\n");
+      exit(1);
+    }
+  };
   phy_ctrl.start_cell_select(found_cell, csel_callback);
   TESTASSERT(not phy_ctrl.is_in_sync());
   TESTASSERT(phy_ctrl.current_state_name() == "selecting_cell");