<p>Pau Espin Pedrol has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/10743">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">osmo-trx: Add osmo_signal to stop whole transceiver chain correctly on error<br><br>Transceiver::stop() can only be called from either CTRL iface thread or<br>from main thread (running osmocom loop). That's because stop attempts to<br>cancel and then join all the other threads, which would then lock if<br>attempting to stop from some of them.<br>As a result, the best option is to indicate to the user of the<br>transceiver option (osmo-trx.cpp) to stop it in a correct fashion by<br>destroying the object from the main thread.<br><br>Change-Id: Iac1d2dbe2328e735db2d4b933cb67b1af1babca1<br>---<br>M CommonLibs/Makefile.am<br>A CommonLibs/osmo_signal.h<br>M Transceiver52M/Transceiver.cpp<br>M Transceiver52M/Transceiver.h<br>M Transceiver52M/osmo-trx.cpp<br>5 files changed, 81 insertions(+), 2 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/43/10743/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/CommonLibs/Makefile.am b/CommonLibs/Makefile.am</span><br><span>index 2332acb..3173397 100644</span><br><span>--- a/CommonLibs/Makefile.am</span><br><span>+++ b/CommonLibs/Makefile.am</span><br><span>@@ -49,4 +49,5 @@</span><br><span>         Logger.h \</span><br><span>   trx_vty.h \</span><br><span>  debug.h \</span><br><span style="color: hsl(120, 100%, 40%);">+     osmo_signal.h \</span><br><span>      config_defs.h</span><br><span>diff --git a/CommonLibs/osmo_signal.h b/CommonLibs/osmo_signal.h</span><br><span>new file mode 100644</span><br><span>index 0000000..4d201d3</span><br><span>--- /dev/null</span><br><span>+++ b/CommonLibs/osmo_signal.h</span><br><span>@@ -0,0 +1,37 @@</span><br><span style="color: hsl(120, 100%, 40%);">+/* (C) 2018 by sysmocom s.f.m.c. GmbH <info@sysmocom.de></span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * Author: Pau Espin Pedrol <pespin@sysmocom.de></span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * All Rights Reserved</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * This program is free software; you can redistribute it and/or modify</span><br><span style="color: hsl(120, 100%, 40%);">+ * it under the terms of the GNU Affero General Public License as published by</span><br><span style="color: hsl(120, 100%, 40%);">+ * the Free Software Foundation; either version 3 of the License, or</span><br><span style="color: hsl(120, 100%, 40%);">+ * (at your option) any later version.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * This program is distributed in the hope that it will be useful,</span><br><span style="color: hsl(120, 100%, 40%);">+ * but WITHOUT ANY WARRANTY; without even the implied warranty of</span><br><span style="color: hsl(120, 100%, 40%);">+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the</span><br><span style="color: hsl(120, 100%, 40%);">+ * GNU Affero General Public License for more details.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * You should have received a copy of the GNU Affero General Public License</span><br><span style="color: hsl(120, 100%, 40%);">+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+/* Generic signalling/notification infrastructure */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+#pragma once</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/core/signal.h></span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/*</span><br><span style="color: hsl(120, 100%, 40%);">+ * Signalling subsystems</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+enum signal_subsystems {</span><br><span style="color: hsl(120, 100%, 40%);">+ SS_TRANSC,</span><br><span style="color: hsl(120, 100%, 40%);">+};</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* SS_TRANSC signals */</span><br><span style="color: hsl(120, 100%, 40%);">+enum SS_TRANSC {</span><br><span style="color: hsl(120, 100%, 40%);">+   S_TRANSC_STOP_REQUIRED, /* Transceiver fatal error, it should be stopped */</span><br><span style="color: hsl(120, 100%, 40%);">+};</span><br><span>diff --git a/Transceiver52M/Transceiver.cpp b/Transceiver52M/Transceiver.cpp</span><br><span>index a1ebb30..d827adb 100644</span><br><span>--- a/Transceiver52M/Transceiver.cpp</span><br><span>+++ b/Transceiver52M/Transceiver.cpp</span><br><span>@@ -27,6 +27,10 @@</span><br><span> #include "Transceiver.h"</span><br><span> #include <Logger.h></span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+extern "C" {</span><br><span style="color: hsl(120, 100%, 40%);">+#include "osmo_signal.h"</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #ifdef HAVE_CONFIG_H</span><br><span> #include "config.h"</span><br><span> #endif</span><br><span>@@ -115,7 +119,7 @@</span><br><span>   : mBasePort(wBasePort), mLocalAddr(TRXAddress), mRemoteAddr(GSMcoreAddress),</span><br><span>     mClockSocket(TRXAddress, wBasePort, GSMcoreAddress, wBasePort + 100),</span><br><span>     mTransmitLatency(wTransmitLatency), mRadioInterface(wRadioInterface),</span><br><span style="color: hsl(0, 100%, 40%);">-    rssiOffset(wRssiOffset),</span><br><span style="color: hsl(120, 100%, 40%);">+    rssiOffset(wRssiOffset), sig_cbfn(NULL),</span><br><span>     mSPSTx(tx_sps), mSPSRx(rx_sps), mChans(chans), mEdge(false), mOn(false), mForceClockInterface(false),</span><br><span>     mTxFreq(0.0), mRxFreq(0.0), mTSC(0), mMaxExpectedDelayAB(0), mMaxExpectedDelayNB(0),</span><br><span>     mWriteBurstToDiskMask(0)</span><br><span>@@ -219,6 +223,17 @@</span><br><span>   return true;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+void Transceiver::setSignalHandler(osmo_signal_cbfn cbfn)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+  if(this->sig_cbfn)</span><br><span style="color: hsl(120, 100%, 40%);">+    osmo_signal_unregister_handler(SS_TRANSC, this->sig_cbfn, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+  if(cbfn) {</span><br><span style="color: hsl(120, 100%, 40%);">+    this->sig_cbfn = cbfn;</span><br><span style="color: hsl(120, 100%, 40%);">+    osmo_signal_register_handler(SS_TRANSC, this->sig_cbfn, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+  }</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*</span><br><span>  * Start the transceiver</span><br><span>  *</span><br><span>@@ -885,8 +900,12 @@</span><br><span> </span><br><span> void Transceiver::driveReceiveRadio()</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-  if (!mRadioInterface->driveReceiveRadio()) {</span><br><span style="color: hsl(120, 100%, 40%);">+  int rc = mRadioInterface->driveReceiveRadio();</span><br><span style="color: hsl(120, 100%, 40%);">+  if (rc == 0) {</span><br><span>     usleep(100000);</span><br><span style="color: hsl(120, 100%, 40%);">+  } else if (rc < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+    LOG(FATAL) << "radio Interface receive failed, requesting stop.";</span><br><span style="color: hsl(120, 100%, 40%);">+    osmo_signal_dispatch(SS_TRANSC, S_TRANSC_STOP_REQUIRED, this);</span><br><span>   } else if (mForceClockInterface || mTransmitDeadlineClock > mLastClockUpdateTime + GSM::Time(216,0)) {</span><br><span>     mForceClockInterface = false;</span><br><span>     writeClockInterface();</span><br><span>diff --git a/Transceiver52M/Transceiver.h b/Transceiver52M/Transceiver.h</span><br><span>index f9b54f0..e250adc 100644</span><br><span>--- a/Transceiver52M/Transceiver.h</span><br><span>+++ b/Transceiver52M/Transceiver.h</span><br><span>@@ -31,6 +31,7 @@</span><br><span> #include <sys/socket.h></span><br><span> </span><br><span> extern "C" {</span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/core/signal.h></span><br><span> #include "config_defs.h"</span><br><span> }</span><br><span> </span><br><span>@@ -128,6 +129,8 @@</span><br><span>   /** accessor for number of channels */</span><br><span>   size_t numChans() const { return mChans; };</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+  void setSignalHandler(osmo_signal_cbfn cbfn);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>   /** Codes for channel combinations */</span><br><span>   typedef enum {</span><br><span>     FILL,               ///< Channel is transmitted, but unused</span><br><span>@@ -177,6 +180,8 @@</span><br><span> </span><br><span>   double rssiOffset;                      ///< RSSI to dBm conversion offset</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+  osmo_signal_cbfn *sig_cbfn;              ///< Registered Signal Handler to announce events.</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>   /** modulate and add a burst to the transmit queue */</span><br><span>   void addRadioVector(size_t chan, BitVector &bits,</span><br><span>                       int RSSI, GSM::Time &wTime);</span><br><span>diff --git a/Transceiver52M/osmo-trx.cpp b/Transceiver52M/osmo-trx.cpp</span><br><span>index 1c40fcf..d01a4cf 100644</span><br><span>--- a/Transceiver52M/osmo-trx.cpp</span><br><span>+++ b/Transceiver52M/osmo-trx.cpp</span><br><span>@@ -55,6 +55,7 @@</span><br><span> #include "convert.h"</span><br><span> #include "trx_vty.h"</span><br><span> #include "debug.h"</span><br><span style="color: hsl(120, 100%, 40%);">+#include "osmo_signal.h"</span><br><span> }</span><br><span> </span><br><span> #define DEFAULT_CONFIG_FILE     "osmo-trx.cfg"</span><br><span>@@ -112,6 +113,20 @@</span><br><span>      return radio;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* Callback function to be called every time we receive a signal from TRANSC */</span><br><span style="color: hsl(120, 100%, 40%);">+static int transc_sig_cb(unsigned int subsys, unsigned int signal,</span><br><span style="color: hsl(120, 100%, 40%);">+                void *handler_data, void *signal_data)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+        switch (signal) {</span><br><span style="color: hsl(120, 100%, 40%);">+     case S_TRANSC_STOP_REQUIRED:</span><br><span style="color: hsl(120, 100%, 40%);">+          gshutdown = true;</span><br><span style="color: hsl(120, 100%, 40%);">+                break;</span><br><span style="color: hsl(120, 100%, 40%);">+     default:</span><br><span style="color: hsl(120, 100%, 40%);">+                break;</span><br><span style="color: hsl(120, 100%, 40%);">+      }</span><br><span style="color: hsl(120, 100%, 40%);">+     return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* Create transceiver core</span><br><span>  *     The multi-threaded modem core operates at multiples of the GSM rate of</span><br><span>  *     270.8333 ksps and consists of GSM specific modulation, demodulation,</span><br><span>@@ -132,6 +147,8 @@</span><br><span>           return -1;</span><br><span>   }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+        transceiver->setSignalHandler(transc_sig_cb);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>    for (size_t i = 0; i < trx->cfg.num_chans; i++) {</span><br><span>              fifo = radio->receiveFIFO(i);</span><br><span>             if (fifo && transceiver->receiveFIFO(fifo, i))</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/10743">change 10743</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/10743"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-trx </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Iac1d2dbe2328e735db2d4b933cb67b1af1babca1 </div>
<div style="display:none"> Gerrit-Change-Number: 10743 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Pau Espin Pedrol <pespin@sysmocom.de> </div>