Attention is currently required from: Hoernchen, fixeria, tnt. pespin has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/osmo-trx/+/43115?usp=email ) Change subject: Transceiver52M: migrate to libosmo-trx ...................................................................... Patch Set 4: (6 comments) File Transceiver52M/Transceiver.cpp: https://gerrit.osmocom.org/c/osmo-trx/+/43115/comment/93cfb83a_cb93993b?usp=... : PS4, Line 764: bi->rssi = (int8_t) lround(-(20.0 * log10(rxFullScale / avg) + rssi_offset)); so there's no nois calculation now? is it not used?BTW, I find this line quite difficult to read with all the operations, parenthesis and casts. Can you split it into a "float rssi = ..." and then "bi->rssi = (int8_t) lround(rssi);" or something like that? https://gerrit.osmocom.org/c/osmo-trx/+/43115/comment/4a4d1af4_cf487583?usp=... : PS4, Line 801: bi->toa256 = (int16_t) lround(ebp.toa * 256.0); I wonder whether doing the multiplication before rounding is correct here. @tnt@246tNt.com and/or @ewild@sysmocom.de may want to review all these changes in this file. https://gerrit.osmocom.org/c/osmo-trx/+/43115/comment/1b0ebaf4_888b97a3?usp=... : PS4, Line 815: bi->burst, float_soft_bit_to_sbit); Can you pass an inline static func to a std::transform?I also wonder whether the change from a loop in a function into a std:transform may hit somehow performance. Maybe it would be wise to do this change in a separate commit. https://gerrit.osmocom.org/c/osmo-trx/+/43115/comment/2a22bc1c_efb59edd?usp=... : PS4, Line 1200: double noise_dbfs = 20.0 * log10(rxFullScale / mStates[chan].mNoiseLev) + rssi_offset; aah noise calculation happens here now, it's only needed for logging, fine. https://gerrit.osmocom.org/c/osmo-trx/+/43115/comment/cf1fcb2a_8cc67d66?usp=... : PS4, Line 1244: msgb_free(msg); Since you are doing a synchrnonous write here: Why allocating and freeing a msgb every time? Can't you reuse it to avoid tons of mem allocating in the heap all the time? Something like msgb_trim() or whatever, and keep the msgb inside the Transceiver object. https://gerrit.osmocom.org/c/osmo-trx/+/43115/comment/046025b1_99c0d962?usp=... : PS4, Line 1311: char command[64]; why was this changed? Looks like not really related to this patch or at least can be done in a separate patch? -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/43115?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: I458ec85c56e2101d073c8c35f11c147191c4ea0c Gerrit-Change-Number: 43115 Gerrit-PatchSet: 4 Gerrit-Owner: fixeria <vyanitskiy@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: Hoernchen <ewild@sysmocom.de> Gerrit-CC: pespin <pespin@sysmocom.de> Gerrit-CC: tnt <tnt@246tNt.com> Gerrit-Attention: Hoernchen <ewild@sysmocom.de> Gerrit-Attention: tnt <tnt@246tNt.com> Gerrit-Attention: fixeria <vyanitskiy@sysmocom.de> Gerrit-Comment-Date: Tue, 04 Aug 2026 11:44:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No