sched: simplify pucch_collision_manager interface

Author: jonathan-srs

lib/scheduler/pucch_scheduling/pucch_allocator_impl.cpp

@@ -259,9 +259,10 @@ void pucch_allocator_impl::alloc_sr_opportunity(cell_slot_resource_allocator& pu
   const pucch_resource* pucch_sr_res = resource_manager.reserve_sr_resource(
       pucch_slot_alloc.slot, crnti, ue_cell_cfg.init_bwp().ul_ded->pucch_cfg.value());
   if (pucch_sr_res == nullptr) {
-    logger.warning("rnti={}: SR occasion allocation for slot={} skipped. Cause: PUCCH F1 ded. resource not available",
-                   crnti,
-                   pucch_slot_alloc.slot);
+    logger.warning(
+        "rnti={}: SR occasion allocation for slot={} skipped. Cause: PUCCH resource configured for SR not available",
+        crnti,
+        pucch_slot_alloc.slot);
     return;
   }
 

lib/scheduler/pucch_scheduling/pucch_collision_manager.cpp

@@ -11,6 +11,7 @@
 #include "pucch_collision_manager.h"
 #include "../support/pucch/pucch_default_resource.h"
 #include "srsran/adt/bounded_bitset.h"
+#include "srsran/adt/expected.h"
 #include "srsran/adt/static_vector.h"
 #include "srsran/ran/pucch/pucch_configuration.h"
 #include "srsran/ran/pucch/pucch_constants.h"
@@ -198,65 +199,26 @@ bool pucch_collision_manager::check_common_to_ded_collision(r_pucch_t r_pucch, u
   return collision_matrix[r_pucch.value()].test(nof_common_res + cell_res_id);
 }
 
-bool pucch_collision_manager::can_alloc_common(slot_point sl, r_pucch_t r_pucch) const
+pucch_collision_manager::alloc_result_t pucch_collision_manager::alloc_common(slot_point sl, r_pucch_t r_pucch)
 {
-  srsran_sanity_check(sl < last_sl_ind + cell_resource_allocator::RING_ALLOCATOR_SIZE,
-                      "PUCCH resource ring-buffer accessed too far into the future");
-
-  const auto& ctx = slots_ctx[sl.to_uint()];
-  const auto& row = collision_matrix[r_pucch.value()];
-
-  return (row & ctx.current_state).none();
-}
-
-bool pucch_collision_manager::can_alloc_ded(slot_point sl, unsigned cell_res_id) const
-{
-  srsran_sanity_check(sl < last_sl_ind + cell_resource_allocator::RING_ALLOCATOR_SIZE,
-                      "PUCCH resource ring-buffer accessed too far into the future");
-  srsran_assert(cell_res_id < cell_cfg.ded_pucch_resources.size(), "cell_res_id out of range");
-
-  const auto& ctx = slots_ctx[sl.to_uint()];
-  const auto& row = collision_matrix[nof_common_res + cell_res_id];
-
-  return (row & ctx.current_state).none();
+  return alloc_resource(sl, r_pucch.value());
 }
 
-void pucch_collision_manager::alloc_common(slot_point sl, r_pucch_t r_pucch)
+pucch_collision_manager::alloc_result_t pucch_collision_manager::alloc_ded(slot_point sl, unsigned cell_res_id)
 {
-  srsran_sanity_check(sl < last_sl_ind + cell_resource_allocator::RING_ALLOCATOR_SIZE,
-                      "PUCCH resource ring-buffer accessed too far into the future");
-
-  auto& ctx = slots_ctx[sl.to_uint()];
-  ctx.current_state.set(r_pucch.value());
-}
-
-void pucch_collision_manager::alloc_ded(slot_point sl, unsigned cell_res_id)
-{
-  srsran_sanity_check(sl < last_sl_ind + cell_resource_allocator::RING_ALLOCATOR_SIZE,
-                      "PUCCH resource ring-buffer accessed too far into the future");
   srsran_assert(cell_res_id < cell_cfg.ded_pucch_resources.size(), "cell_res_id out of range");
-
-  auto& ctx = slots_ctx[sl.to_uint()];
-  ctx.current_state.set(nof_common_res + cell_res_id);
+  return alloc_resource(sl, nof_common_res + cell_res_id);
 }
 
 void pucch_collision_manager::free_common(slot_point sl, r_pucch_t r_pucch)
 {
-  srsran_sanity_check(sl < last_sl_ind + cell_resource_allocator::RING_ALLOCATOR_SIZE,
-                      "PUCCH resource ring-buffer accessed too far into the future");
-
-  auto& ctx = slots_ctx[sl.to_uint()];
-  ctx.current_state.reset(r_pucch.value());
+  free_resource(sl, r_pucch.value());
 }
 
 void pucch_collision_manager::free_ded(slot_point sl, unsigned cell_res_id)
 {
-  srsran_sanity_check(sl < last_sl_ind + cell_resource_allocator::RING_ALLOCATOR_SIZE,
-                      "PUCCH resource ring-buffer accessed too far into the future");
   srsran_assert(cell_res_id < cell_cfg.ded_pucch_resources.size(), "cell_res_id out of range");
-
-  auto& ctx = slots_ctx[sl.to_uint()];
-  ctx.current_state.reset(nof_common_res + cell_res_id);
+  free_resource(sl, nof_common_res + cell_res_id);
 }
 
 /// Collects all PUCCH resources (common + dedicated) from the cell configuration.
@@ -345,3 +307,38 @@ pucch_collision_manager::compute_mux_regions(span<const detail::resource_info> r
   }
   return mux_regions;
 }
