**** BEGIN LOGGING AT Fri May 14 02:59:56 2010
May 14 03:03:58 <denkenz> zhenhua: Btw I haven't had time to look at your patches in depth, I was too busy with Yang's
May 14 03:04:10 <denkenz> zhenhua: However, my feeling is you're on the right track
May 14 03:05:08 <zhenhua> denkenz: okay, and i expect to see your comments so that i can revise and make sure we are in the good shape
May 14 03:05:40 <denkenz> zhenhua: Nod, might not be until next week due to my impending travel
May 14 03:06:42 <zhenhua> denkenz: okay, i can still move forward to implement locally, there're still lots of work and i will update schedule to majid.
May 14 03:07:50 <zhenhua> denkenz: for telephony_str_to_err patch, do u want to resend it to remove strcasecmp part?
May 14 03:08:32 <denkenz> zhenhua: I'm still not sure what that one is supposed to do
May 14 03:09:13 <denkenz> zhenhua: all the modems in the world should support CMEE=1
May 14 03:09:27 <denkenz> its just a matter of sending the right init
May 14 03:09:42 <zhenhua> denkenz: yes, but current decode_at_error simply return 0 error code
May 14 03:10:10 <zhenhua> we need to record the code so DUN client can get that
May 14 03:10:16 <denkenz> yeah, so fix decode_at_error to do the right thing, e.g. report the CME/CMS error code
May 14 03:10:39 <denkenz> comparing messages is a really bad idea
May 14 03:11:15 <denkenz> If you want to handle CMEE=2 inside the emulator and use the messages in common.c that's fine though
May 14 03:11:31 <zhenhua> okay, so it's clear. i just remove string comparing part and it should be done.
May 14 03:11:56 <zhenhua> handle CMEE=2 is not intention of this patch
May 14 03:12:08 <denkenz> yeah, basically just check for CMS/CME prefix and parse the error code
May 14 03:12:44 <zhenhua> beside, i found CEER syntax is 'CEER: ERROR xxx' instead of 'CEER ERROR: xxx'
May 14 03:13:02 <denkenz> Yeah, someone was being stupid :)
May 14 03:13:27 <zhenhua> heh, i am done. yang will ask u questions.
May 14 03:13:29 <denkenz> might have to quirk the error function just like everything else
May 14 03:14:14 <zhenhua> i don't have such plan yet. :-)
May 14 03:14:40 <denkenz> Nod, CEER is pointless anyway
May 14 03:15:11 <denkenz> Unless some modem actually can give us a decent error report
May 14 03:15:59 <yang_office> denkenz: Thank you for your modifications on my code. But I have several parts unclear.
May 14 03:16:13 <denkenz> which?
May 14 03:16:28 <yang_office> 1. the sequence of refresh
May 14 03:16:43 <yang_office> The sequence in spec is not the same with test spec
May 14 03:16:53 <yang_office> So I followed the spec
May 14 03:17:25 <yang_office> So i think you may rollback the last patch
May 14 03:17:30 <denkenz> TS 102.223 Refresh is 6.4.7
May 14 03:18:05 <denkenz> what spec are you using?
May 14 03:18:13 <yang_office> Oh, maybe we are using different version
May 14 03:18:27 <denkenz> ah I see
May 14 03:18:31 <yang_office> 102.223, v9.0.0
May 14 03:18:34 <denkenz> 6.4 and 6.6 have different sequence
May 14 03:18:46 <yang_office> 6.6.13 refresh
May 14 03:18:57 <yang_office> I downloaded the latest one.
May 14 03:19:11 <yang_office> Thus I know the reason.
May 14 03:19:40 <yang_office> That would be fine.
May 14 03:19:49 <yang_office> Second question
May 14 03:20:04 <yang_office> It's about the usage of parse_dataobj()
May 14 03:20:24 <yang_office> I think we can allow to use it several times
May 14 03:20:33 <yang_office> one by one I mean
May 14 03:21:10 <denkenz> we can, but why?
May 14 03:21:27 <yang_office> Current one has a problem: it will move one iter ahead even if all the dataobjs are matched
May 14 03:22:11 <denkenz> Thats what we want, no?
May 14 03:22:14 <yang_office> So we need the code: if (l->next == NULL) break;
May 14 03:23:16 <yang_office> That's to say, we need to read next iter in parse_item_list
May 14 03:23:34 <yang_office> Instead of reading it in dataobj()
May 14 03:23:34 <denkenz> Ok, so I'm actually abusing the system
May 14 03:25:33 <denkenz> ok, I'm actually confused
May 14 03:25:59 <denkenz> why is consuming the iterator in parse_dataobj a problem?
May 14 03:26:32 <yang_office> For example
May 14 03:26:49 <yang_office> We have a case with two dataobj in it
May 14 03:27:15 <yang_office> we can call parse_dataobj() once and pass the two dataobjs as parameters
May 14 03:27:53 <yang_office> We also need to allow to call parse_dataobj() twice, and pass item_0 and item_1 separately.
May 14 03:28:14 <denkenz> where is that useful?
May 14 03:29:31 <yang_office> But it has no reason parse_dataobj() read ahead if it doesn't use it at all.
May 14 03:29:55 <yang_office> Once all the expected dataobjs are matched.
May 14 03:30:07 <denkenz> who cares? :)
May 14 03:31:04 <yang_office> That's natural we think of a parser.
May 14 03:31:53 <yang_office> But anyway, if you don't think this is a problem. I will be fine
May 14 03:32:05 <yang_office> I have the third question
May 14 03:32:06 <denkenz> I don't know, for me its more natural for it to consume it
May 14 03:32:14 <yang_office> item can be empty
May 14 03:32:24 <denkenz> there's no right and wrong here, as long as it works in all cases
May 14 03:33:09 <denkenz> btw, how did your unit tests pass on empty item? :)
May 14 03:33:32 <yang_office> Heh, that's my third question.
May 14 03:33:49 <yang_office> The empty item will have a NULL list
May 14 03:34:08 <denkenz> I swear I tested your patch before making the modifications to parse_item_list and unit tests passed
May 14 03:34:09 <yang_office> But it seems in your solution, your list has one item.
May 14 03:34:46 <yang_office> Right, it passed all the cases.
May 14 03:35:17 <yang_office> I still think we should have a NULL list here.
May 14 03:35:31 <denkenz> ah I see, you ignored the empty case
May 14 03:35:40 <denkenz> Ok, so I actually agree
May 14 03:35:48 <denkenz> However, this has to be done in the main logic
May 14 03:35:56 <denkenz> The reason they did this is:
May 14 03:36:00 <yang_office> My original patch handles this case.
May 14 03:36:16 <denkenz> Item 1 is actually labeled mandatory
May 14 03:36:26 <denkenz> However, you can have empty menus, so the entire thing makes no sense
May 14 03:36:40 <denkenz> hence they invented a NULL item
May 14 03:36:46 <denkenz> utter stupidity on their part
May 14 03:37:02 <denkenz> So, let the list parser remain as is
May 14 03:37:28 <denkenz> However, in the main logic, check to see if the list is of size 1 and the text is NULL / id 0
May 14 03:37:48 <denkenz> In that case free the list, set it to NULL
May 14 03:38:02 <denkenz> all other cases of id 0 / text NULL are actually invalid
May 14 03:38:09 <yang_office> Why not handle this in parse_item_list?
May 14 03:38:22 <denkenz> [22:38] <denkenz> all other cases of id 0 / text NULL are actually invalid
May 14 03:39:39 <yang_office> I think all the item with id==0 shouldn't be added to list
May 14 03:40:57 <denkenz> It won't matter since the command will get rejected
May 14 03:41:44 <denkenz> or should be anyway
May 14 03:44:58 <yang_office> So the modification would be: allow item with id=1 to be added to list.
May 14 03:45:28 <yang_office> After returning the list, check the size is 1 or not, and text is NULL /id 0?
May 14 03:46:22 <yang_office> I still like my original solution, use a gboolean to show if the mandatory item exists. All the empty will not be added to the list.
May 14 03:46:33 <yang_office> Thus a NULL list is returned.
May 14 03:47:22 <yang_office> Use ret to judge parse_item_list() is success or not.
May 14 03:48:20 <denkenz> I'd rather us be pedantic, if the SIM sends us two NULL items that should be an error
May 14 03:48:31 <denkenz> not a successful parse
May 14 03:49:08 <denkenz> If you want to do that inside parse_item_list, that's fine
May 14 03:49:21 <yang_office> Two NULL items are valid, right?
May 14 03:49:57 <denkenz> Not according to the spec: "The identifier is a single byte between '01' and 'FF'. Each item shall have a unique identifier within an Item list."
May 14 03:50:20 <denkenz> as I said, the whole NULL item thing is a special case because the guys who wrote the spec were idiots
May 14 03:50:28 <yang_office> But these two items have no id
May 14 03:50:54 <yang_office> I agree with you;)
May 14 03:51:09 <yang_office> agree with you that they were idiots;)
May 14 03:51:33 <yang_office> But for the ber-tlv that has two NULL items, it's still valid
May 14 03:51:52 <yang_office> If NULL item is valid
May 14 03:51:56 <denkenz> it really shouldn't be according to the rules of Item definition
May 14 03:52:21 <yang_office> But they have the case of NULL item.
May 14 03:52:28 <denkenz> a single one yeah
May 14 03:52:49 <yang_office> ok. Which solution do you prefer?
May 14 03:52:54 <denkenz> How would you treat nul nul nul valid nul nul?
May 14 03:52:56 <yang_office> I can make the patch
May 14 03:53:07 <denkenz> Imo the entire thing should be thrown out
May 14 03:53:21 <yang_office> End with a list that has only one item of valid one
May 14 03:53:57 <yang_office> The first nul one meets the mandatory requirement.
May 14 03:54:05 <yang_office> Anyway, that doesn't matter.
May 14 03:54:23 <yang_office> We should have a way to code with it.
May 14 03:54:29 <denkenz> Lets throw it out for now
May 14 03:54:50 <yang_office> ok, I will make the patch based on this rule.
May 14 03:54:53 <denkenz> if we ever actually meet that foolishness then we come back to it
May 14 03:55:07 <yang_office> Fine. I will make the patch.
May 14 03:55:21 <yang_office> That's all for questions, thank you.
May 14 12:31:56 <balrog-k1n> seems gcc 4.3.2 can't take the array item initializers in test-stkutil.c in setup_menu_nnn tests (or there's something else wrong with my gcc)
May 14 12:51:14 <balrog-k1n> either .items = {{ ... }, { ... }} or .items = { [0] = { ... }, [1] = { ... }} style initialisers work
May 14 13:42:44 <denkenz> balrog-k1n: Send a patch then
May 14 14:16:57 <akiniemi> denkenz: ping
May 14 14:28:13 <denkenz> akiniemi: pong
May 14 15:50:03 <akiniemi> denkenz: about the sms atom...
May 14 15:50:35 <akiniemi> it doesn't seem to set the mms flag with multipart sms
May 14 15:50:43 <akiniemi> should it?
May 14 15:55:23 <denkenz> akiniemi: I'm pretty sure it did
May 14 15:55:54 <denkenz> However, we actually keep track of the last time we set the mms flag
May 14 15:56:45 <denkenz> Maybe we never update it properly
May 14 15:57:01 <akiniemi> denkenz: ok, then I suspect it might be a bug. I will try to take a look
May 14 15:57:40 <denkenz>         if ((g_queue_get_length(sms->txq) > 1) &&
May 14 15:57:41 <denkenz>                         ((ts - sms->last_mms) > 60))
May 14 15:57:43 <denkenz>                 send_mms = 1;
May 14 15:57:50 <denkenz> Sounds like we should also set last_mms too
May 14 15:58:43 <akiniemi> ah, I read that part but missed the bug ;)
May 14 15:59:44 <akiniemi> oh, btw, isimodem now does SMS :)
May 14 16:00:01 <denkenz> sweet
May 14 16:04:19 <denkenz> akiniemi: What is still missing from isimodem btw?
May 14 17:18:18 <akiniemi> denkenz: SIM PIN
May 14 17:18:47 <akiniemi> Also, the hacks we have in the SIM driver to fake EF access is a bit incomplete
May 14 17:19:01 <akiniemi> STK haven't been started yet
May 14 17:19:28 <akiniemi> Also, ssn and call meter not done
May 14 17:20:30 <akiniemi> Then, for the N900, we need a plugin that toggles GPIO lines for modem power up/down
May 14 17:21:49 <akiniemi> In the short term, we will work to make the N900 adaptation work well
May 14 17:22:12 <akiniemi> Then at some point we will start to generalize the isimodem for newer ISI modem APIs...
May 14 17:22:39 <akiniemi> ... adding things like PN_UICC support for a proper SIM driver
May 14 17:34:38 <denkenz> akiniemi: Well the core doesn't have STK, so no biggie
May 14 17:47:21 <pessi> akiniemi: is your single-socket client code in main repo yet?
May 14 17:49:46 <denkenz> pessi: you around to talk about your patches?
May 14 17:51:55 <pessi> sure
May 14 17:52:25 <denkenz> +unsigned int __ofono_modem_add_state_watch(struct ofono_modem *modem,
May 14 17:52:27 <denkenz> +                                       ofono_state_watch_func notify,
May 14 17:52:28 <denkenz> +                                       void *data, ofono_destroy_func destroy);
May 14 17:52:30 <denkenz> +
May 14 17:52:31 <denkenz> +gboolean __ofono_modem_remove_state_watch(struct ofono_modem *modem,
May 14 17:52:33 <denkenz> +                                               unsigned int id);
May 14 17:52:34 <denkenz> +
May 14 17:52:36 <denkenz> I'm not quite sure who is the target for those?
May 14 17:53:07 <pessi> aki wanted atoms to know when modem is online or not
May 14 17:53:33 <denkenz> what's the use case though? They're not used in your later patches
May 14 17:53:34 <pessi> that makes sense for call atom
May 14 17:54:01 <pessi> hm?
May 14 17:54:28 <denkenz> As in, why would I even bother doing a watch instead of just checking the state before doing any action
May 14 17:54:30 <pessi> no, there is nobody using them
May 14 17:55:16 <pessi> aki probably had a use case for them
May 14 17:55:26 <denkenz> e.g. Dial -> get state(), if offline, fail
May 14 17:55:50 <pessi> I can't figure out anything
May 14 17:56:11 <denkenz> heh, ok, so in general, don't add APIs unless you have an immediate use case
May 14 17:56:18 <denkenz> That just confuses everyone :)
May 14 18:00:48 <pessi> the emergency call handling in maemo used similar mechanism
May 14 18:01:18 <pessi> but it was a real pita
May 14 18:01:49 <denkenz> I'm not denying it might be useful, but I don't want to introduce any APIs without reviewing the consumers
May 14 18:02:01 <pessi> what you think about the patch in general? I can ditch the watch and add change_state to at modem drivers
May 14 18:02:33 <denkenz> +enum ofono_modem_state {
May 14 18:02:35 <denkenz> +       OFONO_MODEM_STATE_POWER_OFF,
May 14 18:02:36 <denkenz> +       OFONO_MODEM_STATE_PRE_SIM,
May 14 18:02:38 <denkenz> +       OFONO_MODEM_STATE_OFFLINE,
May 14 18:02:39 <denkenz> +       OFONO_MODEM_STATE_ONLINE,
May 14 18:02:40 <pessi> I'm a bit uneasy about the marriage of the atoms and modem states
May 14 18:02:41 <denkenz> +};
May 14 18:02:42 <denkenz> +
May 14 18:02:44 <denkenz> So I really don't like introducing this
May 14 18:03:19 <denkenz> can't we handle all this internally?
May 14 18:03:28 <pessi> what kind of mechanism you would prefer?
May 14 18:04:03 <denkenz> I mean I'm fine with online() to enable / disable radio
May 14 18:04:26 <pessi> the sim hot swap is problem here I'm trying to solve
May 14 18:04:30 <denkenz> However, I'm not happy with change_state instead of pre_sim / post_sim
May 14 18:04:46 <denkenz> haven't we already solved it?
May 14 18:05:08 <pessi> I don't know
May 14 18:05:16 <denkenz> Andrew's patches already solve sim hotswap
May 14 18:05:33 <denkenz> The only atom affected is sim and voicecall
May 14 18:05:55 <pessi> huh?
May 14 18:06:05 <pessi> all the atoms in post sim state are flushed
May 14 18:06:25 <denkenz> As they should be, right?
May 14 18:06:39 <pessi> yep
May 14 18:06:55 <denkenz> So the only atoms that deal with sim insertion / removal are sim and voicecall
May 14 18:07:04 <denkenz> The online state is entirely separate
May 14 18:07:18 <pessi> other atoms just gets flushed?
May 14 18:07:39 <denkenz> yeah, because hotswap implies removing the SIM first
May 14 18:07:59 <denkenz> Since all of oFono stores settings by IMSI, we have to flush
May 14 18:08:11 <denkenz> Besides, we don't know if the new SIM requires a PIN
May 14 18:08:14 <pessi> so I'm still looking this from the isimodem pow
May 14 18:09:16 <pessi> even if we have sim hotswap, someone has to tell isimodem to go offline mode
May 14 18:09:25 <pessi> so this is not the case with generic atmodem?
May 14 18:09:52 <denkenz> It depends on the modem
May 14 18:10:05 <denkenz> Some don't allow tx/rx on without a SIM
May 14 18:10:12 <denkenz> (probably most)
May 14 18:10:16 <denkenz> Others do
May 14 18:11:04 <denkenz> I'm actually fine if the core triggers online(false) when the SIM is removed
May 14 18:11:56 <denkenz> Or we can assume the driver does it
May 14 18:12:54 <pessi> the point of change_state was to keep drivers simple
May 14 18:13:49 <pessi> so they don't have to keep state and they would have a single function to create atoms 
May 14 18:14:06 <denkenz> no driver keeps state today
May 14 18:14:16 <denkenz> so I'm not sure why this is a concern?
May 14 18:17:01 <pessi> it is just how I did the isimodem driver
May 14 18:17:24 <pessi> because core had atom watch I did not want to duplicate it
May 14 18:18:18 <pessi> and I suspected that all the drivers supporting sim hot swap would also have to add the watch
May 14 18:18:45 <pessi> but perhaps adding call to online(mdoem, false) is better approach here 
May 14 18:18:55 <denkenz> Not really, no watch required for atoms to support hotswap
May 14 18:19:26 <denkenz> so can we take that approach for now? add online to modem driver and an extra populate function
May 14 18:19:51 <pessi> ok
May 14 18:22:10 <pessi> ...and now some stupidd questions about at modems...
May 14 18:24:11 <pessi> can you say at+cops=4 (or something similar) to get modem alive after at+cops=0 ??
May 14 18:24:21 <pessi> or is is completely modem-specific?
May 14 18:24:58 <pessi> at+cops=0 on nokia modems seems to usually misbehave
May 14 18:27:51 <pessi> but -> sauna
May 14 18:30:09 <denkenz> Nope
May 14 18:30:20 <denkenz> AT+COPS=0 takes the modem off the bus in most cases
May 14 18:32:18 <akiniemi> < pessi> akiniemi: is your single-socket client code in main repo yet?
May 14 18:32:25 <akiniemi> pessi: it's in no repo ;)
May 14 18:33:08 <akiniemi> pessi: likely a client will always take two sockets
May 14 18:33:23 <akiniemi> it makes dispatching indications and responses a lot easier
May 14 18:34:04 <akiniemi> I was planning to take client creation away from the driver probe()s and into the isimodem plugin
May 14 18:34:40 <akiniemi> ...so we can share clients between atoms, e.g., all those talking to a single ISA resource share a client
May 14 18:35:51 <akiniemi> But haven't started on this yet
May 14 19:04:21 <pessi> denkenz: ok, that was what I expected...
May 14 19:10:19 <pessi> akiniemi: can we assume all the indications have 0 in trans_id?
May 14 19:10:52 <akiniemi> pessi: I think it's just more elegant to use a dedicated socket to subscribe to them
May 14 19:11:21 <akiniemi> That way, we can map NTF requests easily to subscriptions, too
May 14 19:11:22 <pessi> well, that is true
May 14 19:11:32 <akiniemi> (see my recent patches to gisi client)
May 14 19:11:41 <pessi> ntf use same trans_id?
May 14 19:12:25 <akiniemi> Of course, they could use transid=0, haven't checked actually...
May 14 19:15:11 <akiniemi> s/could/should
**** ENDING LOGGING AT Sat May 15 02:59:56 2010