Attention is currently required from: neels, pespin, lynxis lazus. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/29788 )
Change subject: Add BTS setup ramping to prevent BSC overloading ......................................................................
Patch Set 6:
(12 comments)
File include/osmocom/bsc/bts_setup_ramp.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/582d2812_a5d1d63d PS3, Line 31: BTS_SETUP_RAMP_INIT, /** initial state */
Thank you!
I don't want to be bothersome here, but now it's not a valid doxygen comment style either...
File include/osmocom/bsc/gsm_data.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/dbf95f33_eccd7ed5 PS3, Line 959: struct {
It probably makes more sense to have this struct declared in the bts_ramp_setup. […]
Done
File src/osmo-bsc/bsc_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/b2081acd_69468786 PS4, Line 1326: BTS_NR_STR BTS_NR_STR should not be here anymore.
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/69b2804b_43a2eddd PS4, Line 3088: cfg_bsc_bts_setup_ramping_cmd, What's wrong with spacing here? It was good before.
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/7533bcf2_2afc7085 PS4, Line 3100: bts-setup-ramping-step-interval IMO, 'bts-setup-ramping' and 'step-interval' should be separate things, so in the VTY one could list all parameters quickly:
bts-setup-ramping ? step-interval <0-65535> [help string] step-size <0-65535> [help string]
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/84770946_b6ffde2f PS4, Line 3103: CMD_ATTR_IMMEDIATE) Spacing again.
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/4e281456_f66f6976 PS4, Line 3112: cfg_bsc_bts_setup_ramping_step_size_cmd, Again spacing/alignment problems.
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/c791bfce_ee605695 PS4, Line 3115: Set a step interval Step size or step interval? Copy pasted.
File src/osmo-bsc/bts_setup_ramp.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/2e5da489_75c9db44 PS4, Line 98: && net->bts_setup_ramp.step_size > 0 Extra spacing makes this construct hard to read. Please align properly.
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/6f33077e_f78ce613 PS4, Line 101: net->bts_setup_ramp.active Use bts_setup_ramp_active()?
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/87bd39e9_8ffdad15 PS4, Line 107: net->bts_setup_ramp.active Use !bts_setup_ramp_active()?
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/3720e263_3c1be880 PS4, Line 164: net->bts_setup_ramp.active Why not using the API bts_setup_ramp_active() for that?