sched: improve comments in PUCCH allocator

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

Author: carlo-gal

lib/scheduler/pucch_scheduling/pucch_allocator_impl.cpp

@@ -223,9 +223,10 @@ std::optional<unsigned> pucch_allocator_impl::alloc_common_and_ded_harq_res(cell
     return std::nullopt;
   }
 
-  const bool new_ue_grants_added = ue_grants_it == pucch_grants.end();
+  const bool new_ue_grant_added = ue_grants_it == pucch_grants.end();
 
-  if (new_ue_grants_added) {
+  // Keep track of whether a new ue_grant has been added; in case the function fails, remove it before exiting.
+  if (new_ue_grant_added) {
     pucch_grants.emplace_back(ue_grants{.rnti = rnti});
   }
 
@@ -239,7 +240,7 @@ std::optional<unsigned> pucch_allocator_impl::alloc_common_and_ded_harq_res(cell
     return pucch_common_info.value().pucch_res_indicator;
   }
 
-  if (new_ue_grants_added) {
+  if (new_ue_grant_added) {
     pucch_grants.pop_back();
   }
 
@@ -1654,11 +1655,14 @@ pucch_allocator_impl::multiplex_resources(slot_point                   sl_tx,
                                           const ue_cell_configuration& ue_cell_cfg,
                                           bool                         preserve_res_indicator)
 {
+  // This function implements the multiplexing pseudo-code for PUCCH resources defined in Section 9.2.5, TS 38.213.
+  // Refer to paragraph starting as "Set Q to the set of resources for transmission of corresponding PUCCHs in a single
+  // slot without repetitions where".
   pucch_grant_list mplexed_grants;
 
   std::vector<pucch_grant> resource_set_q;
 
-  // Build the resource set Q.
+  // Build the resource set Q. Refer to Section 9.2.5, TS 38.213.
   if (candidate_grants.harq_resource.has_value()) {
     resource_set_q.emplace_back(candidate_grants.harq_resource.value());
   }
@@ -1680,7 +1684,7 @@ pucch_allocator_impl::multiplex_resources(slot_point                   sl_tx,
 
   sort_res_set_q();
 
-  // This is the implementation of the sudo code for multiplexing the resources provided in Section 9.2.5, TS 38.213.
+  // This is the implementation of the pseudo-code for multiplexing the resources provided in Section 9.2.5, TS 38.213.
   unsigned o_cnt = 0;
   unsigned j_cnt = 0;
   while (j_cnt < resource_set_q.size()) {
@@ -1701,6 +1705,7 @@ pucch_allocator_impl::multiplex_resources(slot_point                   sl_tx,
           return {};
         }
         // Remove the old resources that got merged from the set.
+        // TODO: check if, by using a different data structure, we can achive the deletion more efficiently.
         resource_set_q.erase(resource_set_q.begin() + j_cnt - o_cnt, resource_set_q.begin() + j_cnt + 1);
 
         // Add the new resource (resulting from the previous merge) to the set.
@@ -1718,8 +1723,8 @@ pucch_allocator_impl::multiplex_resources(slot_point                   sl_tx,
     }
   }
 
-  // The PUCCH resource multiplexing algorithm above is specified for the UE. In the GNB, we need to add an extra
-  // resource Format 1 if slot there is a SR opportunity and HARQ bits to be reported with PUCCH Format 1.
+  // The PUCCH resource multiplexing algorithm above is specified from the UE's perspective. In the GNB, we need to add
+  // an extra resource Format 1 if slot there is a SR opportunity and HARQ bits to be reported with PUCCH Format 1.
   if (resource_set_q.size() == 1 and resource_set_q.front().get_format() == pucch_format::FORMAT_1 and
       resource_set_q.front().bits.harq_ack_nof_bits != 0 and
       resource_set_q.front().bits.sr_bits != sr_nof_bits::no_sr) {
@@ -1760,13 +1765,16 @@ pucch_allocator_impl::multiplex_and_allocate_pucch(cell_slot_resource_allocator&
 {
   slot_point sl_ack = pucch_slot_alloc.slot;
 
-  // Find resource needed for the bits to be reported, assuming the resource is not multiplexed.
+  // Find the resources needed for the UCI bits to be reported, assuming the resources are not multiplexed.
   std::optional<pucch_grant_list> candidate_resources =
       get_pucch_res_pre_multiplexing(sl_ack, new_bits, current_grants, ue_cell_cfg);
   if (not candidate_resources.has_value()) {
     return std::nullopt;
   }
 
+  // TODO: Implement optimization step to avoid the multiplexing process if the UCI bits results in the same PUCCH
+  // grants as the previous allocation.
+
   // Multiplex the resources.
   pucch_grant_list multiplexed_grants = multiplex_resources(
       sl_ack, current_grants.rnti, candidate_resources.value(), ue_cell_cfg, preserve_res_indicator);

lib/scheduler/pucch_scheduling/pucch_allocator_impl.h

@@ -173,57 +173,63 @@ class pucch_allocator_impl final : public pucch_allocator
                                                                             const ue_cell_configuration&   ue_cell_cfg,
                                                                             const dci_context_information& dci_info);
 
-  // Helper that allocates a NEW PUCCH HARQ grant (Format 1).
+  // Helper that allocates a NEW PUCCH HARQ grant (Format 0 or 1).
   std::optional<unsigned> allocate_harq_grant(cell_slot_resource_allocator& pucch_slot_alloc,
                                               rnti_t                        crnti,
                                               const ue_cell_configuration&  ue_cell_cfg);
 
-  // Helper that allocates a new PUCCH HARQ grant (Format 2) for CSI.
+  // Helper that allocates a new PUCCH HARQ grant (Format 2/3/4) for CSI.
   void allocate_csi_grant(cell_slot_resource_allocator& pucch_slot_alloc,
                           rnti_t                        crnti,
                           const ue_cell_configuration&  ue_cell_cfg,
                           unsigned                      csi_part1_bits);
 
+  // Implements the main steps of the multiplexing procedure as defined in TS 38.213, Section 9.2.5.
   std::optional<unsigned> multiplex_and_allocate_pucch(cell_slot_resource_allocator& pucch_slot_alloc,
                                                        pucch_uci_bits                new_bits,
                                                        ue_grants&                    current_grants,
                                                        const ue_cell_configuration&  ue_cell_cfg,
                                                        bool                          preserve_res_indicator = false);
 
+  // Computes which resources are expected to be sent, depending on the UCI bits to be sent, before any multiplexing.
   std::optional<pucch_grant_list> get_pucch_res_pre_multiplexing(slot_point                   sl_tx,
                                                                  pucch_uci_bits               new_bits,
                                                                  ue_grants                    ue_current_grants,
                                                                  const ue_cell_configuration& ue_cell_cfg);
 
+  // Execute the multiplexing algorithm as defined in TS 38.213, Section 9.2.5.
   pucch_grant_list multiplex_resources(slot_point                   sl_tx,
                                        rnti_t                       crnti,
                                        pucch_grant_list             candidate_grants,
                                        const ue_cell_configuration& ue_cell_cfg,
                                        bool                         preserve_res_indicator = false);
 
+  // Applies the multiplexing rules depending on the PUCCH resource format, as per TS 38.213, Section 9.2.5.1/2.
   std::optional<pucch_grant> merge_pucch_resources(span<const pucch_grant> resources_to_merge,
                                                    slot_point              slot_harq,
                                                    rnti_t                  crnti,
                                                    const pucch_config&     pucch_cfg,
                                                    bool                    preserve_res_indicator = false);
 
+  // Allocate the PUCCH PDUs in the scheduler output, depending on the new PUCCH grants to be transmitted, and depending
+  // on the PUCCH PDUs currently allocated.
   std::optional<unsigned> allocate_grants(cell_slot_resource_allocator& pucch_slot_alloc,
                                           ue_grants&                    existing_pucchs,
                                           rnti_t                        crnti,
                                           pucch_grant_list              grants_to_tx,
                                           const ue_cell_configuration&  ue_cell_cfg);
 
-  // Fills the PUCCH HARQ grant for common resources.
+  // Fills the PUCCH HARQ PDU 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.
+  // Fills the PUCCH Format 1 PDU.
   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.
+  // Fills the PUCCH Format 2 PDU.
   void fill_pucch_format2_grant(pucch_info&                  pucch_grant,
                                 rnti_t                       crnti,
                                 const pucch_resource&        pucch_ded_res_cfg,