rrc-nr,test: apply reviewer comments

Signed-off-by: Carlo Galiotto <carlo@srs.io>

srsgnb/src/stack/ngap/ngap_ue_proc.cc

@@ -169,7 +169,7 @@ proc_outcome_t ngap_ue_pdu_session_res_setup_proc::init(const asn1::ngap_nr::pdu
 
   // QoS parameter mapping in config in LTE enb
   if (su_req.pdu_session_nas_pdu_present) {
-    if (rrc-> establish_rrc_bearer(ue_ctxt->rnti, su_req.pdu_session_id, su_req.pdu_session_nas_pdu, lcid) ==
+    if (rrc->establish_rrc_bearer(ue_ctxt->rnti, su_req.pdu_session_id, su_req.pdu_session_nas_pdu, lcid) ==
         SRSRAN_SUCCESS) {
       parent->send_pdu_session_resource_setup_response(su_req.pdu_session_id, teid_in, addr_in);
       return proc_outcome_t::success;

srsgnb/src/stack/rrc/cell_asn1_config.cc

@@ -11,11 +11,11 @@
  */
 
 #include "srsgnb/hdr/stack/rrc/cell_asn1_config.h"
+#include "srsenb/hdr/common/common_enb.h"
 #include "srsran/asn1/obj_id_cmp_utils.h"
 #include "srsran/asn1/rrc_nr_utils.h"
 #include "srsran/common/band_helper.h"
 #include <bitset>
-#include <srsenb/hdr/common/common_enb.h>
 
 using namespace asn1::rrc_nr;
 

srsgnb/src/stack/rrc/rrc_nr_ue.cc

@@ -1042,7 +1042,8 @@ void rrc_nr::ue::send_rrc_reconfiguration()
       compute_diff_radio_bearer_cfg(parent->cfg, radio_bearer_cfg, next_radio_bearer_cfg, ies.radio_bearer_cfg);
 
   // If no bearer to add/mod/remove, do not include master_cell_group
-  // Set ies.non_crit_ext_present only if master_cell_group_present = true or
+  // Set ies.non_crit_ext_present (a few lines below) only if
+  // master_cell_group_present == true or ies.non_crit_ext.ded_nas_msg_list_present == true
   if (ies.radio_bearer_cfg_present){
 
     ies.non_crit_ext.master_cell_group_present = true;
@@ -1067,7 +1068,6 @@ void rrc_nr::ue::send_rrc_reconfiguration()
     }
 
     // Update lower layers
-    if (ies.radio_bearer_cfg_present) {
     // add PDCP bearers
     update_pdcp_bearers(ies.radio_bearer_cfg, master_cell_group);
 
@@ -1077,7 +1077,6 @@ void rrc_nr::ue::send_rrc_reconfiguration()
     // add MAC bearers
     update_mac(master_cell_group, false);
   }
-  }
 
   if (nas_pdu_queue.size() > 0) {
     // Pass stored NAS PDUs

srsgnb/src/stack/rrc/test/rrc_nr_test.cc

@@ -161,8 +161,8 @@ void test_rrc_sa_connection()
   test_rrc_nr_connection_establishment(task_sched, rrc_obj, rlc_obj, mac_obj, ngap_obj,0x4601);
   test_rrc_nr_info_transfer(task_sched, rrc_obj, pdcp_obj, ngap_obj, 0x4601);
   test_rrc_nr_security_mode_cmd(task_sched, rrc_obj, pdcp_obj, 0x4601);
-  test_rrc_nr_reconfiguration( task_sched, rrc_obj, pdcp_obj, 0x4601);
+  test_rrc_nr_reconfiguration(task_sched, rrc_obj, pdcp_obj, ngap_obj, 0x4601);
-  test_rrc_nr_2nd_reconfiguration( task_sched, rrc_obj, pdcp_obj, 0x4601);
+  test_rrc_nr_2nd_reconfiguration(task_sched, rrc_obj, pdcp_obj, ngap_obj, 0x4601);
 }
 
 } // namespace srsenb

srsgnb/src/stack/rrc/test/rrc_nr_test_helpers.cc

@@ -178,11 +178,8 @@ void test_rrc_nr_info_transfer(srsran::task_scheduler& task_sched,
   ies_UL.ded_nas_msg_present = true;
 
   // Create an unbounded_octstring object that contains a random NAS message (we simulate a NAS message)
-  // We reuse NAS_UL_msg below to compare the string with the message sent to and unpacked by the gNB
+  // We reuse ies_UL below to compare the string with the message sent to and unpacked by the gNB
-  asn1::unbounded_octstring<false> NAS_UL_msg;
+  ies_UL.ded_nas_msg.from_string("6671f8bc80b1860f29b3a8b3b8563ce6c36a591bb1a3dc6612674448fb958d274426d326356aa9aa");
-  NAS_UL_msg.from_string("6671f8bc80b1860f29b3a8b3b8563ce6c36a591bb1a3dc6612674448fb958d274426d326356aa9aa");
-  ies_UL.ded_nas_msg.resize(NAS_UL_msg.size());
-  memcpy(ies_UL.ded_nas_msg.data(), NAS_UL_msg.data(), NAS_UL_msg.size());
 
   srsran::unique_byte_buffer_t pdu;
   {
@@ -196,7 +193,7 @@ void test_rrc_nr_info_transfer(srsran::task_scheduler& task_sched,
   rrc_obj.write_pdu(rnti, 1, std::move(pdu));
 
   // compare if the actual transmitted matches with the MSG created from the original string
-  TESTASSERT(NAS_UL_msg == ngap.last_pdu);
+  TESTASSERT(ies_UL.ded_nas_msg == ngap.last_pdu);
 }
 
 void test_rrc_nr_security_mode_cmd(srsran::task_scheduler& task_sched,
@@ -257,10 +254,9 @@ void test_rrc_nr_security_mode_cmd(srsran::task_scheduler& task_sched,
 void test_rrc_nr_reconfiguration(srsran::task_scheduler& task_sched,
                                  rrc_nr&                 rrc_obj,
                                  pdcp_nr_rrc_tester&     pdcp,
+                                 ngap_rrc_tester&        ngap,
                                  uint16_t                rnti)
 {
-  TESTASSERT_EQ(srsran::srb_to_lcid(srsran::nr_srb::srb1), pdcp.last_sdu_lcid);
-
   // Test whether there exists the SRB1 initiated in the Connection Establishment
   // We test this as the SRB1 was set up in a different function
   TESTASSERT_EQ(rnti, pdcp.last_sdu_rnti);
@@ -279,7 +275,6 @@ void test_rrc_nr_reconfiguration(srsran::task_scheduler& task_sched,
                 dl_dcch_msg.msg.c1().rrc_recfg().crit_exts.type().value);
   const rrc_recfg_ies_s& reconf_ies = dl_dcch_msg.msg.c1().rrc_recfg().crit_exts.rrc_recfg();
 
-
   // create an unbounded_octstring object that contains the same NAS message as in SecurityModeCommand
   // The RRCreconfiguration reads the SecurityModeCommand NAS msg previously saved in the queue
   asn1::unbounded_octstring<false> NAS_msg;
@@ -304,14 +299,19 @@ void test_rrc_nr_reconfiguration(srsran::task_scheduler& task_sched,
 
   // send message to RRC
   rrc_obj.write_pdu(rnti, 1, std::move(pdu));
+
+  // Verify the NGAP gets notified for the RRCReconfigurationComplete
+  TESTASSERT_EQ(true, ngap.last_rrc_recnf_complete);
 }
 
 void test_rrc_nr_2nd_reconfiguration(srsran::task_scheduler& task_sched,
                                      rrc_nr&                 rrc_obj,
                                      pdcp_nr_rrc_tester&     pdcp,
+                                     ngap_rrc_tester&        ngap,
                                      uint16_t                rnti)
 {
-  TESTASSERT_EQ(srsran::srb_to_lcid(srsran::nr_srb::srb1), pdcp.last_sdu_lcid);
+  // Make sure the NGAP RRCReconfigurationComplete bool is reset to false
+  ngap.last_rrc_recnf_complete = false;
 
   // create an unbounded_octstring object that contains a NAS message (we simulate a random NAS nas)
   asn1::unbounded_octstring<false> NAS_msg;
@@ -383,6 +383,9 @@ void test_rrc_nr_2nd_reconfiguration(srsran::task_scheduler& task_sched,
 
   // send message to RRC
   rrc_obj.write_pdu(rnti, 1, std::move(pdu));
+
+  // Verify the NGAP gets notified for the RRCReconfigurationComplete
+  TESTASSERT_EQ(true, ngap.last_rrc_recnf_complete);
 }
 
 } // namespace srsenb

srsgnb/src/stack/rrc/test/rrc_nr_test_helpers.h

@@ -70,8 +70,11 @@ public:
     memcpy(last_pdu.data(), pdu.data(), pdu.size());
   }
 
+  void ue_notify_rrc_reconf_complete(uint16_t rnti, bool outcome) { last_rrc_recnf_complete = outcome; }
+
   uint16_t            last_sdu_rnti;
   asn1::dyn_octstring last_pdu;
+  bool                last_rrc_recnf_complete = false;
 };
 
 /**
@@ -106,11 +109,13 @@ void test_rrc_nr_security_mode_cmd(srsran::task_scheduler& task_sched,
 void test_rrc_nr_reconfiguration(srsran::task_scheduler& task_sched,
                                  rrc_nr&                 rrc_obj,
                                  pdcp_nr_rrc_tester&     pdcp,
+                                 ngap_rrc_tester&        ngap,
                                  uint16_t                rnti);
 
 void test_rrc_nr_2nd_reconfiguration(srsran::task_scheduler& task_sched,
                                      rrc_nr&                 rrc_obj,
                                      pdcp_nr_rrc_tester&     pdcp,
+                                     ngap_rrc_tester&        ngap,
                                      uint16_t                rnti);
 
 } // namespace srsenb