+
+pucch_collision_manager::alloc_result_t pucch_collision_manager::alloc_resource(slot_point sl, unsigned res_idx)
+{
+  srsran_sanity_check(sl < last_sl_ind + cell_resource_allocator::RING_ALLOCATOR_SIZE,
+                      "PUCCH resource ring-buffer accessed too far into the future");
+
+  // TODO: Take in ul_res_grid as a parameter.
+  // TODO: Compute the difference grid ul_res_grid - ctx.pucch_res_grid.
+  // TODO: Check if the resource collides in the difference grid (with any non-PUCCH UL grant).
+
+  // Check for PUCCH-to-PUCCH collisions using the collision matrix.
+  auto&       ctx = slots_ctx[sl.to_uint()];
+  const auto& row = collision_matrix[res_idx];
+  if ((row & ctx.current_state).any()) {
+    return make_unexpected(alloc_failure_reason::PUCCH_COLLISION);
+  }
+
+  // Allocate the resource.
+  ctx.current_state.set(res_idx);
+  // TODO: mark grants in ul_res_grid.
+  // TODO: mark grants in ctx.pucch_res_grid.
+  return default_success_t();
+}
+
+void pucch_collision_manager::free_resource(slot_point sl, unsigned res_idx)
+{
+  srsran_sanity_check(sl < last_sl_ind + cell_resource_allocator::RING_ALLOCATOR_SIZE,
+                      "PUCCH resource ring-buffer accessed too far into the future");
+
+  auto& ctx = slots_ctx[sl.to_uint()];
+  ctx.current_state.reset(res_idx);
+
+  // TODO: Check if any other resource in the same multiplexing region is still allocated.
+  // If not, free the grants in ul_res_grid and ctx.pucch_res_grid.
+}

lib/scheduler/pucch_scheduling/pucch_collision_manager.h

@@ -15,6 +15,7 @@
 #include "srsran/adt/bounded_bitset.h"
 #include "srsran/adt/bounded_integer.h"
 #include "srsran/adt/circular_array.h"
+#include "srsran/adt/expected.h"
 #include "srsran/adt/span.h"
 #include "srsran/adt/static_vector.h"
 #include "srsran/ran/pucch/pucch_constants.h"
@@ -85,35 +86,34 @@ class pucch_collision_manager
   void slot_indication(slot_point sl_tx);
   void stop();
 
-  /// Checks if two dedicated PUCCH resources collide.
+  /// Check if two dedicated PUCCH resources collide.
   bool check_ded_to_ded_collision(unsigned cell_res_id1, unsigned cell_res_id2) const;
 
-  /// Checks if a common PUCCH resource collides with a dedicated PUCCH resource.
+  /// Check if a common PUCCH resource collides with a dedicated PUCCH resource.
   bool check_common_to_ded_collision(r_pucch_t r_pucch, unsigned cell_res_id) const;
 
-  /// Returns the number of multiplexing regions defined in the cell configuration.
+  /// Return the number of multiplexing regions defined in the cell configuration.
   unsigned nof_mux_regions() const { return mux_matrix.size(); }
 
-  /// \brief Checks if a common PUCCH resource can be allocated at a given slot.
-  /// A resource can be allocated if it is not in use and it does not collide with any other resource already allocated
-  /// at the same slot.
-  bool can_alloc_common(slot_point sl, r_pucch_t r_pucch) const;
-
-  /// \brief Checks if a dedicated PUCCH resource can be allocated at a given slot.
-  /// A resource can be allocated if it is not in use and it does not collide with any other resource already allocated
-  /// at the same slot.
-  bool can_alloc_ded(slot_point sl, unsigned cell_res_id) const;
+  /// Reasons for a PUCCH allocation failure.
+  enum class alloc_failure_reason {
+    PUCCH_COLLISION,
+    UL_GRANT_COLLISION,
+  };
+  using alloc_result_t = error_type<alloc_failure_reason>;
 
-  /// Allocates a common PUCCH resource at a given slot.
-  void alloc_common(slot_point sl, r_pucch_t r_pucch);
+  /// \brief Allocate a common PUCCH resource at a given slot.
+  /// \return Success if the allocation was successful, otherwise an error indicating the reason of failure.
+  alloc_result_t alloc_common(slot_point sl, r_pucch_t r_pucch);
 
-  /// Allocates a dedicated PUCCH resource at a given slot.
-  void alloc_ded(slot_point sl, unsigned cell_res_id);
+  /// \brief Allocate a dedicated PUCCH resource at a given slot.
+  /// \return Success if the allocation was successful, otherwise an error indicating the reason of failure.
+  alloc_result_t alloc_ded(slot_point sl, unsigned cell_res_id);
 
-  /// Frees a common PUCCH resource at a given slot.
+  /// Free a common PUCCH resource at a given slot.
   void free_common(slot_point sl, r_pucch_t r_pucch);
 
-  /// Frees a dedicated PUCCH resource at a given slot.
+  /// Free a dedicated PUCCH resource at a given slot.
   void free_ded(slot_point sl, unsigned cell_res_id);
 
 private:
@@ -152,14 +152,20 @@ class pucch_collision_manager
   // Keeps track of the last slot_point used by the resource manager.
   slot_point last_sl_ind;
 
-  /// Computes the collision matrix for all PUCCH resources in the cell configuration.
+  /// Compute the collision matrix for all PUCCH resources in the cell configuration.
   static cell_resources_t compute_resources(const cell_configuration& cell_cfg);
 
-  /// Computes the collision matrix for all PUCCH resources in the cell configuration.
+  /// Compute the collision matrix for all PUCCH resources in the cell configuration.
   static collision_matrix_t compute_collisions(span<const detail::resource_info> resources);
 
-  /// Computes the multiplexing matrix for all PUCCH resources in the cell configuration.
+  /// Compute the multiplexing matrix for all PUCCH resources in the cell configuration.
   static mux_regions_matrix_t compute_mux_regions(span<const detail::resource_info> resources);
+
+  /// Allocate the PUCCH resource at \ref res_idx at a given slot.
+  alloc_result_t alloc_resource(slot_point sl, unsigned res_idx);
+
+  /// Free the PUCCH resource at \ref res_idx at a given slot.
+  void free_resource(slot_point sl, unsigned res_idx);
 };
 
 } // namespace srsran

lib/scheduler/pucch_scheduling/pucch_resource_manager.cpp

@@ -76,7 +76,15 @@ void pucch_resource_manager::stop()
 bool pucch_resource_manager::is_harq_common_resource_available(slot_point sl, size_t r_pucch)
 {
   srsran_assert(r_pucch < 16, "r_PUCCH must be less than 16");
-  return collision_manager.can_alloc_common(sl, r_pucch);
+  // TODO: Remove this method and use collision_manager.alloc_common directly once it checks for UL grant collisions
+  // correctly.
+  auto res = collision_manager.alloc_common(sl, r_pucch);
+  if (res.has_value()) {
+    // Allocation was successful, free the resource and return true.
+    collision_manager.free_common(sl, r_pucch);
+    return true;
+  }
+  return false;
 }
 
 void pucch_resource_manager::reserve_harq_common_resource(slot_point sl, size_t r_pucch)
