**** BEGIN LOGGING AT Wed Jan 26 02:59:57 2011 Jan 26 03:22:05 ok phonesim is up and running. Should list-modems script show something before running any other tests? **** ENDING LOGGING AT Wed Jan 26 04:23:39 2011 **** BEGIN LOGGING AT Wed Jan 26 06:04:15 2011 Jan 26 08:07:28 About half of the tests pass with phonesim. Just would like to know what kind of test coverage should be met without finding out root cause for each of failing tests one by one Jan 26 08:08:01 When my environment is simple set up of MeeGo 1.1 runtime on Qemu Jan 26 08:09:37 I understand that each test is having it's own purpose, but shouldn't there be higher level documentation what to expect when running for example specified test set in typical (this should be defined) test environment? Jan 26 08:11:05 One of the main purposes of test sets are that the person who runs them, can be sure that his environment is correctly set up and good to go for starting to develope further. Jan 26 08:58:59 padovan: ping ? Jan 26 09:09:57 OlivierG: He won't be up yet. He is 5 hours behind you guys. Jan 26 09:10:37 thx Jan 26 11:51:06 18:33 < balrog-k1n> denkenz: i'll look into the simfs ops cancellation, sorry was away sinze Fri Jan 26 11:51:20 ^^^ was there more discussion around this I missed? Jan 26 11:52:02 akiniemi: Maybe not. I have not followed up on it yet. Jan 26 11:58:03 holtmann: ok Jan 26 11:58:29 holtmann: I see this mainly as a problem with atoms being flushed between different modem states Jan 26 11:58:36 (where flushing == remove()) Jan 26 11:58:54 Yes. And the callbacks still coming, but then the user_data pointers might be invalid. Jan 26 11:59:07 == segfault. Jan 26 11:59:30 Only our aggressive SIM caching prevents us from seeing this on more regular basis. Jan 26 11:59:30 We really don't need that, since what matters is whether the atom has a D-Bus interface registered or not Jan 26 11:59:58 hotlmann: true -- maybe a new config flag for disabling sim cache? Jan 26 12:00:04 s/hotlmann/holtmann Jan 26 12:00:08 There is a bit more to it. Wait until Denis and Andrew are here. I am not looking into that right now. Jan 26 12:02:08 I'll try, not necessarily here that late/early, though ;) Jan 26 13:16:27 OlivierG: pong Jan 26 13:17:51 ping Jan 26 13:18:30 looking for your feedback regarding DUN... i m investigating it and have some questions ;) Jan 26 13:20:31 Ok. Jan 26 13:23:42 The existing code seems to work but i m a little bit unconfortable about some parts are designed. eg: Emulator is "almost" an atom but seems to have a specific behavior Jan 26 13:24:41 i wonder if it could interesting to have a *real* atom... Jan 26 13:27:21 what is this real atom? Jan 26 13:37:53 On my understanding, an atom seems to be an object that can be instanciated on modem, and the instance can use other atoms to interact with the physical modem....maybe i m wrong ;) Jan 26 13:39:08 An atom is pretty abstract. Look at nettime for example. Jan 26 13:39:25 Or the proposed patches for provisioning drivers. Jan 26 13:41:34 i see... so, correct me if i m wrong, an atom must/should be like an "object": instances/interfaces/methods ...and should considered as an "entity" ? Jan 26 13:42:53 so, i m not confortable having code specific to GPRS et Network in the emulator code... Jan 26 13:44:09 shouldn't the emulator send/get information to/from the correct atom ? Jan 26 13:45:08 OlivierG: We don't have any GPRS and netreg code there anymore, I removed them all. Jan 26 13:45:26 padovan: i was not able to get the code this morning ;) Jan 26 13:45:27 See my git tree I posted on the mailing list. Jan 26 13:48:03 got the following message, using git clone :"warning: remote HEAD refers to nonexistent ref, unable to checkout." Jan 26 13:51:09 OlivierG: fixed now. Jan 26 13:55:26 got it... but i can't see any code about DUN or emulator... did you keep the filename ? :( Jan 26 13:55:56 there is a branch dun there Jan 26 13:56:05 ooops Jan 26 14:04:21 padovan: I got the code Jan 26 14:05:41 you kept the way the emulator is created Jan 26 14:06:01 I mean using __ofono_emulator_probe_driver() Jan 26 14:06:40 something is annoying here because in case the dun_gw plugin is not loaded, you will load some code specific to emulator Jan 26 14:07:42 shouldn't emulator service created like for other services? Jan 26 14:07:59 into modem drivers Jan 26 14:16:14 code is much more cleaner ;) ... btw , we have to keep the g_at_server_register() mechanism to be able to handle some specific AT commands (+CGMM, etc...) required. Jan 26 14:59:08 Gzajac: I have to check that, I'm still using the Zhenhua code there, priority was name DUN Server work first. Jan 26 14:59:52 OlivierG: Yeah, we can add them later, now I'm focused on the basic for the Bluetooth DUN Server. Jan 26 15:01:25 padovan: I want the basic first. Then we can extend it. Jan 26 15:06:12 Sure. Jan 26 15:10:12 padovan: ok Jan 26 15:15:03 padovan: We need to start merging patches. If we have too many pending ones, the review gets nasty. Jan 26 15:29:38 padovan: my problem here is: Jan 26 15:29:38 1) Should we create like Zhenzhua a plugin with atom emulator_drivers and trigger the create() and probe() into modem.c Jan 26 15:29:38 2) Or should we create for each modem driver (AT ISI IFX ...): an atom emulator_driver, and trigger the create() and probe() into each modem plugins during post_online() modem or enable() modem? Jan 26 15:32:02 at emulator is a generic thing, it has nothing to do with modem drivers Jan 26 15:33:44 denkenz: ok so it is no a problem to probe it into modem.c right ? Jan 26 15:34:24 The way I would do it is similar to plugins/smart-messaging.c Jan 26 15:34:42 e.g. instantiate a modem watch and create an emulator for the modem Jan 26 15:34:48 The emulator can be a core atom Jan 26 15:35:24 How / when you create an emulator is up to the plugin implementation Jan 26 15:35:33 e.g. on an rfcomm connect is one possible scenario Jan 26 15:36:20 I see Jan 26 15:37:22 We could use an online watch into the plugin Jan 26 15:38:12 I will think a bit more about it. Jan 26 16:21:57 holtmann: Ok, I'm working on the zhenhua's bluetooth patches today, and send for review/merge later. Jan 26 16:22:31 Then we can go ahead with the Emulator and DUN server patches Jan 26 16:30:47 padovan: I should propose a patch for bluetooth supporting multiple servers, adapters and clients tomorrow Jan 26 16:39:26 fdanis: i.e., patches that will replace all zhenhua patches? Jan 26 16:40:28 padovan: no, only for bluetooth server support part Jan 26 17:04:57 holtmann: denkenz: i'm doing the function do clean the storage dir (remove .tmp files)... can I use the field d_type of a struct dirent? Jan 26 17:05:09 or should I got through calling lstat? Jan 26 17:05:38 according to man page: "... Currently, only some file systems (among them: Btrfs, ext2, ext3, and ext4) have full support returning the file type in d_type. All applications must properly handle a return of DT_UNKNOWN." Jan 26 17:06:21 demarchi: You got me here. I am lost. Jan 26 17:06:34 ahn.. ok, let me give you some context Jan 26 17:07:00 currently there's a problem with write_file()/read_file() in src/storage.c Jan 26 17:07:20 if we crash while writing a file Jan 26 17:07:36 or we are shutdown or any other reason Jan 26 17:07:45 we could let dangling .tmp files on storage dir Jan 26 17:10:03 when we are going up again, we have some options: Jan 26 17:10:23 every time we read a file, check if it has .tmp extention and ignore it Jan 26 17:10:37 holtmann: you want me to fix IPCP/ppp iface creation on oFono? Jan 26 17:10:55 or, delete all .tmp files when oFono is going up Jan 26 17:11:19 demarchi: Sorry what does d_type do? Jan 26 17:11:41 denkenz: i can check if it's a regular file or a directory Jan 26 17:11:50 so i can recurse on subdirs Jan 26 17:11:56 ahh Jan 26 17:12:03 but it's not supported by all filesystems Jan 26 17:12:23 So it is quite feasible to run oFono on esoteric file systems Jan 26 17:12:25 the ones tha implement it are btrfs, ext2, ext3, and ext4 (according do man page) Jan 26 17:12:29 padovan: I don't know yet. We need to discuss this further. Jan 26 17:12:36 so I'd just call lstat Jan 26 17:12:59 Or maybe call lstat only if d_type is UNKNOWN or whatever Jan 26 17:13:12 denkenz: i have a third option, that i prefer, but i don't know if it's safe/feasible Jan 26 17:13:27 denkenz: change write_file() so we do not write .tmp files Jan 26 17:13:27 oh> Jan 26 17:13:52 and do what instead? Jan 26 17:13:55 i wrote it... but i have to change also the read_file() Jan 26 17:14:20 denkenz: the idea is to have the first 4 bytes of a file to contain the size Jan 26 17:14:38 then, when writing a file, we write 0 to the first 4 bytes Jan 26 17:14:47 then we write the buffer Jan 26 17:15:06 holtmann: Ok, we have time, I still have DUN/Emulator stuff to finish on oFono first. Jan 26 17:15:11 and finally write the total number of bytes written on position 0 Jan 26 17:15:35 so you have to write, sync, lseek, write, close Jan 26 17:16:03 denkenz: write, sync, pwrite, close Jan 26 17:16:19 And you still have the problem that you potentially are overwriting a file Jan 26 17:16:54 denkenz: why? Jan 26 17:17:21 because we use write_file for things like storage_sync Jan 26 17:17:30 so we're overwriting settings and things like that Jan 26 17:17:40 ahnn... i see Jan 26 17:18:06 I think running a 'find' type thing at startup is just fine Jan 26 17:18:26 denkenz: ok... i personally don't like it too much Jan 26 17:18:39 but i'm doing it right now Jan 26 17:18:56 Well to me it sounds like the re-writing approach is more complicated Jan 26 17:19:01 And won't work for settings Jan 26 17:19:10 I'm open to ideas here though, since I hate the temp files as well Jan 26 17:19:36 denkenz: what if we change read_file to return an error if it has .tmp suffix? Jan 26 17:19:51 what has a .tmp suffix? Jan 26 17:20:10 the path Jan 26 17:20:24 The path should never have a .tmp suffix, oFono uses well known names Jan 26 17:20:38 denkenz: so this should work Jan 26 17:20:51 I'm lost ;) Jan 26 17:20:54 currently implementation of read_file does: Jan 26 17:21:05 ssize_t read_file(unsigned char *buffer, size_t len, const char *path_fmt, ...) Jan 26 17:21:08 { Jan 26 17:21:14 ... Jan 26 17:21:19 va_start(ap, path_fmt); Jan 26 17:21:20 path = g_strdup_vprintf(path_fmt, ap); Jan 26 17:21:20 va_end(ap); Jan 26 17:21:20 fd = TFR(open(path, O_RDONLY)); Jan 26 17:21:29 if we change it to: Jan 26 17:21:34 va_start(ap, path_fmt); Jan 26 17:21:35 path = g_strdup_vprintf(path_fmt, ap); Jan 26 17:21:35 va_end(ap); Jan 26 17:21:52 if (g_str_has_suffix(path, ".tmp") Jan 26 17:21:59 return -1; Jan 26 17:22:06 path will never have .tmp Jan 26 17:22:08 then i think it's sufficient Jan 26 17:22:33 denkenz: unless we are reading the wrong file Jan 26 17:22:45 Then we have a bug and it should be fixed Jan 26 17:22:58 But seriously, are you just being paranoid or do you have an actual bug? Jan 26 17:23:09 i'm being paranoid Jan 26 17:23:14 stop that ;) Jan 26 17:23:41 The core / storage should never read .tmp files Jan 26 17:23:51 we're pretty anal in verifying the path type Jan 26 17:24:07 if this somehow doesn't work, then we need to fix the originating code Jan 26 17:25:09 fixing the originating code was what i was hoping with changing write_file() Jan 26 17:26:34 I still don't think you actually have a problem with temp files Jan 26 17:27:46 Or maybe I'm just blind Jan 26 17:28:03 let me find an example Jan 26 17:29:04 +1 for stop being paranoid. let's solve real problems ;-) Jan 26 17:29:41 denkenz: check sms_assembly_load function Jan 26 17:30:02 it does a scandir Jan 26 17:30:22 then starts to read all files Jan 26 17:31:09 seq = strtol(segments[i]->d_name, &endp, 10); Jan 26 17:31:09 if (*endp != '\0') Jan 26 17:31:09 continue; Jan 26 17:31:12 oh hell... it checks the file name by using strtlol Jan 26 17:31:14 Doesn't this kill it? Jan 26 17:31:22 yeah.. i've just saw that Jan 26 17:31:31 heh Jan 26 17:31:53 denkenz: so i think i have to change the file name on my patches Jan 26 17:31:58 so i can do just like this Jan 26 17:32:05 aki, holtmann, denkenz: so i see two options for adding the sim file op cancelation, one is returning a unique id for each operation in the queue similarly to some_id = g_timeout_add() / g_source_remove(some_id) Jan 26 17:32:31 this is going to clutter a lot of places in the code Jan 26 17:33:23 denkenz: in patch number 3, function sms_tx_load() Jan 26 17:33:30 demarchi: Nod, we tend to do such anal checks everywhere Jan 26 17:33:44 Hence our files are named strangely ;) Jan 26 17:33:56 balrog-k1n: I don't like the first approach, what's the other option? Jan 26 17:34:56 i'm not sure you'll like it either :) Jan 26 17:35:00 the other option is for sim_fs_read_info/sim_fs_read/sim_fs_write to take a new "atom" parameter, so simfs.c can listed for the atom's removal and auto-cancel all the ops in the queue belonging to that atom Jan 26 17:35:12 *listen Jan 26 17:35:31 hmm Jan 26 17:35:51 what if we add another parameter, but do it something like the modem state Jan 26 17:36:00 e.g. pre_sim, post_sim, post_online Jan 26 17:36:21 So if we go from post_online to post_sim, we remove all 'tagged' simfs operations Jan 26 17:37:03 But the atom idea is also not a bad one Jan 26 17:37:12 you mean 'tagged' as "post sim"? Jan 26 17:37:28 so lets say netreg does stuff like Jan 26 17:37:33 that would also work, with the assumption that atoms are only added and removed on sim state changes Jan 26 17:37:48 ofono_sim_read(sim, foo, OFONO_MODEM_STATE_POST_ONLINE) Jan 26 17:37:58 so if we go to Offline mode, that operation gets canceled Jan 26 17:38:06 yeah, that would work Jan 26 17:38:56 I don't have a favorite; what do you prefer? Jan 26 17:39:25 i was going to say the same thing :) Jan 26 17:39:41 let's do the ofono_sim_read(sim, foo, OFONO_MODEM_STATE_POST_ONLINE) Jan 26 17:39:54 so the only advantage of my suggestion is that it can work for things which are not atoms Jan 26 17:40:04 However, we don't really have anything like that today Jan 26 17:40:11 So it is not much of an advantage Jan 26 17:40:24 So I leave this up to you, both are fine with me Jan 26 17:41:22 yeah, but i was thinking even that might not be enough for some future use cases, like for example we get a Send SMS command and we need to read one of the EFs to execute this, so we would want to cancel this if the Send SMS gets cancelled Jan 26 17:42:07 So I think your first suggestion is a required evil Jan 26 17:42:11 e.g. cancelation by id Jan 26 17:43:02 we can do both things Jan 26 17:43:14 Yep, my thinking is the same Jan 26 17:43:23 basically allow cancelation by id for things that truly need it Jan 26 17:43:30 have a sim_fs_op_cancel method for those nasty cases only Jan 26 17:43:42 yep, sounds good Jan 26 17:44:42 Why would we need to read a file for Send SMS though? Jan 26 17:44:47 Are you talking about FDN checks? Jan 26 17:46:08 i just needed an example, perhaps we don't need to do anything like this for Send SMS :) Jan 26 17:46:25 yeah I was going to say you're being paranoid ;) Jan 26 17:47:13 Adding the ids is easy enough, so do that Jan 26 17:47:19 But I'm still doubtful we ever end up using this Jan 26 17:48:02 hmmm another idea would be to instead of passing the modem state as a sim_fs_read_info/sim_fs_read/sim_fs_write parameter, just query the current modem state in simfs.c? Jan 26 17:48:29 good idea ;) Jan 26 17:49:00 but be careful Jan 26 17:50:02 It is better to query the state atom was created in Jan 26 17:50:11 this info should already be available from the atom structure Jan 26 17:50:42 struct ofono_atom { Jan 26 17:50:42 enum ofono_atom_type type; Jan 26 17:50:42 enum modem_state modem_state; Jan 26 17:50:42 yeah, but the user data doesn't always have to be a pointer to the atom struct Jan 26 17:51:12 Yes, but the thing about it is that you might be in this situation: Jan 26 17:51:30 you create an atom in post_sim, but the driver only registers in post_online Jan 26 17:51:42 you issue a sim read in post_online Jan 26 17:51:49 and we go back to offline Jan 26 17:51:59 so we cancel the read erroneously Jan 26 17:52:20 true Jan 26 17:52:44 So maybe doing the atom approach is easier Jan 26 17:52:52 so i think we can't use the current state after all, and we can't use the state of the atom's creation Jan 26 17:53:07 and need to pass an additional parameter Jan 26 17:53:16 Yeap Jan 26 17:54:17 unless you wanna do the same trick we did with gatchat Jan 26 17:54:36 But that is so evil and I'm not sure we need this in the core ;) Jan 26 17:56:58 what did we do in gatchat though? Jan 26 17:57:12 for g_at_chat_register? Jan 26 17:57:37 so we ended up creating facades for GAtChat and calling them GAtChat Jan 26 17:57:53 and hiding the real GAtChat in a private object Jan 26 17:58:12 The facades have a 'group id' and basically when the facade goes away, it cancels all commands with a group id Jan 26 17:58:34 ah Jan 26 17:59:01 We can do something similar Jan 26 17:59:08 well the atom pointer is a kind of "id" like that Jan 26 17:59:34 simfs could just accept any pointer, not only atom pointers, which would be like those "id"s Jan 26 17:59:39 yep, so we could do something like ofono_sim_create_context(sim, atom) Jan 26 17:59:50 ofono_sim_read(context,...) Jan 26 18:00:02 ofono_sim_context_free() Jan 26 18:00:16 and maybe ofono_sim_read -> ofono_sim_context_read() Jan 26 18:00:49 but the ofono_sim_create_context could be implicit, i.e. we can avoid it altogether Jan 26 18:01:06 yes, passing the atom is close enough Jan 26 18:01:48 ofono_sim_read(simfs, anypointer_as_context, ...) although we have more parameters than if we do ofono_sim_create_context(sim, atom) first Jan 26 18:02:27 From an API point of view it might be nicer than arbitrary pointer Jan 26 18:02:35 since this is actually public API Jan 26 18:03:03 So we either make this into an atom parameter for lifetime tracking Jan 26 18:03:07 Or we make a specific context Jan 26 18:06:04 ah right, it's public api Jan 26 18:06:09 and ofono_atom isn't even a public api Jan 26 18:06:47 Yep, that's another good point Jan 26 18:08:04 So maybe we just bite the bullet and make a proper context Jan 26 18:08:16 that is a generalized solution that will work for atoms and non-atoms Jan 26 18:09:50 but are you sure we want every context to be registered first? Jan 26 18:10:17 because if the context don't have to be registered/created, they also don't have to be freed if you don't care about them Jan 26 18:10:40 it might not be that big of a deal in the end Jan 26 18:10:58 ofono_sim_context_create(sim); Jan 26 18:11:05 ofono_sim_context_free(context); Jan 26 18:11:14 ofono_sim_context_read/write(context) Jan 26 18:11:34 So you introduce two extra lines / atom Jan 26 18:11:59 + a place to store the reference Jan 26 18:12:19 but yeah, not a big deal Jan 26 18:12:30 Well we have to do the reference anyway Jan 26 18:12:43 In fact the context is nothing more than a group id Jan 26 18:12:52 exactly :) Jan 26 18:13:12 + a pointer to simfs Jan 26 18:13:27 But its a nice way of hiding the nasties from the user Jan 26 18:24:04 balrog-k1n: ping Jan 26 18:25:21 pong Jeevaka Jan 26 18:28:53 balrog-k1n: are we correctly addressing the null alpha identifier case Jan 26 18:29:11 as per spec, if the alpha identifier is provided by the UICC and is a null data object (i.e. length = '00' and no value part), the Jan 26 18:29:17 terminal should not give any information to the user; Jan 26 18:31:00 Jeevaka: hmmm true... so we just ignore the icon if it is provided but alpha id is null? Jan 26 18:32:38 how are we differentiating the no alpha identifier and nll alpha identifier in allpha_id_set? Jan 26 18:32:46 null* Jan 26 18:33:26 Jeevaka: we aren't, but if i remember correctly we don't have to Jan 26 18:34:28 we "may" give indication to the user if there's no alpha id, but it's not defined how the indication should look anyway Jan 26 18:35:19 what about the null data object(length=00 and no value part) case? Jan 26 18:36:20 how will the agent know when to show some info and when not to show any info? Jan 26 18:36:28 Jeevaka: as i understand, not giving any information to the user is okay in both cases Jan 26 18:37:18 ah ok, got ur point.:) Jan 26 18:37:55 but i think you're right that we need to ignore the icon if alpha id is null Jan 26 18:39:00 want to keep the check in stk.c so that we will be in line with the other checks Jan 26 18:39:57 what do you think? Jan 26 18:40:55 Jeevaka: sounds okay, but there's another tricky case i think, if alpha id is "" (not null) and icon is provided Jan 26 18:41:18 in this case we should treat alpha id normally and send over dbus, right? Jan 26 18:42:44 as per spec, for this specific case it didn't even consider icon id Jan 26 18:42:50 :( Jan 26 18:44:06 now i remember Jan 26 18:44:35 If the terminal receives an icon, and either an empty or no alpha identifier/text string is given by the UICC, than the Jan 26 18:44:38 terminal shall reject the command with general result "Command data not understood by terminal". Jan 26 18:44:45 good old case ;) Jan 26 18:45:17 ah, so we take care of it in stkutil.c already? Jan 26 18:45:22 yes Jan 26 18:46:48 ok, but what if icon id is 0 and alpha id is "" (not null)? i think we should just send this over dbus, too Jan 26 18:49:09 we shouldn't Jan 26 18:49:51 check the second bulletin in 3GPP TS 31.111 section 6.4.12 Jan 26 18:54:25 "6.4.12 SEND USSD"? Jan 26 18:55:00 s Jan 26 18:55:12 its the same for all Jan 26 18:56:12 yes, but it says "if the lpaga identifier is provided by the UICC and is not a null data object, the ME shall use it to inform the user." Jan 26 18:56:19 *alpha Jan 26 18:57:44 ithe ase you described falls in the next one Jan 26 18:57:58 case* Jan 26 18:59:02 correct me if i'm wrong Jan 26 19:00:20 but the next one is about null data object Jan 26 19:01:08 so i think we just need alpha == NULL without a alpha[0] check Jan 26 19:01:30 so the case ur pointing is basically string with len = 1 with oly dcs Jan 26 19:01:52 empty string* Jan 26 19:01:54 yeah, which is not special-cased Jan 26 19:04:06 agreed Jan 26 19:05:57 hmm maybe we should add a comment saying that we are treating "no alpha id" and "null data object alpha id" cases equally because we didn't have a better idea Jan 26 19:07:10 inside stk_alpha_id_set function? Jan 26 19:08:00 yes Jan 26 19:51:16 balrog-k1n: sent v2 Jan 26 19:53:41 Jeevaka: i noticed :) but now alpha will always be non-NULL provided that the attributes were not corrupt Jan 26 19:56:14 yes Jan 26 19:56:55 isn't that how it is supposed to be Jan 26 19:57:25 thats how we are dealing withll the cases Jan 26 19:58:25 either we need to change all the cases to "if text attr fails, then we need to assign the text as we have received in the pcmd Jan 26 19:58:42 or leave it as it is now Jan 26 20:01:25 eg: display text, setup call, play tone etc Jan 26 20:02:54 Jeevaka: but if alpha id was a null data object, or there was no alpha id, we shouldn't call stk_agent_display_action_info Jan 26 20:04:50 as far as i can tell the text ? text : "" change is not needed Jan 26 20:06:11 http://codepad.org/F7cigBmk Jan 26 20:06:13 is this ok Jan 26 20:08:01 Jeevaka: it's would work better, but the alpha ? alpha : "" isn't needed either i believe Jan 26 20:08:05 ignore it Jan 26 20:08:07 it* Jan 26 20:09:19 i think just alpha != NULL check is enough Jan 26 20:09:56 i'd also make the comment "This may be changed" instead of "has to" because the current behaviour is compliant Jan 26 20:12:16 so, basically this is it http://codepad.org/y8IxR4Yy Jan 26 20:12:59 ok? Jan 26 20:13:17 yeah, i think so Jan 26 20:57:43 Are we there yet on the alpha_id thing? Jan 26 21:15:27 we agreed on v3 Jan 26 21:18:08 i'll look into possible alpha id cases tomorrow Jan 26 21:19:31 hm, so icon is not a pointer Jan 26 21:20:34 So is this the case where we get an empty alpha id but an icon? Jan 26 21:24:39 Anyway the patch looks fine to me since in both cases we should not / do not have to inform the user Jan 26 21:28:02 s Jan 26 21:33:34 denkenz: can we move the ofono_clir_option and ofono_cug_option enums from types.h to voicecall.h Jan 26 21:34:33 actually we should get rid of cug_option Jan 26 21:36:08 are we planning to use the boolean to indicate then Jan 26 21:36:49 No, we should just get rid it of it Jan 26 21:37:02 Not like CUG is going to happen this century ;) Jan 26 21:38:57 getting rid of cug involves also change the voice call driver interface change which will result in all the modem's to be changed as well Jan 26 21:39:13 can i go ahead and do the change Jan 26 21:39:43 yeah go for it Jan 26 21:40:02 If we ever need it we add it back Jan 26 21:40:09 ok Jan 26 21:53:56 denkenz: do i need to create 1patch for each driver changes or 1patch for all driver voicecall file changes Jan 26 21:54:11 padovan: about enable and disable methods. What do you mean with "you can't just enable the Jan 26 21:54:21 GPS device when registering the Agent. Jan 26 21:54:22 " Jan 26 21:54:35 Jeevaka: I prefer clear patches over git bisect compatibility Jan 26 21:54:51 So 1 / driver & include is fine Jan 26 21:56:43 padovan: I followed this idea : http://lists.ofono.org/pipermail/ofono/2011-January/007477.html Jan 26 21:57:23 padovan: which says that "Instead of using a Powered property, y, let us use an Agent Jan 26 21:57:29 with file descriptor passing instead." Jan 26 21:57:49 padovan: or you just suggest a readonly property? Jan 26 21:58:29 padovan: and also the gps is turned on when an external client registers an agent. Jan 26 22:00:27 zrafa: I do not think that enable the device when registering the Agent is good idea. Jan 26 22:00:49 the property can be readonly, it is for others see the GPS status. Jan 26 22:01:00 denkenz: what do you think? Jan 26 22:01:00 padovan: and I am not understanding your comments about gpsagent.c?. It is not a test, it is similar to stkagent or smsagent Jan 26 22:02:03 There will ever be one consumer who is interested in the GPS stream Jan 26 22:02:09 padovan: but Denis and Marcel suggested that two weeks ago. And my version 2 of my gps patches does you are suggesting" Jan 26 22:02:13 So having an Enabled / Powered property is pointless Jan 26 22:02:41 You register the agent, we reply OK / error Jan 26 22:02:46 padovan: so you like the previous idea. If so, then we should consider again my version 2 Jan 26 22:02:55 Enable the nmea stream and call the agent's function Jan 26 22:03:07 Or we call release if an error occurred Jan 26 22:04:47 denkenz: I'm asking why we don't need a Enable() method there. Do we really want to enable the device when Registering the Agent? Jan 26 22:05:00 why not? Jan 26 22:05:11 If you're not interested in the data, then don't register the agent Jan 26 22:06:37 yeah, we could do that but RegisterAgent() not a good name to me them. Jan 26 22:07:28 zrafa: my comments about gpsagent.c are wrong. I thought it was something different, I didn't look to the implementation. :( Jan 26 22:08:30 s/not a good/is not a good/ Jan 26 22:08:40 Gah, and s/them/then/ Jan 26 22:08:52 lol, anyhow I'm open to suggestions on the naming Jan 26 22:09:01 in STK we use SelectItem(id, agent) Jan 26 22:09:11 So we can use Enable(agent) or similar Jan 26 22:09:19 But unregistering gets tricky Jan 26 22:09:37 Perhaps Connect(agent) and Disconnect Jan 26 22:09:49 However, this is not really a big deal, we can fight over the names later Jan 26 22:10:10 Yeah, sure. Jan 26 22:13:36 padovan: several use this kind of name for registers an agent: smarmessaging, stk, pushnotification Jan 26 22:14:03 but we can have this method that we don't the name returning the fd to the agent. If we don't to be async here. Jan 26 22:14:40 in english please? :) Jan 26 22:15:22 Dammit, my English is bad today. Let's try again. Jan 26 22:16:33 If we don't care to be async our RegisterAgent() can return the fd to the agent. Jan 26 22:16:57 Then we don't need a extra method only for receive the fd in the Agent API. Jan 26 22:17:14 Am I clear now? ;) Jan 26 22:17:41 so we have an Agent with just a Release() method? Jan 26 22:18:06 Possible, but sort of goes against the Agent idea Jan 26 22:18:25 We don't need the release either. We can just shutdown() the fd and that results in HUP on the client side. Jan 26 22:18:52 hm, good point Jan 26 22:19:08 Then we can do this even simpler ;) Jan 26 22:19:09