pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/18591 )
Change subject: Do not mix public and private BTS features, use libosmocore's API ...................................................................... Patch Set 3: (3 comments) Please add bts_model_get/unset/set_flag() or alike APIs. https://gerrit.osmocom.org/c/osmo-bts/+/18591/3/src/common/bts.c File src/common/bts.c: https://gerrit.osmocom.org/c/osmo-bts/+/18591/3/src/common/bts.c@261 PS3, Line 261: if (~trx->bts->flags & BTS_IMPL_FLAG_MS_PWR_CTRL_DSP) I would really welcome a function helper for this, in order to: * Avoid boolean math in lots of places (and hence having to pay attention to it) * Easily finding where internal features are checked/used/set. Also because it's something all bts_model are using, so it makes sense to have some sort of abstraction to make it easier when adding a new model. https://gerrit.osmocom.org/c/osmo-bts/+/18591/3/src/common/oml.c File src/common/oml.c: https://gerrit.osmocom.org/c/osmo-bts/+/18591/3/src/common/oml.c@176 PS3, Line 176: msgb_tl16v_put(msg, NM_ATT_MANUF_ID, _NUM_BTS_FEAT/8 + 1, bts->features->data); isn't there an API to get the data pointer? https://gerrit.osmocom.org/c/osmo-bts/+/18591/3/src/common/vty.c File src/common/vty.c: https://gerrit.osmocom.org/c/osmo-bts/+/18591/3/src/common/vty.c@845 PS3, Line 845: for (i = 0, no_features = true; i < sizeof(bts->flags) * 8; i++) { features = false, then setting it to true and checking if (!features) is easier to understand :) -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/18591 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Icf792d02323bb73e3b8d46384c7890cb1eb4731e Gerrit-Change-Number: 18591 Gerrit-PatchSet: 3 Gerrit-Owner: Vadim Yanitskiy <vyanits...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Vadim Yanitskiy <vyanits...@sysmocom.de> Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-Comment-Date: Tue, 02 Jun 2020 10:21:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment