Change in osmo-bsc[master]: paging: Add VTY options to calculate T3113 timeout dynamically

Pau Espin Pedrol gerrit-no-reply at lists.osmocom.org
Thu Nov 22 18:06:46 UTC 2018


Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/11654 )

Change subject: paging: Add VTY options to calculate T3113 timeout dynamically
......................................................................


Patch Set 3:

(3 comments)

https://gerrit.osmocom.org/#/c/11654/3/src/osmo-bsc/bsc_vty.c
File src/osmo-bsc/bsc_vty.c:

https://gerrit.osmocom.org/#/c/11654/3/src/osmo-bsc/bsc_vty.c@3893
PS3, Line 3893: 	"Calculate T3113 dynamically based on channel config and load\n"
> I think it would be good to make the name of this command more specific to t3113, for now.
"for now" means having to keep backward compatibility with commands later, and I want to avoid that. So indeed the implementation is so far quite specific to T3113, but same VTY cmd can be used later on for more timers if we decide so, without needing to add more commands or deprecating all ones.


https://gerrit.osmocom.org/#/c/11654/3/src/osmo-bsc/bsc_vty.c@3916
PS3, Line 3916: 	"Calculate T3113 dynamically based on channel config and load\n"
> with the preceding "no", it's ok to here write "Set given timer to non-dynamic and use the default o […]
I didn't completely get what you mean here regarding docs, but anyway that sounds confusing to me when reading because I would read:
NO_STR "Set given timer to non-dynamic and use the default or user provided fixed value"

which in my head would mean:
"Don't set given timer to non-dynamic and use the default or user provided fixed value"

which by boolean !(!a&&b):
"Set given timer to dynamic or don't use default provided fixed value"

And that's basically the opposite of what this command does.


https://gerrit.osmocom.org/#/c/11654/3/src/osmo-bsc/bsc_vty.c@3927
PS3, Line 3927: 		vty_out(vty, "T%d has no dynamic setting%s", d->T, VTY_NEWLINE);
> technically would be ok to ignore this, because setting a non-dynamic timer to non-dynamic is a no-o […]
Sure, but anyway nowadays doesn't make sense, to let's better tell the user probably he's doing something having no effect (so he doesn't expect an effect, or detects he mistyped the timer reference).



-- 
To view, visit https://gerrit.osmocom.org/11654
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fb2969b690151415038631fb6ad059aa6835c7f
Gerrit-Change-Number: 11654
Gerrit-PatchSet: 3
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Comment-Date: Thu, 22 Nov 2018 18:06:46 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181122/28d79bb7/attachment.html>


More information about the gerrit-log mailing list