Attention is currently required from: neels, fixeria, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/30602 )
Change subject: vty: Add check against sensible default value for Ny1 ......................................................................
Patch Set 16:
(8 comments)
File src/osmo-bsc/bts.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/02b3622f_27b56219 PS9, Line 1742: bool ny1_satisfies_requirements = T3105 * ny1 > T3124 + GSM_NY1_REQ_DELTA;
Is this a hard rule of ours or a preference? If there is any leeway, I'd rather leave it as is.
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/1d37c437_221f9e75 PS9, Line 1744: unsigned long ny1_recommended = (T3124 + GSM_NY1_REQ_DELTA)/T3105 + 1;
the standard idiom for integer division rounding up is: […]
I'm not sure I understand... if 'a', 'b' are wlog nonnegative integers and b is nonzero, then 'a/b = rounddown(a/b) + r' with 'r in [0,1)', which goes along with my earlier reply.
Since we are only using unsigned types this should be alright. I can still change it if it's nonetheless important
File src/osmo-bsc/bts.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/05e83da7_2b401911 PS14, Line 537: }
(no need for braces when the body is a single line)
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/059d9674_7d8f6fa0 PS14, Line 1725: otherwise return a suggestion for Ny1
this is still not true
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/d79887c1_16a8e4b6 PS14, Line 1726: the message to print
this also needs to be updated, I guess (I see no msg argument)
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/298a4763_7b98d011 PS14, Line 1729: const struct gsm_lchan *gl = gsm_bts_get_cbch(bts);
(re earlier question, 'lchan' stands for 'logical channel', a sub-division of a BTS timeslot)
thanks :)
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/00ced4a7_f056287f PS14, Line 1738: unsigned long T3105 = osmo_tdef_get(bts->network->T_defs, 3105, OSMO_TDEF_MS, -1);
in osmocom style we are required to declare all variables at the top of the scope
that was my 'reasoning' as well, didn't see it in our guidelines. I've changed it though
File src/osmo-bsc/bts_ipaccess_nanobts_omlattr.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/35711518_f3d8897a PS16, Line 95: (void)
drop the cast to void?
Done