@@ -156,11 +164,10 @@ pucch_resource_manager::reserve_sr_resource(slot_point slot_sr, rnti_t crnti, co
   // the resource had already been allocated, just return it.
   if (res_rnti != crnti) {
     // Check for collisions.
-    if (not collision_manager.can_alloc_ded(slot_sr, cell_res_id)) {
+    if (not collision_manager.alloc_ded(slot_sr, cell_res_id).has_value()) {
       return nullptr;
     }
     res_rnti = crnti;
-    collision_manager.alloc_ded(slot_sr, cell_res_id);
   }
 
   return res_cfg;
@@ -205,11 +212,10 @@ const pucch_resource* pucch_resource_manager::reserve_csi_resource(slot_point
   // the resource had already been allocated, just return it.
   if (res_rnti != crnti) {
     // Check for collisions.
-    if (not collision_manager.can_alloc_ded(slot_csi, cell_res_id)) {
+    if (not collision_manager.alloc_ded(slot_csi, cell_res_id).has_value()) {
       return nullptr;
     }
     res_rnti = crnti;
-    collision_manager.alloc_ded(slot_csi, cell_res_id);
   }
 
   return res_cfg;
@@ -384,59 +390,61 @@ pucch_harq_resource_alloc_record pucch_resource_manager::reserve_next_harq_res_a
 
   // For PUCCH F0 and F2, we don't use the last 2 resources of the PUCCH resource set; these are reserved for CSI and SR
   // slots and should only be picked for through the specific PUCCH resource indicator.
-  const unsigned nof_eligible_resource =
+  const unsigned nof_eligible_resources =
       cell_cfg.is_pucch_f0_and_f2() ? ue_res_set_id_list.size() - 2U : ue_res_set_id_list.size();
-  srsran_assert(nof_eligible_resource >= 1U,
+  srsran_assert(nof_eligible_resources >= 1U,
                 "rnti={}: Not enough eligible resources from PUCCH resource set={}",
                 crnti,
                 pucch_res_set_idx_to_uint(res_set_idx));
 
   // Get context of wanted slot.
   auto& ctx = slots_ctx[slot_harq.to_uint()];
 
-  std::optional<uint8_t> available_r_pucch;
-  for (uint8_t r_pucch = 0; r_pucch != nof_eligible_resource; ++r_pucch) {
-    const unsigned cell_res_id = ue_res_set_id_list[r_pucch].cell_res_id;
+  std::optional<std::pair<uint8_t, unsigned>> available_res;
 
+  // If a resource is already allocated to this RNTI, use it.
+  for (uint8_t r_pucch = 0; r_pucch != nof_eligible_resources; ++r_pucch) {
+    const unsigned cell_res_id = ue_res_set_id_list[r_pucch].cell_res_id;
     srsran_assert(cell_res_id < ctx.ues_using_pucch_res.size(),
                   "PUCCH resource index from PUCCH resource set exceeds the size of the cell resource array");
     auto& res_rnti = ctx.ues_using_pucch_res[cell_res_id];
 
     if (res_rnti == crnti) {
-      // If a resource is already allocated to this RNTI, use it.
-      available_r_pucch = r_pucch;
+      available_res = {r_pucch, cell_res_id};
       break;
     }
+  }
 
-    if (not available_r_pucch.has_value() and res_rnti == rnti_t::INVALID_RNTI and
-        collision_manager.can_alloc_ded(slot_harq, cell_res_id)) {
-      // Else, keep track of the first available resource.
-      available_r_pucch = r_pucch;
+  if (not available_res.has_value()) {
+    // Else, keep track of the first available resource.
+    for (uint8_t r_pucch = 0; r_pucch != nof_eligible_resources; ++r_pucch) {
+      const unsigned cell_res_id = ue_res_set_id_list[r_pucch].cell_res_id;
+      srsran_assert(cell_res_id < ctx.ues_using_pucch_res.size(),
+                    "PUCCH resource index from PUCCH resource set exceeds the size of the cell resource array");
+      auto& res_rnti = ctx.ues_using_pucch_res[cell_res_id];
+
+      if (res_rnti == rnti_t::INVALID_RNTI and collision_manager.alloc_ded(slot_harq, cell_res_id).has_value()) {
+        ctx.ues_using_pucch_res[cell_res_id] = crnti;
+        available_res                        = {r_pucch, cell_res_id};
+        break;
+      }
     }
   }
 
-  const auto& pucch_res_list = pucch_cfg.pucch_res_list;
-
-  // If there is an available resource, allocate it.
-  if (available_r_pucch.has_value()) {
-    const unsigned r_pucch = available_r_pucch.value();
-    // Get the PUCCH resource ID from the PUCCH resource indicator and the PUCCH resource set.
-    const unsigned cell_res_id = ue_res_set_id_list[r_pucch].cell_res_id;
-
+  // If an available resource was found, return it.
+  if (available_res.has_value()) {
     // Search for the PUCCH resource with the correct PUCCH resource ID from the PUCCH resource list.
-    const auto* res_cfg =
-        std::find_if(pucch_res_list.begin(), pucch_res_list.end(), [cell_res_id](const pucch_resource& res) {
+    const auto& pucch_res_list = pucch_cfg.pucch_res_list;
+    const auto* res_cfg        = std::find_if(
+        pucch_res_list.begin(), pucch_res_list.end(), [cell_res_id = available_res->second](const pucch_resource& res) {
           return res.res_id.cell_res_id == cell_res_id;
         });
     srsran_sanity_check(res_cfg != pucch_res_list.end(),
                         "rnti={}: PUCCH resource with cell_res_id={} not found in PUCCH resource list",
                         crnti,
-                        cell_res_id);
+                        available_res->second);
 
-    // Allocate it and return it.
-    ctx.ues_using_pucch_res[cell_res_id] = crnti;
-    collision_manager.alloc_ded(slot_harq, cell_res_id);
-    return pucch_harq_resource_alloc_record{.resource = res_cfg, .pucch_res_indicator = available_r_pucch.value()};
+    return pucch_harq_resource_alloc_record{.resource = res_cfg, .pucch_res_indicator = available_res->first};
   }
   return pucch_harq_resource_alloc_record{.resource = nullptr};
 }
@@ -505,14 +513,11 @@ const pucch_resource* pucch_resource_manager::reserve_harq_resource_by_res_indic
     return res_cfg;
   }
 
-  // Check for collisions.
-  if (not collision_manager.can_alloc_ded(slot_harq, cell_res_id)) {
+  // Allocate the resource to this RNTI.
+  if (not collision_manager.alloc_ded(slot_harq, cell_res_id).has_value()) {
     return nullptr;
   }
-
-  // Allocate the resource to this RNTI.
   res_rnti = crnti;
-  collision_manager.alloc_ded(slot_harq, cell_res_id);
   return res_cfg;
 }
 

tests/unittests/scheduler/uci_and_pucch/pucch_collision_manager_test.cpp

@@ -58,9 +58,11 @@ TEST(pucch_collision_manager_test, common_resources_dont_collide_with_each_other
   slot_point sl(cell_cfg.ul_cfg_common.init_ul_bwp.generic_params.scs, 0);
   col_manager.slot_indication(sl);
   for (unsigned r_pucch = 0; r_pucch < pucch_constants::MAX_NOF_CELL_COMMON_PUCCH_RESOURCES; ++r_pucch) {
-    ASSERT_TRUE(col_manager.can_alloc_common(sl, r_pucch));
-    col_manager.alloc_common(sl, r_pucch);
-    ASSERT_FALSE(col_manager.can_alloc_common(sl, r_pucch));
+    auto res = col_manager.alloc_common(sl, r_pucch);
+    ASSERT_TRUE(res.has_value());
+    res = col_manager.alloc_common(sl, r_pucch);
+    ASSERT_FALSE(res.has_value());
+    ASSERT_EQ(res.error(), pucch_collision_manager::alloc_failure_reason::PUCCH_COLLISION);
   }
 }