UHD: stability improvements

lib/src/phy/rf/rf_uhd_generic.h

@@ -21,7 +21,7 @@ private:
   uhd::usrp::multi_usrp::sptr     usrp                         = nullptr;
   const uhd::fs_path              TREE_DBOARD_RX_FRONTEND_NAME = "/mboards/0/dboards/A/rx_frontends/A/name";
   const std::chrono::milliseconds FE_RX_RESET_SLEEP_TIME_MS    = std::chrono::milliseconds(2000UL);
-  uhd::stream_args_t              stream_args;
+  uhd::stream_args_t              stream_args                  = {};
   double                          lo_freq_tx_hz                = 0.0;
   double                          lo_freq_rx_hz                = 0.0;
 
@@ -30,11 +30,21 @@ private:
     // Destroy any previous USRP instance
     usrp = nullptr;
 
+    Debug("Making USRP object with args '" << dev_addr.to_string() << "'");
+
     UHD_SAFE_C_SAVE_ERROR(this, usrp = uhd::usrp::multi_usrp::make(dev_addr);)
   }
 
-  uhd_error set_tx_subdev(const std::string& string) { UHD_SAFE_C_SAVE_ERROR(this, usrp->set_tx_subdev_spec(string);) }
+  uhd_error set_tx_subdev(const std::string& string)
-  uhd_error set_rx_subdev(const std::string& string) { UHD_SAFE_C_SAVE_ERROR(this, usrp->set_rx_subdev_spec(string);) }
+  {
+    Info("Setting tx_subdev_spec to '" << string << "'");
+    UHD_SAFE_C_SAVE_ERROR(this, usrp->set_tx_subdev_spec(string);)
+  }
+  uhd_error set_rx_subdev(const std::string& string)
+  {
+    Info("Setting rx_subdev_spec to '" << string << "'");
+    UHD_SAFE_C_SAVE_ERROR(this, usrp->set_rx_subdev_spec(string);)
+  }
 
   uhd_error test_ad936x_device(uint32_t nof_channels)
   {
@@ -104,8 +114,14 @@ private:
   }
 
 public:
