**** BEGIN LOGGING AT Tue Mar 23 02:59:57 2010
**** BEGIN LOGGING AT Tue Mar 23 11:58:15 2010
Mar 23 16:37:21 <denkenz> holtmann: Wtf is up with the MCC:MNC operator name? Is some modem so stupid it doesn't report proper COPS name?
Mar 23 16:39:18 <holtmann> We don't wanna have ConnMan work around it. oFono should always export an operator name.
Mar 23 16:39:34 <holtmann> If it has to construct it like MCC:MNC, then that is fine, too.
Mar 23 16:40:06 <denkenz> Agreed, but I'm surprised that the COPS name is not being reported
Mar 23 16:42:01 <holtmann> Modem is broken.
Mar 23 16:43:05 <denkenz> Has anyone seen if it is a character set issue or maybe another command needs to be used?
Mar 23 16:43:59 <holtmann> Martin can answer that for you I assume.
Mar 23 16:45:54 <denkenz> holtmann: Also, I think we're ready for the .20 tag
Mar 23 16:49:05 <holtmann> We have until tomorrow, but I can build the release today if that helps.
Mar 23 16:49:44 <denkenz> up to you, we can include the mcc/mnc fix if it is important
Mar 23 17:49:22 <holtmann> denkenz: Yes. It is important to include that one. Otherwise we have a broken UI right now.
Mar 23 17:49:34 <holtmann> And I don't want the UI designers try to workaround it.
Mar 23 18:39:03 <denkenz> holtmann: Do you have this hardware to test or only martin has it?
Mar 23 18:39:12 <denkenz> holtmann: I have a prototype patch
Mar 23 18:41:45 <holtmann> Only Martin has it.
Mar 23 18:43:44 <denkenz> ok then I send it to him for testing
Mar 23 18:52:48 <kristenc> denkenz: I have to disagree with you that you should not be posting changes to the list.  Clearly even simple changes can cause problems.  I'd reply to the list with this to have it be recorded publically, but clearly you guys aren't interested in hearing what I have to say.  I can disagree with you guys and keep working on this, but I'm not happy about it.
Mar 23 18:54:15 <kristenc> I've never worked on any other project where commits where not sent to the mailing list for review.  all commits.
Mar 23 18:54:20 <kristenc> seems like very bad practice.
Mar 23 18:55:30 <denkenz> kristenc: I see no point for commit messages, git does the same
Mar 23 18:55:47 <denkenz> If the maintainer screwed something up, then send a fix, no big deal
Mar 23 18:55:52 <denkenz> We are human too
Mar 23 18:56:19 <kristenc> denkenz: exactly my point.  I should not have to monitor the repo to see if some of my code got broken.
Mar 23 18:56:42 <kristenc> and with the one line commit messages, it's not easy to just scan the log to see what happened.
Mar 23 18:57:20 <kristenc> for example, on the mistake I caught, I think the commit message just said something like remove pointless something or another.
Mar 23 18:57:43 <denkenz> My point is this is not 'your' code once it is upstream
Mar 23 18:57:57 <kristenc> for the same reason we don't do attachments to the mailing list.  It isn't workabout to assume we're all monitoring the repo.
Mar 23 18:58:30 <kristenc> denkenz: of course not, and I am obviously not claiming that.
Mar 23 18:58:48 <denkenz> [13:56] <kristenc> denkenz: exactly my point.  I should not have to monitor the repo to see if some of my code got broken.
Mar 23 18:59:24 <denkenz> If someone broke something, fix it
Mar 23 18:59:27 <kristenc> denkenz: by "my" I mean code I am going to be working on.
Mar 23 18:59:58 <kristenc> denkenz: and how much time would you like me to waste trying to figure out what went wrong because you guys won't accept public review of your patches?
Mar 23 19:00:25 <holtmann> kristenc: If you monitor commits, then you can subscribe to the RSS feed from gitweb from git.kernel.org.
Mar 23 19:00:29 <kristenc> I would have moved on to the next phase of development assuming everything still worked.
Mar 23 19:00:53 <kristenc> holtmann: I don't want to monitor commits.  we have a mailing list.  it is there for patches to be reviewed on.
Mar 23 19:01:20 <denkenz> Again, refactoring commits are not going there, its pointless
Mar 23 19:01:50 <denkenz> I can generate 20-30 on good days, and most nobody is going to understand them anyway
Mar 23 19:02:20 <kristenc> again, you are missing the obvious problem, which was that it was not pointless, because had you posted that patch for review, which you assumed was simple, somebody might have noticed the problem.
Mar 23 19:02:25 <holtmann> kristenc: Let me make this clear. Your patches broke the build for our PRC people.
Mar 23 19:03:10 <kristenc> holtmann: I am not interested in discussing whether or not their was code that needed to be patched.  I am interested in discussing whether it is good practice in an open source project to commit without review.
Mar 23 19:03:17 <holtmann> kristenc: And we expect you to monitor the upstream GIT tree and always work against that tree. If we have to do changes in the code you currently work, you need to look into that.
Mar 23 19:03:56 <holtmann> And yes, there will be cases where I and Denis to bad changes. That happens.
Mar 23 19:04:23 <kristenc> holtmann: patching against latest tree, yep, perfectly reasonable.  expecting me to monitor the git repo, nope.
Mar 23 19:04:51 <kristenc> holtmann: of course you will make bad changes.  That is expected and normal.  This is my point.
Mar 23 19:04:56 <denkenz> shrug, I don't see why not
Mar 23 19:05:04 <denkenz> not like you need to monitor the entire tree
Mar 23 19:05:08 <kristenc> this is why it's important to have an open review.
Mar 23 19:07:40 <kristenc> we have a difference of opinion.  
Mar 23 19:09:57 <holtmann> kristenc: We expect you to monitor the tree and see if changes happen to code that you are currently working on.
Mar 23 19:10:15 <holtmann> We might apply patches from other people where these thing slip through.
Mar 23 19:10:48 <holtmann> I have re-read the patch that broke this 5 times, before I committed it. I still missed the logic that you implied with the variable initialization to NULL.
Mar 23 19:11:05 <holtmann> So this happens. And I bet you it will happen again, even with public review.
Mar 23 19:12:11 <kristenc> holtmann: I agree it will happen again.  Please don't think that I'm implying that I think you were an idiot for breaking the code.  I don't think that!  I'm trying to convince you guys that public review gives you the best opportunity for catching our very human errors.
Mar 23 19:13:12 <holtmann> So we have to ways here, we either make you redo the whole code until we are 100% happy. Or we start working on it as a collaboration.
Mar 23 19:13:24 <kristenc> as ofono grows in developers and scale, it seems that this very well established model is the best for the long term.
Mar 23 19:13:40 <holtmann> The first one will happen from now on since the basic support is merged.
Mar 23 19:14:15 <holtmann> I don't want big massive patches after people have been working on it for 12 month. They are impossible to review and nobody can follow them.
Mar 23 19:14:28 <kristenc> holtmann: obviously I'm not saying we don't collaborate.  Again, all I am saying is that we should have all patches sent to the mailing list so we get as many eyes as possible on them.  Even if you think they are simple, they could benefit from review.
Mar 23 19:15:17 <holtmann> It will not happen for cleanup patches that Denis and I have to do on a regular basis. It is just not practical.
Mar 23 19:15:33 <kristenc> holtmann: yes, that is understood by everyone.  that is not what I am advocating either.
Mar 23 19:16:05 <denkenz> What are you advocating?
Mar 23 19:16:16 <kristenc> when I was a kernel maintainer for my subsystem, all cleanup patches when to the list.  and if it touched a part of code that I knew somebody actively worked in, I cc:d them directly.
Mar 23 19:16:32 <kristenc> all other kernel maintainers I worked with (gregkh, jgarzik) had the same policy.
Mar 23 19:16:35 <kristenc> it worked well for us.
Mar 23 19:16:45 <kristenc> it prevented mistakes many times.
Mar 23 19:17:22 <kristenc> denkenz: <kristenc> holtmann: I agree it will happen again.  Please don't think that I'm implying that I think you were an idiot for breaking the code.  I don't think that!  I'm trying to convince you guys that public review gives you the best opportunity for catching our very human errors.
Mar 23 19:17:22 <kristenc> <kristenc> as ofono grows in developers and scale, it seems that this very well established model is the best for the long term.
Mar 23 19:17:41 <denkenz> We don't have anyone upstream though?
Mar 23 19:17:53 <denkenz> Think of us as Linus
Mar 23 19:18:31 <kristenc> patches go to a collective.  upstream has nothing to do with it.
Mar 23 19:18:53 <lpotter> I personally dont care about all the patches, and dont care to see patch spam :)
Mar 23 19:19:19 <kristenc> hit the delete button.
Mar 23 19:20:07 <lpotter> more like shift-move delete
Mar 23 19:20:23 <denkenz> The development model you describe makes sense for linux kernel, for oFono it does not right now
Mar 23 19:20:29 <denkenz> maybe we scale to that point, but not right now
Mar 23 19:20:56 <kristenc> it's ok, I can see you are not convinced by my arguments.  I can keep working with your system, I just feel obligated to point out what i feel is not the best process.  I can get back to work now
Mar 23 19:24:54 <holtmann> kristenc: I review Denis changes every day after I pull ofono.git. And I assume he does the same with my changes.
Mar 23 19:25:23 <holtmann> That is already a lot of workload that we do besides patch review on the mailing list. Especially Denis has a big workload with STK patches already.
Mar 23 19:26:09 <holtmann> We are not making our life more complicated by sending cleanup patches to the mailing list. That model doesn't scale right now until we have 10 more people working on it full time.
Mar 23 19:26:19 * kristenc is done discussing this.
Mar 23 19:27:13 <kristenc> don't worry, I'm in "disagree and commit" mode.
Mar 23 21:26:39 <denkenz> holtmann: remind me
Mar 23 21:26:41 <denkenz> +       SS_GSM_ALL_FORWARDINGS = 002,
Mar 23 21:26:42 <denkenz> +       SS_GSM_ALL_COND_FORWARDINGS = 004,
Mar 23 21:26:46 <denkenz> Is that an octal syntax?
Mar 23 21:45:00 <holtmann> No. Just prefixed with zeros.
Mar 23 21:45:20 <holtmann> 002 == 2 ;)
Mar 23 21:53:29 <denkenz> You sure?
Mar 23 21:53:55 <denkenz> How do you prefix octal numbers then?
Mar 23 21:54:38 <denkenz> No, I'm right
Mar 23 21:54:44 <denkenz> anything preceded with 0 is octal
Mar 23 22:00:14 <holtmann> Hah. Funny. Learned something new again.
Mar 23 22:02:01 <jprvita> but 002 == 2 still holds, hehe
Mar 23 22:04:38 <denkenz> Yeah, but by luck rather than design
Mar 24 00:58:52 <zhenhua1> denkenz: ping
Mar 24 00:59:13 <denkenz> pong
Mar 24 01:00:13 <zhenhua1> for command 'D', all following char should belong the command line. i did look at the spec.
Mar 24 01:00:24 <zhenhua1> and i think we should include ';'
Mar 24 01:00:58 <zhenhua1> you need ';' to know whether it is a voice call or data call.
Mar 24 01:01:31 <zhenhua1> shall i change it to while (buf[i] != '\0' || buf[i] != ';')? 
Mar 24 01:01:35 <denkenz> The include ';' part is definitely true
Mar 24 01:01:47 <denkenz> But not the eat entire command line part
Mar 24 01:02:35 <zhenhua1> all chars followed after 'D' are considered part of the call, right?
Mar 24 01:03:03 <denkenz> up to and including ';'
Mar 24 01:03:08 <denkenz> But not afterwards
Mar 24 01:04:00 <zhenhua1> okay. so while (buf[i] != '\0' || buf[i] != ';') should be fine.
Mar 24 01:04:14 <denkenz> yep, but test to make sure
Mar 24 01:04:35 <zhenhua1> sure. i have test cases for D
Mar 24 01:04:58 <zhenhua1> and for  if (*prefix == 'A' || *prefix == 'Z')
Mar 24 01:04:58 <zhenhua1>                 return -1;
Mar 24 01:05:12 <zhenhua1> i need to return -1 to indicate to ignore the rest line.
Mar 24 01:05:26 <denkenz> How about you simply return strlen
Mar 24 01:05:32 <denkenz> or strlen + 1 or whatever
Mar 24 01:06:16 <zhenhua1> for 'D' the rest command line should be ignored, right?
Mar 24 01:06:24 <denkenz> nope
Mar 24 01:06:32 <zhenhua1> return strlen + 1 is not correct, i think
Mar 24 01:07:00 <zhenhua1> how about ATAAAA
Mar 24 01:07:21 <denkenz> That one is just a single ATA, rest ignored
Mar 24 01:07:38 <denkenz> ATD555;A is valid
Mar 24 01:07:44 <denkenz> Though probably doesn't make sense
Mar 24 01:07:52 <zhenhua1> the rest cause us to send a result error
Mar 24 01:08:12 <denkenz> However, ATD5555A is not ATD followed by ATA
Mar 24 01:08:35 <denkenz> The V250 syntax is fucked, but that's life
Mar 24 01:09:40 <denkenz> The set of characters for ATD is basically union from 27007 and V250, some are actually not present, like some alphabetical ones
Mar 24 01:09:41 <zhenhua1> can u explain how ATAAAA, the rest 'AAA' is ignored? if you return 3, the rest will send a final error result, right?
Mar 24 01:10:12 <denkenz> ATAAAA -> AAAA -> A + return 4
Mar 24 01:10:20 <denkenz> So the last 3 are ignored
Mar 24 01:12:00 <zhenhua1> i am stupid and don't think it's right.
Mar 24 01:12:34 <zhenhua1> we should return something like -1 in this case
Mar 24 01:12:55 <denkenz> Why?
Mar 24 01:13:05 <denkenz> Besides making the code more complicated
Mar 24 01:13:11 <zhenhua1>  if (consumed == 0) {
Mar 24 01:13:11 <zhenhua1>                           g_at_server_send_final(server,
Mar 24 01:13:11 <zhenhua1>                                                   G_AT_SERVER_RESULT_ERROR);
Mar 24 01:13:11 <zhenhua1>                           break;
Mar 24 01:13:11 <zhenhua1>                   }
Mar 24 01:13:31 <zhenhua1> because the rest AAA cause consumed is zero, right?
Mar 24 01:13:47 <zhenhua1> and it would send out final error code
Mar 24 01:13:58 <denkenz> Nope, AAA is never seen
Mar 24 01:14:16 <denkenz> We execute A and consume the entire command line
Mar 24 01:14:24 <denkenz> Only 1 result code is ever produced
Mar 24 01:14:29 <zhenhua1> got u, understand now
Mar 24 01:14:49 <zhenhua1> strlen(buf)
Mar 24 01:15:56 <zhenhua1> +			while (g_ascii_isdigit(buf[++i])) > +				prefix[i] = buf[i];  Is there some limit on the number?  Can we limit it to max int or something?  
Mar 24 01:16:03 <denkenz> Also, it sounds like ATE=? and ATE? are not really kosher according to V250
Mar 24 01:16:18 <zhenhua1> but MBM does support that
Mar 24 01:16:19 <denkenz> But if we want to support that form it is fine
Mar 24 01:16:40 <zhenhua1> Huawei EM770W reports error
Mar 24 01:16:56 <zhenhua1> but MBM returns like E: (0-1), E:1
Mar 24 01:17:17 <denkenz> Nod, but just make sure we don't get into trouble because of it
Mar 24 01:17:24 <zhenhua1> how to limit S parameters? I don't know.
Mar 24 01:17:26 <denkenz> Otherwise I'm fine doing it
Mar 24 01:17:39 <zhenhua1> we have S3, S10 xxx
Mar 24 01:17:52 <denkenz> I suggest limiting it to two digits
Mar 24 01:17:59 <denkenz> And leave it at that for now
Mar 24 01:18:18 <zhenhua1> okay
Mar 24 01:18:55 <zhenhua1> oh, that's a bug. prefix size should be 4 instead of 3
Mar 24 01:19:22 <denkenz> Actually nevermind, its just a char prefix
Mar 24 01:19:54 <zhenhua1> it can overflowed
Mar 24 01:20:01 <zhenhua1> s/can/can be
Mar 24 01:20:43 <denkenz> You can overflow the prefix you allocated, yes ;)
Mar 24 01:21:22 <denkenz> However, you're fine rejecting anything that is 3 digits long or longer
Mar 24 01:21:44 <denkenz> Nobody will care in the end, 99 S-registers is enough
Mar 24 01:21:54 <zhenhua1> yes
Mar 24 01:22:22 <denkenz> But again, remember to make a comment about that
Mar 24 01:22:51 <zhenhua1> okay
Mar 24 02:07:15 <ygu5_ubuntu> denkenz: Do you think all the test cases for stk in 102.384 need to be implemented?
Mar 24 02:07:50 <denkenz> Yes
Mar 24 02:12:03 <ygu5_ubuntu> ok. I just write a small perl program to help me format the data ;)
Mar 24 02:12:27 <denkenz> Don't forget 31.124 too
Mar 24 02:15:05 <ygu5_ubuntu> Sure.
Mar 24 02:17:50 <denkenz> martin_lab: Ok pushed that patch now, can you also run an operator scan just to make sure that part still works?
Mar 24 02:42:00 <holtmann> denkenz: So I assume we are ready for the release now?
Mar 24 02:53:30 <denkenz> holtmann: I'd say so
**** ENDING LOGGING AT Wed Mar 24 02:59:57 2010