Attention is currently required from: fixeria, pespin.
11 comments:
File src/pcu_l1_if.cpp:
Patch Set #1, Line 77: uint8_t trx_no;
"trx_no" but "ts", let's better standarize variable names please.
Done
Patch Set #1, Line 81: struct gsm_e1_subslot e1_link;
type is subslot but name is link? this looks weird tbh.
This is inherited from osmo-bsc, there the name e1_link is very common. The struct just contains the e1 line number along with the timeslot number and the subslot number.
Patch Set #1, Line 84: static LLIST_HEAD(e1_ccu_table);
some comment describing this wuld be welcome.
Done
Patch Set #1, Line 981: int pcu_l1if_get_ccu_conn_pars(struct gsm_e1_subslot *e1_link, uint8_t pdch_ts, uint8_t pdch_trx_no)
trx should go before ts.
Done
Patch Set #1, Line 987: memcpy(e1_link, &e1_ccu_conn_pars->e1_link, sizeof(*e1_link));
this looks a bit weird. Why copying the content if you can provide a const pointer to it? […]
Done
Patch Set #1, Line 992: LOGP(DL1IF, LOGL_ERROR, "(PDCH-TS:%u, TRX:%u) cannot find E1 connection parameters for CCU\n", pdch_ts,
Indeed, would be nice to have the format consistent with LOGPDCH().
Done
Patch Set #1, Line 1003: if (e1_ccu_ind->e1_ts_ss == 255) {
don't we have defined for all these numbers?
As far as I know not. The 255 is also used in osmo-bsc:bts_trx_vty.c like this. The two variables are also only used for logging.
Patch Set #1, Line 1012: "(PDCH-TS:%u, TRX:%u) new E1 CCU communication parameters for CCU (E1-line:%u, E1-TS:%u, E1-SS:%u, rate:%ukbps)\n",
same, prefix formatting.
Done
Patch Set #1, Line 1017: llist_for_each_entry(e1_ccu_conn_pars, &e1_ccu_table, entry) {
If you passed a pointer to the pcu_l1if_get_ccu_conn_pars() above you could get a pointer to e1_ccu_ […]
Done
Patch Set #1, Line 1027: e1_ccu_conn_pars = talloc_zero(tall_pcu_ctx, struct e1_ccu_conn_pars);
You can probably move this into an _alloc() function.
I don't think that this is necessary. This is the only location where we need to alloc the connection parameters.
Patch Set #1, Line 1271: pcu_prim->u.info_ind
copy-paste, should be 'e1_ccu_ind' instead
Done
To view, visit change 31174. To unsubscribe, or for help writing mail filters, visit settings.