**** BEGIN LOGGING AT Mon Mar 29 02:59:57 2010 Mar 29 16:08:16 holtmann: Gah, so what is this style rule for indenting function arguments on the next line? Mar 29 16:08:34 There is none. Mar 29 16:08:38 holtmann: I indent right after the opening brace, but you do much more Mar 29 16:09:03 I tend to push them to the end of the line. Either way is fine. Mar 29 16:09:19 Heh Mar 29 16:09:41 Lets agree on something, otherwise I might re-indent it the other way 6 months from now Mar 29 16:11:47 denkenz: I just sent several patches to ML using my gmail account (gyagp0@gmail.com), due that I don't have vpn. But I got the message from ofono: "Your message to ofono awaits moderator approval". Can you see the patches? Mar 29 16:12:21 No, you need to register to the mailing list first Mar 29 16:13:08 ok. Mar 29 16:13:23 denkenz: The reason why I indent to the end of the line is that for me it becomes more readable this way. However I am not going to push anybody towards this. It is a pure personal thing. Minimum is 2 tabs. Everything is up to the developer. Mar 29 16:13:46 I normally don't re-indent them. Just got side tracked in the gsmdial code. Mar 29 16:13:59 denkenz: Funny story from the GACT exercise. Mar 29 16:14:19 So with the Novatel cards, if you use *99* magic. The PPP setup works fine. Mar 29 16:14:41 When you use CGDATA, then it says CONNECT and PPP starts, but it never finishes. Mar 29 16:15:10 If you then go back to *99* it doesn't work either anymore. You need to unplug the dongle/ Mar 29 16:16:03 ygu5_home: You can register multiple email addresses with the mailing list and set them to NO MAIL. That way, you get only one copy, but can post from multiple addresses. Mar 29 16:16:15 lol Mar 29 16:16:49 holtmann: Thank you, I'm trying to do so :) Mar 29 16:17:10 denkenz: I haven't debugged it yet. Mar 29 16:17:24 So is it a case of different ppp options being negotiated? Mar 29 16:17:28 Maybe it needs CGAUTO=0 or it is totally broken. Mar 29 16:18:10 Different options would be fine, but why does a subsequent *99* doesn't work. Seems the internal state machine in the firmware gets screwed up. Mar 29 16:18:32 Once you tried CGDATA, the hardware is not usable anymore. Mar 29 16:18:38 However PPP does start. Mar 29 16:18:45 That's awesome Mar 29 16:19:24 At some point we should just blog about this and embarass the hell out of these companies Mar 29 16:21:24 The problem seems to be all the code path that are normally not used or error path. They are full of bugs. Mar 29 16:22:35 Yeah, who would ever follow the standard and issue a CGDATA anyway ;) Mar 29 16:22:44 That's unpossible Mar 29 16:23:27 Only thing I haven't tried is CGAUTO=0 setting. Mar 29 16:23:47 Almost no dongle in the world actually supports network-initiated context activation Mar 29 16:24:02 And those that do only support it in synthetic environment Mar 29 16:25:19 I think Kristen needs to debug this one. Mar 29 16:25:40 So in principle it looks good, but this is another stupid bug :( Mar 29 16:25:40 She doesn't have that hardware Mar 29 16:25:53 And the other problem is I can't find any Novatel cards to buy anywhere Mar 29 16:25:57 I might have to resort to Ebay Mar 29 16:26:13 The Sierra one should support CGDATA. So not using -l option might cause similar issues. Mar 29 16:26:40 They all have different firmware though Mar 29 16:27:23 True. I am just saying with need to test with -l and without. Mar 29 16:27:39 Until yesterday it just wasn't hooked up and nobody tried. Mar 29 16:27:42 Of course, especially since the default behavior should be CGDATA Mar 29 16:27:48 *99 is a hack Mar 29 16:28:02 ModemManager is using that as default ;) Mar 29 16:29:09 Shrug, it very well might be necessary for some cards Mar 29 16:29:32 However, the handset modems are usually much better tested Mar 29 17:33:22 holtmann: So mbm should now correctly report technology in netreg, give it a spin, need to do the same for hso now Mar 29 17:34:46 + if ((status == 1 || status == 5) && (tech == -1)) Mar 29 17:34:46 + tech = nd->tech; Mar 29 17:34:58 The braces around tech == -1 are not needed ;) Mar 29 17:35:16 Oops, fixing Mar 29 17:35:32 Not that big of a deal actually. Mar 29 17:36:44 And personally I would have done it the other way around. Like tech not set, and condition, then set tech. Mar 29 17:36:54 However that is really a personal choice. Mar 29 17:37:50 self-optimizing compiler in me, the status should be checked first since tech == -1 almost always :) Mar 29 17:38:52 I figured that much. Mar 29 18:11:04 - while (p->channel && (p->read_so_far < len)) { Mar 29 18:11:08 + while (p->channel && p->parse_ready && (p->read_so_far < len)) { Mar 29 18:11:23 No braces around the p->read_so_far < len check please. Mar 29 18:11:42 which one is that one? Mar 29 18:12:56 Patches on the mailing list for AT server. However it is already wrong in the current tree. Mar 29 18:13:16 I'm actually fine with it that way Mar 29 18:13:58 Still a bad idea. Everybody needs to understand which operators have priority over the other one. Mar 29 18:14:38 Sure, but that one is more readable with parens Mar 29 18:15:21 Then leave it like it. However it should be clear to everybody that && and || is the lowest priority. Otherwise we end up in braces nightmare. Mar 29 19:15:04 holtmann: So the chap auth patch from Pekka looks good to me Mar 29 19:16:24 And I'm seriously scared at that gsize cast Mar 29 19:22:20 Ah never mind, that is an inout parameter Mar 29 19:26:30 Patch looks good. However I am not sure why it is this way in the first place. Let Kristen comment on it. Mar 29 19:26:50 Registered to GPRS network, roaming=false Mar 29 19:26:50 : > AT+CGDCONT=1,"IP","internet.com"\r Mar 29 19:26:51 : < \r\nERROR\r\n Mar 29 19:26:51 Unable to define context Mar 29 19:27:02 That is with a Huawei modem when the context is left active. Mar 29 19:28:14 Only happens if you use AT+CGACT=1,1 Mar 29 19:28:38 And same issue with CGDATA. Mar 29 19:28:40 : > AT+CGDATA="PPP",1\r Mar 29 19:28:41 : < \r\nCONNECT\r\n Mar 29 19:28:41 oops -- found acked option 2 we didn't request Mar 29 19:28:44 And then it is stuck. Mar 29 19:29:04 Says "ipcp finished" and we get nowhere. Mar 29 19:30:57 I have to manually do AT-CGACT=0,1 to allow another CGDCONT call. Mar 29 19:31:43 [14:27] That is with a Huawei modem when the context is left active. Mar 29 19:31:48 What exactly does that mean? Mar 29 19:32:09 The ctrl-c and PPP code doesn't work. So PPP link gets terminated and context is still active. Mar 29 19:32:36 With *99* it auto-deactivates the context. Mar 29 19:32:57 And the Huawei modem doesn't let you modify CGDCONT while you have an active context. Which makes sense. Mar 29 19:34:55 I can switch between *99* and CGDATA mode without any problems with this Modem. Of course the CGACT=0,1 handling is missing in the cleanup. Mar 29 19:35:38 Seems like proper behavior. However our current PPP code is not able to handle CGDATA :( Mar 29 19:37:05 Kristen never added it, so that's perfectly understandable Mar 29 19:38:15 That part is fine. Still don't know why CGDATA and PPP fails. Mar 29 19:39:10 Probably because Qualcomm never made it work ): Mar 29 19:39:56 Huawei/Novatel/Sierra/Option are all Qualcomm firmware Mar 29 19:40:08 I don't think so. I think it is our mistake in the PPP code. Mar 29 19:40:52 We need better debugging support for the LCP stuff. It is hard to debug this right now. And using strace all the time is not an option. Mar 29 19:40:55 Then I go back to my earlier question, are the lcp options different between CGDATA and *99 Mar 29 19:41:17 They shouldn't be different. Only think I can think about is the auth stuff. Mar 29 19:41:40 So why is CGDATA not working, while *99 is ;) Mar 29 19:44:07 I assume some PPP stuff. I have to add some debugging code in ppp_cp.c that does something proper. The pppcp_trace() thing is useless. Mar 29 19:45:09 If you see Kristen coming online, ask her to test without the -l option. I will be offline for the rest of the day., Mar 29 21:13:00 kristenc: Please review the patch on the mailing list for the digest. Mar 29 21:13:20 And start testing without the -l option. There is something weird going on when using CGDATA command. Mar 29 21:21:19 kristenc: http://logs.nslu2-linux.org/livelogs/ofono.txt in case you missed the discussion Mar 30 00:58:20 kristenc: ping Mar 30 01:21:23 denkenz: ping Mar 30 01:21:47 denkenz: i am fine with most of your comments. Mar 30 01:23:11 denkenz: but i want to update read_pos before call at_command_notify, that's why i move send_final into the parse_extended and parse_basic. Mar 30 01:25:30 denkenz: for variable name, i still perfer read_line or cur_line, because it is widely used in parse_line and send_final. we can do a simple cast if meet REPEAT_LAST. what do you think? Mar 30 01:34:53 zhenhua1: I still prefer the controlling logic to be in server_parse_line as opposed to thrown around 5 functions Mar 30 01:35:38 zhenhua1: Maybe it isn't possible, but see what you can come up with Mar 30 01:35:43 denkenz: but we need to update read_pos before call command notify, right? Mar 30 01:35:54 zhenhua1: Maybe, maybe not Mar 30 01:36:08 denkenz: because the send_final may be called before read_pos is updated otherwise Mar 30 01:36:36 zhenhua1: Again, with your current setup sure, but maybe the setup should be re-thought Mar 30 01:37:26 any good idea? maybe we should reorg the code? Mar 30 01:38:45 zhenhua1: One idea is to simply set some boolean variables in parse_line / send_final' Mar 30 01:39:09 To figure out whether send_final was called sync or async Mar 30 01:40:22 yes. so? what it is for? Mar 30 01:43:04 That way you can localize the logic to two functions, as opposed to 4 or 5 Mar 30 01:44:57 Again, your way might end up better, you have to try and see Mar 30 01:46:15 zhenhua1: Also, feel free to just throw away bytes arriving on the channel when you're processing a request Mar 30 01:47:02 zhenhua1: And for kicks I'd like to know how other modems handle it :) Mar 30 01:47:12 throw away? so we don't put them into read buffer. right? Mar 30 01:47:54 i only have qt-extended code, and i cann't compile it due to qmake error. Mar 30 01:48:33 zhenhua1: Yeah throw away, abortability is an optional feature, so we can ignore it for now Mar 30 01:48:38 i really think 4-5 functions are better than 2. Mar 30 01:48:54 okay, i will change the code Mar 30 01:48:55 zhenhua1: Trust me, they're not ;) Mar 30 01:49:20 and this 2 function will be very big Mar 30 01:50:38 merge send_result and send_final, for that one, i am fine. Mar 30 01:51:04 how to merge other 4 functions? Mar 30 01:52:07 send_ext_final doesn't need to invoke parse_line, because it is an error Mar 30 01:53:16 It might be fine as is Mar 30 01:54:11 how about rename read_line to cur_line, so be consistent with cur_pos Mar 30 01:58:34 last line would make more sense to me, but that's fine Mar 30 02:03:21 heh, so i plan to use cur_line and cur_pos. hope it's fine. Mar 30 02:04:16 or last_line and last_pos? Mar 30 02:17:34 zhenhua1: I already told you my preference Mar 30 02:17:56 zhenhua1: If you think you know better, try it, but don't be surprised if I change it ;) Mar 30 02:18:13 sure. that's fine. ;-) Mar 30 02:33:51 denkenz: adding a flag is not helpful I think. you need to move cursor to point to the next command in the command line. i don't find better way now. :-( Mar 30 02:34:51 Think some more ;) Mar 30 02:44:28 denkenz: one idea from my colleague is to use g_main_loop_run to handle such async function call Mar 30 02:46:58 denkenz: yes, it should be better, let me draft some code. ;-) Mar 30 02:48:38 don't even go there ;) Mar 30 02:49:09 re-entrant event loops are about as evil as it gets Mar 30 02:50:09 errr. okay, let me think others. Mar 30 02:50:38 i cannot simply set a flag and use stupid while loop to wait. Mar 30 02:51:19 nah, do something like server->in_parse_line = TRUE; parse_command .... send_final -> if (in_parse_line) ... Mar 30 02:51:42 denkenz: When bootstrap-configure the latest code, I met the following error: Mar 30 02:51:50 checking for CAPNG... no Mar 30 02:51:50 configure: error: Capabilities library is required Mar 30 02:52:04 install capng devel Mar 30 02:52:19 holtmann added this to latest configure today in preparation for something Mar 30 02:53:08 Is this for ppp? And is this necessary? Mar 30 02:53:27 No, it is for privilege dropping and yes it will be necessary **** ENDING LOGGING AT Tue Mar 30 02:59:57 2010