Attention is currently required from: neels, fixeria, dexter.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31497 )
Change subject: pcu_sock: activate/deactivate PDCH on pcu reconnect ......................................................................
Patch Set 5:
(3 comments)
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/1565225a_6f6796c5 PS2, Line 801: printf("l1sap_chan_rel(trx,gsm_lchan2chan_nr(ts->lchan));\n");
either keep this printf, or if it should be dropped, drop it in a separate patch?
Ack
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/968de04c_416d056b PS2, Line 801: && ts->pchan_on_init == GSM_PCHAN_OSMO_DYN
Why does this apply only to dynamic timeslots? I think I understand why, but I'd like to see a code […]
Ericsson only has fully-dynamic timeslots, AFAIR. But yes, mentioning that here might make sense for the most common reader of this code who has no clue about ericsson specifics.
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/880ebec0_1f12759b PS2, Line 941: bts = llist_entry(state->net->bts_list.next, struct gsm_bts, list);
rather use llist_first_entry_or_null() […]
I actually think it's a serious problem that only one BTS appears to be suppored here. Not only that, but there's an implicit assumption that the first BTS is the Ericsson BTS served by the PCU?
In general, a BSC-colocated PCU can of course serve any number of BTSs. Until we address that general problem/shortcoming in our current code, we shouldn't make a blind assumption, but we should make sure that the user can operate this only in a safe configuration. In the worst case, we could check if thre are more than one (ericsson) BTS configured in the BSC, and refuse to operate the PCU socket with an associated error message.
-
laforge