osmo-gsm-tester/ea797ca92816bc9fbc8220c6481...

321 lines
12 KiB
Plaintext

{
"comments": [
{
"key": {
"uuid": "fa98f980_587c3573",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 116,
"author": {
"id": 1000005
},
"writtenOn": "2017-05-02T11:45:39Z",
"side": 1,
"message": "elif?",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_5858b5fa",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 116,
"author": {
"id": 1000074
},
"writtenOn": "2017-05-02T12:00:29Z",
"side": 1,
"message": "Agree",
"parentUuid": "fa98f980_587c3573",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_d88f4574",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 129,
"author": {
"id": 1000005
},
"writtenOn": "2017-05-02T11:45:39Z",
"side": 1,
"message": "this being called from is_connected() would raise a KeyError if the I_NETREG was not in the list. Instead, is_connected() should probably return False?\n\nSo maybe return None in case there is no I_NETREG. Maybe this works:\n\n nr \u003d self.dbus_obj().get(I_NETREG)\n if nr is None:\n return None\n\nWhat do you think?",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_78465925",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 129,
"author": {
"id": 1000074
},
"writtenOn": "2017-05-02T12:00:29Z",
"side": 1,
"message": "Makes sense",
"parentUuid": "fa98f980_d88f4574",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_18dd4d15",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 129,
"author": {
"id": 1000005
},
"writtenOn": "2017-05-03T11:23:26Z",
"side": 1,
"message": "though I noticed now that dbus_obj() has no get() function \u003e:(\n\nIn current master I added code to check whether an interface is really present by trying to get via [] and catching an exception (because when I get the signal that an interface was added, sometimes the interface is not present in the dbus object yet -- it\u0027s racy). Maybe we can use that function here.\n\n(And now that I think of it we could change that function to use the \u0027in\u0027 operator instead, in case that works, probably for a separate patch)",
"parentUuid": "fa98f980_78465925",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_f88c0982",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 138,
"author": {
"id": 1000005
},
"writtenOn": "2017-05-02T11:45:39Z",
"side": 1,
"message": "are \u0027registered\u0027 and \u0027roaming\u0027 fixed definitions from ofono? I hope so. Especially when we\u0027re using the same one several times (\u0027roaming\u0027), I would prefer using constants defined above, sort of like the I_NETREG.",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_984a1d26",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 138,
"author": {
"id": 1000074
},
"writtenOn": "2017-05-02T12:00:29Z",
"side": 1,
"message": "Yes, they are string tokens defined by ofono, see: https://github.com/intgr/ofono/blob/master/doc/network-api.txt#L78\n\nNot sure if it\u0027s that worth moving it into a constant as I would expect it not to change anyway.",
"parentUuid": "fa98f980_f88c0982",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_38ee9123",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 140,
"author": {
"id": 1000005
},
"writtenOn": "2017-05-02T11:45:39Z",
"side": 1,
"message": "hmm, I see a name duality arising: register and connect. Maybe we should rename all of them to \u0027register\u0027?",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_b847e11c",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 140,
"author": {
"id": 1000074
},
"writtenOn": "2017-05-02T12:00:29Z",
"side": 1,
"message": "I agree. I can create a 2nd patch to do the connect()-\u003eregister()",
"parentUuid": "fa98f980_38ee9123",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_f8c0892c",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 140,
"author": {
"id": 1000005
},
"writtenOn": "2017-05-03T11:23:26Z",
"side": 1,
"message": "yes, thx",
"parentUuid": "fa98f980_b847e11c",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_18e9cd1a",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 148,
"author": {
"id": 1000005
},
"writtenOn": "2017-05-02T11:45:39Z",
"side": 1,
"message": "what if the nitb arg reflects another network than the one we\u0027re connected to? I assume we usually want to disconnect and re-connect.",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_d854a5cc",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 148,
"author": {
"id": 1000074
},
"writtenOn": "2017-05-02T12:00:29Z",
"side": 1,
"message": "Yes, this is not really detailed but for now it\u0027s enough. Even in connect() we should actually pass some Network code to which we should be connecting to, or None in case we want to use auto + default network. Then we should use all the Operator API from ofono, but I think all this can wait for later steps and for now consider we always try to connect automatically to default network.",
"parentUuid": "fa98f980_18e9cd1a",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_b8c60148",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 148,
"author": {
"id": 1000005
},
"writtenOn": "2017-05-03T11:23:26Z",
"side": 1,
"message": "In current master I already cycle powered and online to false and then true here ... so I went with the paradigm \"connect() means re-connect\" and that connect() is the First Step (tm) one takes to start using a modem. We could also move those clear-the-state things to a separate start() or reset() function, but test scripts might easily forget to call it, so I would still call the reset function from here (until a need arises to have them separate, my guess being there will not be any)...\n\nbottom line is, when you rebase to master you will see that there isn\u0027t really a need for this \u0027if\u0027 anymore.\n\ns/connect/register/g",
"parentUuid": "fa98f980_d854a5cc",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_58f3550a",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 154,
"author": {
"id": 1000005
},
"writtenOn": "2017-05-02T11:45:39Z",
"side": 1,
"message": "could call get_netreg_status() less often...",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_f85169d9",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 154,
"author": {
"id": 1000074
},
"writtenOn": "2017-05-02T12:00:29Z",
"side": 1,
"message": "Do you think is really an issue? it\u0027s not like it\u0027s taking seconds to do it.",
"parentUuid": "fa98f980_58f3550a",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_d8c3c536",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 154,
"author": {
"id": 1000005
},
"writtenOn": "2017-05-03T11:23:26Z",
"side": 1,
"message": "the issue really is that you obtain the same value twice, meaning you could technically get two different values. Something like this would be more, I don\u0027t know, \"safe\":\n\n def is_connected(netreg_status\u003dNone):\n if netreg_status is None:\n netreg_status \u003d self.get_netreg_status()\n return netreg_status \u003d\u003d ...\n\n def foo():\n ns \u003d self.get_netreg_status()\n if self.is_connected(ns):\n self.dbg(\u0027registered\u0027, netreg_status\u003dns)\n\nyou are right that it\u0027s a little bit overboard for this less important place, nevertheless I\u0027d prefer this coding style to be used all the time.\n\ns/connect/register/g",
"parentUuid": "fa98f980_f85169d9",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_78f819e6",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 156,
"author": {
"id": 1000005
},
"writtenOn": "2017-05-02T11:45:39Z",
"side": 1,
"message": "so far I\u0027ve been using RuntimeError (but we should also probably use more fine grained exceptions, like introduce an OfonoExn or ModemExn...)",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
},
{
"key": {
"uuid": "fa98f980_3863f1b2",
"filename": "src/osmo_gsm_tester/ofono_client.py",
"patchSetId": 1
},
"lineNbr": 156,
"author": {
"id": 1000074
},
"writtenOn": "2017-05-02T12:00:29Z",
"side": 1,
"message": "Agree. I will use RuntimeError for now.",
"parentUuid": "fa98f980_78f819e6",
"revId": "ea797ca92816bc9fbc8220c64813e1a040f8be76",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326",
"unresolved": false
}
]
}