osmo-bsc[master]: Add support for Access Control Class ramping.
gerrit-no-reply at lists.osmocom.org
Mon Feb 26 13:27:50 UTC 2018
Patch Set 7:
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.
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
PS7, Line 55: 8_t barred_t2;
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.
Line 950: bool acc_ramping_enabled;
why not make the 'enabled' flage a member of struct acc_ramp?
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-Owner: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
More information about the gerrit-log