Change in osmo-trx[master]: add support for xtrx

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/14018 )

Change subject: add support for xtrx
......................................................................


Patch Set 1:

(11 comments)

Some comments after quick review.

https://gerrit.osmocom.org/#/c/14018/1/Transceiver52M/device/xtrx/XTRXDevice.cpp
File Transceiver52M/device/xtrx/XTRXDevice.cpp:

https://gerrit.osmocom.org/#/c/14018/1/Transceiver52M/device/xtrx/XTRXDevice.cpp@44
PS1, Line 44: 	LOG(INFO) << "creating XTRX device:"
You should be using category DEV here, not MAIN (see other devices).


https://gerrit.osmocom.org/#/c/14018/1/Transceiver52M/device/xtrx/XTRXDevice.cpp@162
PS1, Line 162: 	return NORMAL;
This NORMAL thing here makes no sense in the context of this function afaik.


https://gerrit.osmocom.org/#/c/14018/1/Transceiver52M/device/xtrx/XTRXDevice.cpp@167
PS1, Line 167: 	if (device) {
Drop {}


https://gerrit.osmocom.org/#/c/14018/1/Transceiver52M/device/xtrx/XTRXDevice.cpp@175
PS1, Line 175: 	if (started) {
Drop {}


https://gerrit.osmocom.org/#/c/14018/1/Transceiver52M/device/xtrx/XTRXDevice.cpp@212
PS1, Line 212: 	if (loopback) {
Drop {}


https://gerrit.osmocom.org/#/c/14018/1/Transceiver52M/device/xtrx/XTRXDevice.cpp@231
PS1, Line 231: 	if (started) {
early return: if (!started) reutrn false;


https://gerrit.osmocom.org/#/c/14018/1/Transceiver52M/device/xtrx/XTRXDevice.cpp@278
PS1, Line 278: 	LOG(NOTICE) << "Setting TX gain to " << dB << " dB.";
LOGCHAN


https://gerrit.osmocom.org/#/c/14018/1/Transceiver52M/device/xtrx/XTRXDevice.cpp@282
PS1, Line 282: 		LOG(ERR) << "Error setting TX gain res: " << res;
LOGCHAN. LOGCHAN everywhere where "chan" is passed as parameter in this file.


https://gerrit.osmocom.org/#/c/14018/1/Transceiver52M/device/xtrx/XTRXDevice.cpp@319
PS1, Line 319: 	int res = xtrx_recv_sync_ex(device, &ri);
Looks like we need to add smpl_buf usage in here (see my latest commits merged).


https://gerrit.osmocom.org/#/c/14018/1/debian/osmo-trx-xtrx.install
File debian/osmo-trx-xtrx.install:

https://gerrit.osmocom.org/#/c/14018/1/debian/osmo-trx-xtrx.install@3
PS1, Line 3: /usr/bin/osmo-trx-xtrx
why some strt with / and some doesn't? Unify style if possible.


https://gerrit.osmocom.org/#/c/14018/1/doc/examples/osmo-trx-xtrx/osmo-trx-xtrx.cfg
File doc/examples/osmo-trx-xtrx/osmo-trx-xtrx.cfg:

https://gerrit.osmocom.org/#/c/14018/1/doc/examples/osmo-trx-xtrx/osmo-trx-xtrx.cfg@21
PS1, Line 21:   tx-path BAND1
Are you sure this belongs here? looks copied from LMS file.



-- 
To view, visit https://gerrit.osmocom.org/14018
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iad73e0611e7951d5bcfcc918063cc3778cb1dd8f
Gerrit-Change-Number: 14018
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-CC: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Comment-Date: Mon, 13 May 2019 16:27:58 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190513/9c017057/attachment.htm>