sched: fix unittests and related pucch_allocator code

Signed-off-by: Carlo Galiotto <[email protected]>

Author: carlo-gal

include/srsran/scheduler/scheduler_slot_handler.h

@@ -482,6 +482,12 @@ struct prach_occasion_info {
 
 /// Info about PUCCH used resource.
 struct pucch_info {
+  /// This information only is used by the scheduler.
+  struct context {
+    unsigned id        = MAX_PUCCH_PDUS_PER_SLOT;
+    bool     is_common = false;
+  };
+
   rnti_t                   crnti;
   const bwp_configuration* bwp_cfg;
   pucch_format             format;
@@ -496,6 +502,8 @@ struct pucch_info {
   };
   /// In case the PUCCH will contain CSI bits, this struct contains information how those bits are to be decoded.
   std::optional<csi_report_configuration> csi_rep_cfg;
+
+  context pdu_context;
 };
 
 struct ul_sched_result {

lib/scheduler/pucch_scheduling/pucch_allocator.h

@@ -24,8 +24,13 @@ struct pucch_uci_bits {
   /// Number of SR info bits that should have been reported in the removed PUCCH grant.
   sr_nof_bits sr_bits{sr_nof_bits::no_sr};
   /// Number of CSI Part 1 info bits that should have been reported in the removed PUCCH grant.
-  unsigned csi_part1_bits{0};
+  unsigned csi_part1_nof_bits{0};
   // TODO: add extra bits for CSI Part 2.
+
+  [[nodiscard]] unsigned get_total_bits() const
+  {
+    return harq_ack_nof_bits + sr_nof_bits_to_uint(sr_bits) + csi_part1_nof_bits;
+  }
 };
 
 /// PUCCH scheduling interface.

lib/scheduler/pucch_scheduling/pucch_allocator_impl.cpp

@@ -87,17 +87,6 @@ class pucch_allocator_impl final : public pucch_allocator
     unsigned r_pucch;
   };
 
-  struct uci_bits {
-    unsigned    harq_ack_bits  = 0U;
-    sr_nof_bits sr_bits        = sr_nof_bits::no_sr;
-    unsigned    csi_part1_bits = 0U;
-
-    [[nodiscard]] unsigned get_total_bits() const
-    {
-      return harq_ack_bits + sr_nof_bits_to_uint(sr_bits) + csi_part1_bits;
-    }
-  };
-
   // At the moment, we only supports PUCCH resource set index 0 and 1.
   enum class pucch_res_set_idx : uint8_t { set_0 = 0, set_1 };
 
@@ -106,21 +95,31 @@ class pucch_allocator_impl final : public pucch_allocator
     uint8_t           pucch_res_ind = 0;
   };
 
+  /// Defines the type of resource.
+  /// HARQ indicates the HAR-ACK resource (it can carry HARQ-ACK and/or SR and/or CSI bits).
+  /// SR indicates the resource dedicated for SR (it can carry SR and HARQ-ACK bits).
+  /// CSI indicates the resource dedicated for CSI (it can carry CSI and SR bits).
   enum class pucch_grant_type { harq_ack, sr, csi };
 
+  /// \brief Defines a PUCCH grant (and its relevant information) currently allocated to a given UE.
+  /// It is used internally to keep track of the UEs' allocations of the PUCCH grants with dedicated resources.
   class pucch_grant
   {
   public:
     pucch_grant_type type;
     // Only relevant for HARQ-ACK resources.
     harq_res_id           harq_id;
-    pucch_format          format;
-    uci_bits              bits;
+    pucch_uci_bits        bits;
     const pucch_resource* pucch_res_cfg = nullptr;
 
+    [[nodiscard]] pucch_format get_format() const
+    {
+      return pucch_res_cfg != nullptr ? pucch_res_cfg->format : pucch_format::NOF_FORMATS;
+    }
     [[nodiscard]] ofdm_symbol_range get_symbols() const;
   };
 
+  /// \brief List of possible PUCCH grants that allocated to a UE, at a given slot.
   class pucch_grant_list
   {
   public:
@@ -129,30 +128,31 @@ class pucch_allocator_impl final : public pucch_allocator
     std::optional<pucch_grant> csi_resource;
     unsigned                   nof_grants = 0;
 
-    [[nodiscard]] uci_bits get_uci_bits() const;
+    [[nodiscard]] pucch_uci_bits get_uci_bits() const;
+    [[nodiscard]] bool           is_emtpy() const;
   };
 
   /// Keeps track of the PUCCH grants (common + dedicated) for a given UE.
   struct ue_grants {
     rnti_t rnti;
-    bool   has_common_pucch;
-
+    // Information about the common PUCCH grant.
+    bool has_common_pucch = false;
+    // List of PUCCH grants with dedicated resources.
     pucch_grant_list pucch_grants;
   };
 
   using slot_pucch_grants = static_vector<ue_grants, MAX_PUCCH_PDUS_PER_SLOT>;
 
