**** BEGIN LOGGING AT Wed Mar 31 02:59:56 2010
Mar 31 03:02:36 <denkenz> However, you do want to make sure read_count is updated properly
Mar 31 03:07:18 <denkenz> I fixed that one for you, we still have to potentially address echo if crap like AT+FOO\rAT+BAR\r comes in
Mar 31 03:11:22 <ygu5_ubuntu> denkenz: You mentioned in comments that parse_dataobj_common_byte_array() is not adequate for it doesn't do boundary check. Need I modify this function in the patch? If so, how?
Mar 31 03:12:56 <denkenz> For the ones that are fixed size arrays I'd rather you do the checking in a dedicated function
Mar 31 03:13:13 <denkenz> Then have common_byte_array do g_mallocs for the ones that need it
Mar 31 03:14:27 <ygu5_ubuntu> ok, I see.
Mar 31 03:14:49 <ygu5_ubuntu> I have noticed your change on the code. I will rework my patches based on your change.
Mar 31 03:16:24 <ygu5_ubuntu> One more change would be moving comprehension_tlv_iter_next(iter) to the if (comprehension_tlv_iter_get_tag(iter) == entry->type). Do you think so?
Mar 31 03:17:26 <denkenz> Don't see how that would work
Mar 31 03:18:11 <ygu5_ubuntu> If the tag are the same, we should move iter forward whatever it returns true or false.
Mar 31 03:19:10 <denkenz> You mean if we detect invalid format of the tag
Mar 31 03:19:29 <ygu5_ubuntu> yes
Mar 31 03:19:31 <denkenz> Yeah that sounds reasonable
Mar 31 03:19:55 <ygu5_ubuntu> ok. I will do that in another patch. 
Mar 31 03:20:18 <ygu5_ubuntu> By the way, do you agree on stk_common_byte_array?
Mar 31 03:20:39 <ygu5_ubuntu> Or should I have different struct with similar fields?
Mar 31 03:21:10 <ygu5_ubuntu> I mean struct stk_common_byte_array for parse_dataobj_common_byte_array()
Mar 31 03:21:48 <denkenz> I don't mind how you did it already actually, passing in the array and len
Mar 31 03:21:57 <denkenz> make the array a **ptr and you'd be fine
Mar 31 03:22:24 <denkenz> Then we can decide how to best declare the data structures
Mar 31 03:24:08 <denkenz> Don't worry if we get this wrong the first time, we will see how it works out once we define the proactive command structures
Mar 31 03:24:27 <ygu5_ubuntu> You mean **array, then alloc memory inside the function?
Mar 31 03:24:32 <denkenz> yeap
Mar 31 03:25:09 <ygu5_ubuntu> I will use struct stk_common_byte_array if we don't know the size of array.
Mar 31 03:25:25 <ygu5_ubuntu> I will make the patches.
Mar 31 07:56:45 <ygu5_ubuntu> balrog-k2n: Your question reminds me to combine those two assignments for has_cause to a single one :)
Mar 31 18:22:57 <denkenz> Ok, we have to come up with a better name: +void ofono_sim_remove_removed_watch(struct ofono_sim *sim, unsigned int id);
Mar 31 18:23:54 <holtmann> I would propose use "hotswap" for this.
Mar 31 18:25:03 <akiniemi> holtmann: +1
Mar 31 18:25:28 <denkenz> how about dead watch, we have to handle sim dead conditions too
Mar 31 18:26:23 <denkenz> akiniemi: I'm about to apply the USSD Response patch, now is the time to comment if you care either way ;)
Mar 31 18:28:47 <holtmann> Since we split them, I am okay with dead_watch as well.
Mar 31 18:29:38 <holtmann> Maybe we should just have a simple ofono_sim_register_event() function. And then we can register for certain SIM events.
Mar 31 18:31:23 <denkenz> So I'm fine with that too, we have similar thing for atom watches
Mar 31 18:31:59 <holtmann> I actually dislike the watch id stuff for sim.
Mar 31 18:32:19 <denkenz> sorry?
Mar 31 18:32:22 <holtmann> In general all events should be tied to the sim atom. So when removing the sim atom we clear all the events.
Mar 31 18:32:33 <holtmann> That keeps the plugin code simpler.
Mar 31 18:32:44 <denkenz> Nope, won't work for voicecalls
Mar 31 18:33:05 <holtmann> The manual removing of ready/dead watches seems not a valid use case.
Mar 31 18:33:07 <denkenz> Besides, watches already work that way
Mar 31 18:35:10 <holtmann> Voice calls might be different, but I prefer not to make it too complicated for sim atoms.
Mar 31 18:35:20 <holtmann> And especially all the plugins.
Mar 31 18:35:58 <holtmann> If you think it is needed, then that is fine.
Mar 31 18:37:14 <holtmann> +       if (rlen != len * 2 || len < 2 || (response[len - 2] != 0x90 &&
Mar 31 18:37:14 <holtmann> +                               response[len - 2] != 0x91) ||
Mar 31 18:37:14 <holtmann> +                       (response[len - 2] == 0x90 && response[len - 1] != 0))
Mar 31 18:37:14 <holtmann> +               return;
Mar 31 18:37:19 <holtmann> And this is not how we do it.
Mar 31 18:37:35 <denkenz> Actually it is useful, consider CBS being powered off
Mar 31 18:37:37 <holtmann> Break this down to multiple if (failed) return statements.
Mar 31 18:37:46 <denkenz> It should remove the netreg watch
Mar 31 18:37:55 <holtmann> My brain produces a core dump ;)
Mar 31 18:38:30 <denkenz> lol, which one is that?
Mar 31 18:39:14 <holtmann> Last patch.
Mar 31 18:40:05 <denkenz> yeah 3 if statements would be better
Mar 31 18:42:47 <holtmann> Send an example to the mailing list.
Mar 31 18:43:32 <denkenz> balrog-k2n: So your sim_inserted just has one flaw, you're starting the sim init procedure before the sim atom has registered
Mar 31 18:43:54 <denkenz> It works by magic with AT one, but in general that is bad ;)
Mar 31 18:47:40 <akiniemi> denkenz: That USSD patch looks ok to me
Mar 31 18:48:43 <denkenz> ok, then it is going in, I feel it'll make the UI interaction on transactions a bit easier
Mar 31 18:50:50 <denkenz> We still have the problem that we don't indicate whether the transaction is over or not...
Mar 31 18:55:25 <akiniemi> denkenz: um, shouldn't there be a signal for idle state?
Mar 31 18:55:44 <denkenz> There is a signal, and in theory the signal is emitted before the method return
Mar 31 18:55:56 <denkenz> So with decent bindings there's no need to check for the property value
Mar 31 18:56:05 <denkenz> However, are UI writers that smart?
Mar 31 18:56:43 <akiniemi> denkenz: heh, you know I'm a big advocate of giving them even higher level "bindings" ;)
Mar 31 18:57:24 <denkenz> Even guys writing the higher-level bindings might not be that smart
Mar 31 18:58:14 <denkenz> Anyway, I leave it alone for now
Mar 31 19:19:56 <akiniemi> What does the hso driver expect to see in modem.conf?
Mar 31 19:20:15 <denkenz> hso driver works with udev
Mar 31 19:22:14 <akiniemi> What are those ControlPort and ApplicationPort for?
Mar 31 19:22:35 <balrog-k2n> for the idle state you only get a PropertyChanged, i agree this isn't really consistent
Mar 31 19:22:56 <denkenz> akiniemi: The HSO hardware is weird, unsolicited notifications only come on a particular port
Mar 31 19:23:49 <denkenz> akiniemi: Hence we have to give the atom(s) the right tty or they won't be useable
Mar 31 19:24:12 <akiniemi> denkenz: I can see five, which one is "right"?
Mar 31 19:24:53 <denkenz> akiniemi: I dunno, the one labeled Application I think, follow the logic in plugins/udev.c and plugins/hso.c
Mar 31 19:24:55 <holtmann> They have hso_type names in sysfs.
Mar 31 19:25:58 <denkenz> balrog-k2n: Yeah, so my thought was maybe we need an extra boolean argument on Respond/Initiate
Mar 31 19:26:12 <denkenz> balrog-k2n: But the solution might be uglier than just listening to the signal
Mar 31 19:32:34 <balrog-k2n> denkenz: you mean an extra return value?
Mar 31 19:32:56 <denkenz> sorry, yes, extra return argument
Mar 31 19:34:01 <balrog-k2n> this makes sense. i can add it, but i'm not sure it's so useful either
Mar 31 19:34:22 <denkenz> Yeah leave it for now
Mar 31 19:34:36 <balrog-k2n> okay
Mar 31 19:44:38 <akiniemi> Ooh, my Nokia Booklet 3G just sent an SMS :)
Mar 31 19:45:14 <denkenz> heh
Mar 31 19:56:52 <akiniemi> ...but I had to hard code the tty devices
Mar 31 19:57:08 <denkenz> poke around in plugins/ofono.rules
Mar 31 19:57:20 <denkenz> Maybe you just need to install the proper device id in there
Mar 31 19:58:14 <akiniemi> Nope, it's there already
Mar 31 20:00:07 <denkenz> Are ofono.rules installed?
Mar 31 20:00:20 <denkenz> Why is your device not being automagically recognized?
Mar 31 20:23:27 <denkenz> kristenc: How do I obtain the current tun device name for connect callback purposes?
Mar 31 20:51:30 <kristenc> denkenz: I don't have any way to do that.  I guess I should add that to the list of things we pass to the connect callback.
Mar 31 20:52:42 <denkenz> kristenc: will this work?
Mar 31 20:52:44 <denkenz> +       ppp->connect_cb(ppp, G_AT_PPP_CONNECT_SUCCESS,
Mar 31 20:52:45 <denkenz> +                       pppcp->ppp->net->if_name,
Mar 31 20:52:47 <denkenz> +                       ip[0] ? ip : NULL,
Mar 31 20:52:48 <denkenz> +                       dns1[0] ? dns1 : NULL,
Mar 31 20:52:50 <denkenz> +                       dns2[0] ? dns2 : NULL,
Mar 31 20:53:54 <kristenc> denkenz: I was actually thinking that we should create a structure to contain all the things we pass in, that way if we have to expand it over time we don't have to change the api.
Mar 31 20:56:00 <kristenc> plus I guess I just dislike functions with a lot of parameters, but that's a personal taste thing and not that important.
Mar 31 20:56:32 <denkenz> In general I agree, but gratuitious structures are evil as well
Mar 31 20:57:52 <denkenz> btw, the ppp argument isn't needed, and the dns arguments can be sent as an array
Mar 31 20:58:00 <denkenz> So that function becomes fairly small argument wise
Mar 31 21:01:28 <kristenc> I would keep the ppp argument just in case we need to do something in the future that requires access to that structure.  I think my cutoff for when I start thinking there are too many parameters is about 5, so in my mind this wouldn't be gratuitous.  But it's not worth arguing about to me, however you want to patch it is fine by me.
Mar 31 21:02:38 <kristenc> functionally what you have above works just fine.
Mar 31 21:04:03 <denkenz> point is, ppp argument can be kept in user_data
Mar 31 21:04:44 <denkenz> And we'd be 4 arguments, however, the original question still stands
Mar 31 21:04:56 <denkenz> will this work? +                       pppcp->ppp->net->if_name,
Mar 31 22:01:01 <holtmann> denkenz: So the if_name that comes when creating the tun/tap device should be "ppp%d". The actual created interface needs to re-read from the kernel.
Mar 31 22:01:49 <holtmann> If TUNSETIFF is doing this already, we are good.
Mar 31 22:01:59 <denkenz> holtmann: Well is it?
Mar 31 22:02:19 <holtmann> I don't know.
Mar 31 22:02:32 <holtmann> print the name and we see.
Mar 31 22:03:22 <kristen> it isn't - it is just using the default of tunX
Mar 31 22:06:25 <denkenz> holtmann: btw, how do I make the cdc-wdmX devices visible to udev plugin?
Mar 31 22:07:02 <holtmann> We need to check for proper sysfs subsystem attributes.
Mar 31 22:07:33 <denkenz> yeah, how
Mar 31 22:07:46 <denkenz> Extremely helpful today :)
Mar 31 22:08:53 <denkenz> e.g. udev_enumerate_add_match_subsystem(enumerate, "net")
Mar 31 22:09:04 <denkenz> I need to add something like that for the cdc ones
Mar 31 22:16:10 <denkenz> kristenc: So I get this on MBM
Mar 31 22:16:12 <denkenz> ofonod[2894]: Data: > AT+CGDCONT=1,"IP","internet2.voicestream.com"\r
Mar 31 22:16:13 <denkenz> ofonod[2894]: Data: < AT+CGDCONT=1,"IP","internet2.voicestream.com"\r
Mar 31 22:16:15 <denkenz> ofonod[2894]: Data: < \r\nOK\r\n
Mar 31 22:16:16 <denkenz> ofonod[2894]: Data: > AT+CGDATA="PPP",1\r
Mar 31 22:16:18 <denkenz> ofonod[2894]: Data: < AT+CGDATA="PPP",1\r
Mar 31 22:16:19 <denkenz> ofonod[2894]: Data: < \r\nCONNECT\r\n~\777}#\700!}!}!} }9}#}%\702#}%}(}"}'}"}"}&} } } } }%}&#\701Nny5~
Mar 31 22:16:21 <denkenz> lcp: pppcp_open_event: current state 0:INITIAL
Mar 31 22:16:22 <denkenz> lcp: pppcp_up_event: current state 1:STARTING
Mar 31 22:16:24 <denkenz> lcp: pppcp_initialize_restart_count: current state 1:STARTING
Mar 31 22:16:25 <denkenz> lcp: pppcp_send_configure_request: current state 1:STARTING
Mar 31 22:16:27 <denkenz> lcp: pppcp_process_configure_request: current state 6:REQSENT
Mar 31 22:16:28 <denkenz> lcp: pppcp_rcr_plus_event: current state 6:REQSENT
Mar 31 22:16:30 <denkenz> lcp: pppcp_send_configure_ack: current state 6:REQSENT
Mar 31 22:16:31 <denkenz> lcp: pppcp_process_configure_ack: current state 8:ACKSENT
Mar 31 22:16:32 <denkenz> oops -- found acked option 2 we didn't request
Mar 31 22:16:34 <denkenz> lcp: pppcp_rca_event: current state 8:ACKSENT
Mar 31 22:16:36 <denkenz> lcp: pppcp_initialize_restart_count: current state 8:ACKSENT
Mar 31 22:16:37 <denkenz> error -1 setting ifr
Mar 31 22:16:39 <denkenz> ofonod[2894]: Modem: < \r\n+CIEV: 2,4\r\n
Mar 31 22:16:40 <denkenz> ofonod[2894]: Modem: < \r\n+CIEV: 2,3\r\n
Mar 31 22:16:42 <denkenz> ofonod[2894]: Modem: < \r\n+CIEV: 2,4\r\n
Mar 31 22:16:43 <denkenz> ipcp: pppcp_process_configure_request: current state 0:INITIAL
Mar 31 22:16:45 <denkenz> ipcp: pppcp_rcr_minus_event: current state 0:INITIAL
Mar 31 22:16:46 <denkenz> Illegal event 7 while in state 0
Mar 31 22:16:48 <denkenz> Then the above 3 lines just repeat
Mar 31 22:47:54 <denkenz> holtmann, kristenc, kristen: http://pastebin.com/pejSbQ9e
Mar 31 22:49:21 <holtmann> denkenz: I need to look into it myself. Don't remember all udev details.
Mar 31 22:50:24 <holtmann> denkenz: Btw. there is a TUNGETIFF ioctl. That should give you the assigned ifname.
Mar 31 22:50:26 <denkenz> ppp code is so bad it causes valgrind to crash :P
Mar 31 22:55:05 <kristen> denkenz: is the ifr error problem consistent?  I don't see it in your pastebin
Mar 31 22:55:23 <denkenz> kristen: The ifr error is from me not running root, ignore that
Mar 31 22:55:52 <kristen> denkenz: ok
Mar 31 22:55:59 <denkenz> kristen: Though the code should actually handle that and give up
Mar 31 23:00:12 <kristen> denkenz: looks like we are getting some options that we either want to reject, or they are unacceptable.  I don't have pppcp_trace() on send_configure_nak, but I can see by the state that is what we are doing.  So we are in this circle where we are getting configure requests from the modem, but never getting anything that meets our criteria.  If you had an strace of the session we could see what it was sending us.
Mar 31 23:00:41 <kristen> On the todo list still is to implement the max-failure counter.
Mar 31 23:01:02 <kristen> (which would tell us to just give up when we fail to negotiate after some number of tries)
Mar 31 23:02:07 <holtmann> kristen: Can you add code that just prints the option name and number.
Mar 31 23:02:25 <holtmann> We really wanna have debug logs for what the modem tries to negotiate.
Mar 31 23:02:41 <holtmann> A lot of the options we might just simply be able to accept.
Mar 31 23:03:14 <denkenz> i can submit most of my code if anyone wants it
Mar 31 23:03:51 <kristen> in this case it isn't needed - if we had found an option we didn't recognize, we would have seen an error printed since we already do have debugging code that prints that out.
Mar 31 23:04:07 <kristen> so the problem is that we aren't getting a response to a config option that we need.
Mar 31 23:04:59 <holtmann> kristen: I prefer that we just log all the options we are getting.
Mar 31 23:05:07 <holtmann> We wanna see what the modem wants from us.
Mar 31 23:05:27 <holtmann> Every modem might be different and I wanna have proper traces of this. Going through binary straces is just waste of time.
Mar 31 23:05:39 <holtmann> denkenz: It won't hurt. Just commit the code.
Mar 31 23:06:03 <kristen> holtmann: not a waste of time IMO, I like strace logs.  I can put this on my todo list.
Mar 31 23:07:51 <holtmann> Lets do better debug output first. We are starting to test this with various modem. And I wanna have other people testing this as well. So this is important.
Mar 31 23:08:29 <kristen> perhaps you should do the patch?  you seem to have a pretty clear idea what you want.
Mar 31 23:15:43 <holtmann> kristen: I just want to have the option name and number in the debug output. So we can easily check what the modem wants from us.
Mar 31 23:15:53 <holtmann> Going through strace all the time is not feasible.
Mar 31 23:30:57 <denkenz> So I have my changes working on HSO hardware and I even get the tun device properly identified
Mar 31 23:32:58 <denkenz> e.g. I don't get the same crash as on mbm
Mar 31 23:49:55 <holtmann> denkenz: Then go ahead and push the changes.
Mar 31 23:51:06 <denkenz> pushed
Mar 31 23:51:22 <denkenz> -       gc = ofono_gprs_context_create(modem, 0, "mbm", data->modem_port);
Mar 31 23:51:24 <denkenz> +       gc = ofono_gprs_context_create(modem, 0, "atmodem", data->data_port);
Mar 31 23:51:36 <denkenz> holtmann: That is all that is needed to have spectacular crashes on mbm
Apr 01 00:27:05 <zhenhua> denkenz: any thought about how to integrate gatserver with core?
Apr 01 00:48:19 <denkenz> zhenhua: So lets start by adding CreateEmulator() and DestroyEmulator() methods on the Modem interface
Apr 01 00:48:46 <denkenz> zhenhua: Then maybe an emulator watch type functions where each atom can register handlers it supports
Apr 01 00:49:23 <zhenhua> modem interface is the dbus interface like /atgen0, right?
Apr 01 00:49:38 <denkenz> yeah
Apr 01 00:50:00 <denkenz> see how feasible that is for say HFP
Apr 01 00:50:00 <zhenhua> so can you share what you are intended to do next?
Apr 01 00:50:25 <denkenz> Honestly I don't know, but integrating emulation into the core is the logical next step
Apr 01 00:50:58 <denkenz> Or maybe the emulator should be its own atom
Apr 01 00:51:15 <zhenhua> i don't know, that's why i am asking u.
Apr 01 00:51:33 <zhenhua> at least, we should make some prototypes before check into upstream, i think.
Apr 01 00:51:37 <denkenz> We're entering the wide unknown here, we have to try a few things
Apr 01 00:51:54 <denkenz> So yeah, prototype a couple of approaches
Apr 01 00:52:17 <zhenhua> it may not enough to register a simple atom watch, i guess.
Apr 01 00:53:49 <denkenz> it could work actually, we have multiple history atoms too
Apr 01 00:54:04 <denkenz> so emulator as an atom wouldn't really be out of the question
Apr 01 00:54:36 <zhenhua> okay. hope it works, i will dig into a bit deeper.
Apr 01 00:55:24 <denkenz> yep, I need time to think about it more, it might also be a good idea to create a few unit tests for gatserver
Apr 01 00:56:42 <zhenhua> connect to test-server and then talk with server?
Apr 01 00:57:08 <zhenhua> i means act as client side.
Apr 01 00:57:46 <denkenz> yeah, I'm looking for any corner cases we might have missed
Apr 01 00:58:53 <zhenhua> yeah, we have, like AT+CMGS, i plan to write cases for it.
Apr 01 00:59:17 <denkenz> That is also something that is needed, the whole prompt part
Apr 01 01:00:01 <zhenhua> i know, we don't handle such cases right now
Apr 01 01:00:52 <zhenhua> actually, i think modem emulator is quite similar with phonesim
Apr 01 01:02:09 <denkenz> That is another thing, we might have to re-implement phonesim using GAtServer
Apr 01 01:02:46 <denkenz> For now poke around in the core and see if you can come up with any clever solutions
Apr 01 01:02:47 <zhenhua> err... is it in our POR?
Apr 01 01:02:51 <denkenz> Not yet ;)
Apr 01 01:03:00 <zhenhua> okay
**** ENDING LOGGING AT Thu Apr 01 02:59:57 2010