integrated new observer pattern into phy controller fsm

lib/include/srslte/adt/observer.h

@@ -33,33 +33,78 @@ using observer_id                = std::size_t;
 const size_t invalid_observer_id = std::numeric_limits<observer_id>::max();
 
 template <typename... Args>
-class base_observable
+class observer;
+template <typename T>
+struct is_observer : public std::false_type {};
+template <typename... Args2>
+struct is_observer<observer<Args2...> > : public std::true_type {};
+
+//! Type-erasure of Observer
+template <typename... Args>
+class observer
 {
 public:
   using callback_t = std::function<void(Args...)>;
+  template <typename Callable>
+  using enable_if_callback_t =
+      typename std::enable_if<std::is_convertible<Callable, callback_t>::value and
+                              not is_observer<typename std::decay<Callable>::type>::value>::type;
+
+  observer() = default;
 
   //! Subscribe Observer that is a callback
-  template <typename Callable>
-  typename std::enable_if<std::is_convertible<Callable, callback_t>::value, observer_id>::type
-  subscribe(Callable&& callable)
-  {
-    return subscribe_common(callable);
-  }
+  template <typename Callable, typename Check = enable_if_callback_t<Callable> >
+  observer(Callable&& callable) : callback(std::forward<Callable>(callable))
+  {}
+
+  template <typename Observer,
+            typename TCheck =
+                typename std::enable_if<not std::is_convertible<Observer, callback_t>::value, observer_id>::type>
+  observer(Observer& observer) : callback([&observer](Args... args) { observer.trigger(std::forward<Args>(args)...); })
+  {}
 
-  //! Subscribe Observer type with method Observer::trigger(Args...)
   template <typename Observer>
-  typename std::enable_if<not std::is_convertible<Observer, callback_t>::value, observer_id>::type
-  subscribe(Observer& observer)
+  observer(Observer& observer, void (Observer::*trigger_method)(Args...)) :
+    callback([&observer, trigger_method](Args... args) { (observer.*trigger_method)(std::forward<Args>(args)...); })
+  {}
+
+  void operator()(Args... args)
   {
-    return subscribe_common([&observer](Args... args) { observer.trigger(std::forward<Args>(args)...); });
+    if (callback) {
+      callback(std::forward<Args>(args)...);
+    }
   }
 
-  //! Subscribe Observer type with custom trigger method
-  template <typename Observer>
-  observer_id subscribe(Observer& observer, void (Observer::*trigger_method)(Args...))
+  explicit operator bool() const { return static_cast<bool>(callback); }
+
+  void reset() { callback = nullptr; }
+
+private:
+  callback_t callback;
+};
+
+template <typename... Args>
+class base_observable
+{
+public:
+  using this_observer_t = observer<Args...>;
+
+  //! Subscribe Observer that is a callback
+  template <typename... Args2>
+  observer_id subscribe(Args2&&... args)
   {
-    return subscribe_common(
-        [&observer, trigger_method](Args... args) { (observer.*trigger_method)(std::forward<Args>(args)...); });
+    size_t id = 0;
+    for (auto& slot : observers) {
+      if (not static_cast<bool>(slot)) {
+        // empty slot found
+        slot = this_observer_t{std::forward<Args2>(args)...};
+        return id;
+      }
+      id++;
+    }
+    // append to end of list
+    observers.emplace_back(std::forward<Args2>(args)...);
+    return observers.size() - 1;
   }
 
   //! Unsubscribe Observer
@@ -87,34 +132,15 @@ public:
   void dispatch(Args... args)
   {
     for (auto& obs_callback : observers) {
-      if (obs_callback) {
-        obs_callback(std::forward<Args>(args)...);
-      }
+      obs_callback(std::forward<Args>(args)...);
     }
   }
 
 protected:
-  using observer_list_t = std::deque<callback_t>;
+  using observer_list_t = std::deque<this_observer_t>;
 
   ~base_observable() = default;
 
-  template <typename Callable>
-  observer_id subscribe_common(Callable&& callable)
-  {
-    size_t id = 0;
-    for (auto& slot : observers) {
-      if (not static_cast<bool>(slot)) {
-        // empty slot found
-        slot = std::forward<Callable>(callable);
-        return id;
-      }
-      id++;
-    }
-    // append to end of list
-    observers.emplace_back(std::forward<Callable>(callable));
-    return observers.size() - 1;
-  }
-
   observer_list_t observers;
 };
 
@@ -122,6 +148,9 @@ template <typename... Args>
 class observable : public base_observable<Args...>
 {};
 
+template <typename Event>
+using event_observer = observer<const Event&>;
+
 //! Special case of observable for event types
 template <typename Event>
 class event_dispatcher : public base_observable<const Event&>

lib/test/adt/observer_test.cc

@@ -22,6 +22,9 @@
 #include "srslte/adt/observer.h"
 #include "srslte/common/test_common.h"
 