-  class res_manager_garbage_collector
-  {
-  public:
-    res_manager_garbage_collector(pucch_resource_manager& res_manager_) : res_manager(res_manager_){};
-
+  /// \brief Collects the information of what PUCCH cell resources have been allocated to a UE at given slot.
+  /// This info is only used during the allocation, and the PUCCH allocator is called for a new UE or new allocation.
+  struct res_manager_garbage_collector {
     bool                    harq_set_0 = false;
     bool                    harq_set_1 = false;
     bool                    csi        = false;
     bool                    sr         = false;
     pucch_resource_manager& res_manager;
 
+    res_manager_garbage_collector(pucch_resource_manager& res_manager_) : res_manager(res_manager_){};
     void reset();
     void release_resource(slot_point slot_tx, rnti_t crnti, const ue_cell_configuration& ue_cell_cfg);
   };
@@ -168,7 +168,7 @@ class pucch_allocator_impl final : public pucch_allocator
                                              pucch_common_params           pucch_params);
 
   std::optional<pucch_common_params> find_common_and_ded_harq_res_available(cell_slot_resource_allocator& pucch_alloc,
-                                                                            ue_grants* existing_grants,
+                                                                            ue_grants& existing_grants,
                                                                             rnti_t     rnti,
                                                                             const ue_cell_configuration&   ue_cell_cfg,
                                                                             const dci_context_information& dci_info);
@@ -184,34 +184,14 @@ class pucch_allocator_impl final : public pucch_allocator
                           const ue_cell_configuration&  ue_cell_cfg,
                           unsigned                      csi_part1_bits);
 
-  // Fills the PUCCH HARQ grant for common resources.
-  void fill_pucch_harq_common_grant(pucch_info& pucch_info, rnti_t rnti, const pucch_res_alloc_cfg& pucch_res);
-
-  // Fills the PUCCH Format 1 grant.
-  void fill_pucch_ded_format1_grant(pucch_info&           pucch_grant,
-                                    rnti_t                crnti,
-                                    const pucch_resource& pucch_ded_res_cfg,
-                                    unsigned              harq_ack_bits,
-                                    sr_nof_bits           sr_bits);
-
-  // Fills the PUCCH Format 2 grant.
-  void fill_pucch_format2_grant(pucch_info&                  pucch_grant,
-                                rnti_t                       crnti,
-                                const pucch_resource&        pucch_ded_res_cfg,
-                                const ue_cell_configuration& ue_cell_cfg,
-                                unsigned                     nof_prbs,
-                                unsigned                     harq_ack_bits,
-                                sr_nof_bits                  sr_bits,
-                                unsigned                     csi_part1_bits);
-
   std::optional<unsigned> multiplex_and_allocate_pucch(cell_slot_resource_allocator& pucch_slot_alloc,
-                                                       uci_bits                      new_bits,
+                                                       pucch_uci_bits                new_bits,
                                                        ue_grants&                    current_grants,
                                                        const ue_cell_configuration&  ue_cell_cfg,
                                                        bool                          preserve_res_indicator = false);
 
   std::optional<pucch_grant_list> get_pucch_res_pre_multiplexing(slot_point                   sl_tx,
-                                                                 uci_bits                     new_bits,
+                                                                 pucch_uci_bits               new_bits,
                                                                  ue_grants                    ue_current_grants,
                                                                  const ue_cell_configuration& ue_cell_cfg);
 
@@ -233,6 +213,26 @@ class pucch_allocator_impl final : public pucch_allocator
                                           pucch_grant_list              grants_to_tx,
                                           const ue_cell_configuration&  ue_cell_cfg);
 
+  // Fills the PUCCH HARQ grant for common resources.
+  void fill_pucch_harq_common_grant(pucch_info& pucch_info, rnti_t rnti, const pucch_res_alloc_cfg& pucch_res);
+
+  // Fills the PUCCH Format 1 grant.
+  void fill_pucch_ded_format1_grant(pucch_info&           pucch_grant,
+                                    rnti_t                crnti,
+                                    const pucch_resource& pucch_ded_res_cfg,
+                                    unsigned              harq_ack_bits,
+                                    sr_nof_bits           sr_bits);
+
+  // Fills the PUCCH Format 2 grant.
+  void fill_pucch_format2_grant(pucch_info&                  pucch_grant,
+                                rnti_t                       crnti,
+                                const pucch_resource&        pucch_ded_res_cfg,
+                                const ue_cell_configuration& ue_cell_cfg,
+                                unsigned                     nof_prbs,
+                                unsigned                     harq_ack_bits,
+                                sr_nof_bits                  sr_bits,
+                                unsigned                     csi_part1_bits);
+
   /// ////////////  Private helpers   //////////////
 
   void remove_unsed_pucch_res(slot_point                   sl_tx,

lib/scheduler/pucch_scheduling/pucch_allocator_impl.h

@@ -273,10 +273,17 @@ pucch_harq_resource_alloc_record pucch_resource_manager::reserve_next_harq_res_a
                 "Indexing of PUCCH resource set exceeds the size of the cell resource array");
   span<resource_tracker> slot_ue_res_array(&slot_res_array[ue_first_res_id], ue_res_id_set_for_harq.size());
 
