<p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641">View Change</a></p><p>39 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp">File Transceiver52M/device/ipc/IPCDevice.cpp:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@66">Patch Set #7, Line 66:</a> <code style="font-family:monospace,monospace">       //m_IPC_stream_rx.resize(chans);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Can be dropped?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@82">Patch Set #7, Line 82:</a> <code style="font-family:monospace,monospace"> //unsigned int i;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Drop it</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@94">Patch Set #7, Line 94:</a> <code style="font-family:monospace,monospace">        for (auto i : shm_io_rx_streams)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">That sounds like c++11, do we want to depend on it? (do we already depend on it?)</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">We do, Tom introduced c++11 in 2017 and no one complained about it for 3 years.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@210">Patch Set #7, Line 210:</a> <code style="font-family:monospace,monospace">  /* FIXME: this is actually the sps value, not the sample rate!</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">What about this?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is how it currently works and there is no way to change it because the other ipc side implemented it this way and we can't really do much about it for the uhd test tool since the actual sample rate depends on the device and other settings and is dynamically looked up.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@227">Patch Set #7, Line 227:</a> <code style="font-family:monospace,monospace">       /* FIXME: clock ref part of config, not open */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">ACK, can be fixed later.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@375">Patch Set #7, Line 375:</a> <code style="font-family:monospace,monospace">               //              shm_io_tx_streams.push_back(ipc_shm_init_producer(shm_dec->channels[i]->dl_stream));</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Can be dropped</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@510">Patch Set #7, Line 510:</a> <code style="font-family:monospace,monospace">      //struct ipc_sk_if *ipc_prim = (struct ipc_sk_if *) msg->data;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Drop</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@653">Patch Set #7, Line 653:</a> <code style="font-family:monospace,monospace">int IPCDevice::ipc_sock_write(struct osmo_fd *bfd)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">So AFAIU we reimplement all the sock stuff here and we have almost same implementation used for the  […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Because they are different after all and depending on a test tool implementation for internal implementation details does not make sense.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@735">Patch Set #7, Line 735:</a> <code style="font-family:monospace,monospace">              /* _after_ we send it, we can deueue */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">And third typo: dequeue</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@837">Patch Set #7, Line 837:</a> <code style="font-family:monospace,monospace">void IPCDevice::manually_poll_sock_fds()</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">We should really make all this async later in the future.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">No, because it can't be async, because we can't process messages out of order, so we could only asynchronously not handle any events while waiting.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@1163">Patch Set #7, Line 1163:</a> <code style="font-family:monospace,monospace">                    //expect_smpls = len - avail_smpls;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Can be dropped</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.h">File Transceiver52M/device/ipc/ipc-driver-test.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.h@23">Patch Set #7, Line 23:</a> <code style="font-family:monospace,monospace">/* 8 channels are plenty */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Ugh that's one per TRX right? Cannot we have more with some devices? (I know, I may have been the on […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">This tool is a hack for a b210, what do I care about devices no one owns that might or might not work and happen to have more than 8 distinct channels?!</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.h@30">Patch Set #7, Line 30:</a> <code style="font-family:monospace,monospace">   struct osmo_fd conn_bfd; /* fd for connection to lcr */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">lcr? that's probably some leftover from some old code.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c">File Transceiver52M/device/ipc/ipc-driver-test.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@70">Patch Set #7, Line 70:</a> <code style="font-family:monospace,monospace">//enum { DMAIN,</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Can br dropped.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@79">Patch Set #7, Line 79:</a> <code style="font-family:monospace,monospace">    [DDEV] = {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Wrong indent.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@91">Patch Set #7, Line 91:</a> <code style="font-family:monospace,monospace">#ifdef __cplusplus</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Why is this here? Shouldn't it be a lot higher?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">This file was not always a standaloe tool, but used to be included in a cpp file and rand as a thread so it allowed replacing direct function calls to the uhddevice with ipc channels during initial development.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@199">Patch Set #7, Line 199:</a> <code style="font-family:monospace,monospace">          /* FIXME: dynamc chan limit, currently 8 */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">typo dynamic: […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@254">Patch Set #7, Line 254:</a> <code style="font-family:monospace,monospace">volatile int ul_running = 0;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">better use bool with true/false for boolean, it's clearer then that it's a boolean variable.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@261">Patch Set #7, Line 261:</a> <code style="font-family:monospace,monospace">       pthread_setname_np(pthread_self(), "uplink rx");</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I'd avoid spaces in thread names, it's confusing and you save chars (only 15 available): UplinkRx</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@418">Patch Set #7, Line 418:</a> <code style="font-family:monospace,monospace">       //osmo_signal_register_handler(SS_GLOBAL, IPC_if_signal_cb, NULL);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Can be dropped?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@496">Patch Set #7, Line 496:</a> <code style="font-family:monospace,monospace"> //ipc_sock_close()</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Can be uncommented?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_chan.c">File Transceiver52M/device/ipc/ipc_chan.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_chan.c@52">Patch Set #7, Line 52:</a> <code style="font-family:monospace,monospace">fprintf</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Agree with all log related comments from laforge.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">done.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_chan.c@193">Patch Set #7, Line 193:</a> <code style="font-family:monospace,monospace">             /* _after_ we send it, we can deueue */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">typo: dequeue</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_chan.c@265">Patch Set #7, Line 265:</a> <code style="font-family:monospace,monospace">     //IPC_tx_info_ind();</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">what about this?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_shm.h">File Transceiver52M/device/ipc/ipc_shm.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_shm.h@31">Patch Set #7, Line 31:</a> <code style="font-family:monospace,monospace"> volatile struct ipc_shm_raw_stream *this_stream; // plus num_buffers at end</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I don't see num_buffers anywhere nearby?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_shm.c">File Transceiver52M/device/ipc/ipc_shm.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_shm.c@34">Patch Set #7, Line 34:</a> <code style="font-family:monospace,monospace">#define SAMPLE_SIZE_BYTE sizeof(uint16_t) * 2</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">better adding () around here.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_shm.c@36">Patch Set #7, Line 36:</a> <code style="font-family:monospace,monospace">struct ipc_shm_io *ipc_shm_init_consumer(struct ipc_shm_stream *s)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">A bit of documentation in functions would be welcome at some point.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_shm.c@67">Patch Set #7, Line 67:</a> <code style="font-family:monospace,monospace">fprintf</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">likewise here in this function, please use the osmocom logging framework. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_shm.c@120">Patch Set #7, Line 120:</a> <code style="font-family:monospace,monospace">            if (r->buf_ptrs)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">free already checks for NULL, no need to check.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_sock.c">File Transceiver52M/device/ipc/ipc_sock.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_sock.c@70">Patch Set #7, Line 70:</a> <code style="font-family:monospace,monospace">        //struct ipc_sk_if *ipc_prim = (struct ipc_sk_if *) msg->data;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This line can be dropped?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_sock.c@200">Patch Set #7, Line 200:</a> <code style="font-family:monospace,monospace">               /* _after_ we send it, we can deueue */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">typo: dequeue</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_sock.c@270">Patch Set #7, Line 270:</a> <code style="font-family:monospace,monospace">     //IPC_tx_info_ind();</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Can be dropped?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/shm.h">File Transceiver52M/device/ipc/shm.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/shm.h@33">Patch Set #7, Line 33:</a> <code style="font-family:monospace,monospace">      pthread_cond_t cf;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">May be worth documenting all these conditional variables.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/shm.c">File Transceiver52M/device/ipc/shm.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/shm.c@40">Patch Set #7, Line 40:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">#ifdef ENCDECDEBUG<br>              fprintf(stderr, "decode: smpl_buf %d at offset %u\n", i, stream_raw->buffer_offset[i]);<br>#endif<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">would make sense to #define a DEBUGENCDEC() macro to avoid #ifdef/endif around every line of related […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/uhdwrap.h">File Transceiver52M/device/ipc/uhdwrap.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/uhdwrap.h@46">Patch Set #7, Line 46:</a> <code style="font-family:monospace,monospace">   //      bool start() override;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Drop all the commented lines.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">No. They serve as as reminder that you can coveniently wrap start and stop here to debug stuff.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/uhd/UHDDevice.cpp">File Transceiver52M/device/uhd/UHDDevice.cpp:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/uhd/UHDDevice.cpp@155">Patch Set #7, Line 155:</a> <code style="font-family:monospace,monospace"> osmo_cpu_sched_vty_apply_localthread();</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Non related, must be in a separate commit. BTW, why does it fail? It shouldn't... […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">No, this code is used by the uhd ipc test backend, which does not have a vty or anthing else, that is why it fails and that is why it is part of this patchset.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/configure.ac">File configure.ac:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/configure.ac@246">Patch Set #7, Line 246:</a> <code style="font-family:monospace,monospace">AM_CONDITIONAL(DEVICE_UHD, [test "x$with_uhd" == "xyes"])</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This change looks non related, please submit a separate patch. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is a related change because it defaults to uhd which means you can't quite build the ipc backend without uhd, even though is has no deps on uhd.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/contrib/osmo-trx.spec.in">File contrib/osmo-trx.spec.in:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/contrib/osmo-trx.spec.in@239">Patch Set #7, Line 239:</a> <code style="font-family:monospace,monospace"># FIXME: missing: osmo-trx-ipc.cfg</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Please add the file to the commit instead of adding a FIXME here.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/debian/osmo-trx-ipc.install">File debian/osmo-trx-ipc.install:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/debian/osmo-trx-ipc.install@4">Patch Set #7, Line 4:</a> <code style="font-family:monospace,monospace">/usr/share/doc/osmo-trx/examples/osmo-trx-ipc/osmo-trx-ipc.cfg /usr/share/doc/osmo-trx/examples/osmo-trx-ipc/</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">So the cfg file actually exists in the commit, then drop the FIXME line I commented on ;)</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">yep.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-trx/+/19641">change 19641</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/c/osmo-trx/+/19641"/><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-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 </div>
<div style="display:none"> Gerrit-Change-Number: 19641 </div>
<div style="display:none"> Gerrit-PatchSet: 9 </div>
<div style="display:none"> Gerrit-Owner: Hoernchen <ewild@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Hoernchen <ewild@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 17 Aug 2020 15:21:05 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Comment-In-Reply-To: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>