Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/14167 )
Change subject: Add rate_ctr support to store/retrieve SDR errors through VTY ...................................................................... Patch Set 1: (5 comments) Mostly cosmetic comments, not merge blockers. https://gerrit.osmocom.org/#/c/14167/1/CommonLibs/osmo_signal.h File CommonLibs/osmo_signal.h: https://gerrit.osmocom.org/#/c/14167/1/CommonLibs/osmo_signal.h@30 PS1, Line 30: SS_DEVICE cosmetic: missing comma. https://gerrit.osmocom.org/#/c/14167/1/CommonLibs/osmo_signal.h@49 PS1, Line 49: uint32_t Do we really need fixed-size types for that? Why not generic unsigned int? https://gerrit.osmocom.org/#/c/14167/1/CommonLibs/trx_rate_ctr.cpp File CommonLibs/trx_rate_ctr.cpp: https://gerrit.osmocom.org/#/c/14167/1/CommonLibs/trx_rate_ctr.cpp@69 PS1, Line 69: (uint32_t)-1 As far as I understand, it's UINT32_MAX, right? https://gerrit.osmocom.org/#/c/14167/1/CommonLibs/trx_rate_ctr.cpp@163 PS1, Line 163: size_t i; New line is missing here. Alternatively we can declare this counter in the 'for' loop statement, as it is not used anywhere else: for (size_t i = 0, ...) AFAIR, it's allowed in C++. https://gerrit.osmocom.org/#/c/14167/1/Transceiver52M/device/lms/LMSDevice.cpp File Transceiver52M/device/lms/LMSDevice.cpp: https://gerrit.osmocom.org/#/c/14167/1/Transceiver52M/device/lms/LMSDevice.cpp@594 PS1, Line 594: cosmetic: ws -- To view, visit https://gerrit.osmocom.org/14167 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: I78b158141697e5714d04db8b9ccc96f31f34f439 Gerrit-Change-Number: 14167 Gerrit-PatchSet: 1 Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-CC: Vadim Yanitskiy <axilira...@gmail.com> Gerrit-Comment-Date: Sat, 25 May 2019 11:03:16 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
