**** BEGIN LOGGING AT Fri Jul 23 02:59:56 2010 Jul 23 05:12:55 holtmann: what is rfcomm control socket? shall i create this socket after adding DUN record to server? **** BEGIN LOGGING AT Fri Jul 23 15:16:14 2010 Jul 23 15:21:17 holtmann: [PATCH] watch: Free service data in service_reply is gdbus related, so you should have a look Jul 23 15:24:49 zhenhua: You need to create the RFCOMM server listen socket before adding SDP. If you use dynamic RFCOMM channel assignment that is the only way to know the channel. And otherwise it is good as well to avoid a race condition. Jul 23 16:33:42 balrog-k1n: I really question the need for EndSession, can't we send that automatically if the agent dies? Jul 23 16:33:49 balrog-k1n: Otherwise, what's the point? Jul 23 16:57:05 denkenz: first, if there's only a default agent if it unregisters and reregisters there's a chance that it misses some command Jul 23 16:57:48 and then the primary agent would also have to disconnect from the bus just to unregister because there's no function for that, and then when it makes a new selection would need to re-rexport the object Jul 23 16:58:27 I don't really buy this Jul 23 16:58:37 which part? Jul 23 16:58:38 The default agent will never go above 1 level down or so Jul 23 16:58:49 So GoBack is plenty sufficient Jul 23 16:59:35 And the SIM knows when the user session is over, so 99% of the time the UI shouldn't bother with sending a User Terminated Session Jul 23 16:59:40 The less confusion the better Jul 23 16:59:54 well, yeah, assuming the card uses the GoBack response the canonical way etc, it could be use perhaps Jul 23 17:00:01 i'm not sure if Set Up Call allows go back Jul 23 17:00:48 GoBack and Terminate are two different responses, i wouldn't like to tell a user we don't support one of them Jul 23 17:01:12 The user doesn't care Jul 23 17:01:55 And we do support both of them, just the user doesn't see it Jul 23 17:02:42 we do? if there's only the default agent and the card issues Display Text, how do you send a EndSession? Jul 23 17:03:30 As I said, in that scenario, GoBack is just as good Jul 23 17:03:36 In fact, it has the same meaning Jul 23 17:03:45 that's assuming a lot about the application on the card Jul 23 17:04:04 it may be have a totally different meaning Jul 23 17:04:07 Seriously, if the UI has to *somehow* know to send an EndSession vs GoBack based on the SIM App running Jul 23 17:04:13 Then we already lost Jul 23 17:04:13 If there is no agent, then we just go with a default. And nothing gets to the user. Jul 23 17:04:17 Pack up and go home ;) Jul 23 17:04:57 not based on the app running Jul 23 17:05:10 Then what, you expect the user to know? Jul 23 17:05:35 most phones have a red "hang up" button, and some kind of left arrow or backspace Jul 23 17:05:41 these are different things Jul 23 17:06:28 Again, I understand your concern, but right now I'm totally unconvinced Jul 23 17:06:32 the "hang up" button just terminates everything, the go back thing usually takes you to the screen you have been seeing before Jul 23 17:07:10 The EndSession is only truly useful in multi-level menus, these are only going to be an issue on user sessions Jul 23 17:07:11 beside adding things in the api that can be ignore id the UI writer doesn't want to use them does no harm whatsoever Jul 23 17:07:30 It does, because you have to explain it to them Jul 23 17:07:33 it doesn't have to be menus, you may be shown a series of text screens or something Jul 23 17:08:00 you have to explain to them only the important things, the other things they can ignore Jul 23 17:08:15 Sorry, I don't view it this way Jul 23 17:08:17 though i think this is actually important Jul 23 17:08:20 That is what makes most APIs suck Jul 23 17:08:38 I would really try go get away with not explicitly doing it. Lets try really hard to do this implicit. And only if can not make it work, then we re-think our approach. Jul 23 17:08:42 i think it makes the software suck if it doesn't support something others do :) Jul 23 17:09:20 My point here is that you have not established a definite need Jul 23 17:09:26 Until you do, leave it out Jul 23 17:09:32 This has been oFono's philosophy from day 1 Jul 23 17:10:57 well there are two things, one is what the popular applications on popular SIMs use and i don't know much about this because i've never really made use of any other apps, the other thing is supporting the spec Jul 23 17:11:33 and yet other thing is supporting what the vendors want (for example the Refresh thing, which we don't even have a way to test even if we implement it) Jul 23 17:11:58 You're proving my point for me, you do not know Jul 23 17:12:07 For example, Qtopia STK *never* uses EndSession Jul 23 17:12:15 and still worked fine with every Sim we threw at it Jul 23 17:12:47 balrog-k1n: The main idea is pretty simple. We start really small. And only when we see the need, then we extended/change it. Jul 23 17:13:07 We _do not_ know, so until we know, we leave it out Jul 23 17:13:19 There are plenty of things we had to go back and add after the fact Jul 23 17:13:23 Nothing is set in stone, but I am here with Denis. Lets try to not do it in the beginning. Adding something is easier than removing. Jul 23 17:13:36 But if we did it right away, we'd have to change way more Jul 23 17:13:49 And majority of the time we find we don't actually need it at all Jul 23 17:14:20 ok, i guess we can wait until someone complains about no way to issue a EndSession, but there are already a bunch of stull like callbarring that are highly hypothetical and we do them because they are part of GSM Jul 23 17:14:47 Btw, Nokia guys have already commented on how good our Call Barring API is Jul 23 17:14:54 And there *is* a way to test Refresh Jul 23 17:16:25 denkenz: is it ok to use UnregisterAgent for the primary agent though? Jul 23 17:17:01 Personally I'd leave it out until we need this case Jul 23 17:18:02 i think that's just making life difficult to the UI writer because they have to re export the agent every time they make a selection from the main menu Jul 23 17:18:46 in my client i'm using the same object for the default and the primary agent, Jul 23 17:19:05 so if i want to go back to main menu i have to unexport the agent object completely Jul 23 17:19:06 I don't know, but my feeling is that the SIM sends us an end session anyway Jul 23 17:19:10 thus unregistering both agents Jul 23 17:19:31 yes, it does, but only if we tell it we want to go back to main menu Jul 23 17:19:37 which is what EndSession is for Jul 23 17:20:05 you could emulate it with sending N GoBack() response but that's just ugly Jul 23 17:20:11 responses* Jul 23 17:20:52 Honestly, I'd like us to implement the proposal as it stands first Jul 23 17:21:08 As I said, I am all for this as well. Jul 23 17:21:10 Then have more people play with the API Jul 23 17:21:15 Then we decide Jul 23 17:21:32 Then in a few weeks we revisit this. Then we have more knowledge what works and what doesn't. Jul 23 17:22:06 I know you want this done and buried, but this is too tricky Jul 23 17:22:12 It requires time for everyone to absorb Jul 23 17:22:38 And it might be that the EndSession stuff has to be solved differently / better Jul 23 17:22:54 Until we have a prototype, everything is just conjecture Jul 23 17:23:19 what would the prototype need to look like? Jul 23 17:23:46 * balrog-k1n notes he has two working clients so kind of know what's good for the UI writer and what just complicates things Jul 23 17:24:06 Your web client is a start Jul 23 17:24:13 Ideally I'd like a minimal agent in python-qt Jul 23 17:24:32 That might be more accessible to myself / UI guys Jul 23 17:24:35 the other one i have is python text based Jul 23 17:25:10 fair enough, then clean it up and submit it too Jul 23 17:25:11 We might need some proving in either Qt or GTK+ that is something like how Android or the iPhone does it. Jul 23 17:25:15 however there are just things that can't be done with the current api, or could be done more simply Jul 23 17:25:48 Fair enough, you're speedy gonzales and way ahead of us Jul 23 17:26:43 So your job is to get us to the same or nearly same level ;) Jul 23 17:27:16 denkenz: imagine a really simple use case where you want a phone that works like the old nokia dumb-phones Jul 23 17:27:16 balrog-k1n: Maybe it is just good enough since the UI will never expose this anyway. Jul 23 17:27:58 my question is what should the UI call on the dbus api when the user presses the red "hang up" button while in a sim application Jul 23 17:28:21 Hangup button means end all apps Jul 23 17:28:32 So the STK App is ended, removed from bus and we get an end session automagically Jul 23 17:28:50 yes, but the UI immediately goes to the home screen, right? Jul 23 17:29:01 sure Jul 23 17:29:05 and it doesn't want to unregister the default agent, there's no reason to do that Jul 23 17:29:25 That is what I would assume. So we are somehow in a RequestSelection() callback. Just return an error. And then it will unregister the agent. Jul 23 17:29:27 and it would be a race condition because it might just miss a command whlie it unregisters and immediately registers again Jul 23 17:29:51 Hold on here for a second. Jul 23 17:29:54 holtmann: ok, that's what i'm doing currently Jul 23 17:29:55 Yeah, so again you're doing the argument of using a 'default' and 'user session' agent Jul 23 17:30:01 The same one Jul 23 17:30:07 i'm returning org.ofono.Error.EndSession Jul 23 17:30:19 I still question that use case ;) Jul 23 17:30:45 denkenz: ok, i thought you had suggested using the same agent obejct for both yesterday :) Jul 23 17:30:56 denkenz: I was assuming a method call, but an error is just fine. Any error will actually do. Jul 23 17:31:08 I suggested many things, doesn't mean half of them were right ;) Jul 23 17:31:23 balrog-k1n: Using the same object is potentially possible. Up to the UI implementation. Jul 23 17:31:36 Just to counter argument, what if you do this and the UI gets killed by an OOM condition / segfault Jul 23 17:31:39 balrog-k1n: I wouldn't highly recommend it, but possible. Jul 23 17:31:40 You're up shit creek Jul 23 17:31:50 So I think that is a bad idea Jul 23 17:32:11 denkenz: then both agents fall off the dbus, what's wrong with that? Jul 23 17:32:40 Yes, that is fine. oFono will get the D-Bus notification that one of the agents unexpected died. That is fine. Jul 23 17:32:52 We handle that in BlueZ all the time nicely and clean up after the agent. Jul 23 17:33:28 denkenz: We even clean up after the users that just use something like StartDiscovery. If the process doesn't stick around for the result. We just cancel everything it tried to do. Jul 23 17:33:28 yeah, the patches i sent also handle that.. the problem is that denkenz suggest that disconnecting from dbus be the *only* way to unregister the primary agent Jul 23 17:33:31 Problem here is that possibilities are endless, my case assumes that the agent is in the home screen, not sim app Jul 23 17:33:47 Primary agent being user session agent ;) Jul 23 17:33:57 holtmann: yes Jul 23 17:34:18 denkenz: Just returning EndSession error is fine with me. Then oFono can release the agent after that. Jul 23 17:34:20 probably should call it "user initiated session agent" Jul 23 17:36:10 actually i can remove the references to org.ofono.Error.EndSession from the code (and docs) and it would still work because any unknown response is also treated as EndSession Jul 23 17:36:25 but it will be relying on undocumented features then Jul 23 17:36:42 IMO the typical use case is 2 agents, a reduced one in Homescreen for the basics like DisplayText and SelectItem only Jul 23 17:36:43 Then an STK app with a fully featured one Jul 23 17:37:29 denkenz: I am with Andrew on this one. Using an error instead of a method call makes perfect sense to me. To signal the core that we are no longer interested in doing this. Jul 23 17:37:33 denkenz: i agree that's the most advanced case, but may not be the case for meego Jul 23 17:38:08 It works nicely and we have done that in BlueZ as well for Reject cases. They come via an error. Jul 23 17:38:14 certainly no point to go that advanced if you want to implement a old nokia-style dumbphone Jul 23 17:38:42 denkenz: Check BlueZ RequestPinCode. We have: Jul 23 17:38:46 Possible errors: org.bluez.Error.Rejected Jul 23 17:38:47 org.bluez.Error.Canceled Jul 23 17:38:51 And honestly, the worst consequence here is that the user will have to press back 3 times instead of 1 Jul 23 17:39:35 well, yeah, but then you can't go around saying you want an easy api, or be nice to the UI writer :) Jul 23 17:39:36 And of course if the process that started the user initiated STK session dies, then the EndSession is implied. And we just cleanup. Jul 23 17:42:52 i guess i can remove the references to org.ofono.Error.EndSession and we can say in the docs that "Any unknown response will trigger end of session" Jul 23 17:43:36 balrog-k1n: I am against an extra method for EndSession since that seems overhead, but Error.EndSession is perfectly fine with me. Jul 23 17:43:59 well, yeah, but then you can't go around saying you want an easy api, or be nice to the UI writer :) Jul 23 17:43:59 And of course if the process that started the user initiated STK session dies, then the EndSession is implied. And we just cleanup. Jul 23 17:44:00 * denkenz has quit (Read error: Connection reset by peer) Jul 23 17:44:00 i guess i can remove the references to org.ofono.Error.EndSession and we can say in the docs that "Any unknown response will trigger end of session" Jul 23 17:44:01 balrog-k1n: I am against an extra method for EndSession since that seems overhead, but Error.EndSession is perfectly fine with me. Jul 23 17:44:03 * denkenz (~denkenz@p1-17-rb5.clm.centur Jul 23 17:44:25 Or course the docs should also note that any unknown error will result in EndSession and then the Release() call for the agent. Jul 23 17:44:41 I'm not arguing about a method call for end session Jul 23 17:44:53 I'm basically arguing about the need for it period Jul 23 17:45:03 No method. Jul 23 17:45:23 We need it. We can't rely on leaving D-Bus. That bus is shared for the process. Jul 23 17:45:28 i'm not arguing for a method call either Jul 23 17:45:41 I'm unconvinced Jul 23 17:45:51 Even the iPhone doesn't do it Jul 23 17:45:53 I think the error is good idea. With the additional notes about that any other error will cause the same effect plus Release(). Jul 23 17:46:34 denkenz: So for bus disconnect detection we have the problem that the bus might be used for other parts as well or internally by a library that is linked. Jul 23 17:47:03 That causes issues since the bus connection is shared. No matter what. So as long as one piece inside the process holds a reference to the bus, it will not disconnect. Jul 23 17:47:17 The EndSession error is still not natural Jul 23 17:47:27 But of course if we just disconnect from the bus, then ofonod will clean up after you and do the EndSession. Jul 23 17:47:30 If you want to do it this way then Unregistering the Agent is better Jul 23 17:47:42 denkenz: Disagree here. Yes it is. Jul 23 17:47:56 We have been doing that since ever within BlueZ. Jul 23 17:48:21 And you can't unregister the agent. That is for the default agent only. Jul 23 17:48:38 Dude, there's maybe 1 person in the world who knows BlueZ Agents and UI better than I do Jul 23 17:48:43 So you would need a method that would do TerminateSelectItem(). Jul 23 17:48:48 And seriously, I disagree ;) Jul 23 17:49:01 i guess UnregisterAgent can be for both things Jul 23 17:49:17 balrog-k1n: No. That would be bad API design and not really logical. Jul 23 17:49:19 but it doesn't hurt documenting it in the docs that any unknown error triggers end of session Jul 23 17:49:35 denkenz: Sorry but I disagree with you here. The error seems just fine to me. Jul 23 17:50:22 And with BlueZ we use different errors to trigger different results. Jul 23 17:50:53 BlueZ only uses Rejected and Canceled Jul 23 17:51:01 Almost the same meaning Jul 23 17:51:13 Yes. They end up causing different responses on the lower level. Jul 23 17:51:23 And that is intentional. Jul 23 17:51:35 Except Johan removed this in the meantime. Jul 23 17:51:48 Fair enough, for us GoBack is not necessarily the only response either Jul 23 17:52:21 denkenz: Anyhow, any unexpected response from an agent needs to be treated as EndSession anyway. And it will result in Release() callback to release the agent. Jul 23 17:52:45 So why not give the agent a clear documented way with Error.EndSession. Jul 23 17:53:19 The agent is basically telling us. So my user has no clue what he was doing, I am giving up. Do what you think is best. And oFono core does Release(). Jul 23 17:53:42 That is what GoBack is for Jul 23 17:53:45 Feel free to call org.ofono.Error.IAmAStupidUIAndClueless if you want. Jul 23 17:54:07 Or the big red hangup button Jul 23 17:54:23 GoBack and the red button are different, at least on the phones i've used Jul 23 17:54:26 EndSession is a very specific use case, and to me it is not natural to return an error Jul 23 17:54:50 For me it is GoBack and EndSession as errors are just fine to me. Jul 23 17:55:19 Fair enough, but I want to make up my mind when I see the UI Jul 23 17:55:43 Sure. And I bet you that we will have UI designs that never call EndSession. They force the user into multiple GoBack. Jul 23 17:55:52 I'm just not convinced it is really required, but I have been wrong before and will freely admit if this is the case Jul 23 17:56:23 I am not sure if I am right either. I just can acknowledge that it seems like a good idea to me. Jul 23 17:56:38 At least right now it makes sense. Jul 23 17:57:26 Sure, and I can see it working this way, don't get me wrong Jul 23 17:58:15 For now I just want the API in the proposal, then we can go happy modifying it Jul 23 17:58:20 i'm not sure that the EndSession is the best way to do this, but i'm sure leaving disconnecting from the bus as the only option is also pretty bad Jul 23 17:58:21 denkenz: For the iPhone argument. What do they do when pressing the home button. They might have to translate that to EndSession somehow. Just thinking out loud. Jul 23 17:59:20 They do, but I suspect their SimApp is actually running in the home screen Jul 23 17:59:29 Settings only displays the Main Menu Jul 23 17:59:31 balrog-k1n: Just go ahead and implement the EndSession error handling. And the extra notes in the docs what happens if you return some unknown error. Jul 23 17:59:42 Then we see how this goes. Jul 23 17:59:50 Lets do it this way Jul 23 18:00:01 Lets implement the proposal, then we do each one as a separate patch Jul 23 18:00:13 Also the more documentation we have how things are interaction the better. Especially getting the Release() call from the core etc. Jul 23 18:00:19 The patches on the list are already way too big Jul 23 18:00:25 holtmann: current code i sent to the list does this exactly (EndSession or any unknown response ends up in end of session and subsequently Release() being called) Jul 23 18:00:52 Just for my sanity Jul 23 18:02:00 balrog-k1n: Add some documentation on the behavior. More documentation the better. Jul 23 18:02:09 denkenz: ok, i can remove this and resend, have you seen anything else that i should change while i'm doin gthis? Jul 23 18:02:30 Yeah, we need to move most of the agent stuff to stkagent.c/h Jul 23 18:02:41 though in this case removing org.ofono.Error.EndSession only saves 2 lines Jul 23 18:02:52 Yeah, forget EndSession, leave it in there Jul 23 18:03:00 Yes, leave it in the code. Jul 23 18:03:02 I'm more worried about UnregisterAgent behavior and other things Jul 23 18:03:14 If we decide later on I was on crack, then at least the GIT log shows that. Jul 23 18:03:52 Lets document the desired behavior for all of these. And we can review it. Jul 23 18:04:12 I actually have to leave in 10 minutes. So I will be only looking at this later tonight. Jul 23 18:04:22 The iPhone definitely does menus outside the Settings app Jul 23 18:04:49 Sweet. How do you trigger it? Jul 23 18:04:57 Maybe I just have the wrong SIM cards :( Jul 23 18:05:09 another question, the agent interface in the docs is currently org.ofono.SimToolkitAgent while in the code it's org.ofono.SimApplicationAgent, should i send a patch to change it in dbus.h? Jul 23 18:05:15 Mostly a gut feeling, but the UI design is vastly different Jul 23 18:05:16 or change the docs? Jul 23 18:05:51 Honestly I don't care about the name, do you feel strongly about one vs the other? Jul 23 18:05:58 We stick with SimToolkit I would assume. So we need to change the code. Jul 23 18:06:15 Just take the shorter one for now ;) Jul 23 18:06:24 ok Jul 23 18:06:40 Yeah, so lets add a rule Jul 23 18:06:46 * balrog-k1n 's client uses SimApplicationAgent :) Jul 23 18:06:49 If there's an API proposal, follow it to the letter Jul 23 18:07:02 If you want to change it, do that first or after Jul 23 18:07:08 Don't just mix and match Jul 23 18:07:28 denkenz: it's already in the tree though Jul 23 18:07:38 What is? Jul 23 18:08:08 balrog-k1n: Fix the code to match the API docs. Jul 23 18:08:17 #define OFONO_SIM_APP_INTERFACE OFONO_SERVICE ".SimApplicationAgent" in include/dbus.h Jul 23 18:08:44 i'll send a patch then Jul 23 18:08:57 Then fix it or the proposal Jul 23 18:09:10 my feeling is SimApplication may make more sense to the poor reader Jul 23 18:10:01 Honestly it doesn't matter to me, SimToolkit might be more recognizable Jul 23 18:10:22 okay, SimToolkitAgent then Jul 23 18:10:38 Which ever you pick, just be consistent ;) Jul 23 18:12:48 + void (*cmd_send)(struct ofono_stk *stk, DBusMessage *call); Jul 23 18:12:49 + void (*cmd_cb)(struct ofono_stk *stk, enum stk_agent_result result, Jul 23 18:12:49 + DBusMessage *reply); Jul 23 18:12:49 + struct ofono_stk *stk; Jul 23 18:12:49 +}; Jul 23 18:13:22 So I suggest moving cmd_send and cmb_cb typedefs to stkagent.h and make them proper void * based ones Jul 23 18:13:37 I also suggest getting rid of stk member if at all possible Jul 23 18:13:47 + struct stk_app_agent *primary_agent; Jul 23 18:13:47 + struct stk_app_agent *default_agent; Jul 23 18:13:54 Name primary agent into user_session_agent Jul 23 18:14:16 the previous versions didn't have the stk member, but now you need it because there are two different agents Jul 23 18:14:29 s/primary_agent/session_agent/ Jul 23 18:14:37 You guys confuse the hell out of me otherwise ;) Jul 23 18:15:16 i can change struct ofono_stk *stk to user_data if you want to avoid reference to ofono_stk Jul 23 18:15:25 Yes please Jul 23 18:15:30 but since it's the stk agent i think we can assume it'll only be used with stk Jul 23 18:15:41 I think the agent can be written without depending on the stk data structures Jul 23 18:16:07 And don't be so sure, we might have to call the stk agent from voicecall Jul 23 18:16:35 +static void stk_agent_request_cancel(struct ofono_stk *stk) Jul 23 18:16:35 +{ Jul 23 18:16:35 + if (stk->primary_agent) Jul 23 18:16:35 + app_agent_request_cancel(stk->primary_agent); Jul 23 18:16:36 + Jul 23 18:16:36 + if (stk->default_agent) Jul 23 18:16:36 + app_agent_request_cancel(stk->default_agent); Jul 23 18:16:37 +} Jul 23 18:16:37 + Jul 23 18:16:37 yeah, i can avoid depending on the members of stk, but the using struct ofono_stk does no harm imho (it's always a pointer) Jul 23 18:16:49 so it just avoids the casts from void * Jul 23 18:17:05 Just an idea, but you might want to simply use a current_agent pointer Jul 23 18:17:11 makes things a bit simpler Jul 23 18:17:20 I suggest all casting to be done inside stk.c Jul 23 18:17:33 Leave stkagent with just userdata Jul 23 18:17:51 yeah, but some calls like DisplayText can actually remain active on one agent while the other one is doing something so there's no "current" Jul 23 18:18:17 my thinking is that if we can avoid this casting altogether then why not :) Jul 23 18:19:04 No, if another command arrives we cancel the previous agent Jul 23 18:19:50 Even if its the weird immediate response display text thing Jul 23 18:20:01 the spec says the opposite Jul 23 18:20:44 Quote? Jul 23 18:21:14 Anyway, I am off until later now. Jul 23 18:21:27 + * "If the text is to be sustained, the terminal shall display Jul 23 18:21:27 + * the text of applicable DISPLAY TEXT commands beyond the sending Jul 23 18:21:27 + * of the TERMINAL RESPONSE and possibly beyond the end of the Jul 23 18:21:29 + * proactive session. Jul 23 18:21:29 etc Jul 23 18:21:57 The terminal shall continue to display the text until one of Jul 23 18:21:57 the following events occurs: Jul 23 18:21:57 - Jul 23 18:21:57 a subsequent proactive command is received containing display data; Jul 23 18:22:06 Considering just about every proactive command contains display data... Jul 23 18:22:35 the current code actually does this, Jul 23 18:23:14 So then there's no problem ;) Jul 23 18:23:24 but if your default agent is executing a DisplayText, and your "current" agent is suddenly the session agent, you need to cancel the command on the other agent Jul 23 18:23:29 not the "current" agent Jul 23 18:23:54 err? How does that happen? Jul 23 18:25:05 it shouldn't happen with a sane ui Jul 23 18:25:16 Lol, so make sure it doesn't Jul 23 18:25:19 well, you're right, it actually can't happen at all Jul 23 18:25:30 Even if we're in DisplayText and some UI calls SelectItem Jul 23 18:25:35 Then we cancel the previous agent Jul 23 18:26:18 + if (dbus_message_get_args(msg, NULL, Jul 23 18:26:18 + DBUS_TYPE_BYTE, &selection, Jul 23 18:26:19 + DBUS_TYPE_OBJECT_PATH, &agent_path, Jul 23 18:26:19 + DBUS_TYPE_INVALID) == FALSE) Jul 23 18:26:19 + return __ofono_error_invalid_args(msg); Jul 23 18:26:19 + Jul 23 18:26:20 + /* TODO */ Jul 23 18:26:20 + Jul 23 18:26:20 + return NULL; Jul 23 18:26:25 yeah, and actually you can't call SelectItem when you're in DisplayText because the menu has no items Jul 23 18:26:35 No biggie, but return error_not_implemented here Jul 23 18:26:59 the second patch implements this Jul 23 18:27:51 I know, but put yourself in my shoes Jul 23 18:28:08 Reviewing a 400 line patch is hard enough, keeping context between 5 is impossible ;) Jul 23 18:28:31 Hence my preference for simple self-contained ones Jul 23 18:28:54 yes, makes sense Jul 23 18:29:08 (though it's hard enough working with patches on the same file already) Jul 23 18:29:51 git-am -i doesn't help splitting such changes Jul 23 18:29:55 I know, but if you get into a habit it becomes easier Jul 23 18:30:08 e.g. make skeleton that just returns not implemented Jul 23 18:30:15 Then fill in one function, commit, another, commit Jul 23 18:31:01 This makes it more likely I accepted them too Jul 23 18:31:29 personally i'd probably prefer reviewing a single big patch (seeing how the functions are used etc) Jul 23 18:31:52 obviously i'm not against splitting in patches :) Jul 23 18:32:17 Well I know I prefer doing the simpler chunks Jul 23 18:32:23 I think Marcel prefers the same Jul 23 18:32:40 So just to make the process easier and faster I'd encourage you to do that Jul 23 18:33:04 It also makes it easier for me to accept m/n patches as opposed to having to reject the entire thing Jul 23 18:34:08 Ok so break out the agent stuff into stkagent.c/h and resubmit Jul 23 18:35:26 ok Jul 23 18:35:44 to reiterate, we want no references to struct ofono_stk in these files? Jul 23 18:36:26 From my looking at patch 1 only, I think we can get away with no references to ofono_stk there Jul 23 18:36:54 Also, it might be that we turn StkAgent into a more generic Agent, there's alot of overlap Jul 23 18:37:19 okay then Jul 23 18:37:19 So if we can, lets do the generic callback / userdata approach for now Jul 23 18:38:14 Feel free to typedef inside stkagent.h to make the code easier Jul 23 18:38:28 I think in this case it is fine Jul 23 19:03:31 balrog-k1n: One other thing Jul 23 19:03:32 + DBusPendingCall *call; Jul 23 19:03:33 + guint watch; Jul 23 19:03:33 + guint cmd_timeout; Jul 23 19:03:47 You can happily get rid of cmd_timeout by setting the timeout on the pending call Jul 23 19:04:17 That way cleaning up the pending call cancels the timeout as well Jul 23 19:06:59 denkenz: i was thinking about it too, don't remember anymore why i decided against, will see again Jul 23 19:07:05 gotta go for now Jul 23 19:07:18 nod Jul 23 22:31:07 denkenz: ping Jul 23 22:31:31 padovan: pong Jul 23 22:32:51 denkenz: about the DUN client work, you said that it should be a atom, does this means new modem in the drivers directory? Jul 23 22:33:01 yep Jul 23 22:33:54 denkenz: and should we keep the PAN similar API that you and Marcel dicussed? Jul 23 22:34:18 No, forget PAN Jul 23 22:34:29 denkenz: so we need a plugin to handle the driver right? Jul 23 22:34:34 We thought that was a good idea when dun client was a separate daemon Jul 23 22:35:12 With oFono based approach the DataConnectionManager API is reused for DUN Jul 23 22:35:24 And yes, you'd need a dun.c in plugins or something Jul 23 22:36:02 Probably same setup like hfp.c but without the need for a BlueZ agent Jul 23 22:36:33 then DUN will be like a new protocol to the DataConnectionManager? Jul 23 22:38:29 We're basically counting on the fact that nobody really needs any custom dial strings for DUN Jul 23 22:38:47 So we effectively can re-use the atmodem gprs-context driver to do the work for us Jul 23 22:39:11 Just a matter of faking some stuff Jul 23 22:39:17 I see. Jul 23 22:46:55 denkenz: found the problem with the make distcheck failing on ofono/types.h -- the way the links are generated is broken -- if you run it from a VPATH directory it will fail Jul 23 22:53:24 inakypg: Post a patch to ML, holtmann deals with the build system Jul 23 22:53:43 inakypg: I'm just a monkey who does make distcheck :) Jul 23 22:54:16 denkenz: I need to twist holtmann's arm into building *always* from a VPATH directory. He always gets caught in these since I know him... Jul 23 22:54:24 ...and that's a long time already :) Jul 23 22:54:54 I have no idea what you just said, for me and automake, ignorance is bliss :) Jul 23 23:06:42 denkenz: fixed, it was a typo: Jul 23 23:07:05 - $(AM_V_GEN)$(LN_S) $(abs_top_srcdir)/$< $@ Jul 23 23:07:06 + $(AM_V_GEN)$(LN_S) $(abs_top_builddir)/$< $@ Jul 23 23:07:09 I'll send a patch too Jul 23 23:11:28 Yep, again I happily let holtmann deal with it :) Jul 23 23:11:47 I do all the hard work, he does the releases and gets credit ;) Jul 23 23:18:04 inakypg: Ok I do the rest on Monday, quitting time here Jul 23 23:35:09 aww Jul 24 02:12:30 padovan: I think you need plugins/dun.c and also drivers/dunmodem/netreg.c and drivers/dunmodem/sim.c for the fake NET and SIM atom drivers. Jul 24 02:15:51 holtmann: you know how to get oFono to work?? Jul 24 02:16:12 luke-jr: Yes. I just daily for Internet connectivity on my laptop :) Jul 24 02:16:17 s/just/use/ Jul 24 02:16:21 holtmann: any tips? Jul 24 02:16:28 mine refuses to list the modem Jul 24 02:16:53 that is, in org.ofono.Manager.GetProperties Jul 24 02:18:57 Which modem? Jul 24 02:19:24 phonet? Jul 24 02:19:52 isimodem I guess Jul 24 02:20:46 debug output ends with ofonod[6068]: plugins/usbpnmodem.c:usbpn_status_cb() Phonet link phonet0 (4) is up Jul 24 02:28:46 luke-jr: Wrong person? Ask one of the Nokia guys. Jul 24 02:29:15 :/ Jul 24 02:29:19 who are they? Jul 24 02:29:39 I have used my N900 via USB cable. Works fine. You just need a recent enough kernel. Jul 24 02:29:56 Fedora 13 kernels were just fine. Jul 24 02:43:52 inakypg: Not seeing the patch for VPATH build on the mailing list. **** ENDING LOGGING AT Sat Jul 24 02:59:57 2010