Patch Set 7: (6 comments)
looks great, aside from some cosmetic issues. The uint16_t is an optional idea (isn't it simpler?) as is enabled-in-struct-acc_ramp, but all other comments I would like to see adressed. thanks. https://gerrit.osmocom.org/#/c/6324/7//COMMIT_MSG Commit Message: Line 17: access-control-class-ramping-step-interval (<30,600>|dynamic) 30-600 not 30,600? same below? Line 29: ramping stepm with a 'dyanmic' step interval. This means the dynamic https://gerrit.osmocom.org/#/c/6324/7/include/osmocom/bsc/acc_ramp.h File include/osmocom/bsc/acc_ramp.h: PS7, Line 55: 8_t barred_t2; : uint I would have used one uint16_t bit mask in order to make it simpler to set the bit for one given ACC? Maybe I'm thinking too complicated ;) Line 77: * Initialize an acc_ramp data structure. We're not using doxygen inside applications at this point, but as you're adding extensive API docs it makes sense to go for doxygen syntax, even if it's not generating any docs yet. https://gerrit.osmocom.org/#/c/6324/7/include/osmocom/bsc/gsm_data.h File include/osmocom/bsc/gsm_data.h: Line 950: bool acc_ramping_enabled; why not make the 'enabled' flage a member of struct acc_ramp? https://gerrit.osmocom.org/#/c/6324/7/src/libbsc/acc_ramp.c File src/libbsc/acc_ramp.c: Line 23: #include <errno.h> sttbool as you're using bool? -- To view, visit https://gerrit.osmocom.org/6324 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0a5ac3a08f992f326435944f17e0a9171911afb0 Gerrit-PatchSet: 7 Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Owner: Stefan Sperling <ssperl...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Stefan Sperling <ssperl...@sysmocom.de> Gerrit-Reviewer: Vadim Yanitskiy <axilira...@gmail.com> Gerrit-HasComments: Yes
