Attention is currently required from: Hoernchen, pespin, tnt. fixeria 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/be111e64_cada4a94?usp=email : PS4, Line 764: bi->rssi = (int8_t) lround(-(20.0 * log10(rxFullScale / avg) + rssi_offset)); > so there's no nois calculation now? Noise calculation was moved to `Transceiver::logRxBurst()`. It's not needed for TRXD BURST.ind because it's never sent over the wire. > I find this line quite difficult to read with all the operations, parenthesis > and casts. Can you split it into ... Moved the calculation into an inline function. https://gerrit.osmocom.org/c/osmo-trx/+/43115/comment/85477170_0e078302?usp=email : 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. […] It's correct. With the current logic, e.g. `ebp.toa = -0.3` becomes `-77` (`-0.3 * 256.0 = -76.8`). If you multiply after `lround()`, you loose the fractional part: `ebp.toa = -0.3` becomes `0 * 256.0 = 0`. https://gerrit.osmocom.org/c/osmo-trx/+/43115/comment/94133de8_9a477ab1?usp=email : 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 […] `std::transform` is a template function, and at least with `-O2` the compiler does inline `float_soft_bit_to_sbit()` (I checked). That `inline` itself is not necessary, since it's up to the compiler, but it does not hurt either. This whole construction should be equivalent to a plain for-loop in terms of performance. https://gerrit.osmocom.org/c/osmo-trx/+/43115/comment/64bb5437_904b4815?usp=email : 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. Done https://gerrit.osmocom.org/c/osmo-trx/+/43115/comment/db6dc920_7aa8bd2a?usp=email : PS4, Line 1244: msgb_free(msg); > Since you are doing a synchrnonous write here: Why allocating and freeing a > msgb every time? Can't y […] Good idea. Changed to pre-allocate msgbs for each channel. https://gerrit.osmocom.org/c/osmo-trx/+/43115/comment/5dc8b3ef_45e66dec?usp=email : 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 separ […] This change was unintentional, reverted back. -- 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 <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: Hoernchen <[email protected]> Gerrit-CC: pespin <[email protected]> Gerrit-CC: tnt <[email protected]> Gerrit-Attention: Hoernchen <[email protected]> Gerrit-Attention: tnt <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Comment-Date: Tue, 04 Aug 2026 17:50:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <[email protected]>