+static_assert(srslte::is_observer<srslte::observer<int> >::value, "is_observer<> meta-function failed");
+static_assert(not srslte::is_observer<std::function<void(int)> >::value, "is_observer<> meta-function failed");
+
 struct M {
   M() = default;
   explicit M(int v) : val(v) {}

srsue/hdr/stack/rrc/phy_controller.h

@@ -55,17 +55,13 @@ public:
   explicit phy_controller(phy_interface_rrc_lte* phy_, srslte::task_sched_handle task_sched_);
 
   // PHY procedures interfaces
-  bool start_cell_select(const phy_cell_t& phy_cell);
-  bool start_cell_search();
+  bool start_cell_select(const phy_cell_t& phy_cell, srslte::event_observer<bool> observer);
+  bool start_cell_search(srslte::event_observer<cell_srch_res> observer);
   void cell_search_completed(cell_search_ret_t cs_ret, phy_cell_t found_cell);
   void cell_selection_completed(bool outcome);
   void in_sync();
   void out_sync() { trigger(out_sync_ev{}); }
 
-  // Event Observers
-  srslte::event_dispatcher<cell_srch_res> cell_search_observers;
-  srslte::event_dispatcher<bool>          cell_selection_observers;
-
   // state getters
   bool cell_is_camping() { return phy->cell_is_camping(); }
   bool is_in_sync() const { return is_in_state<in_sync_st>(); }
@@ -79,7 +75,7 @@ public:
 
     struct wait_csel_res {};
     struct wait_in_sync {
-      void enter(selecting_cell* f, const cell_sel_res& ev);
+      void enter(selecting_cell* f);
     };
 
     explicit selecting_cell(phy_controller* parent_);
@@ -118,8 +114,10 @@ public:
   };
 
 private:
-  phy_interface_rrc_lte*    phy = nullptr;
-  srslte::task_sched_handle task_sched;
+  phy_interface_rrc_lte*                  phy = nullptr;
+  srslte::task_sched_handle               task_sched;
+  srslte::event_observer<bool>            cell_selection_observer;
+  srslte::event_dispatcher<cell_srch_res> cell_search_observers;
 
 protected:
   state_list<unknown_st, in_sync_st, out_sync_st, searching_cell, selecting_cell> states{this,

srsue/src/stack/rrc/phy_controller.cc

@@ -45,7 +45,7 @@ void phy_controller::in_sync()
  *    PHY Cell Select Procedure
  *************************************/
 
-bool phy_controller::start_cell_select(const phy_cell_t& phy_cell)
+bool phy_controller::start_cell_select(const phy_cell_t& phy_cell, srslte::event_observer<bool> observer)
 {
   if (is_in_state<selecting_cell>()) {
     log_h->warning("Failed to launch cell selection as it is already running\n");
@@ -56,6 +56,7 @@ bool phy_controller::start_cell_select(const phy_cell_t& phy_cell)
     log_h->warning("Failed to launch cell selection. Current state: %s\n", current_state_name().c_str());
     return false;
   }
+  cell_selection_observer = std::move(observer);
   return true;
 }
 
@@ -96,12 +97,12 @@ void phy_controller::selecting_cell::exit(phy_controller* f)
   // Signal result back to FSM that called cell selection
   bool result = csel_res.result;
   f->task_sched.defer_task([f, result]() {
-    f->cell_selection_observers.dispatch(result);
-    f->cell_selection_observers.unsubscribe_all();
+    f->cell_selection_observer(result);
+    f->cell_selection_observer.reset();
   });
 }
 
-void phy_controller::selecting_cell::wait_in_sync::enter(selecting_cell* f, const cell_sel_res& ev)
+void phy_controller::selecting_cell::wait_in_sync::enter(selecting_cell* f)
 {
   f->wait_in_sync_timer.set(wait_sync_timeout_ms, [f](uint32_t tid) { f->parent_fsm()->trigger(timeout_ev{}); });
   f->wait_in_sync_timer.run();
@@ -112,7 +113,7 @@ 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()
+bool phy_controller::start_cell_search(srslte::event_observer<cell_srch_res> observer)
 {
   if (is_in_state<searching_cell>()) {
     fsmInfo("Cell search already launched.\n");
@@ -123,6 +124,7 @@ bool phy_controller::start_cell_search()
     fsmWarning("Failed to launch cell search\n");
     return false;
   }
+  cell_search_observers.subscribe(observer);
   return true;
 }
 

srsue/src/stack/rrc/rrc_procedures.cc

@@ -46,11 +46,10 @@ 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()) {
+  if (not rrc_ptr->phy_ctrl->start_cell_search(rrc_ptr->cell_searcher)) {
     Warning("Failed to initiate Cell Search.\n");
     return proc_outcome_t::error;
   }
-  rrc_ptr->phy_ctrl->cell_search_observers.subscribe(rrc_ptr->cell_searcher);
   return proc_outcome_t::yield;
 }
 
