Attention is currently required from: neels, pespin, fixeria.
14 comments:
Patchset:
I hope I've addressed all your comments.
I'll revisited all your comments tomorrow again to ensure I'ven't missed something.
File include/osmocom/bsc/bts_setup_ramp.h:
Patch Set #3, Line 31: BTS_SETUP_RAMP_INIT, /** initial state */
(osmo-bsc doesn't generate doxygen, but just saying: […]
Thank you!
File include/osmocom/bsc/nm_bts_ramp.h:
Because there can be multiple declarations of the function, but usually only one implementation.
Thank you for the explanation.
File src/osmo-bsc/bsc_vty.c:
Patch Set #1, Line 1284: "allow-bts-configuration <0-65535>",
(i also prefer 'bts <0-255> foo')
Done
Patch Set #1, Line 2659: vty_out(vty, " bts ramp limit %d within %d seconds%s",
I've changed it as fixeria suggested.
Done
Patch Set #1, Line 3087: "bts ramp limit <0-999> within <0-999> seconds",
It may not have a complex configuration NOW, but it may LATER, because we add more features and ways […]
Done
File src/osmo-bsc/bsc_vty.c:
Patch Set #3, Line 3091: "Number of BTS"
missing dot.
Done
File src/osmo-bsc/bts_setup_ramp.c:
Patch Set #3, Line 42: /* inform all MOs */
My comment regarding moving this to a generic address was not applied nor got feedback.
Done
Patch Set #3, Line 62: * \brief Unblock a BTS from BTS setup ramping to allow it to setup and configure.
please don't use '\brief', our doxygen config has AUTOBRIEF set
Done
Patch Set #3, Line 77: * \brief Timer callback and called by bts_setup_ramp_deactivate
drop \brief and end the line with a period (same below)
Done
Patch Set #3, Line 201: llist_del(&bts->bts_setup_ramp.list);
what's the content of &bts->bts_setup_ramp. […]
It's empty afterwards.
File src/osmo-bsc/nm_bb_transc_fsm.c:
Patch Set #1, Line 345: case NM_EV_RAMP_GO:
This has not yet been applied.
I've implemented it together with a libosmocore (ignore_invalid_event_mask).
Without the libosmocore patch I don't really see the point because it would require to add NM_EV_RAMP_GO in every fsm or it would need to check the state of the fsm before dispatching the signal.
File src/osmo-bsc/nm_bts_fsm.c:
Patch Set #1, Line 92: struct gsm_bts *bts,
Nope, not addressed.
IMHO this is fine. last example of 6.2) looks the same.
https://www.kernel.org/doc/html/latest/process/coding-style.html
File src/osmo-bsc/nm_bts_ramp.c:
Patch Set #1, Line 42: /* inform all MOs */
It may make sense to have this moved into a function in nm_common_fsm. […]
Done
To view, visit change 29788. To unsubscribe, or for help writing mail filters, visit settings.