Attention is currently required from: pespin.
Hoernchen has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-trx/+/30966 )

Change subject: ms: adjust float<->integral type conversion
......................................................................


Patch Set 1:

(6 comments)

Commit Message:

https://gerrit.osmocom.org/c/osmo-trx/+/30966/comment/d07eba64_4b45966d
PS1, Line 11: split the two cases.
> can you pinpoint where did you "split the two cases"? I'm either not 
> understand correctly or I didn' […]
Take a closer look at the () + args, as explained in the message...


File Transceiver52M/Complex.h:

https://gerrit.osmocom.org/c/osmo-trx/+/30966/comment/8cc4c948_cc3e3bb8
PS1, Line 32:   typedef Real value_type;
> why this typedef?
To make the template work with the useless hand-rolled osmo-trx complex class 
that is not std::complex. Unfortunately osmo-trx has a Complex<T> typedefed as 
"complex" that is not really compatible with the plain old std::complex that is 
being used everywhere and which should be removed at some point, just like the 
weird "vector" class..


File Transceiver52M/ms/ms.h:

https://gerrit.osmocom.org/c/osmo-trx/+/30966/comment/6be5b02b_b8e820d8
PS1, Line 58: namespace cvt_internal
> please put some new line spacing between blocks inside here, otherwise it's 
> hard to read.
Done


https://gerrit.osmocom.org/c/osmo-trx/+/30966/comment/edb8fe93_7b79e265
PS1, Line 66: template <typename DST_T, typename ST>
> I don't think I'm following here. […]
() + args!

There still is only one three type template with proper type deduction that 
serves as the public interface, see usage pattern changes in the other files.


https://gerrit.osmocom.org/c/osmo-trx/+/30966/comment/22626425_8a1b894e
PS1, Line 72: template <typename ST> void convert_and_scale_i(float *dst, const 
float *src, unsigned int src_len, ST scale)
> you have the template in a new line in functions above, but here in the same 
> one.
Fixed by adjusting the clang-format file.


File Transceiver52M/ms/ms_upper.cpp:

https://gerrit.osmocom.org/c/osmo-trx/+/30966/comment/a2e54fa0_b5f8a3d2
PS1, Line 465:  fesetround(FE_TOWARDZERO);
> can you explain why are you dropping this?
Original hack related to rounding misadventures.



--
To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/30966
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Change-Id: I93f077f967daf2ed382d12cc20a54846b3688634
Gerrit-Change-Number: 30966
Gerrit-PatchSet: 1
Gerrit-Owner: Hoernchen <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Comment-Date: Fri, 13 Jan 2023 13:24:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <[email protected]>
Gerrit-MessageType: comment

Reply via email to