Change in osmo-bsc[master]: vty: Add check against sensible default value for Ny1 - gerrit-log

3 Jan 2023

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

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: If3f96a6bd4f9ae32b6421de43c1c5a5d64482089
Gerrit-Change-Number: 30602
Gerrit-PatchSet: 16
Gerrit-Owner: arehbein &lt;arehbein(a)sysmocom.de&gt;
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt;
Gerrit-CC: neels &lt;nhofmeyr(a)sysmocom.de&gt;
Gerrit-CC: pespin &lt;pespin(a)sysmocom.de&gt;
Gerrit-Attention: neels &lt;nhofmeyr(a)sysmocom.de&gt;
Gerrit-Attention: fixeria &lt;vyanitskiy(a)sysmocom.de&gt;
Gerrit-Attention: pespin &lt;pespin(a)sysmocom.de&gt;
Gerrit-Comment-Date: Tue, 03 Jan 2023 00:19:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein &lt;arehbein(a)sysmocom.de&gt;
Comment-In-Reply-To: neels &lt;nhofmeyr(a)sysmocom.de&gt;
Comment-In-Reply-To: pespin &lt;pespin(a)sysmocom.de&gt;
Comment-In-Reply-To: fixeria &lt;vyanitskiy(a)sysmocom.de&gt;
Gerrit-MessageType: comment