Change in osmo-bsc[master]: Add a bts_model->bts_init() and trx_init() call-back function

[pespin]

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/21825 )

Change subject: Add a bts_model->bts_init() and trx_init() call-back function
......................................................................


Patch Set 1:

(2 comments)

I have the feeling you just hooked the function calls to the first place you found which made your stuff work, it may be worth giving it a second thought IMHO.

https://gerrit.osmocom.org/c/osmo-bsc/+/21825/1/src/osmo-bsc/bts.c 
File src/osmo-bsc/bts.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/21825/1/src/osmo-bsc/bts.c@513 
PS1, Line 513: 	if (model->trx_init) {
I would rather hook all this into vty_go_parent cb so that it is called at the end of config (you can check if you are currently reading a file or in config mode somehow iirc), or in main()


https://gerrit.osmocom.org/c/osmo-bsc/+/21825/1/src/osmo-bsc/bts_trx.c 
File src/osmo-bsc/bts_trx.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/21825/1/src/osmo-bsc/bts_trx.c@123 
PS1, Line 123: 	if (bts->model && bts->model->trx_init) {
I find a bit strange that you call trx_init() here and in the previous file too. Looks like prone to double-call and I don't see why you need it in both places.



-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/21825
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Icbad9cdc12221c9ad997267d77e5414edcbac538
Gerrit-Change-Number: 21825
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Tue, 22 Dec 2020 10:28:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20201222/83ce3c3c/attachment.htm>