b100: made async callback safer, other tweaks (still issues)

4 files changed, 41 insertions, 19 deletions

diff --git a/host/lib/usrp/b100/b100_ctrl.cpp b/host/lib/usrp/b100/b100_ctrl.cpp
index 22512f413..53054edf7 100644
--- a/host/lib/usrp/b100/b100_ctrl.cpp
+++ b/host/lib/usrp/b100/b100_ctrl.cpp
@@ -34,9 +34,8 @@ bool b100_ctrl_debug = false;
 
 class b100_ctrl_impl : public b100_ctrl {
 public:
-    b100_ctrl_impl(uhd::transport::zero_copy_if::sptr ctrl_transport, const async_cb_type &async_cb):
+    b100_ctrl_impl(uhd::transport::zero_copy_if::sptr ctrl_transport):
         sync_ctrl_fifo(2),
-        _async_cb(async_cb),
         _ctrl_transport(ctrl_transport),
         _seq(0)
     {
@@ -86,6 +85,11 @@ public:
         return boost::uint16_t(words[0]);
     }
 
+    void set_async_cb(const async_cb_type &async_cb){
+        boost::mutex::scoped_lock lock(_async_mutex);
+        _async_cb = async_cb;
+    }
+
 private:
     int send_pkt(boost::uint16_t *cmd);
 
@@ -97,7 +101,7 @@ private:
 
     uhd::transport::zero_copy_if::sptr _ctrl_transport;
     boost::uint8_t _seq;
-    boost::mutex _ctrl_mutex;
+    boost::mutex _ctrl_mutex, _async_mutex;
 };
 
 /***********************************************************************
@@ -206,8 +210,8 @@ void b100_ctrl_impl::viking_marauder_loop(boost::barrier &spawn_barrier) {
     set_thread_priority_safe();
 
     while (not boost::this_thread::interruption_requested()){
-        managed_recv_buffer::sptr rbuf = _ctrl_transport->get_recv_buff();
-        if(!rbuf.get()) continue; //that's ok, there are plenty of villages to pillage!
+        managed_recv_buffer::sptr rbuf = _ctrl_transport->get_recv_buff(1.0);
+        if(rbuf.get() == NULL) continue; //that's ok, there are plenty of villages to pillage!
         const boost::uint16_t *pkt_buf = rbuf->cast<const boost::uint16_t *>();
 
         if(pkt_buf[0] >> 8 == CTRL_PACKET_HEADER_MAGIC) {
@@ -216,16 +220,27 @@ void b100_ctrl_impl::viking_marauder_loop(boost::barrier &spawn_barrier) {
             unpack_ctrl_pkt(pkt_buf, pkt);
 
             if(pkt.pkt_meta.seq != boost::uint8_t(_seq - 1)) {
-                throw uhd::runtime_error("Sequence error on control channel");
+                UHD_MSG(error)
+                    << "Sequence error on control channel." << std::endl
+                    << "Exiting control loop." << std::endl
+                ;
+                return;
             }
             if(pkt.pkt_meta.len > (CTRL_PACKET_LENGTH - CTRL_PACKET_HEADER_LENGTH)) {
-                throw uhd::runtime_error("Control channel packet length too long");
+                UHD_MSG(error)
+                    << "Control channel packet length too long" << std::endl
+                    << "Exiting control loop." << std::endl
+                ;
+                return;
             }
 
             //push it onto the queue
-            sync_ctrl_fifo.push_with_wait(pkt.data);
+            sync_ctrl_fifo.push_with_pop_on_full(pkt.data);
+        }
+        else{ //otherwise let the async callback handle it
+            boost::mutex::scoped_lock lock(_async_mutex);
+            if (not _async_cb.empty()) _async_cb(rbuf);
         }
-        else _async_cb(rbuf); //let the async callback handle it
     }
 }
 
@@ -237,6 +252,6 @@ bool b100_ctrl_impl::get_ctrl_data(ctrl_data_t &pkt_data, double timeout){
 /***********************************************************************
  * Public make function for b100_ctrl interface
  **********************************************************************/
