Attention is currently required from: neels, pespin, fixeria. 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 19:
(7 comments)
File src/osmo-bsc/bts.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/4a80a88d_79abf804 PS9, Line 1729: struct gsm_lchan *gl = gsm_bts_get_cbch(bts);
According to the specs, T3124 is set depending on the channel type (I assume 'lchan' stands for 'lin […]
Done
File src/osmo-bsc/bts.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/8aba729d_6ad38f09 PS18, Line 535: if (!gsm_bts_check_ny1(bts, LOGL_ERROR)) return -EINVAL;
put return always in a separate line.
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/9220b3e8_7eaa269f PS18, Line 1726: const struct gsm_lchan *gl = gsm_bts_get_cbch(bts);
I'm still not understanding why are you taking a CBCH channel here, seems totally not related. […]
I removed the check.
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/e0a4ab5e_c63b3fce PS18, Line 1729: if (gl) {
no need for {} here. Linter should already have warned about it.
I used 'lint_diff.sh' from our Docker tests locally (I think that's where I got it from) and didn't get anything. Looking at it again, I guess I have to run it after creating the commit (seems like it runs on the latest commit, not on local changes, which I was assuming).
Apart from that, does Jenkins fail if the linter warns?
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/294beef5_38231b59 PS18, Line 1730: T3124 = (gl->type == GSM_LCHAN_SDCCH) ? GSM_T3124_SDCCH : GSM_T3124_OTHER_CH;
this is always going to be false since gl is CBCH.
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/6bb07845_431fe884 PS18, Line 1732: /* Take the higher lower bound to be safe */
you can then move the comment to the "else" line (1731)
Done
File src/osmo-bsc/bts_ipaccess_nanobts_omlattr.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/069ed66a_041df112 PS18, Line 95: gsm_bts_check_ny1(bts, LOGL_NOTICE);
I really see no reason for having different log levels.
This is the additional check during runtime (vs. startup), so should this function also exit with an error if the check isn't successful? My thinking was if we don't exit, it shouldn't count as an error message.
-
arehbein