Change in osmo-trx[master]: osmo-trx-ipc
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/19641 ) Change subject: osmo-trx-ipc .. Patch Set 11: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 11 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-Comment-Date: Tue, 25 Aug 2020 09:27:29 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-trx[master]: osmo-trx-ipc
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/19641 ) Change subject: osmo-trx-ipc .. Patch Set 11: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 11 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-Comment-Date: Tue, 25 Aug 2020 08:52:59 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-trx[master]: osmo-trx-ipc
Hoernchen has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/19641 ) Change subject: osmo-trx-ipc .. Patch Set 11: looks like i didn't push it a week ago after all... -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 11 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-Comment-Date: Mon, 24 Aug 2020 23:01:56 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-trx[master]: osmo-trx-ipc
Hello pespin, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 to look at the new patch set (#11). Change subject: osmo-trx-ipc .. osmo-trx-ipc This adds a IPC backend that uses shared memory interface to communicate with (proprietary) devices. Requires config file option dev-args ipc_msock=/path/to/socket to specify the master socket the ipc backend should connect to. If UHD is avaialble the ipc-driver-test tool can be used to test the backend with a uhd device, this was so far only tested with a b2xx. Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 --- M .gitignore M Transceiver52M/Makefile.am M Transceiver52M/device/Makefile.am A Transceiver52M/device/ipc/IPCDevice.cpp A Transceiver52M/device/ipc/IPCDevice.h A Transceiver52M/device/ipc/Makefile.am A Transceiver52M/device/ipc/ipc-driver-test.c A Transceiver52M/device/ipc/ipc-driver-test.h A Transceiver52M/device/ipc/ipc_chan.c A Transceiver52M/device/ipc/ipc_chan.h A Transceiver52M/device/ipc/ipc_shm.c A Transceiver52M/device/ipc/ipc_shm.h A Transceiver52M/device/ipc/ipc_sock.c A Transceiver52M/device/ipc/ipc_sock.h A Transceiver52M/device/ipc/shm.c A Transceiver52M/device/ipc/shm.h A Transceiver52M/device/ipc/uhdwrap.cpp A Transceiver52M/device/ipc/uhdwrap.h M Transceiver52M/device/uhd/UHDDevice.cpp M Transceiver52M/device/uhd/UHDDevice.h M configure.ac M contrib/jenkins.sh M contrib/osmo-trx.spec.in M contrib/systemd/Makefile.am A contrib/systemd/osmo-trx-ipc.service M debian/control A debian/osmo-trx-ipc.install M debian/rules M doc/examples/Makefile.am A doc/examples/osmo-trx-ipc/osmo-trx-ipc.cfg 30 files changed, 3,765 insertions(+), 10 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/41/19641/11 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 11 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-MessageType: newpatchset
Change in osmo-trx[master]: osmo-trx-ipc
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/19641 ) Change subject: osmo-trx-ipc .. Patch Set 10: ping? Would be good if feedback was incorporated so this can finally be merged. -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 10 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-Comment-Date: Mon, 24 Aug 2020 14:28:35 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-trx[master]: osmo-trx-ipc
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/19641 ) Change subject: osmo-trx-ipc .. Patch Set 10: (4 comments) https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_shm.c File Transceiver52M/device/ipc/ipc_shm.c: https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_shm.c@43 PS10, Line 43: malloc Can we use talloc here? (just wondering) https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_sock.c File Transceiver52M/device/ipc/ipc_sock.c: https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_sock.c@59 PS10, Line 59: LOGP(DMAIN, LOGL_ERROR, "Received unknown IPC msg type %d\n", msg_type); 0x%02x https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_sock.c@59 PS10, Line 59: DMAIN DDEV? https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_sock.c@269 PS10, Line 269: IPC_tx_info_ind Why is it commented out? If it's not needed - please drop it. -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 10 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-Comment-Date: Mon, 17 Aug 2020 18:00:13 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-trx[master]: osmo-trx-ipc
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/19641 ) Change subject: osmo-trx-ipc .. Patch Set 10: Also, shouldn't we use thread-safe logging? (see CommonLibs/Logger.h) -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 10 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-Comment-Date: Mon, 17 Aug 2020 17:45:37 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-trx[master]: osmo-trx-ipc
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/19641 ) Change subject: osmo-trx-ipc .. Patch Set 10: (10 comments) https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_chan.c File Transceiver52M/device/ipc/ipc_chan.c: https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_chan.c@66 PS10, Line 66: DMAIN DDEV? https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_chan.c@99 PS10, Line 99: DMAIN DDEV? https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_chan.c@130 PS10, Line 130: DMAIN DDEV? https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_chan.c@138 PS10, Line 138: DMAIN DDEV? https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_chan.c@168 PS10, Line 168: "message type (%d) with ZERO " Not critical, but I still find formatting of this statement looking odd. https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_chan.c@225 PS10, Line 225: DMAIN DDEV? https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_chan.c@230 PS10, Line 230: DMAIN DDEV? https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_chan.c@248 PS10, Line 248: DMAIN DDEV? https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc_chan.c@256 PS10, Line 256: DMAIN DDEV? https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/doc/examples/osmo-trx-ipc/osmo-trx-ipc.cfg File doc/examples/osmo-trx-ipc/osmo-trx-ipc.cfg: https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/doc/examples/osmo-trx-ipc/osmo-trx-ipc.cfg@17 PS10, Line 17: egprs disable duplicate, see line #24 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 10 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-Comment-Date: Mon, 17 Aug 2020 17:44:13 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-trx[master]: osmo-trx-ipc
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/19641 ) Change subject: osmo-trx-ipc .. Patch Set 10: Code-Review+1 (2 comments) https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc-driver-test.c File Transceiver52M/device/ipc/ipc-driver-test.c: https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc-driver-test.c@245 PS10, Line 245: volatile bool ul_running = 0; = false https://gerrit.osmocom.org/c/osmo-trx/+/19641/10/Transceiver52M/device/ipc/ipc-driver-test.c@252 PS10, Line 252: pthread_setname_np(pthread_self(), "uplink_rx"); I was following CamelCase convention in osmo-trx because it uses less characters (only 15 allowed), but fine here for now. -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 10 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-Comment-Date: Mon, 17 Aug 2020 15:40:50 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-trx[master]: osmo-trx-ipc
Hello pespin, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 to look at the new patch set (#10). Change subject: osmo-trx-ipc .. osmo-trx-ipc This adds a IPC backend that uses shared memory interface to communicate with (proprietary) devices. Requires config file option dev-args ipc_msock=/path/to/socket to specify the master socket the ipc backend should connect to. If UHD is avaialble the ipc-driver-test tool can be used to test the backend with a uhd device, this was so far only tested with a b2xx. Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 --- M .gitignore M Transceiver52M/Makefile.am M Transceiver52M/device/Makefile.am A Transceiver52M/device/ipc/IPCDevice.cpp A Transceiver52M/device/ipc/IPCDevice.h A Transceiver52M/device/ipc/Makefile.am A Transceiver52M/device/ipc/ipc-driver-test.c A Transceiver52M/device/ipc/ipc-driver-test.h A Transceiver52M/device/ipc/ipc_chan.c A Transceiver52M/device/ipc/ipc_chan.h A Transceiver52M/device/ipc/ipc_shm.c A Transceiver52M/device/ipc/ipc_shm.h A Transceiver52M/device/ipc/ipc_sock.c A Transceiver52M/device/ipc/ipc_sock.h A Transceiver52M/device/ipc/shm.c A Transceiver52M/device/ipc/shm.h A Transceiver52M/device/ipc/uhdwrap.cpp A Transceiver52M/device/ipc/uhdwrap.h M Transceiver52M/device/uhd/UHDDevice.cpp M Transceiver52M/device/uhd/UHDDevice.h M configure.ac M contrib/jenkins.sh M contrib/osmo-trx.spec.in M contrib/systemd/Makefile.am A contrib/systemd/osmo-trx-ipc.service M debian/control A debian/osmo-trx-ipc.install M debian/rules M doc/examples/Makefile.am A doc/examples/osmo-trx-ipc/osmo-trx-ipc.cfg 30 files changed, 3,769 insertions(+), 10 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/41/19641/10 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 10 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-MessageType: newpatchset
Change in osmo-trx[master]: osmo-trx-ipc
Hoernchen has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-trx/+/19641 )
Change subject: osmo-trx-ipc
..
Patch Set 9:
(39 comments)
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp
File Transceiver52M/device/ipc/IPCDevice.cpp:
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@66
PS7, Line 66: //m_IPC_stream_rx.resize(chans);
> Can be dropped?
Done
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@82
PS7, Line 82: //unsigned int i;
> Drop it
Done
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@94
PS7, Line 94: for (auto i : shm_io_rx_streams)
> That sounds like c++11, do we want to depend on it? (do we already depend on
> it?)
We do, Tom introduced c++11 in 2017 and no one complained about it for 3 years.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@210
PS7, Line 210: /* FIXME: this is actually the sps value, not the sample rate!
> What about this?
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.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@227
PS7, Line 227: /* FIXME: clock ref part of config, not open */
> ACK, can be fixed later.
Done
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@375
PS7, Line 375: //
shm_io_tx_streams.push_back(ipc_shm_init_producer(shm_dec->channels[i]->dl_stream));
> Can be dropped
Done
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@510
PS7, Line 510: //struct ipc_sk_if *ipc_prim = (struct ipc_sk_if *) msg->data;
> Drop
Done
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@653
PS7, Line 653: int IPCDevice::ipc_sock_write(struct osmo_fd *bfd)
> So AFAIU we reimplement all the sock stuff here and we have almost same
> implementation used for the […]
Because they are different after all and depending on a test tool
implementation for internal implementation details does not make sense.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@735
PS7, Line 735: /* _after_ we send it, we can deueue */
> And third typo: dequeue
Done
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@837
PS7, Line 837: void IPCDevice::manually_poll_sock_fds()
> We should really make all this async later in the future.
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.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@1163
PS7, Line 1163: //expect_smpls = len - avail_smpls;
> Can be dropped
Done
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.h
File Transceiver52M/device/ipc/ipc-driver-test.h:
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.h@23
PS7, Line 23: /* 8 channels are plenty */
> Ugh that's one per TRX right? Cannot we have more with some devices? (I know,
> I may have been the on […]
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?!
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.h@30
PS7, Line 30: struct osmo_fd conn_bfd; /* fd for connection to lcr */
> lcr? that's probably some leftover from some old code.
Done
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c
File Transceiver52M/device/ipc/ipc-driver-test.c:
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@70
PS7, Line 70: //enum { DMAIN,
> Can br dropped.
Done
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@79
PS7, Line 79: [DDEV] = {
> Wrong indent.
Done
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@91
PS7, Line 91: #ifdef __cplusplus
> Why is this here? Shouldn't it be a lot higher?
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.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@199
PS7, Line 199: /* FIXME: dynamc chan limit, currently 8 */
> typo
Change in osmo-trx[master]: osmo-trx-ipc
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-trx/+/19641 )
Change subject: osmo-trx-ipc
..
Patch Set 8: Code-Review-1
(37 comments)
Mostly few lines general cleanup and a couple things which should go in a
seaprate commit. Should be quick to apply and then I'm fine with it.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp
File Transceiver52M/device/ipc/IPCDevice.cpp:
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@66
PS7, Line 66: //m_IPC_stream_rx.resize(chans);
Can be dropped?
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@82
PS7, Line 82: //unsigned int i;
Drop it
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@94
PS7, Line 94: for (auto i : shm_io_rx_streams)
That sounds like c++11, do we want to depend on it? (do we already depend on
it?)
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@210
PS7, Line 210: /* FIXME: this is actually the sps value, not the sample rate!
What about this?
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@227
PS7, Line 227: /* FIXME: clock ref part of config, not open */
ACK, can be fixed later.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@375
PS7, Line 375: //
shm_io_tx_streams.push_back(ipc_shm_init_producer(shm_dec->channels[i]->dl_stream));
Can be dropped
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@510
PS7, Line 510: //struct ipc_sk_if *ipc_prim = (struct ipc_sk_if *) msg->data;
Drop
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@653
PS7, Line 653: int IPCDevice::ipc_sock_write(struct osmo_fd *bfd)
So AFAIU we reimplement all the sock stuff here and we have almost same
implementation used for the Driver in a seaprate C file? Why?
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@735
PS7, Line 735: /* _after_ we send it, we can deueue */
And third typo: dequeue
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@837
PS7, Line 837: void IPCDevice::manually_poll_sock_fds()
We should really make all this async later in the future.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/IPCDevice.cpp@1163
PS7, Line 1163: //expect_smpls = len - avail_smpls;
Can be dropped
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.h
File Transceiver52M/device/ipc/ipc-driver-test.h:
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.h@23
PS7, Line 23: /* 8 channels are plenty */
Ugh that's one per TRX right? Cannot we have more with some devices? (I know, I
may have been the one myself writing this line when doing the prototype a while
ago).
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.h@30
PS7, Line 30: struct osmo_fd conn_bfd; /* fd for connection to lcr */
lcr? that's probably some leftover from some old code.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c
File Transceiver52M/device/ipc/ipc-driver-test.c:
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@70
PS7, Line 70: //enum { DMAIN,
Can br dropped.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@79
PS7, Line 79: [DDEV] = {
Wrong indent.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@91
PS7, Line 91: #ifdef __cplusplus
Why is this here? Shouldn't it be a lot higher?
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@199
PS7, Line 199: /* FIXME: dynamc chan limit, currently 8 */
typo dynamic:
We are using "8" as limit in several places, may be worth adding a define for
it.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@254
PS7, Line 254: volatile int ul_running = 0;
better use bool with true/false for boolean, it's clearer then that it's a
boolean variable.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@261
PS7, Line 261: pthread_setname_np(pthread_self(), "uplink rx");
I'd avoid spaces in thread names, it's confusing and you save chars (only 15
available): UplinkRx
https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc-driver-test.c@418
PS7, Line 418: //osmo_signal_register_handler(SS_GLOBAL, IPC_if_signal_cb,
NULL);
Can be dropped?
Change in osmo-trx[master]: osmo-trx-ipc
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 to look at the new patch set (#9). Change subject: osmo-trx-ipc .. osmo-trx-ipc This adds a IPC backend that uses shared memory interface to communicate with (proprietary) devices. Requires config file option dev-args ipc_msock=/path/to/socket to specify the master socket the ipc backend should connect to. If UHD is avaialble the ipc-driver-test tool can be used to test the backend with a uhd device, this was so far only tested with a b2xx. Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 --- M .gitignore M Transceiver52M/Makefile.am M Transceiver52M/device/Makefile.am A Transceiver52M/device/ipc/IPCDevice.cpp A Transceiver52M/device/ipc/IPCDevice.h A Transceiver52M/device/ipc/Makefile.am A Transceiver52M/device/ipc/ipc-driver-test.c A Transceiver52M/device/ipc/ipc-driver-test.h A Transceiver52M/device/ipc/ipc_chan.c A Transceiver52M/device/ipc/ipc_chan.h A Transceiver52M/device/ipc/ipc_shm.c A Transceiver52M/device/ipc/ipc_shm.h A Transceiver52M/device/ipc/ipc_sock.c A Transceiver52M/device/ipc/ipc_sock.h A Transceiver52M/device/ipc/shm.c A Transceiver52M/device/ipc/shm.h A Transceiver52M/device/ipc/uhdwrap.cpp A Transceiver52M/device/ipc/uhdwrap.h M Transceiver52M/device/uhd/UHDDevice.cpp M Transceiver52M/device/uhd/UHDDevice.h M configure.ac M contrib/jenkins.sh M contrib/osmo-trx.spec.in M contrib/systemd/Makefile.am A contrib/systemd/osmo-trx-ipc.service M debian/control A debian/osmo-trx-ipc.install M debian/rules M doc/examples/Makefile.am A doc/examples/osmo-trx-ipc/osmo-trx-ipc.cfg 30 files changed, 3,790 insertions(+), 10 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/41/19641/9 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 9 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-MessageType: newpatchset
Change in osmo-trx[master]: osmo-trx-ipc
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 to look at the new patch set (#8). Change subject: osmo-trx-ipc .. osmo-trx-ipc This adds a IPC backend that uses shared memory interface to communicate with (proprietary) devices. Requires config file option dev-args ipc_msock=/path/to/socket to specify the master socket the ipc backend should connect to. If UHD is avaialble the ipc-driver-test tool can be used to test the backend with a uhd device, this was so far only tested with a b2xx. Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 --- M .gitignore M Transceiver52M/Makefile.am M Transceiver52M/device/Makefile.am A Transceiver52M/device/ipc/IPCDevice.cpp A Transceiver52M/device/ipc/IPCDevice.h A Transceiver52M/device/ipc/Makefile.am A Transceiver52M/device/ipc/ipc-driver-test.c A Transceiver52M/device/ipc/ipc-driver-test.h A Transceiver52M/device/ipc/ipc_chan.c A Transceiver52M/device/ipc/ipc_chan.h A Transceiver52M/device/ipc/ipc_shm.c A Transceiver52M/device/ipc/ipc_shm.h A Transceiver52M/device/ipc/ipc_sock.c A Transceiver52M/device/ipc/ipc_sock.h A Transceiver52M/device/ipc/shm.c A Transceiver52M/device/ipc/shm.h A Transceiver52M/device/ipc/uhdwrap.cpp A Transceiver52M/device/ipc/uhdwrap.h M Transceiver52M/device/uhd/UHDDevice.cpp M Transceiver52M/device/uhd/UHDDevice.h M configure.ac M contrib/jenkins.sh M contrib/osmo-trx.spec.in M contrib/systemd/Makefile.am A contrib/systemd/osmo-trx-ipc.service M debian/control A debian/osmo-trx-ipc.install M debian/rules M doc/examples/Makefile.am A doc/examples/osmo-trx-ipc/osmo-trx-ipc.cfg 30 files changed, 3,790 insertions(+), 10 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/41/19641/8 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 8 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-MessageType: newpatchset
Change in osmo-trx[master]: osmo-trx-ipc
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/19641 ) Change subject: osmo-trx-ipc .. Patch Set 7: (3 comments) https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_chan.c File Transceiver52M/device/ipc/ipc_chan.c: https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_chan.c@52 PS7, Line 52: fprintf can we migrate all of those fprintf/printf statements to the osmocom logging API, please? https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_shm.c File Transceiver52M/device/ipc/ipc_shm.c: https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/ipc_shm.c@67 PS7, Line 67: fprintf likewise here in this function, please use the osmocom logging framework. and in fact, osmo_panic() might be a good idea if you want to log something + exit. https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/shm.c File Transceiver52M/device/ipc/shm.c: https://gerrit.osmocom.org/c/osmo-trx/+/19641/7/Transceiver52M/device/ipc/shm.c@40 PS7, Line 40: #ifdef ENCDECDEBUG : fprintf(stderr, "decode: smpl_buf %d at offset %u\n", i, stream_raw->buffer_offset[i]); : #endif would make sense to #define a DEBUGENCDEC() macro to avoid #ifdef/endif around every line of related logging. The macro can then either be #defined away, or #defined to fprintf - or even anything else. -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 7 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-Comment-Date: Mon, 17 Aug 2020 10:04:30 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-trx[master]: osmo-trx-ipc
Hoernchen has abandoned this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/19664 ) Change subject: osmo-trx-ipc .. Abandoned -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19664 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Iefae70cc079a0174f48309f9ef25157c530e5c32 Gerrit-Change-Number: 19664 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-MessageType: abandon
Change in osmo-trx[master]: osmo-trx-ipc
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 to look at the new patch set (#7). Change subject: osmo-trx-ipc .. osmo-trx-ipc This adds a IPC backend that uses shared memory interface to communicate with (proprietary) devices. Requires config file option dev-args ipc_msock=/path/to/socket to specify the master socket the ipc backend should connect to. If UHD is avaialble the ipc-driver-test tool can be used to test the backend with a uhd device, this was so far only tested with a b2xx. Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 --- M .gitignore M Transceiver52M/Makefile.am M Transceiver52M/device/Makefile.am A Transceiver52M/device/ipc/IPCDevice.cpp A Transceiver52M/device/ipc/IPCDevice.h A Transceiver52M/device/ipc/Makefile.am A Transceiver52M/device/ipc/ipc-driver-test.c A Transceiver52M/device/ipc/ipc-driver-test.h A Transceiver52M/device/ipc/ipc_chan.c A Transceiver52M/device/ipc/ipc_chan.h A Transceiver52M/device/ipc/ipc_shm.c A Transceiver52M/device/ipc/ipc_shm.h A Transceiver52M/device/ipc/ipc_sock.c A Transceiver52M/device/ipc/ipc_sock.h A Transceiver52M/device/ipc/shm.c A Transceiver52M/device/ipc/shm.h A Transceiver52M/device/ipc/uhdwrap.cpp A Transceiver52M/device/ipc/uhdwrap.h M Transceiver52M/device/uhd/UHDDevice.cpp M Transceiver52M/device/uhd/UHDDevice.h M configure.ac M contrib/jenkins.sh M contrib/osmo-trx.spec.in M contrib/systemd/Makefile.am A contrib/systemd/osmo-trx-ipc.service M debian/control A debian/osmo-trx-ipc.install M debian/rules M doc/examples/Makefile.am A doc/examples/osmo-trx-ipc/osmo-trx-ipc.cfg 30 files changed, 3,823 insertions(+), 10 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/41/19641/7 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 7 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-MessageType: newpatchset
Change in osmo-trx[master]: osmo-trx-ipc
Hoernchen has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/19664 ) Change subject: osmo-trx-ipc .. Patch Set 1: yay. -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19664 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Iefae70cc079a0174f48309f9ef25157c530e5c32 Gerrit-Change-Number: 19664 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Comment-Date: Sat, 15 Aug 2020 22:47:38 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-trx[master]: osmo-trx-ipc
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 to look at the new patch set (#6). Change subject: osmo-trx-ipc .. osmo-trx-ipc This adds a IPC backend that uses shared memory interface to communicate with (proprietary) devices. Requires config file option dev-args ipc_msock=/path/to/socket to specify the master socket the ipc backend should connect to. If UHD is avaialble the ipc-driver-test tool can be used to test the backend with a uhd device, this was so far only tested with a b2xx. Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 --- M .gitignore M Transceiver52M/Makefile.am M Transceiver52M/device/Makefile.am A Transceiver52M/device/ipc/IPCDevice.cpp A Transceiver52M/device/ipc/IPCDevice.h A Transceiver52M/device/ipc/Makefile.am A Transceiver52M/device/ipc/ipc-driver-test.c A Transceiver52M/device/ipc/ipc-driver-test.h A Transceiver52M/device/ipc/ipc_chan.c A Transceiver52M/device/ipc/ipc_chan.h A Transceiver52M/device/ipc/ipc_shm.c A Transceiver52M/device/ipc/ipc_shm.h A Transceiver52M/device/ipc/ipc_sock.c A Transceiver52M/device/ipc/ipc_sock.h A Transceiver52M/device/ipc/shm.c A Transceiver52M/device/ipc/shm.h A Transceiver52M/device/ipc/uhdwrap.cpp A Transceiver52M/device/ipc/uhdwrap.h M Transceiver52M/device/uhd/UHDDevice.cpp M Transceiver52M/device/uhd/UHDDevice.h M configure.ac M contrib/jenkins.sh M contrib/osmo-trx.spec.in M contrib/systemd/Makefile.am A contrib/systemd/osmo-trx-ipc.service M debian/control A debian/osmo-trx-ipc.install M debian/rules M doc/examples/Makefile.am A doc/examples/osmo-trx-ipc/osmo-trx-ipc.cfg 30 files changed, 3,815 insertions(+), 10 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/41/19641/6 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 6 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-MessageType: newpatchset
Change in osmo-trx[master]: osmo-trx-ipc
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/19641 ) Change subject: osmo-trx-ipc .. Patch Set 5: thanks for squashing. In case it was forgotten/missed: The debian packaging for osmo-trx-ipc appears to still be missing. -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 5 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-Comment-Date: Sat, 15 Aug 2020 07:42:23 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-trx[master]: osmo-trx-ipc
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 to look at the new patch set (#5). Change subject: osmo-trx-ipc .. osmo-trx-ipc This adds a IPC backend that uses shared memory interface to communicate with (proprietary) devices. Requires config file option dev-args ipc_msock=/path/to/socket to specify the master socket the ipc backend should connect to. If UHD is avaialble the ipc-driver-test tool can be used to test the backend with a uhd device, this was so far only tested with a b2xx. Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 --- M .gitignore M Transceiver52M/Makefile.am M Transceiver52M/device/Makefile.am A Transceiver52M/device/ipc/IPCDevice.cpp A Transceiver52M/device/ipc/IPCDevice.h A Transceiver52M/device/ipc/Makefile.am A Transceiver52M/device/ipc/ipc-driver-test.c A Transceiver52M/device/ipc/ipc-driver-test.h A Transceiver52M/device/ipc/ipc_chan.c A Transceiver52M/device/ipc/ipc_chan.h A Transceiver52M/device/ipc/ipc_shm.c A Transceiver52M/device/ipc/ipc_shm.h A Transceiver52M/device/ipc/ipc_sock.c A Transceiver52M/device/ipc/ipc_sock.h A Transceiver52M/device/ipc/shm.c A Transceiver52M/device/ipc/shm.h A Transceiver52M/device/ipc/uhdwrap.cpp A Transceiver52M/device/ipc/uhdwrap.h M Transceiver52M/device/uhd/UHDDevice.cpp M Transceiver52M/device/uhd/UHDDevice.h M configure.ac M contrib/jenkins.sh M contrib/osmo-trx.spec.in M contrib/systemd/Makefile.am A contrib/systemd/osmo-trx-ipc.service 25 files changed, 3,753 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/41/19641/5 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 5 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-MessageType: newpatchset
Change in osmo-trx[master]: osmo-trx-ipc
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 to look at the new patch set (#4). Change subject: osmo-trx-ipc .. osmo-trx-ipc This adds a IPC backend that uses shared memory interface to communicate with (proprietary) devices. Requires config file option dev-args ipc_msock=/path/to/socket to specify the master socket the ipc backend should connect to. If UHD is avaialble the ipc-driver-test tool can be used to test the backend with a uhd device, this was so far only tested with a b2xx. Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 --- M .gitignore M Transceiver52M/Makefile.am M Transceiver52M/device/Makefile.am A Transceiver52M/device/ipc/IPCDevice.cpp A Transceiver52M/device/ipc/IPCDevice.h A Transceiver52M/device/ipc/Makefile.am A Transceiver52M/device/ipc/ipc-driver-test.c A Transceiver52M/device/ipc/ipc-driver-test.h A Transceiver52M/device/ipc/ipc_chan.c A Transceiver52M/device/ipc/ipc_chan.h A Transceiver52M/device/ipc/ipc_shm.c A Transceiver52M/device/ipc/ipc_shm.h A Transceiver52M/device/ipc/ipc_sock.c A Transceiver52M/device/ipc/ipc_sock.h A Transceiver52M/device/ipc/shm.c A Transceiver52M/device/ipc/shm.h A Transceiver52M/device/ipc/uhdwrap.cpp A Transceiver52M/device/ipc/uhdwrap.h M Transceiver52M/device/uhd/UHDDevice.cpp M Transceiver52M/device/uhd/UHDDevice.h M configure.ac M contrib/jenkins.sh M contrib/osmo-trx.spec.in M contrib/systemd/Makefile.am A contrib/systemd/osmo-trx-ipc.service 25 files changed, 3,753 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/41/19641/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 4 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-MessageType: newpatchset
Change in osmo-trx[master]: osmo-trx-ipc
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 to look at the new patch set (#3). Change subject: osmo-trx-ipc .. osmo-trx-ipc This adds a IPC backend that uses shared memory interface to communicate with (proprietary) devices. Requires config file option dev-args ipc_msock=/path/to/socket to specify the master socket the ipc backend should connect to. If UHD is avaialble the ipc-driver-test tool can be used to test the backend with a uhd device, this was so far only tested with a b2xx. Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 --- M .gitignore M Transceiver52M/Makefile.am M Transceiver52M/device/Makefile.am A Transceiver52M/device/ipc/IPCDevice.cpp A Transceiver52M/device/ipc/IPCDevice.h A Transceiver52M/device/ipc/Makefile.am A Transceiver52M/device/ipc/ipc-driver-test.c A Transceiver52M/device/ipc/ipc-driver-test.h A Transceiver52M/device/ipc/ipc_chan.c A Transceiver52M/device/ipc/ipc_chan.h A Transceiver52M/device/ipc/ipc_shm.c A Transceiver52M/device/ipc/ipc_shm.h A Transceiver52M/device/ipc/ipc_sock.c A Transceiver52M/device/ipc/ipc_sock.h A Transceiver52M/device/ipc/shm.c A Transceiver52M/device/ipc/shm.h A Transceiver52M/device/ipc/uhdwrap.cpp A Transceiver52M/device/ipc/uhdwrap.h M Transceiver52M/device/uhd/UHDDevice.cpp M Transceiver52M/device/uhd/UHDDevice.h M configure.ac M contrib/jenkins.sh M contrib/osmo-trx.spec.in M contrib/systemd/Makefile.am A contrib/systemd/osmo-trx-ipc.service 25 files changed, 3,753 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/41/19641/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 3 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-CC: laforge Gerrit-MessageType: newpatchset
Change in osmo-trx[master]: osmo-trx-ipc
fixeria has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-trx/+/19641 )
Change subject: osmo-trx-ipc
..
Patch Set 2:
(7 comments)
https://gerrit.osmocom.org/c/osmo-trx/+/19641/2/Transceiver52M/device/ipc/ipc-driver-test.c
File Transceiver52M/device/ipc/ipc-driver-test.c:
https://gerrit.osmocom.org/c/osmo-trx/+/19641/2/Transceiver52M/device/ipc/ipc-driver-test.c@72
PS2, Line 72: [DMAIN] = {
Inconsistent formatting.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/2/Transceiver52M/device/ipc/ipc-driver-test.c@74
PS2, Line 74: color
Why three tabs here?
https://gerrit.osmocom.org/c/osmo-trx/+/19641/2/Transceiver52M/device/ipc/ipc-driver-test.c@240
PS2, Line 240: *
formatting
https://gerrit.osmocom.org/c/osmo-trx/+/19641/2/Transceiver52M/device/ipc/ipc_chan.c
File Transceiver52M/device/ipc/ipc_chan.c:
https://gerrit.osmocom.org/c/osmo-trx/+/19641/2/Transceiver52M/device/ipc/ipc_chan.c@7
PS2, Line 7: Permission to use, copy, modify, and/or distribute this
software for any purpose with or without fee is hereby granted.
The formatting of this header is inconsistent compared to the other license
headers introduced by this change. In some files 4 spaces are used as spacing,
in others they are absent. The usual approach is ' * TEXT'.
https://gerrit.osmocom.org/c/osmo-trx/+/19641/2/Transceiver52M/device/ipc/ipc_chan.c@50
PS2, Line 50: %d
chan_nr is unsigned, so %u
https://gerrit.osmocom.org/c/osmo-trx/+/19641/2/Transceiver52M/device/ipc/ipc_chan.c@54
PS2, Line 54: fprintf
BTW, why not LOGP()?
https://gerrit.osmocom.org/c/osmo-trx/+/19641/2/Transceiver52M/device/ipc/ipc_chan.c@134
PS2, Line 134: LOGP(DMAIN, LOGL_INFO,
: "IPC socket not created, "
: "dropping message\n");
Is it how clang-format formats the code? Looks very odd, I am sure it would
take less than 120 chars...
--
To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756
Gerrit-Change-Number: 19641
Gerrit-PatchSet: 2
Gerrit-Owner: Hoernchen
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria
Gerrit-CC: laforge
Gerrit-Comment-Date: Fri, 14 Aug 2020 08:46:25 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Change in osmo-trx[master]: osmo-trx-ipc
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/19641 ) Change subject: osmo-trx-ipc .. Patch Set 2: we are missing to create a debian sub-package either here or in a follow-up patch. -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 2 Gerrit-Owner: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-Comment-Date: Fri, 14 Aug 2020 05:43:35 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-trx[master]: osmo-trx-ipc
Hoernchen has uploaded a new patch set (#2). ( https://gerrit.osmocom.org/c/osmo-trx/+/19641 ) Change subject: osmo-trx-ipc .. osmo-trx-ipc This adds a IPC backend that uses shared memory interface to communicate with (proprietary) devices. Requires config file option dev-args ipc_msock=/path/to/socket to specify the master socket the ipc backend should connect to. If UHD is avaialble the ipc-driver-test tool can be used to test the backend with a uhd device, this was so far only tested with a b2xx. Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 --- M Transceiver52M/Makefile.am M Transceiver52M/device/Makefile.am A Transceiver52M/device/ipc/IPCDevice.cpp A Transceiver52M/device/ipc/IPCDevice.h A Transceiver52M/device/ipc/Makefile.am A Transceiver52M/device/ipc/ipc-driver-test.c A Transceiver52M/device/ipc/ipc-driver-test.h A Transceiver52M/device/ipc/ipc_chan.c A Transceiver52M/device/ipc/ipc_chan.h A Transceiver52M/device/ipc/ipc_shm.c A Transceiver52M/device/ipc/ipc_shm.h A Transceiver52M/device/ipc/ipc_sock.c A Transceiver52M/device/ipc/ipc_sock.h A Transceiver52M/device/ipc/shm.c A Transceiver52M/device/ipc/shm.h A Transceiver52M/device/ipc/uhdwrap.cpp A Transceiver52M/device/ipc/uhdwrap.h M Transceiver52M/device/uhd/UHDDevice.cpp M Transceiver52M/device/uhd/UHDDevice.h M configure.ac 20 files changed, 3,660 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/41/19641/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/19641 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ice63d3499026293ade8aad675ff7a883bcdd5756 Gerrit-Change-Number: 19641 Gerrit-PatchSet: 2 Gerrit-Owner: Hoernchen Gerrit-CC: Jenkins Builder Gerrit-MessageType: newpatchset
