Change in osmo-trx[master]: LMSDevice: make use of dev-args in osmo-trx.cfg

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Pau Espin Pedrol gerrit-no-reply at lists.osmocom.org
Tue Dec 11 17:37:05 UTC 2018


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

Change subject: LMSDevice: make use of dev-args in osmo-trx.cfg
......................................................................


Patch Set 2: Code-Review-1

(6 comments)

https://gerrit.osmocom.org/#/c/12245/2/Transceiver52M/device/lms/LMSDevice.cpp
File Transceiver52M/device/lms/LMSDevice.cpp:

https://gerrit.osmocom.org/#/c/12245/2/Transceiver52M/device/lms/LMSDevice.cpp@25
PS2, Line 25: #include <boost/algorithm/string.hpp>
I'm actually in process of getting rid of boost in osmo-trx code, since we only use it in URSP1 (and can be easily dropped completelly). Can you use something else?

See https://gerrit.osmocom.org/#/c/osmo-trx/+/12088/ and grep for "boost".


https://gerrit.osmocom.org/#/c/12245/2/Transceiver52M/device/lms/LMSDevice.cpp@103
PS2, Line 103:  *  \return index of first matching item or -1 (no match) */
s/item/device, I was first confused with item meaning the first key or value.


"Index of first matching item in the list or -1 (no match".


https://gerrit.osmocom.org/#/c/12245/2/Transceiver52M/device/lms/LMSDevice.cpp@109
PS2, Line 109: 	boost::split(filters, args, boost::is_any_of(","));
As said, please use something else. I think there's some code you can reuse in ./Transceiver52M/osmo-trx.cpp, see comma_delimited_to_vector() in there.

Feel free to improve it by passing a new param "separator" and then you can use it to first break by "," and later by "=" if required.


https://gerrit.osmocom.org/#/c/12245/2/Transceiver52M/device/lms/LMSDevice.cpp@159
PS2, Line 159: 		delete [] info_list;
I know in other places is "delete []", but I think it's better having "delete[]" (as in https://en.cppreference.com/w/cpp/memory/new/operator_delete)


https://gerrit.osmocom.org/#/c/12245/2/Transceiver52M/device/lms/LMSDevice.cpp@163
PS2, Line 163: 	LOGC(DDEV, INFO) << "Using device: " << info_list[dev_id];
May be worth printing the index too.


https://gerrit.osmocom.org/#/c/12245/2/tests/Transceiver52M/LMSDeviceTest.cpp
File tests/Transceiver52M/LMSDeviceTest.cpp:

https://gerrit.osmocom.org/#/c/12245/2/tests/Transceiver52M/LMSDeviceTest.cpp@43
PS2, Line 43: 	delete [] info_list;
same (delete[])



-- 
To view, visit https://gerrit.osmocom.org/12245
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: Ib9aaa066a01bf9de3f78234d7ada884d6f28c852
Gerrit-Change-Number: 12245
Gerrit-PatchSet: 2
Gerrit-Owner: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Comment-Date: Tue, 11 Dec 2018 17:37:05 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181211/39c93cf9/attachment.htm>


More information about the gerrit-log mailing list