-  rf_uhd_generic(){};
+  rf_uhd_generic() { Info("RF UHD Generic instance constructed"); }
-  virtual ~rf_uhd_generic(){};
+  virtual ~rf_uhd_generic()
+  {
+    rx_stream = nullptr;
+    tx_stream = nullptr;
+    usrp      = nullptr;
+    Debug("RF UHD closed Ok");
+  }
   uhd_error usrp_make(const uhd::device_addr_t& dev_addr_, uint32_t nof_channels) override
   {
     uhd::device_addr_t dev_addr = dev_addr_;
@@ -154,7 +170,6 @@ public:
 
     // Set transmitter subdev spec if specified
     if (not tx_subdev.empty()) {
-      printf("Setting tx_subdev_spec to '%s'\n", tx_subdev.c_str());
       err = set_tx_subdev(tx_subdev);
       if (err != UHD_ERROR_NONE) {
         return err;
@@ -163,7 +178,6 @@ public:
 
     // Set receiver subdev spec if specified
     if (not rx_subdev.empty()) {
-      printf("Setting rx_subdev_spec to '%s'\n", rx_subdev.c_str());
       err = set_rx_subdev(tx_subdev);
       if (err != UHD_ERROR_NONE) {
         return err;
@@ -247,6 +261,7 @@ public:
   }
   uhd_error set_time_unknown_pps(const uhd::time_spec_t& timespec) override
   {
+    Debug("Setting Time at next PPS...");
     UHD_SAFE_C_SAVE_ERROR(this, usrp->set_time_unknown_pps(timespec);)
   }
   uhd_error get_time_now(uhd::time_spec_t& timespec) override
@@ -255,6 +270,7 @@ public:
   }
   uhd_error set_sync_source(const std::string& source) override
   {
+    Debug("Setting PPS source to '" << source << "'")
 #if UHD_VERSION < 3140099
         UHD_SAFE_C_SAVE_ERROR(this, usrp->set_clock_source(source); usrp->set_time_source(source);)
 #else
@@ -267,17 +283,28 @@ public:
   }
   uhd_error set_master_clock_rate(double rate) override
   {
+    Debug("Setting master clock rate to " << rate / 1e6 << " MHz");
     UHD_SAFE_C_SAVE_ERROR(this, usrp->set_master_clock_rate(rate);)
   }
-  uhd_error set_rx_rate(double rate) override { UHD_SAFE_C_SAVE_ERROR(this, usrp->set_rx_rate(rate);) }
+  uhd_error set_rx_rate(double rate) override
-  uhd_error set_tx_rate(double rate) override { UHD_SAFE_C_SAVE_ERROR(this, usrp->set_tx_rate(rate);) }
+  {
+    Debug("Setting Rx Rate to " << rate / 1e6 << "MHz");
+    UHD_SAFE_C_SAVE_ERROR(this, usrp->set_rx_rate(rate);)
+  }
+  uhd_error set_tx_rate(double rate) override
+  {
+    Debug("Setting Tx Rate to " << rate / 1e6 << "MHz");
+    UHD_SAFE_C_SAVE_ERROR(this, usrp->set_tx_rate(rate);)
+  }
   uhd_error set_command_time(const uhd::time_spec_t& timespec) override
   {
     UHD_SAFE_C_SAVE_ERROR(this, usrp->set_command_time(timespec);)
   }
   uhd_error get_rx_stream(size_t& max_num_samps) override
   {
-    UHD_SAFE_C_SAVE_ERROR(this, rx_stream = nullptr; rx_stream = usrp->get_rx_stream(stream_args);
+    Debug("Creating Rx stream");
+    UHD_SAFE_C_SAVE_ERROR(
+        this, rx_stream = nullptr; rx_stream = usrp->get_rx_stream(stream_args);
         max_num_samps = rx_stream->get_max_num_samps();
         if (max_num_samps == 0UL) {
           last_error = "The maximum number of receive samples is zero.";
@@ -286,19 +313,31 @@ public:
   }
   uhd_error get_tx_stream(size_t& max_num_samps) override
   {
-    UHD_SAFE_C_SAVE_ERROR(this, tx_stream = nullptr; tx_stream = usrp->get_tx_stream(stream_args);
+    Debug("Creating Tx stream");
+    UHD_SAFE_C_SAVE_ERROR(
+        this, tx_stream = nullptr; tx_stream = usrp->get_tx_stream(stream_args);
         max_num_samps = tx_stream->get_max_num_samps();
         if (max_num_samps == 0UL) {
           last_error = "The maximum number of transmit samples is zero.";
           return UHD_ERROR_VALUE;
         })
   }
-  uhd_error set_tx_gain(size_t ch, double gain) override { UHD_SAFE_C_SAVE_ERROR(this, usrp->set_tx_gain(gain, ch);) }
+  uhd_error set_tx_gain(size_t ch, double gain) override
-  uhd_error set_rx_gain(size_t ch, double gain) override { UHD_SAFE_C_SAVE_ERROR(this, usrp->set_rx_gain(gain, ch);) }
+  {
+    Debug("Setting channel " << ch << " Tx gain to " << gain << " dB");
+    UHD_SAFE_C_SAVE_ERROR(this, usrp->set_tx_gain(gain, ch);)
+  }
+  uhd_error set_rx_gain(size_t ch, double gain) override
+  {
+    Debug("Setting channel " << ch << " Rx gain to " << gain << " dB");
+    UHD_SAFE_C_SAVE_ERROR(this, usrp->set_rx_gain(gain, ch);)
+  }
   uhd_error get_rx_gain(double& gain) override { UHD_SAFE_C_SAVE_ERROR(this, gain = usrp->get_rx_gain();) }
   uhd_error get_tx_gain(double& gain) override { UHD_SAFE_C_SAVE_ERROR(this, gain = usrp->get_tx_gain();) }
   uhd_error set_tx_freq(uint32_t ch, double target_freq, double& actual_freq) override
   {
+    Debug("Setting channel " << ch << " Tx frequency to " << target_freq / 1e6 << " MHz");
+
     // Create Tune request
     uhd::tune_request_t tune_request(target_freq);
 
@@ -314,6 +353,7 @@ public:
   }
   uhd_error set_rx_freq(uint32_t ch, double target_freq, double& actual_freq) override
   {
+    Debug("Setting channel " << ch << " Rx frequency to " << target_freq / 1e6 << " MHz");
 
     // Create Tune request
     uhd::tune_request_t tune_request(target_freq);

lib/src/phy/rf/rf_uhd_imp.cc

@@ -111,6 +111,8 @@ static const std::chrono::milliseconds RF_UHD_IMP_ASYNCH_MSG_SLEEP_MS = std::chr
 static const uint32_t RF_UHD_IMP_MAX_RX_TRIALS = 100;
 
 struct rf_uhd_handler_t {
+  size_t id;
+
   std::string                            devname;
   std::shared_ptr<rf_uhd_safe_interface> uhd = nullptr;
 
@@ -122,6 +124,8 @@ struct rf_uhd_handler_t {
   bool                                    dynamic_master_rate = true;
   uint32_t                                nof_rx_channels     = 0;
   uint32_t                                nof_tx_channels     = 0;
+  std::array<double, SRSLTE_MAX_CHANNELS> tx_freq             = {};
+  std::array<double, SRSLTE_MAX_CHANNELS> rx_freq             = {};
 
   srslte_rf_error_handler_t    uhd_error_handler     = nullptr;
   void*                        uhd_error_handler_arg = nullptr;
@@ -143,6 +147,10 @@ struct rf_uhd_handler_t {
 #endif /* HAVE_ASYNC_THREAD */
 };
 
+// Store UHD Handler instances as shared pointer to avoid new/delete
+static std::map<size_t, std::shared_ptr<rf_uhd_handler_t> > rf_uhd_map;
+static size_t                                               uhd_handler_counter = 0;
+
 #if UHD_VERSION < 31100
 static void (*handler)(const char*);
 
@@ -277,10 +285,8 @@ static void* async_thread(void* h)
 }
 #endif
 
-static inline void uhd_free(rf_uhd_handler_t* h)
+static inline void uhd_free(rf_uhd_handler_t* handler)
 {
-  rf_uhd_handler_t* handler = (rf_uhd_handler_t*)h;
-
   // NULL handler, return
   if (handler == nullptr) {
     return;
@@ -294,7 +300,8 @@ static inline void uhd_free(rf_uhd_handler_t* h)
   }
 #endif
 
-  delete handler;
+  // Erase element from MAP
+  rf_uhd_map.erase(handler->id);
 }
 
 void rf_uhd_suppress_stdout(void* h)
@@ -453,6 +460,7 @@ const char* rf_uhd_devname(void* h)
 bool rf_uhd_rx_wait_lo_locked(void* h)
 {
   rf_uhd_handler_t* handler = (rf_uhd_handler_t*)h;
+  Debug("Waiting for Rx LO Locked");
 
   // wait for clock source to lock
   std::string sensor_name = "lo_locked";
@@ -519,7 +527,15 @@ int rf_uhd_stop_rx_stream(void* h)
   rf_uhd_handler_t*            handler = (rf_uhd_handler_t*)h;
   std::unique_lock<std::mutex> lock(handler->rx_mutex);
 
-  return rf_uhd_stop_rx_stream_unsafe(handler);
+  if (rf_uhd_stop_rx_stream_unsafe(handler) < SRSLTE_SUCCESS) {
+    return SRSLTE_ERROR;
+  }
+
+  // Make sure the Rx stream is flushed
+  lock.unlock(); // Flush has its own lock
+  rf_uhd_flush_buffer(h);
+
+  return SRSLTE_SUCCESS;
 }
 
 void rf_uhd_flush_buffer(void* h)
@@ -560,21 +576,8 @@ int rf_uhd_open(char* args, void** h)
   return rf_uhd_open_multi(args, h, 1);
 }
 
-int rf_uhd_open_multi(char* args, void** h, uint32_t nof_channels)
+static int uhd_init(rf_uhd_handler_t* handler, char* args, uint32_t nof_channels)
 {
-  // Check valid handler pointer
-  if (h == nullptr) {
-    return SRSLTE_ERROR_INVALID_INPUTS;
-  }
-
-  if (nof_channels > SRSLTE_MAX_CHANNELS) {
-    ERROR("Error opening UHD: maximum number of channels exceeded (%d>%d)", nof_channels, SRSLTE_MAX_CHANNELS);
-    return SRSLTE_ERROR;
-  }
-
-  rf_uhd_handler_t* handler = new rf_uhd_handler_t;
-  *h                        = handler;
-
   // Disable fast-path (U/L/O) messages
   setenv("UHD_LOG_FASTPATH_DISABLE", "1", 0);
 
@@ -676,6 +679,32 @@ int rf_uhd_open_multi(char* args, void** h, uint32_t nof_channels)
   }
   handler->current_master_clock = device_addr.cast("master_clock_rate", 0.0);
 
+  // Parse default frequencies
+  for (uint32_t i = 0; i < nof_channels; i++) {
+    // Parse Tx frequency
+    if (i == 0 and device_addr.has_key("tx_freq")) {
+      handler->tx_freq[i] = device_addr.cast("tx_freq", handler->tx_freq[i]);
+      device_addr.pop("tx_freq");
+    } else {
+      std::string key = "tx_freq" + std::to_string(i);
+      if (device_addr.has_key(key)) {
+        handler->tx_freq[i] = device_addr.cast(key, handler->tx_freq[i]);
+        device_addr.pop(key);
+      }
+    }
+
+    // Parse Rx frequency
+    if (i == 0 and device_addr.has_key("rx_freq")) {
+      handler->rx_freq[i] = device_addr.cast("rx_freq", handler->rx_freq[i]);
+    } else {
+      std::string key = "rx_freq" + std::to_string(i);
+      if (device_addr.has_key(key)) {
+        handler->rx_freq[i] = device_addr.cast("rx_freq" + std::to_string(i), handler->rx_freq[i]);
+        device_addr.pop(key);
+      }
+    }
+  }
+
   // Set dynamic master clock rate configuration
   if (device_addr.has_key("type")) {
     handler->dynamic_master_rate = RH_UHD_IMP_FIX_MASTER_CLOCK_RATE_DEVICE_LIST.count(device_addr["type"]) == 0;
@@ -706,7 +735,6 @@ int rf_uhd_open_multi(char* args, void** h, uint32_t nof_channels)
   // Make USRP
   if (handler->uhd->usrp_make(device_addr, nof_channels) != UHD_ERROR_NONE) {
     print_usrp_error(handler);
-    uhd_free(handler);
     return SRSLTE_ERROR;
   }
 
@@ -777,6 +805,7 @@ int rf_uhd_open_multi(char* args, void** h, uint32_t nof_channels)
   handler->nof_rx_channels = nof_channels;
   handler->nof_tx_channels = nof_channels;
 
+  // Set default Tx/Rx rates
   if (handler->uhd->set_rx_rate(handler->rx_rate) != UHD_ERROR_NONE) {
     print_usrp_error(handler);
     return SRSLTE_ERROR;
@@ -786,6 +815,7 @@ int rf_uhd_open_multi(char* args, void** h, uint32_t nof_channels)
     return SRSLTE_ERROR;
   }
 
+  // Reset timestamps
   if (nof_channels > 1 and clock_src != "gpsdo") {
     handler->uhd->set_time_unknown_pps(uhd::time_spec_t());
   }
@@ -800,6 +830,27 @@ int rf_uhd_open_multi(char* args, void** h, uint32_t nof_channels)
     return SRSLTE_ERROR;
   }
 
+  // Tune LOs if the default frequency is provided
+  bool require_wait_rx_lock = false;
+  for (uint32_t i = 0; i < nof_channels; i++) {
+    if (std::isnormal(handler->rx_freq[i])) {
+      if (handler->uhd->set_rx_freq(i, handler->rx_freq[i], handler->rx_freq[i]) != UHD_ERROR_NONE) {
+        print_usrp_error(handler);
+        return SRSLTE_ERROR;
+      }
+      rf_uhd_rx_wait_lo_locked(handler);
+      require_wait_rx_lock = true;
+    }
+  }
+  for (uint32_t i = 0; i < nof_channels; i++) {
+    if (std::isnormal(handler->tx_freq[i])) {
+      if (handler->uhd->set_tx_freq(i, handler->tx_freq[i], handler->tx_freq[i]) != UHD_ERROR_NONE) {
+        print_usrp_error(handler);
+        return SRSLTE_ERROR;
+      }
+    }
+  }
+
   // Populate RF device info
   uhd::gain_range_t tx_gain_range;
   uhd::gain_range_t rx_gain_range;
@@ -832,6 +883,37 @@ int rf_uhd_open_multi(char* args, void** h, uint32_t nof_channels)
   return SRSLTE_SUCCESS;
 }
 
+int rf_uhd_open_multi(char* args, void** h, uint32_t nof_channels)
+{
+  // Check valid handler pointer
+  if (h == nullptr) {
+    return SRSLTE_ERROR_INVALID_INPUTS;
+  }
+
+  if (nof_channels > SRSLTE_MAX_CHANNELS) {
+    ERROR("Error opening UHD: maximum number of channels exceeded (%d>%d)", nof_channels, SRSLTE_MAX_CHANNELS);
+    return SRSLTE_ERROR;
+  }
+
+  // Create UHD handler
+  rf_uhd_map[uhd_handler_counter] = std::make_shared<rf_uhd_handler_t>();
+  rf_uhd_handler_t* handler       = rf_uhd_map[uhd_handler_counter].get();
+  handler->id                     = uhd_handler_counter;
+  uhd_handler_counter++;
+  *h = handler;
+
+  // Initialise UHD handler
+  if (uhd_init(handler, args, nof_channels) < SRSLTE_SUCCESS) {
+    ERROR("uhd_init failed, freeing...");
+    // Free/Close UHD handler properly
+    uhd_free(handler);
+    *h = nullptr;
+    return SRSLTE_ERROR;
+  }
+
+  return SRSLTE_SUCCESS;
+}
+
 int rf_uhd_close(void* h)
 {
   // Makes sure Tx is ended
@@ -852,7 +934,7 @@ int rf_uhd_close(void* h)
 static inline void rf_uhd_set_master_clock_rate_unsafe(rf_uhd_handler_t* handler, double rate)
 {
   // Set master clock rate if it is allowed and change is required
-  if (handler->dynamic_master_rate && handler->current_master_clock != rate) {
+  if (handler->dynamic_master_rate and handler->current_master_clock != rate) {
     if (handler->uhd->set_master_clock_rate(rate) != UHD_ERROR_NONE) {
       print_usrp_error(handler);
     }
@@ -1078,44 +1160,61 @@ srslte_rf_info_t* rf_uhd_get_info(void* h)
 
   return info;
 }
+static bool rf_uhd_set_freq_ch(rf_uhd_handler_t* handler, uint32_t ch, double& freq, bool is_tx)
+{
+  double& curr_freq = (is_tx) ? handler->tx_freq[ch] : handler->rx_freq[ch];
+
+  // Skip if frequency is unchanged
+  if (round(freq) == round(curr_freq)) {
+    return false;
+  }
+
+  // Set frequency
+  if (is_tx) {
+    if (handler->uhd->set_tx_freq(ch, freq, curr_freq) != UHD_ERROR_NONE) {
+      print_usrp_error(handler);
+    }
+  } else {
+    if (handler->uhd->set_rx_freq(ch, freq, curr_freq) != UHD_ERROR_NONE) {
+      print_usrp_error(handler);
+    }
+  }
+  return true;
+}
 
 double rf_uhd_set_rx_freq(void* h, uint32_t ch, double freq)
 {
+  bool require_rx_wait_lo_locked = false;
+
   rf_uhd_handler_t* handler = (rf_uhd_handler_t*)h;
   if (ch < handler->nof_rx_channels) {
-    if (handler->uhd->set_rx_freq(ch, freq, freq) != UHD_ERROR_NONE) {
+    require_rx_wait_lo_locked |= rf_uhd_set_freq_ch(handler, ch, freq, false);
-      print_usrp_error(handler);
-      return SRSLTE_ERROR;
-    }
   } else {
     for (uint32_t i = 0; i < handler->nof_rx_channels; i++) {
-      if (handler->uhd->set_rx_freq(i, freq, freq) != UHD_ERROR_NONE) {
+      require_rx_wait_lo_locked |= rf_uhd_set_freq_ch(handler, i, freq, false);
-        print_usrp_error(handler);
-        return SRSLTE_ERROR;
-      }
     }
   }
+
+  // wait for LO Locked
+  if (require_rx_wait_lo_locked) {
     rf_uhd_rx_wait_lo_locked(handler);
-  return freq;
+  }
+
+  return handler->rx_freq[ch % handler->nof_rx_channels];
 }
 
 double rf_uhd_set_tx_freq(void* h, uint32_t ch, double freq)
 {
   rf_uhd_handler_t* handler = (rf_uhd_handler_t*)h;
   if (ch < handler->nof_tx_channels) {
-    if (handler->uhd->set_tx_freq(ch, freq, freq) != UHD_ERROR_NONE) {
+    rf_uhd_set_freq_ch(handler, ch, freq, true);
-      print_usrp_error(handler);
-      return SRSLTE_ERROR;
-    }
   } else {
     for (uint32_t i = 0; i < handler->nof_tx_channels; i++) {
-      if (handler->uhd->set_tx_freq(i, freq, freq) != UHD_ERROR_NONE) {
+      rf_uhd_set_freq_ch(handler, i, freq, true);
-        print_usrp_error(handler);
-        return SRSLTE_ERROR;
     }
   }
-  }
+
-  return freq;
+  return handler->tx_freq[ch % handler->nof_tx_channels];
 }
 
 void rf_uhd_get_time(void* h, time_t* secs, double* frac_secs)

lib/src/phy/rf/rf_uhd_safe.h

@@ -122,6 +122,7 @@ public:
   virtual uhd_error get_time_now(uhd::time_spec_t& timespec)                             = 0;
   uhd_error         start_rx_stream(double delay)
   {
+    Debug("Starting Rx stream");
     uhd::time_spec_t time_spec;
     uhd_error        err = get_time_now(time_spec);
     if (err != UHD_ERROR_NONE) {
@@ -137,7 +138,9 @@ public:
   }
   uhd_error stop_rx_stream()
   {
+    Debug("Stopping Rx stream");
     UHD_SAFE_C_SAVE_ERROR(this, uhd::stream_cmd_t stream_cmd(uhd::stream_cmd_t::STREAM_MODE_STOP_CONTINUOUS);
+                          stream_cmd.stream_now = true;
                           rx_stream->issue_stream_cmd(stream_cmd);)
   }
   virtual uhd_error set_sync_source(const std::string& source)                                         = 0;
@@ -147,9 +150,7 @@ public:
   virtual uhd_error set_tx_rate(double rate)                                                           = 0;
   virtual uhd_error set_command_time(const uhd::time_spec_t& timespec)                                 = 0;
   virtual uhd_error get_rx_stream(size_t& max_num_samps)                                               = 0;
-  virtual uhd_error destroy_rx_stream() { UHD_SAFE_C_SAVE_ERROR(this, rx_stream = nullptr;) }
   virtual uhd_error get_tx_stream(size_t& max_num_samps)                                               = 0;
-  virtual uhd_error destroy_tx_stream() { UHD_SAFE_C_SAVE_ERROR(this, rx_stream = nullptr;) }
   virtual uhd_error set_tx_gain(size_t ch, double gain)                                                = 0;
   virtual uhd_error set_rx_gain(size_t ch, double gain)                                                = 0;
   virtual uhd_error get_rx_gain(double& gain)                                                          = 0;

lib/src/radio/test/benchmark_radio.cc

@@ -332,6 +332,7 @@ static void* radio_thread_run(void* arg)
     radio_args.nof_carriers = 1;
     radio_args.device_args  = radios_args[r].empty() ? "auto" : radios_args[r];
     radio_args.rx_gain      = agc_enable ? -1 : rf_gain;
+    radio_args.tx_gain      = agc_enable ? -1 : rf_gain;
     radio_args.device_name  = radio_device;
 
     if (radio_h[r]->init(radio_args, &phy) != SRSLTE_SUCCESS) {

srsue/src/phy/sync.cc

@@ -125,6 +125,10 @@ void sync::stop()
     q->stop();
   }
   running = false;
+
+  // Reset (stop Rx stream) as soon as possible to avoid base-band Rx buffer overflow
+  radio_h->reset();
+
   wait_thread_finish();
 }