-  // Check first if there is any PUCCH resource is available.
+  // Check first if there is any PUCCH resource already used by this UE.
   auto* available_resource = std::find_if(slot_ue_res_array.begin(),
                                           slot_ue_res_array.end(),
-                                          [](const resource_tracker res) { return res.rnti == rnti_t::INVALID_RNTI; });
+                                          [crnti](const resource_tracker res) { return res.rnti == crnti; });
+
+  // If not, find an available resource.
+  if (available_resource == slot_ue_res_array.end()) {
+    available_resource = std::find_if(slot_ue_res_array.begin(),
+                                      slot_ue_res_array.end(),
+                                      [](const resource_tracker res) { return res.rnti == rnti_t::INVALID_RNTI; });
+  }
 
   const auto& pucch_res_list = pucch_cfg.pucch_res_list;
 

lib/scheduler/pucch_scheduling/pucch_resource_manager.cpp

@@ -184,6 +184,7 @@ std::optional<uci_allocation> uci_allocator_impl::alloc_uci_harq_ue(cell_resourc
 {
   // [Implementation-defined] We restrict the number of HARQ bits per PUCCH that are expected to carry CSI reporting to
   // 2 , until the PUCCH allocator supports more than this.
+  // TODO: remove this, as with the new refactor we are not constrained by this anymore.
   static const uint8_t max_harq_bits_per_uci = 2U;
 
   const slot_point          pdsch_slot = res_alloc[k0].slot;
@@ -262,15 +263,15 @@ void uci_allocator_impl::multiplex_uci_on_pusch(ul_sched_info&                pu
   const pucch_uci_bits pucch_uci = pucch_alloc.remove_ue_uci_from_pucch(slot_alloc, crnti, ue_cell_cfg);
 
   // In case there are no UCI bits from PUCCH, then there is no UCI to be multiplexed on the PUSCH.
-  if (pucch_uci.harq_ack_nof_bits == 0 and pucch_uci.csi_part1_bits == 0) {
+  if (pucch_uci.harq_ack_nof_bits == 0 and pucch_uci.csi_part1_nof_bits == 0) {
     return;
   }
 
   // We assume that at this point, there are no existing UCI grants in the PUSCH; allocate a new one.
   uci_info& uci = pusch_grant.uci.emplace();
   uci.alpha     = ue_cell_cfg.cfg_dedicated().ul_config.value().init_ul_bwp.pusch_cfg.value().uci_cfg.value().scaling;
 
-  if (pucch_uci.csi_part1_bits != 0) {
+  if (pucch_uci.csi_part1_nof_bits != 0) {
     // The number of bits is computed based on the CSI report configuration.
     add_csi_to_uci_on_pusch(uci.csi.emplace(uci_info::csi_info()), ue_cell_cfg);
   }

lib/scheduler/uci_scheduling/uci_allocator_impl.cpp

@@ -111,7 +111,7 @@ class scheduler_con_res_msg4_test : public base_scheduler_conres_test,
   void enqueue_random_number_of_rach_indications()
   {
     rach_indication_message rach_ind{to_du_cell_index(0), next_slot_rx(), {{0, 0, {}}}};
-    unsigned                nof_preambles = test_rgen::uniform_int<unsigned>(1, 10);
+    auto                    nof_preambles = test_rgen::uniform_int<unsigned>(1, 10);
     for (unsigned i = 0; i != nof_preambles; ++i) {
       rach_ind.occasions[0].preambles.push_back({i, to_rnti((uint16_t)rnti + 1 + i), phy_time_unit{}});
     }
@@ -173,12 +173,12 @@ TEST_P(scheduler_con_res_msg4_test, while_ue_is_in_fallback_then_common_pucch_is
 
   // Wait for ConRes + Msg4 PDCCH, PDSCH and PUCCH to be scheduled.
   ASSERT_TRUE(this->run_slot_until([this]() {
-    for (const auto& pucch : this->last_sched_res_list[to_du_cell_index(0)]->ul.pucchs) {
-      if (pucch.crnti == rnti and pucch.format == pucch_format::FORMAT_1 and pucch.format_1.harq_ack_nof_bits > 0) {
-        return true;
-      }
-    }
-    return false;
+    return std::any_of(this->last_sched_res_list[to_du_cell_index(0)]->ul.pucchs.begin(),
+                       this->last_sched_res_list[to_du_cell_index(0)]->ul.pucchs.end(),
+                       [rnti = this->rnti](const pucch_info& pucch) {
+                         return pucch.crnti == rnti and pucch.format == pucch_format::FORMAT_1 and
+                                pucch.format_1.harq_ack_nof_bits > 0;
+                       });
   }));
 
   // Enqueue SRB1 data; with the UE in fallback mode, and after the MSG4 has been delivered, both common and dedicated
@@ -225,25 +225,31 @@ TEST_P(scheduler_con_res_msg4_test, while_ue_is_in_fallback_then_common_pucch_is
           return true;
         }
       }
+      return false;
     }
     // Case of 3 PUCCH grants.
     else if (this->last_sched_res_list[to_du_cell_index(0)]->ul.pucchs.size() == 3) {
       for (const auto& pucch : this->last_sched_res_list[to_du_cell_index(0)]->ul.pucchs) {
         if (pucch.crnti == rnti and pucch.format == pucch_format::FORMAT_1) {
-          if (pucch.format_1.sr_bits == sr_nof_bits::no_sr) {
-            if (pucch.format_1.harq_ack_nof_bits > 0) {
-              pucch.resources.second_hop_prbs.empty() ? pucch_res_ptrs.f1_ded_ptr    = &pucch
-                                                      : pucch_res_ptrs.f1_common_ptr = &pucch;
+          if (pucch.format_1.sr_bits == sr_nof_bits::no_sr and pucch.format_1.harq_ack_nof_bits > 0 and
+              not pucch.resources.second_hop_prbs.empty()) {
+            pucch_res_ptrs.f1_common_ptr = &pucch;
+          } else if (pucch.format_1.sr_bits == sr_nof_bits::one and pucch.format_1.harq_ack_nof_bits > 0) {
+            // We cannot tell whether it's a f1_ded_sr_ptr or f1_ded_ptr yet only from the bits and format. But this is
+            // not important for the test. We only need to know that both pointers are set.
+            if (pucch_res_ptrs.f1_ded_ptr == nullptr and pucch_res_ptrs.f1_ded_sr_ptr == nullptr) {
+              pucch_res_ptrs.f1_ded_ptr = &pucch;
+            } else if (pucch_res_ptrs.f1_ded_ptr != nullptr and pucch_res_ptrs.f1_ded_sr_ptr == nullptr) {
+              pucch_res_ptrs.f1_ded_sr_ptr = &pucch;
             }
-          } else {
-            pucch_res_ptrs.f1_ded_sr_ptr = &pucch;
           }
         }
         if (pucch_res_ptrs.f1_common_ptr != nullptr and pucch_res_ptrs.f1_ded_ptr != nullptr and
             pucch_res_ptrs.f1_ded_sr_ptr != nullptr) {
           return true;
         }
       }
+      return false;
     }
 
     return false;

tests/unittests/scheduler/scheduler_ue_fallback_mode_test.cpp

@@ -14,6 +14,7 @@
 #include "lib/scheduler/config/sched_config_manager.h"
 #include "srsran/du/du_cell_config_helpers.h"
 #include "srsran/ran/duplex_mode.h"
+#include "srsran/ran/pucch/pucch_info.h"
 #include "srsran/scheduler/config/csi_helper.h"
 #include "srsran/scheduler/config/logical_channel_config_factory.h"
 #include "srsran/scheduler/config/sched_cell_config_helpers.h"
@@ -257,6 +258,12 @@ inline uplink_config make_test_ue_uplink_config(const config_helpers::cell_confi
         config_helpers::generate_k2_candidates(cyclic_prefix::NORMAL, params.tdd_ul_dl_cfg_common.value());
   }
 
+  // Compute the max UCI payload per format.
+  pucch_cfg.format_max_payload[pucch_format_to_uint(pucch_format::FORMAT_1)] = 2U;
+  const auto& res_f2 = std::get<pucch_format_2_3_cfg>(res_basic_f2.format_params);
+  pucch_cfg.format_max_payload[pucch_format_to_uint(pucch_format::FORMAT_2)] = get_pucch_format2_max_payload(
+      res_f2.nof_prbs, res_f2.nof_symbols, to_max_code_rate_float(pucch_cfg.format_2_common_param.value().max_c_rate));
+
   // > SRS config.
   ul_config.init_ul_bwp.srs_cfg.emplace(config_helpers::make_default_srs_config(params));