**** BEGIN LOGGING AT Tue Nov 23 02:59:57 2010
Nov 23 10:41:28 <demarchi> denkenz: with the addition of band selection to radio settings, i think it doesn't make sense to return _error_not_implemented() if query_rat_mode is not implemented. I'm adding a value _UNKNOWN to the mode enum. This is returned in case method is not implemented in driver.
Nov 23 10:41:34 <demarchi> denkenz: do you agree?
Nov 23 10:42:12 <denkenz> If query is not implemented, simply default to 'Auto' and be done with it
Nov 23 10:43:29 <demarchi> denkenz: the same for frequency band?
Nov 23 10:44:57 <denkenz> ok, so I'm not following
Nov 23 10:45:13 <denkenz> band selection and frequency band is the same thing, no?
Nov 23 10:45:51 <demarchi> denkenz: yes, but i was talking about the return value in case query_rat_mode() is not implemented
Nov 23 10:46:10 <demarchi> i.e. querying the mode (umts, any, etc)
Nov 23 10:46:38 <demarchi> in radio_get_properties, there's this condition:
Nov 23 10:46:43 <demarchi> 	if (!rs->driver->query_rat_mode)
Nov 23 10:46:44 <demarchi> 		return __ofono_error_not_implemented(msg);
Nov 23 10:47:05 <denkenz> Ah I see
Nov 23 10:47:12 <denkenz> it is probably safe to leave that condition in there
Nov 23 10:47:27 <demarchi> but now there isn't only the rat_mode... there's fast_dormancy and also frequency_band
Nov 23 10:47:27 <denkenz> But yeah, if query_rat_mode is not implemented, defaulting to any is safe
Nov 23 10:48:35 <denkenz> doing the band selection preference is essentially the same as the fast dormancy condition
Nov 23 10:49:13 <denkenz> the query_rat_mode being null is really a separate issue
Nov 23 10:49:52 <demarchi> do you mean the rat_mode is a mandatory property and fast_dormancy and band selection are optional?
Nov 23 10:51:01 <denkenz> Yeah, I'm sort of saying that in a roundabout way ;)
Nov 23 10:51:41 <demarchi> so i'll let that condition in radio_get_properties
Nov 23 10:51:52 <denkenz> Nod, don't worry about that for now
Nov 23 10:52:00 <denkenz> If we need to relax this later, we do it in a separate patch anyway
Nov 23 10:52:24 <demarchi> ok
Nov 23 10:52:26 <demarchi> thanks
Nov 23 13:05:45 <demarchi> denkenz: were these errors intentionally left out of the docs?
Nov 23 13:05:47 <demarchi> http://codepad.org/3tSpyVLY
Nov 23 13:09:10 <denkenz> Intentionally? no
Nov 23 13:09:40 <denkenz> Feel free to submit a patch
Nov 23 13:09:57 <demarchi> ok, i'm going to do
Nov 23 14:51:14 <akiniemi> denkenz: ping?
Nov 23 14:51:26 <denkenz> akiniemi: pong
Nov 23 14:53:28 <akiniemi> Do you mind splitting HSPA up so that HSDPA and HSUPA are separate techs?
Nov 23 14:53:47 <denkenz> I thought we agreed not to do this?
Nov 23 14:54:07 <akiniemi> Distinguishing between the two seems to be a requirement in a test bench...
Nov 23 14:54:31 <denkenz> In what manner is it exposed?
Nov 23 14:54:54 <akiniemi> HSDPA, HSUPA, HSPA (both)
Nov 23 14:55:09 <akiniemi> Rémi has a pair of patches out, and I was about to push, actually
Nov 23 14:55:17 <holtmann> The API is meant for the UI showing either 3G or 3.5G and nothing else.
Nov 23 14:55:33 <akiniemi> It's likely we might also need HSPA+ for 3.9G or whatever...
Nov 23 14:55:36 <holtmann> So the patch is not what we want. We do want to just differ between UTMS.
Nov 23 14:55:47 <holtmann> And HSPA etc.
Nov 23 14:56:02 <holtmann> So even HSPA+ or whatever that is, would be 3.5G.
Nov 23 14:56:51 <akiniemi> holtmann: still not that difficult to group them in the UI
Nov 23 14:57:13 <holtmann> That is not the point behind the API.
Nov 23 14:57:59 <akiniemi> holtmann: I know, I was originally proposing using 2g, 2.5g, etc as values
Nov 23 14:58:11 <holtmann> We changed this back in March this to make it on purpose a more simpler API.
Nov 23 14:58:26 <akiniemi> But we're limiting what can be shown in the UI by doing this, maybe too much
Nov 23 14:58:55 <akiniemi> At least too much when it comes to what can be implemented using oFono in cert tests
Nov 23 14:59:50 <akiniemi> For instance, I can see operators wanting to show HSPA+ differently from plain ol' HSPA
Nov 23 14:59:54 <holtmann> What does have certification tests to do with the UI now. This property is intended for the UI.
Nov 23 15:00:36 <akiniemi> We need to be able to generate a proprietary AT cmd response as part of a test set by an unnamed operator ;)
Nov 23 15:00:51 <akiniemi> It needs to distinguish between different flavors of HSPA
Nov 23 15:01:17 <holtmann> The UI does not see the difference.
Nov 23 15:01:26 <akiniemi> Also, UI specs are flaky -- they might indeed want to show HSDPA and HSUPA and both in a different way
Nov 23 15:01:32 <holtmann> HSUPA and HSDPA and HSPA+ and HSPA are all the same.
Nov 23 15:01:52 <holtmann> I don't believe that any UI will show the difference between HSDPA and HSUPA.
Nov 23 15:02:03 <akiniemi> holtmann: So you say, but you are not a UI designer, or an operator asking for this stuff
Nov 23 15:02:12 <holtmann> Showing the difference between 3G and 3.5G is already overkill/.
Nov 23 15:02:20 <akiniemi> Huh?
Nov 23 15:02:23 <holtmann> Which operator wants to show this in the UI?
Nov 23 15:02:29 <akiniemi> All?
Nov 23 15:02:42 <akiniemi> They absolutely want you to see you're on high speed
Nov 23 15:02:45 <denkenz> This doesn't even work on any AT modem in existence
Nov 23 15:02:46 <holtmann> My iPhone for example does not do that. It just shows 3G.
Nov 23 15:03:02 <denkenz> The only modem that supports this is ISI
Nov 23 15:03:34 <akiniemi> Anyway, it is trivial to group all h* into a 3.5G icon, but the reverse is not
Nov 23 15:03:59 <akiniemi> Here we are again stepping on the wrong side of the "minimal" principle, IMHO
Nov 23 15:04:26 <holtmann> Why not implement the AT command stuff into oFono as part of the dialup/modem emulator support. Then you have access to whatever network information you need.
Nov 23 15:04:41 <holtmann> We should not complicated the D-Bus API. It is simple and enough.
Nov 23 15:06:19 <akiniemi> holtmann: you really want battery info, or kbd input code in oFono?
Nov 23 15:07:48 <holtmann> This is CIND and CBC etc. So they are pretty much standard ones?
Nov 23 15:16:48 <denkenz> akiniemi: Btw, we do want this in oFono, please see the AT Modem Emulator task in the TODO
Nov 23 16:20:47 <demarchi> denkenz: did you see my patches about text telephony?
Nov 23 16:21:01 <demarchi> denkenz: i have a change on their documentation
Nov 23 16:21:11 <demarchi> the returned errors were wrong too
Nov 23 16:21:59 <denkenz> demarchi: So I only had two complaints
Nov 23 16:22:07 <denkenz> I really want the first patch separated a bit
Nov 23 16:22:21 <denkenz> And I want you to use the CACHED flag in the get_properties implementation
Nov 23 16:23:03 <demarchi> ok
Nov 23 16:23:18 <demarchi> i'll rework it a bit then
Nov 23 16:23:49 <denkenz> Nod, I was planning to apply them and fix them up, but if you were planning to resubmit I let you do it
Nov 23 16:24:08 <demarchi> how do you want it to be split?
Nov 23 16:28:08 <demarchi> on possible way imo would be split the registration and the addition of Powered property
Nov 23 16:28:16 <demarchi> s/on/one/
Nov 23 16:29:00 <denkenz> let me check
Nov 23 16:29:47 <denkenz> So I want one patch for include/text-telephony.h
Nov 23 16:29:58 <denkenz> One patch for doc/ + Makefile
Nov 23 16:30:37 <denkenz> One patch for dbus.h
Nov 23 16:30:46 <denkenz> And the rest in the final patch
Nov 23 16:31:13 <denkenz> Also, do you really need the single line change inside modem.c?
Nov 23 16:31:42 <denkenz> Ah actually nevermind
Nov 23 16:31:48 <denkenz> I want the modem.c change in a separate patch too ;)
Nov 23 16:36:19 <demarchi> :-o
Nov 23 16:36:51 <demarchi> so, i'll let doc + Makefile as the last patch
Nov 23 16:37:06 <denkenz> Actually, to be clear, the Makefile changes should be in each of the above patches
Nov 23 16:37:18 <demarchi> otherwise it will be easy to break build
Nov 23 16:37:40 <denkenz> So one Makefile change for the include file
Nov 23 16:37:46 <denkenz> One Makefile change for the doc file
Nov 23 16:37:52 <denkenz> one makefile change to add text-telephony.c
Nov 23 16:38:04 <denkenz> Not in a separate patch, just grouped with those patches
Nov 23 16:40:53 <demarchi> anyway, i'm just seeing a breakage if i split the patches this way
Nov 23 16:41:07 <demarchi> let me try
Nov 23 22:36:30 <Jeevaka> denkenz: few information required on call forwarding properties
Nov 23 22:36:50 <denkenz> shoot
Nov 23 22:38:06 <Jeevaka> currently, we are signalling the property change only for voice even if the data and fax are active
Nov 23 22:38:52 <denkenz> Yep, because we don't support Fax or Data, so no point
Nov 23 22:39:44 <Jeevaka> ok
Nov 23 22:42:53 <Jeevaka> Do we need to provide the support for data and fax unconditional indicators ?
Nov 23 22:43:20 <denkenz> I don't think so, the only reason we deal with that stuff is for 22.030 MMI codes
Nov 23 22:43:31 <denkenz> For writing to the SIM sticking with voice is enough
Nov 23 22:46:01 <Jeevaka> ok, got it
Nov 23 23:17:10 <balrog-k1n> denkenz: out of curiosity why's the forward declaration in http://lists.ofono.org/pipermail/ofono/2010-November/005796.html bad style?
Nov 23 23:17:39 <denkenz> forward declarations of static functions are pointless
Nov 23 23:18:06 <balrog-k1n> they make your code compile :)
Nov 23 23:18:19 <denkenz> The only reason to do that is for queues of some sort
Nov 23 23:18:30 <denkenz> Otherwise moving the function around is the better choice
Nov 23 23:18:32 <balrog-k1n> plus there's another one just above that one
Nov 23 23:18:52 <balrog-k1n> well yeah, you can usually move the function but it generates a bigger patch
Nov 23 23:19:32 <denkenz> yeah, that one probably used to have a reason for being forward declared
Nov 23 23:19:40 <denkenz> But after GAtIO refactoring it has disappeared
Nov 23 23:19:46 <denkenz> So that is bad style actually
Nov 23 23:27:52 <denkenz> Actually there is a circular dependency, so that one is still fine
Nov 23 23:28:09 <denkenz> But basically whenever I see a forward declaration of a static, alarm bells go off
Nov 23 23:31:12 <balrog-k1n> i also dislike useless forward declarations in code, but when i edit somebody's code i'm assuming they may have ordered the functions in some way and i don't want to try to find out
Nov 23 23:32:45 <denkenz> Generally that is safe assumption, but we're totally pedantic about this
Nov 23 23:33:33 <denkenz> otherwise even with these (very good) intentions you degenerate the code quality over time
Nov 23 23:33:49 <balrog-k1n> true
**** ENDING LOGGING AT Wed Nov 24 02:59:58 2010