From 7cd4f45d623db3e10f4790e47302a402d6d55dd8 Mon Sep 17 00:00:00 2001 From: Francisco Paisana Date: Tue, 18 Aug 2020 13:33:23 +0100 Subject: [PATCH] fsm refactor. - avoid triggering directly a subfsm - improved metafunctions - other cleanups --- lib/include/srslte/common/fsm.h | 194 ++++++++++++++------------ lib/test/common/fsm_test.cc | 58 +++++--- srsenb/hdr/stack/rrc/rrc_mobility.h | 12 +- srsenb/src/stack/rrc/rrc_mobility.cc | 18 +-- srsue/src/stack/rrc/phy_controller.cc | 2 +- 5 files changed, 161 insertions(+), 123 deletions(-) diff --git a/lib/include/srslte/common/fsm.h b/lib/include/srslte/common/fsm.h index 6e79d82ae..10fabf733 100644 --- a/lib/include/srslte/common/fsm.h +++ b/lib/include/srslte/common/fsm.h @@ -46,11 +46,25 @@ namespace srslte { //! Forward declarations template -class fsm_t; +class base_fsm_t; +template +class composite_fsm_t; + +//! Check if type T is an FSM +template +using is_fsm = std::is_base_of, T>; + +//! Check if type T is a composite FSM +template +struct is_composite_fsm : public std::false_type {}; +template +struct is_composite_fsm::value>::type> { + const static bool value = T::is_nested; +}; namespace fsm_details { -//! Meta-function to filter transition list by types +//! Meta-function to filter transition_list by types template struct filter_transition_type; template @@ -75,34 +89,16 @@ struct state_name_visitor { std::string name = "invalid"; }; -//! Enable/Disable meta-function if is part of state list +//! Enable/Disable SFINAE meta-function to check if is part of state list template using enable_if_fsm_state = typename std::enable_if(), T>::type; template using disable_if_fsm_state = typename std::enable_if(), T>::type; template -constexpr bool is_fsm() -{ - return std::is_base_of, FSM>::value; -} - +using enable_if_subfsm = typename std::enable_if::value>::type; template -constexpr typename std::enable_if(), bool>::type is_subfsm() -{ - return FSM::is_nested; -} - -template -constexpr typename std::enable_if(), bool>::type is_subfsm() -{ - return false; -} - -template -using enable_if_subfsm = typename std::enable_if()>::type; -template -using disable_if_subfsm = typename std::enable_if()>::type; +using disable_if_subfsm = typename std::enable_if::value>::type; //! Metafunction to determine if FSM can hold given State type template @@ -163,7 +159,7 @@ template struct state_traits { static_assert(FSM::template can_hold_state(), "FSM type does not hold provided State\n"); using state_t = State; - using is_subfsm = std::integral_constant()>; + using is_subfsm = std::integral_constant::value>; //! enter new state. enter is called recursively for subFSMs template @@ -279,7 +275,7 @@ struct trigger_visitor { enable_if_subfsm operator()(CurrentState& s) { // Enter here for SubFSMs - result = s.trigger(std::forward(ev)); + result = s.process_event(std::forward(ev)); if (not result) { result = call_react(s); } @@ -300,29 +296,24 @@ struct trigger_visitor { } // namespace fsm_details -template -class nested_fsm_t; - -//! CRTP Class for all non-nested FSMs template -class fsm_t +class base_fsm_t { -protected: - using base_t = fsm_t; - template - using subfsm_t = nested_fsm_t; - public: + using derived_t = Derived; + //! get access to derived protected members from the base class derived_view : public Derived { public: - using derived_t = Derived; // propagate user fsm methods using Derived::states; using typename Derived::transitions; }; + template + using transition_table = type_list; + //! Params of a state transition template ; template - struct from_any_state { + struct to_state { using dest_state_t = DestState; using event_t = Event; constexpr static bool (Derived::*guard_fn)(const Event&) = GuardFn; @@ -376,11 +367,6 @@ public: using is_match = std::is_same; }; - template - using transition_table = type_list; - - static const bool is_nested = false; - //! Struct used to store FSM states template struct state_list : public std::tuple { @@ -389,7 +375,7 @@ public: static_assert(not type_list_contains(), "An FSM cannot contain itself as state\n"); template - state_list(fsm_t* f, Args&&... args) : tuple_base_t(std::forward(args)...) + state_list(base_fsm_t* f, Args&&... args) : tuple_base_t(std::forward(args)...) { if (not Derived::is_nested) { // If Root FSM, call initial state enter method @@ -440,26 +426,6 @@ public: size_t current_idx = 0; }; - explicit fsm_t(srslte::log_ref log_) : log_h(log_) {} - - // Push Events to FSM - template - bool trigger(Ev&& e) - { - if (trigger_locked) { - scheduled_event(std::forward(e), typename std::is_lvalue_reference::type{}); - return false; - } - trigger_locked = true; - bool ret = process_event(std::forward(e)); - while (not pending_events.empty()) { - pending_events.front()(); - pending_events.pop_front(); - } - trigger_locked = false; - return ret; - } - template bool is_in_state() const { @@ -495,7 +461,57 @@ public: template constexpr static bool can_hold_state() { - return fsm_details::fsm_state_list_type >::template can_hold_type(); + return fsm_details::fsm_state_list_type >::template can_hold_type(); + } + +protected: + // Access to CRTP derived class + derived_view* derived() { return static_cast(this); } + + const derived_view* derived() const { return static_cast(this); } + + template + bool process_event(Ev&& e) + { + fsm_details::trigger_visitor visitor{derived(), std::forward(e)}; + srslte::visit(visitor, derived()->states); + return visitor.result; + } +}; + +template +class composite_fsm_t; + +//! CRTP Class for all non-nested FSMs +template +class fsm_t : public base_fsm_t +{ +protected: + using base_t = fsm_t; + template + using subfsm_t = composite_fsm_t; + +public: + static const bool is_nested = false; + + explicit fsm_t(srslte::log_ref log_) : log_h(log_) {} + + // Push Events to FSM + template + bool trigger(Ev&& e) + { + if (trigger_locked) { + scheduled_event(std::forward(e), typename std::is_lvalue_reference::type{}); + return false; + } + trigger_locked = true; + bool ret = process_event(std::forward(e)); + while (not pending_events.empty()) { + pending_events.front()(); + pending_events.pop_front(); + } + trigger_locked = false; + return ret; } void set_fsm_event_log_level(srslte::LOG_LEVEL_ENUM e) { fsm_event_log_level = e; } @@ -527,53 +543,55 @@ public: } protected: - // Access to CRTP derived class - derived_view* derived() { return static_cast(this); } - - const derived_view* derived() const { return static_cast(this); } + using base_fsm_t::derived; + using base_fsm_t::process_event; template - bool process_event(Ev&& e) - { - fsm_details::trigger_visitor visitor{derived(), std::forward(e)}; - srslte::visit(visitor, derived()->states); - return visitor.result; - } - - template - void scheduled_event(Ev&& e, std::true_type) + void scheduled_event(Ev&& e, std::true_type t) { pending_events.emplace_back([this, e]() { process_event(e); }); } template - void scheduled_event(Ev&& e, std::false_type) + void scheduled_event(Ev&& e, std::false_type t) { pending_events.emplace_back(std::bind([this](Ev& e) { process_event(std::move(e)); }, std::move(e))); } - srslte::log_ref log_h; - srslte::LOG_LEVEL_ENUM fsm_event_log_level = LOG_LEVEL_INFO; - bool trigger_locked = false; - std::list > pending_events; + srslte::log_ref log_h; + srslte::LOG_LEVEL_ENUM fsm_event_log_level = LOG_LEVEL_INFO; + bool trigger_locked = false; + std::deque > pending_events; }; template -class nested_fsm_t : public fsm_t +class composite_fsm_t : public base_fsm_t { public: - using base_t = nested_fsm_t; + using base_t = composite_fsm_t; using parent_t = ParentFSM; static const bool is_nested = true; - explicit nested_fsm_t(ParentFSM* parent_fsm_) : fsm_t(parent_fsm_->get_log()), fsm_ptr(parent_fsm_) {} - nested_fsm_t(nested_fsm_t&&) = default; - nested_fsm_t& operator=(nested_fsm_t&&) = default; + explicit composite_fsm_t(ParentFSM* parent_fsm_) : fsm_ptr(parent_fsm_) {} + composite_fsm_t(composite_fsm_t&&) noexcept = default; + composite_fsm_t& operator=(composite_fsm_t&&) noexcept = default; - // Get pointer to outer FSM in case of HSM + // Get pointer to outer FSM in case of HFSM const parent_t* parent_fsm() const { return fsm_ptr; } parent_t* parent_fsm() { return fsm_ptr; } + srslte::log_ref get_log() const { return parent_fsm()->get_log(); } + + // Push Events to root FSM + template + bool trigger(Ev&& e) + { + return parent_fsm()->trigger(std::forward(e)); + } + + // Push events to this subFSM + using base_fsm_t::process_event; + protected: using parent_fsm_t = ParentFSM; diff --git a/lib/test/common/fsm_test.cc b/lib/test/common/fsm_test.cc index ace54f30a..4a79b41ef 100644 --- a/lib/test/common/fsm_test.cc +++ b/lib/test/common/fsm_test.cc @@ -30,7 +30,7 @@ struct ev2 {}; std::vector calls; template -void call_log_helper(State* state, srslte::log_ref& log_h, const char* type) +void call_log_helper(State* state, srslte::log_ref log_h, const char* type) { std::string callname = srslte::get_type_name() + "::" + type; log_h->info("%s custom called\n", callname.c_str()); @@ -64,22 +64,22 @@ public: struct state_inner { void enter(fsm2* f) { - call_log_helper(this, f->log_h, "enter"); + call_log_helper(this, f->get_log(), "enter"); f->parent_fsm()->inner_enter_counter++; } }; struct state_inner2 { - void enter(fsm2* f) { call_log_helper(this, f->log_h, "enter"); } - void exit(fsm2* f) { call_log_helper(this, f->log_h, "exit"); } + void enter(fsm2* f) { call_log_helper(this, f->get_log(), "enter"); } + void exit(fsm2* f) { call_log_helper(this, f->get_log(), "exit"); } }; - explicit fsm2(fsm1* f_) : nested_fsm_t(f_) {} + explicit fsm2(fsm1* f_) : composite_fsm_t(f_) {} fsm2(fsm2&&) = default; fsm2& operator=(fsm2&&) = default; - ~fsm2() { log_h->info("%s being destroyed!", get_type_name(*this).c_str()); } + ~fsm2() { get_log()->info("%s being destroyed!", get_type_name(*this).c_str()); } - void enter(fsm1* f) { call_log_helper(this, f->log_h, "enter"); } - void exit(fsm1* f) { call_log_helper(this, f->log_h, "exit"); } + void enter(fsm1* f) { call_log_helper(this, get_log(), "enter"); } + void exit(fsm1* f) { call_log_helper(this, get_log(), "exit"); } private: void inner_action1(state_inner& s, const ev1& e); @@ -147,17 +147,17 @@ void fsm1::state1::exit(fsm1* f) // FSM event handlers void fsm1::fsm2::inner_action1(state_inner& s, const ev1& e) { - call_log_helper(this, log_h, "inner_action1"); + call_log_helper(this, get_log(), "inner_action1"); } void fsm1::fsm2::inner_action2(state_inner& s, const ev2& e) { - call_log_helper(this, log_h, "inner_action2"); + call_log_helper(this, get_log(), "inner_action2"); } void fsm1::fsm2::inner_action3(state_inner2& s, const ev2& e) { - log_h->info("fsm2::state_inner2::react called\n"); + get_log()->info("fsm2::state_inner2::react called\n"); } void fsm1::action1(idle_st& s, const ev1& e) @@ -181,8 +181,8 @@ void fsm1::action3(state1& s, const ev2& ev) namespace srslte { namespace fsm_details { -static_assert(is_fsm(), "invalid metafunction\n"); -static_assert(is_subfsm(), "invalid metafunction\n"); +static_assert(is_fsm::value, "invalid metafunction\n"); +static_assert(is_composite_fsm::value, "invalid metafunction\n"); static_assert(type_list_size(typename filter_transition_type >::type{}) > 0, "invalid filter metafunction\n"); static_assert( @@ -308,7 +308,7 @@ protected: row< idle_st, procstate1, launch_ev >, upd< procstate1, procevent1, &proc1::handle_success, &proc1::is_success >, upd< procstate1, procevent1, &proc1::handle_failure, &proc1::is_failure >, - from_any_state< idle_st, complete_ev > + to_state< idle_st, complete_ev > // +------------+-------------+----------------+------------------------+--------------------+ >; // clang-format on @@ -462,7 +462,7 @@ protected: row< emm_ta_updating_initiated, emm_registered, tau_outcome_ev >, row< emm_ta_updating_initiated, emm_deregistered, tau_reject_other_cause_ev >, row< emm_deregistered_initiated, emm_deregistered, detach_accept_ev >, - from_any_state< emm_deregistered, power_off_ev > + to_state< emm_deregistered, power_off_ev > // +-----------------------------+-------------------------+-----------------------------+ >; // clang-format on @@ -541,18 +541,29 @@ struct fsm3 : public srslte::fsm_t { struct st1 {}; struct st2 { int counter = 0; - void enter(fsm3* fsm) { counter++; } + void enter(fsm3* fsm) + { + counter++; + fsm->events.push_back(fsm->current_state_name()); + } }; fsm3() : base_t(srslte::log_ref{"TEST"}) {} + std::vector events; + protected: - void handle_ev1(st1& s, const ev1& ev) { trigger(ev2{}); } + void handle_ev1(st1& s, const ev1& ev) + { + trigger(ev2{}); + events.push_back(current_state_name() + "::action"); // still in st1 + } void handle_ev2(st2& s, const ev2& ev) { if (s.counter < 2) { trigger(ev1{}); } + events.push_back(current_state_name() + "::action"); // still in st2 } state_list states{this}; @@ -560,8 +571,8 @@ protected: using transitions = transition_table< // Start Target Event Action // +------------------------+-------------------------+-------------------+--------------------+ - row< st1, st2, ev1, &fsm3::handle_ev1>, - row< st2, st1, ev2, &fsm3::handle_ev2> + row< st1, st2, ev1, &fsm3::handle_ev1 >, + row< st2, st1, ev2, &fsm3::handle_ev2 > // +------------------------+-------------------------+-------------------+--------------------+ >; // clang-format on @@ -570,11 +581,20 @@ protected: int test_fsm_self_trigger() { fsm3 fsm; + TESTASSERT(fsm.events.empty()); TESTASSERT(fsm.is_in_state()); fsm.trigger(ev1{}); TESTASSERT(fsm.is_in_state()); + TESTASSERT(fsm.events.size() == 6); + TESTASSERT(fsm.events[0] == "st1::action"); + TESTASSERT(fsm.events[1] == "st2"); + TESTASSERT(fsm.events[2] == "st2::action"); + TESTASSERT(fsm.events[3] == "st1::action"); + TESTASSERT(fsm.events[4] == "st2"); + TESTASSERT(fsm.events[5] == "st2::action"); + return SRSLTE_SUCCESS; } diff --git a/srsenb/hdr/stack/rrc/rrc_mobility.h b/srsenb/hdr/stack/rrc/rrc_mobility.h index 254894327..a7401c1b9 100644 --- a/srsenb/hdr/stack/rrc/rrc_mobility.h +++ b/srsenb/hdr/stack/rrc/rrc_mobility.h @@ -189,12 +189,12 @@ private: state_list states{this}; // clang-format off using transitions = transition_table< - // Start Target Event Action Guard - // +-------------------+------------------+------------------------------+---------+---------------------+ - from_any_state< idle_st, srslte::failure_ev >, - row< wait_ho_req_ack_st, status_transfer_st, srslte::unique_byte_buffer_t, nullptr, &fsm::send_ho_cmd >, - row< wait_ho_req_ack_st, idle_st , srslte::unique_byte_buffer_t > - // +-------------------+------------------+------------------------------+---------+---------------------+ + // Start Target Event Action Guard + // +-------------------+------------------+------------------------------+---------+---------------------+ + to_state< idle_st, srslte::failure_ev >, + row< wait_ho_req_ack_st, status_transfer_st, srslte::unique_byte_buffer_t, nullptr, &fsm::send_ho_cmd >, + row< wait_ho_req_ack_st, idle_st , srslte::unique_byte_buffer_t > + // +-------------------+------------------+------------------------------+---------+---------------------+ >; // clang-format on }; diff --git a/srsenb/src/stack/rrc/rrc_mobility.cc b/srsenb/src/stack/rrc/rrc_mobility.cc index f7bd75fd1..882a0367a 100644 --- a/srsenb/src/stack/rrc/rrc_mobility.cc +++ b/srsenb/src/stack/rrc/rrc_mobility.cc @@ -1017,8 +1017,8 @@ bool rrc::ue::rrc_mobility::needs_intraenb_ho(idle_st& s, const ho_meas_report_e void rrc::ue::rrc_mobility::s1_source_ho_st::wait_ho_req_ack_st::enter(s1_source_ho_st* f, const ho_meas_report_ev& ev) { - f->log_h->console("Starting S1 Handover of rnti=0x%x to 0x%x.\n", f->parent_fsm()->rrc_ue->rnti, ev.target_eci); - f->log_h->info("Starting S1 Handover of rnti=0x%x to 0x%x.\n", f->parent_fsm()->rrc_ue->rnti, ev.target_eci); + f->get_log()->console("Starting S1 Handover of rnti=0x%x to 0x%x.\n", f->parent_fsm()->rrc_ue->rnti, ev.target_eci); + f->get_log()->info("Starting S1 Handover of rnti=0x%x to 0x%x.\n", f->parent_fsm()->rrc_ue->rnti, ev.target_eci); f->report = ev; bool success = f->parent_fsm()->start_ho_preparation(f->report.target_eci, f->report.meas_obj->meas_obj_id, false); @@ -1035,14 +1035,14 @@ bool rrc::ue::rrc_mobility::s1_source_ho_st::send_ho_cmd(wait_ho_req_ack_st& { asn1::cbit_ref bref(container->msg, container->N_bytes); if (rrchocmd.unpack(bref) != asn1::SRSASN_SUCCESS) { - log_h->warning("Unpacking of RRC HOCommand was unsuccessful\n"); - log_h->warning_hex(container->msg, container->N_bytes, "Received container:\n"); + get_log()->warning("Unpacking of RRC HOCommand was unsuccessful\n"); + get_log()->warning_hex(container->msg, container->N_bytes, "Received container:\n"); return false; } } if (rrchocmd.crit_exts.type().value != c1_or_crit_ext_opts::c1 or rrchocmd.crit_exts.c1().type().value != ho_cmd_s::crit_exts_c_::c1_c_::types_opts::ho_cmd_r8) { - log_h->warning("Only handling r8 Handover Commands\n"); + get_log()->warning("Only handling r8 Handover Commands\n"); return false; } @@ -1052,18 +1052,18 @@ bool rrc::ue::rrc_mobility::s1_source_ho_st::send_ho_cmd(wait_ho_req_ack_st& asn1::cbit_ref bref(&rrchocmd.crit_exts.c1().ho_cmd_r8().ho_cmd_msg[0], rrchocmd.crit_exts.c1().ho_cmd_r8().ho_cmd_msg.size()); if (dl_dcch_msg.unpack(bref) != asn1::SRSASN_SUCCESS) { - log_h->warning("Unpacking of RRC DL-DCCH message with HO Command was unsuccessful.\n"); + get_log()->warning("Unpacking of RRC DL-DCCH message with HO Command was unsuccessful.\n"); return false; } } if (dl_dcch_msg.msg.type().value != dl_dcch_msg_type_c::types_opts::c1 or dl_dcch_msg.msg.c1().type().value != dl_dcch_msg_type_c::c1_c_::types_opts::rrc_conn_recfg) { - log_h->warning("HandoverCommand is expected to contain an RRC Connection Reconf message inside\n"); + get_log()->warning("HandoverCommand is expected to contain an RRC Connection Reconf message inside\n"); return false; } asn1::rrc::rrc_conn_recfg_s& reconf = dl_dcch_msg.msg.c1().rrc_conn_recfg(); if (not reconf.crit_exts.c1().rrc_conn_recfg_r8().mob_ctrl_info_present) { - log_h->warning("HandoverCommand is expected to have mobility control subfield\n"); + get_log()->warning("HandoverCommand is expected to have mobility control subfield\n"); return false; } @@ -1077,7 +1077,7 @@ bool rrc::ue::rrc_mobility::s1_source_ho_st::send_ho_cmd(wait_ho_req_ack_st& void rrc::ue::rrc_mobility::s1_source_ho_st::status_transfer_st::enter(s1_source_ho_st* f) { - f->log_h->info("HandoverCommand of rnti=0x%x handled successfully.\n", f->parent_fsm()->rrc_ue->rnti); + f->get_log()->info("HandoverCommand of rnti=0x%x handled successfully.\n", f->parent_fsm()->rrc_ue->rnti); // TODO: Do anything with MeasCfg info within the Msg (e.g. update ue_var_meas)? diff --git a/srsue/src/stack/rrc/phy_controller.cc b/srsue/src/stack/rrc/phy_controller.cc index 5889cef34..08353202d 100644 --- a/srsue/src/stack/rrc/phy_controller.cc +++ b/srsue/src/stack/rrc/phy_controller.cc @@ -60,7 +60,7 @@ void phy_controller::in_sync() trigger(in_sync_ev{}); } -phy_controller::selecting_cell::selecting_cell(phy_controller* parent_) : nested_fsm_t(parent_) +phy_controller::selecting_cell::selecting_cell(phy_controller* parent_) : composite_fsm_t(parent_) { wait_in_sync_timer = parent_fsm()->task_sched.get_unique_timer(); }