osmo-gsm-tester[master]: ms: Create a starter for virtphy and mobile application

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Pau Espin Pedrol gerrit-no-reply at lists.osmocom.org
Tue Mar 6 18:04:13 UTC 2018


Patch Set 5:

(1 comment)

https://gerrit.osmocom.org/#/c/6917/5/src/osmo_ms_driver/starter.py
File src/osmo_ms_driver/starter.py:

Line 107:     def start_mobile(self, loop):
> You are the maintainer but I wouldn't do it:
* What's the issue with it being generic given that is called from a known class? Usually you have 1 class per file, so if you know the object, just grep for "start(" in the class file.

* I asked you to call it start() because it was already being called start_mobile(), if you had called it launch_process_mobile() I would have pointed out that it would make more sense to have it called launch_process(), because imho if you are in class Foo, it doesn't make sense to have a method do_foo() but better call it do(), as foo is already known from the object. In our objects used for tests we usually have a public API called start() because we don't care about whether it creates a process or not underneath, everything is managed internally by the object. Even more, in the Modem (ofono) class, we only have a connect() signal which does all the initialization and starts registration, because so far we didn't need other steps (basically because the modem attempts to connect to the network as quick as it is powered on). That being said, if other implementations require more fine grained actions/steps, we can improve the API used in osmo-gsm-tester tests.

So, to keep it in a similar fashion with other existing objects (bts, bsc, msc), for future inclusion it would be nice to have it called start() too, and add that API to the Modem object (in ofono we'd probably keep doing everything in the connect() anyway).

As it's not yet merged with the other osmo-gsm-tester stuff (different directory) there no hard requirement on how to call it, but for later inclusion it may ease already providing similar naming.


-- 
To view, visit https://gerrit.osmocom.org/6917
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5c6d742842d7f3e0a1858436ef3f8634d8c0582d
Gerrit-PatchSet: 5
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-HasComments: Yes



More information about the gerrit-log mailing list