@@ -98,11 +97,10 @@ proc_outcome_t rrc::cell_search_proc::handle_cell_found(const phy_interface_rrc_
 
   // set new serving cell in PHY
   state = state_t::phy_cell_select;
-  if (not rrc_ptr->phy_ctrl->start_cell_select(rrc_ptr->meas_cells.serving_cell().phy_cell)) {
+  if (not rrc_ptr->phy_ctrl->start_cell_select(rrc_ptr->meas_cells.serving_cell().phy_cell, rrc_ptr->cell_searcher)) {
     Error("Couldn't start phy cell selection\n");
     return proc_outcome_t::error;
   }
-  rrc_ptr->phy_ctrl->cell_selection_observers.subscribe(rrc_ptr->cell_searcher);
   return proc_outcome_t::yield;
 }
 
@@ -511,11 +509,10 @@ proc_outcome_t rrc::cell_selection_proc::start_serv_cell_selection()
   Info("Not camping on serving cell %s. Selecting it...\n", rrc_ptr->meas_cells.serving_cell().to_string().c_str());
 
   state = search_state_t::serv_cell_camp;
-  if (not rrc_ptr->phy_ctrl->start_cell_select(rrc_ptr->meas_cells.serving_cell().phy_cell)) {
+  if (not rrc_ptr->phy_ctrl->start_cell_select(rrc_ptr->meas_cells.serving_cell().phy_cell, rrc_ptr->cell_selector)) {
     Error("Failed to launch PHY Cell Selection\n");
     return proc_outcome_t::error;
   }
-  rrc_ptr->phy_ctrl->cell_selection_observers.subscribe(rrc_ptr->cell_selector);
   serv_cell_select_attempted = true;
   return proc_outcome_t::yield;
 }
@@ -553,11 +550,11 @@ proc_outcome_t rrc::cell_selection_proc::start_cell_selection()
       Info("Selected cell: %s\n", rrc_ptr->meas_cells.serving_cell().to_string().c_str());
 
       state = search_state_t::cell_selection;
-      if (not rrc_ptr->phy_ctrl->start_cell_select(rrc_ptr->meas_cells.serving_cell().phy_cell)) {
+      if (not rrc_ptr->phy_ctrl->start_cell_select(rrc_ptr->meas_cells.serving_cell().phy_cell,
+                                                   rrc_ptr->cell_selector)) {
         Error("Failed to launch PHY Cell Selection\n");
         return proc_outcome_t::error;
       }
-      rrc_ptr->phy_ctrl->cell_selection_observers.subscribe(rrc_ptr->cell_selector);
       return proc_outcome_t::yield;
     }
   }
@@ -1492,11 +1489,10 @@ srslte::proc_outcome_t rrc::ho_proc::step()
 
     Info("Starting cell selection of target cell %s\n", target_cell->to_string().c_str());
 
-    if (not rrc_ptr->phy_ctrl->start_cell_select(target_cell->phy_cell)) {
+    if (not rrc_ptr->phy_ctrl->start_cell_select(target_cell->phy_cell, rrc_ptr->ho_handler)) {
       Error("Failed to launch the selection of target cell %s\n", target_cell->to_string().c_str());
       return proc_outcome_t::error;
     }
-    rrc_ptr->phy_ctrl->cell_selection_observers.subscribe(rrc_ptr->ho_handler);
     state = wait_phy_cell_select_complete;
   }
   return proc_outcome_t::yield;

srsue/test/upper/rrc_phy_ctrl_test.cc

@@ -83,8 +83,7 @@ int test_phy_ctrl_fsm()
   TESTASSERT(phy_ctrl.is_in_sync());
 
   // TEST: Correct initiation of Cell Search state
-  TESTASSERT(phy_ctrl.start_cell_search());
-  phy_ctrl.cell_search_observers.subscribe(csearch_tester);
+  TESTASSERT(phy_ctrl.start_cell_search(csearch_tester));
   TESTASSERT(not phy_ctrl.is_in_sync());
 
   // TEST: Cell Search only listens to a cell search result event
@@ -112,8 +111,7 @@ int test_phy_ctrl_fsm()
   phy_ctrl.out_sync();
 
   // TEST: Correct initiation of Cell Select state
-  phy_ctrl.start_cell_select(found_cell);
-  phy_ctrl.cell_selection_observers.subscribe(csel_tester);
+  phy_ctrl.start_cell_select(found_cell, csel_tester);
   TESTASSERT(not phy_ctrl.is_in_sync());
   TESTASSERT(phy_ctrl.current_state_name() == "selecting_cell");
 
@@ -135,8 +133,7 @@ int test_phy_ctrl_fsm()
 
   // TEST: Cell Selection with timeout being reached
   csel_tester.result = -1;
-  TESTASSERT(phy_ctrl.start_cell_select(found_cell));
-  phy_ctrl.cell_selection_observers.subscribe(csel_tester);
+  TESTASSERT(phy_ctrl.start_cell_select(found_cell, csel_tester));
   TESTASSERT(not phy_ctrl.is_in_sync());
   phy_ctrl.cell_selection_completed(true);
   TESTASSERT(phy_ctrl.current_state_name() == "selecting_cell");