-b100_ctrl::sptr b100_ctrl::make(uhd::transport::zero_copy_if::sptr ctrl_transport, const async_cb_type &async_cb){
-    return sptr(new b100_ctrl_impl(ctrl_transport, async_cb));
+b100_ctrl::sptr b100_ctrl::make(uhd::transport::zero_copy_if::sptr ctrl_transport){
+    return sptr(new b100_ctrl_impl(ctrl_transport));
 }
diff --git a/host/lib/usrp/b100/b100_ctrl.hpp b/host/lib/usrp/b100/b100_ctrl.hpp
index eda194ece..6c3217b11 100644
--- a/host/lib/usrp/b100/b100_ctrl.hpp
+++ b/host/lib/usrp/b100/b100_ctrl.hpp
@@ -37,10 +37,12 @@ public:
     /*!
      * Make a USRP control object from a data transport
      * \param ctrl_transport a USB data transport
-     * \param async_cb the callback for async messages
      * \return a new b100 control object
      */
-    static sptr make(uhd::transport::zero_copy_if::sptr ctrl_transport, const async_cb_type &async_cb);
+    static sptr make(uhd::transport::zero_copy_if::sptr ctrl_transport);
+
+    //! set an async callback for messages
+    virtual void set_async_cb(const async_cb_type &async_cb) = 0;
 
     /*!
      * Write a byte vector to an FPGA register
diff --git a/host/lib/usrp/b100/b100_impl.cpp b/host/lib/usrp/b100/b100_impl.cpp
index 2c90bc873..d1048d4c8 100644
--- a/host/lib/usrp/b100/b100_impl.cpp
+++ b/host/lib/usrp/b100/b100_impl.cpp
@@ -198,13 +198,11 @@ b100_impl::b100_impl(const device_addr_t &device_addr){
     ////////////////////////////////////////////////////////////////////
     // Create controller objects
     ////////////////////////////////////////////////////////////////////
-    _fpga_ctrl = b100_ctrl::make(_ctrl_transport, boost::bind(&b100_impl::handle_async_message, this, _1));
+    _fpga_ctrl = b100_ctrl::make(_ctrl_transport);
+    this->enable_gpif(true); //TODO best place to put this?
     this->check_fpga_compat(); //check after making control
     _fpga_i2c_ctrl = i2c_core_100::make(_fpga_ctrl, B100_REG_SLAVE(3));
     _fpga_spi_ctrl = spi_core_100::make(_fpga_ctrl, B100_REG_SLAVE(2));
-    //init GPIF stuff TODO: best place to put this
-    this->enable_gpif(true);
-    this->reset_gpif(6);
 
     ////////////////////////////////////////////////////////////////////
     // Initialize the properties tree
@@ -219,7 +217,7 @@ b100_impl::b100_impl(const device_addr_t &device_addr){
     ////////////////////////////////////////////////////////////////////
     // setup the mboard eeprom
     ////////////////////////////////////////////////////////////////////
-    const mboard_eeprom_t mb_eeprom = mboard_eeprom_t(*_fx2_ctrl, mboard_eeprom_t::MAP_B000);
+    const mboard_eeprom_t mb_eeprom(*_fx2_ctrl, mboard_eeprom_t::MAP_B000);
     _tree->create<mboard_eeprom_t>(mb_path / "eeprom")
         .set(mb_eeprom)
         .subscribe(boost::bind(&b100_impl::set_mb_eeprom, this, _1));
@@ -401,7 +399,8 @@ b100_impl::b100_impl(const device_addr_t &device_addr){
 }
 
 b100_impl::~b100_impl(void){
-    /* NOP */
+    //set an empty async callback now that we deconstruct
+    _fpga_ctrl->set_async_cb(b100_ctrl::async_cb_type());
 }
 
 void b100_impl::check_fw_compat(void){
diff --git a/host/lib/usrp/b100/io_impl.cpp b/host/lib/usrp/b100/io_impl.cpp
index cb8d035a2..894876a7c 100644
--- a/host/lib/usrp/b100/io_impl.cpp
+++ b/host/lib/usrp/b100/io_impl.cpp
@@ -101,12 +101,18 @@ void b100_impl::io_init(void){
     _tx_otw_type.shift = 0;
     _tx_otw_type.byteorder = uhd::otw_type_t::BO_BIG_ENDIAN;
 
+    //TODO best place to put this?
+    this->reset_gpif(6);
+
     //set the expected packet size in USB frames
     _fpga_ctrl->poke32(B100_REG_MISC_RX_LEN, 4);
 
     //create new io impl
     _io_impl = UHD_PIMPL_MAKE(io_impl, (_data_transport, _rx_dsps.size()));
 
+    //now its safe to register the async callback
+    _fpga_ctrl->set_async_cb(boost::bind(&b100_impl::handle_async_message, this, _1));
+
     //init some handler stuff
     _io_impl->recv_handler.set_vrt_unpacker(&vrt::if_hdr_unpack_le);
     _io_impl->recv_handler.set_converter(_rx_otw_type);