dexter 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:

(2 comments)

I am not through yet. I have only looked at debug.h, gsm_data.h so far... So 
far I have only a few cosmetic hints.

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:

/* Whether to use call waiting on the network */
bool call_waiting;

/* Whether to use lcls on the network */
bool lcls_disable;

char *sms_db_file_path;

Also I wonder why there is negative logic. I guess you want to have lcls 
enabled by default? I wonder if this is a good idea. We probably might want to 
have it disabled by default to keep the default config as simple as possible.


https://gerrit.osmocom.org/c/osmo-msc/+/24236/7/include/osmocom/msc/mncc.h
File include/osmocom/msc/mncc.h:

https://gerrit.osmocom.org/c/osmo-msc/+/24236/7/include/osmocom/msc/mncc.h@164
PS7, Line 164:  struct osmo_gcr_parsed gcr;
There is probably a better place for this. Maybe below callref?



--
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 08:50:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to