Attention is currently required from: arehbein, fixeria.
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/30539 )

Change subject: vty: Add support for Ny1 configuration
......................................................................


Patch Set 13:

(3 comments)

File include/osmocom/bsc/gsm_data.h:

https://gerrit.osmocom.org/c/osmo-bsc/+/30539/comment/44bb3e2d_602ea8ee
PS13, Line 50: /* Requirements: We want ny1 to be as low as possible, while 
respecting T3105 * ny1 > T3124 + delta
isn't it named "Ny1" in the specs? better use same formatting so that people 
grepping in the code fins it easily, specially since it's a short name.


https://gerrit.osmocom.org/c/osmo-bsc/+/30539/comment/1ba0e424_f486f5cf
PS13, Line 52: #define GSM_NY1_DEFAULT (unsigned long)((GSM_T3124_MAX + 
GSM_NY1_REQ_DELTA)/GSM_T3105_DEFAULT + 1)
I see no reason to use the case here. At least better add an extra parenthesis 
surrounding the whole expression in that case, not sure if there can be 
unintended problems with the cast if some more stuff is used.


File tests/nanobts_omlattr/nanobts_omlattr_test.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/30539/comment/fd35b2d2_dc75c917
PS13, Line 130:         { .T = -3105, .default_val = GSM_NY1_DEFAULT, .val = 
GSM_NY1_DEFAULT, .min_val = 0, .max_val = UINT8_MAX, .unit = OSMO_TDEF_CUSTOM,
we usually put negative ones at the end



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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I6318cceb4ebdce50005e39e2e9323c1c8433250a
Gerrit-Change-Number: 30539
Gerrit-PatchSet: 13
Gerrit-Owner: arehbein <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-CC: pespin <[email protected]>
Gerrit-Attention: arehbein <[email protected]>
Gerrit-Attention: fixeria <[email protected]>
Gerrit-Comment-Date: Tue, 03 Jan 2023 09:53:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to