keith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/24236 )
Change subject: Add support for LCLS to the MSC ...................................................................... Patch Set 7: (5 comments) Thanks for review! https://gerrit.osmocom.org/c/osmo-msc/+/24236/7/include/osmocom/msc/gsm_data.h File include/osmocom/msc/gsm_data.h: https://gerrit.osmocom.org/c/osmo-msc/+/24236/7/include/osmocom/msc/gsm_data.h@264 PS7, Line 264: bool lcls_disable; > Maybe this integrates better when you put this below call_waiting? I would > write it like this: […] A long long time ago, in a land far far away, (https://gerrit.osmocom.org/c/osmo-msc/+/13421) LaF0rge said: "I would rather say it is "permit lcls" than "enable lcls", as whether or not LCLS can be used depends on a lot of other conditions, not just this newly introduced flag." Based on that logic, this was the result. We don't just magically "enable" lcls with this flag, but we CAN disable it. As you later saw, it is disabled by default. https://gerrit.osmocom.org/c/osmo-msc/+/24236/7/src/libmsc/msc_vty.c File src/libmsc/msc_vty.c: https://gerrit.osmocom.org/c/osmo-msc/+/24236/7/src/libmsc/msc_vty.c@503 PS7, Line 503: "lcls-disable", > Maybe use DEFUN_ATTR(...., CMD_ATTR_IMMEDIATE). […] Ack https://gerrit.osmocom.org/c/osmo-msc/+/24236/7/src/libmsc/msc_vty.c@506 PS7, Line 506: gsmnet->lcls_disable = 1; > maybe use "true" - its also declared as bool Ack https://gerrit.osmocom.org/c/osmo-msc/+/24236/7/src/libmsc/msc_vty.c@514 PS7, Line 514: gsmnet->lcls_disable = 0; > same here, better use "false" Ack https://gerrit.osmocom.org/c/osmo-msc/+/24236/7/src/osmo-msc/msc_main.c File src/osmo-msc/msc_main.c: https://gerrit.osmocom.org/c/osmo-msc/+/24236/7/src/osmo-msc/msc_main.c@261 PS7, Line 261: net->lcls_disable = 1; > better use "true" Ack -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/24236 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I705c860e51637b4537cad65a330ecbaaca96dd5b Gerrit-Change-Number: 24236 Gerrit-PatchSet: 7 Gerrit-Owner: keith <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <[email protected]> Gerrit-Reviewer: neels <[email protected]> Gerrit-CC: pespin <[email protected]> Gerrit-Comment-Date: Wed, 06 Oct 2021 14:24:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: dexter <[email protected]> Gerrit-MessageType: comment
