Attention is currently required from: dexter.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/28896
)
Change subject: abis_om2000: duplicate nmstate of bb_trxc mo to trx mo
......................................................................
Patch Set 1:
(8 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-bsc/+/28896/comment/1252bd37_30f85a79
PS1, Line 9: In OM2000 a separate bb_trxc MO does not exist to archive better
"does not exit. To achive"
Missing dot
Typo in "archive".
https://gerrit.osmocom.org/c/osmo-bsc/+/28896/comment/78831608_250a8938
PS1, Line 10: compatibilty towards classic ABIS and its MOs lets mirror the nmstate of
Abis
let's
https://gerrit.osmocom.org/c/osmo-bsc/+/28896/comment/a2281b72_281f9b6d
PS1, Line 11: the trx MO to the trxc_mo to make it look like if it were present.
BB_TRANSC MO to the RCARRIER MO
File src/osmo-bsc/abis_om2000.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/28896/comment/6d4c3908_f1552f05
PS1, Line 1011: switch (mo_state) {
Move this to a function: mo2nm_state(uint8_t mo_state).
Then, "nsd.new_state.availability = mo2nm_state(mo_state);"
https://gerrit.osmocom.org/c/osmo-bsc/+/28896/comment/ddc40cab_4bccd52d
PS1, Line 1032: duplicate_trxc_nm_state(bts, mo);
This looks wrong, you should emit signals for both NM objects not only one?
You could do something like this after emitting the signal:
if (mo->class == OM2K_MO_CLS_TRXC) {
struct gsm_nm_state *bbtransc_nm_state = nm_state = &trx->bb_transc.mo.nm_state;
/* Update current state before emitting signal: */
bbtransc_nm_state->availability = nsd.new_state.availability;
osmo_signal_dispatch(SS_NM, S_NM_STATECHG, &nsd);
}
https://gerrit.osmocom.org/c/osmo-bsc/+/28896/comment/eb830fe4_207f08a6
PS1, Line 1058: duplicate_trxc_nm_state(bts, mo);
Same, simply explicitly set bb_transc_nm_state->operational = new_state.operational;
https://gerrit.osmocom.org/c/osmo-bsc/+/28896/comment/ea0e51bd_1294e48c
PS1, Line 1059: }
Not directly related to this patch but indirectly related to have stuff working as
expected: AFAICT you are missing a "osmo_signal_dispatch(SS_NM, S_NM_STATECHG,
&nsd);" here.
https://gerrit.osmocom.org/c/osmo-bsc/+/28896/comment/06e4ee92_b9d2332c
PS1, Line 2245: otfp->trx->bb_transc.mo.nm_state.administrative =
NM_STATE_UNLOCKED;
Not directly related to this patch but indirectly related to have stuff working as
expected: AFAICT you are missing a "osmo_signal_dispatch(SS_NM, S_NM_STATECHG,
&nsd);" here.
--
To view, visit
https://gerrit.osmocom.org/c/osmo-bsc/+/28896
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I4611d8af16a30725308bd527098b12a356bfde9f
Gerrit-Change-Number: 28896
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 02 Aug 2022 13:57:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment