**** BEGIN LOGGING AT Tue May 11 02:59:56 2010
May 11 12:04:27 <feitingen> i cant seem to power up my freerunner modem with ofono, is there a special procedure for this?
May 11 14:09:32 <denkenz> feitingen: This was a bug fixed sometime after 0.20, see commit 15d93ad0b9da5f60afaafe942bd4d05f250318a3
May 11 14:13:41 <feitingen> oh, thanks, i'll get it from git instead then
May 11 14:56:41 * Djdas is away: Away
May 11 15:59:02 <denkenz> balrog-k1n: here?
May 11 18:51:57 <feitingen> denkenz: i grabbed the newest git today, it still does not want to power on the calypso :(
May 11 18:52:54 <denkenz> feitingen: Honestly I haven't tried in a while, try export OFONO_AT_DEBUG=1 and restart ofono
May 11 18:53:05 <denkenz> see at least where it fails
May 11 18:53:22 <feitingen> thanks
May 11 19:00:13 <feitingen> here is what happens: http://pastebin.com/qQNSuavu
May 11 19:04:47 <denkenz> interesting
May 11 19:04:58 <denkenz> I've never seen a CFUN=1 return an ERROR
May 11 19:05:14 <denkenz> Do you have a SIM in there?
May 11 20:42:04 <balrog-k1n> denkenz: now i am
May 11 20:43:48 <denkenz> balrog-k1n: So I took a look at the ber tlv builder stuff again
May 11 20:44:25 <denkenz> I'm pretty much fine with the stk builder api, except for the static buffer
May 11 20:44:45 <denkenz> I really don't want the higher layers to deal with allocating the right buffer size
May 11 20:45:48 <denkenz> Also, my impression of the ber_tlv builder was that it was too complicated with some corner cases I wasn't sure were properly covered
May 11 20:46:41 <denkenz> e.g. ber_tlv_builder_optimize didn't end up calling set_length on the parent...
May 11 20:46:42 <balrog-k1n> i don't see the buffer allocation as problematic, but we could use something liek gbytearray or a static buffer of 500 bytes in simutil.c until we see bigger TLVs in use
May 11 20:47:20 <denkenz> See what I'm afraid of is that the modems will decode the commands for us
May 11 20:47:27 <balrog-k1n> gbytearray will complicate things a lot imho
May 11 20:47:34 <balrog-k1n> yeah, the wavecom modems do this
May 11 20:47:35 <denkenz> And then send the decoded bits, like the mbm does
May 11 20:47:48 <denkenz> Which makes static buffers impractical
May 11 20:48:07 <balrog-k1n> why is it impractical?
May 11 20:48:34 <denkenz> Because something like display text can be 100 bytes or 16k
May 11 20:48:53 <denkenz> and a setup menu can be 16k * n bytes
May 11 20:49:15 <balrog-k1n> i'm pretty sure there's a limit below 256 bytes for all STK payloads
May 11 20:49:35 <balrog-k1n> it even says something to that effect in the description of Set Up Menu,
May 11 20:49:54 <balrog-k1n> that you need to limit the text for each entry if you have too many entries to fit in the limits
May 11 20:50:08 <balrog-k1n> i.e. the length of the items
May 11 20:50:57 <denkenz> The spec says this, but I really wonder if that is true
May 11 20:51:04 <balrog-k1n> +CSIM also has this limit
May 11 20:52:15 <denkenz> Where does it say that about +CSIM?
May 11 20:52:16 <balrog-k1n> the length of the APDU / RPDU (what is sent / received with +CSIM) wouldn't fit in the byte length otherwise
May 11 20:54:30 <balrog-k1n> even allocating a 32k buffer would be lighter on the system that gbytearray i think
May 11 20:54:45 <balrog-k1n> than*
May 11 20:54:54 <denkenz> Yeah glib stuff is pretty crap
May 11 20:56:02 <denkenz> so assuming you're correct on the 256 byte limit
May 11 20:56:44 <denkenz> Then stk pdu builder should be even simpler
May 11 20:57:09 <denkenz> Right now I am just failing to see the use case for a generic ber/ctlv builder
May 11 20:58:01 <balrog-k1n> also, now that i think of it, when the modem fetches a proactive command, it'll be notified about the command using the two status bytes from sim, the first byte is 0x91 and the second byte is the length (what needs to be used in the next FETCH command)
May 11 20:58:37 <denkenz> Yeah you're right about the 256 byte limit
May 11 20:59:07 <balrog-k1n> the driver for the modems that decode PDUs in firmware will need to build the BER-TLV + the CTLVs in it
May 11 20:59:11 <denkenz> Been a while since I looked at 11.11
May 11 20:59:34 <denkenz> Sure, they need to build the proactive command ones
May 11 20:59:40 <denkenz> Not a generic ber tlv..
May 11 20:59:47 <balrog-k1n> we don't need to use the generic utilities, but my opinion is that it only makes code clearer
May 11 21:00:03 <balrog-k1n> and symmetric for encoding and decoding, thus easy to understand
May 11 21:00:27 <denkenz> So I agree in the general case 100%
May 11 21:00:42 <denkenz> However, here it seems we're doing more work than really necessary
May 11 21:00:55 <denkenz> and it is actually hard to get it right
May 11 21:01:32 <balrog-k1n> more work code wise?
May 11 21:01:51 <denkenz> Just take ber_tlv_builder_optimize, it never calls set_length on the parent
May 11 21:01:59 <balrog-k1n> performance wise i don't see any place that could be simplified (a lot) by assuming we only need it for stk
May 11 21:02:30 <balrog-k1n> _optimize will only ever be called for the highest level BER-TLV
May 11 21:02:51 <denkenz> So what if I have two nested ber-tlvs?
May 11 21:02:57 <balrog-k1n> well, it can be called for the embedded BER-TLVs but it can't memmove so there's no gain
May 11 21:03:11 <balrog-k1n> then the children BER-TLVs are unoptimised
May 11 21:03:19 <balrog-k1n> or we would need to memmove which you didn't like :)
May 11 21:04:16 <denkenz> so you're relying on the 0xff padding part right?
May 11 21:04:22 <balrog-k1n> yeah
May 11 21:04:43 <denkenz> okay, that was the piece I was missing
May 11 21:04:56 <balrog-k1n> i don't think the memmoves were too bad, a memmove on a string cost the same time as a strlen
May 11 21:06:45 <denkenz> Another thing I noticed, the append_text stuff didn't really check for overflow
May 11 21:07:34 <balrog-k1n> oh yes, that is one part i haven't made very defensive
May 11 21:07:46 <balrog-k1n> i can fix it
May 11 21:08:07 <balrog-k1n> the other appends also need to check length but it doesn't seem really practical
May 11 21:10:34 <denkenz> I think we do need the boundary checking for all of this though
May 11 21:10:49 <denkenz> Especially if we decide to make an emulator on it later
May 11 21:11:57 <balrog-k1n> ok, i'll add the error checks
May 11 21:12:15 <denkenz> So I'm pretty much fine accepting the BER / CTLV builder part
May 11 21:12:27 <balrog-k1n> cool!
May 11 21:12:56 <denkenz> I still think we can get away without it, but since its more or less there might as well
May 11 21:13:57 <denkenz> One thing you might want to think about is  combining ber_tlv_iter and ber_tlv_builder
May 11 21:14:05 <balrog-k1n> the thing is even if we did, it wouldn't help much, at most we could remove the multi-byte tag and length (only have one or two byte length)
May 11 21:15:00 <balrog-k1n> so making one struct instead of two?
May 11 21:15:29 <denkenz> yeah, have builder* and iter* functions share the same structure
May 11 21:15:43 <denkenz> sort of like how DBus does it
May 11 21:15:58 <denkenz> the ber_tlv ones differ by 1 member each
May 11 21:16:46 <balrog-k1n> true
May 11 21:17:04 <denkenz> anyway, just a thought
May 11 21:17:22 <denkenz> +               builder->pdu[builder->pos] = 0x7f;
May 11 21:17:24 <denkenz> +               builder->pdu[builder->pos + 1] = (cr ? 0x80 : 0) | (tag >> 8);
May 11 21:17:26 <denkenz> +               builder->pdu[builder->pos + 2] = tag & 0xff;
May 11 21:17:27 <denkenz> +               builder->pdu[builder->pos + 3] = 0;
May 11 21:17:28 <denkenz> What is that last = 0 for?
May 11 21:18:34 <balrog-k1n> it sets the lengths byte to zero,
May 11 21:19:05 <denkenz> in function set_tag?
May 11 21:19:27 <denkenz> I'm confused :)
May 11 21:19:30 <balrog-k1n> so that if you just start a new ctlv, it'll be a valid zero-length ctlv,
May 11 21:19:42 <balrog-k1n> later when you set the tag, it'll still be a valid zero-length ctlv
May 11 21:20:08 <denkenz> shouldn't the whole thing be memset to 0?
May 11 21:21:06 <balrog-k1n> i'm not sure, at that point we don't know what the whole thing's length is
May 11 21:21:37 <denkenz> see, if it is a short tag you don't mess with the length, but on a long tag you do
May 11 21:21:58 <denkenz> hence why I'm asking
May 11 21:22:06 <balrog-k1n> we might want to memset to 0xff, the padding bytes
May 11 21:22:21 <balrog-k1n> yeah, if it's a short tag, then the first two bytes have been zeroed in _next
May 11 21:22:30 <balrog-k1n> the tag byte and the length byte
May 11 21:22:41 <balrog-k1n> if we set a long tag, the length byte moves
May 11 21:23:12 <balrog-k1n> well, yes, it may make sense to zero the if we set a short tag, too
May 11 21:23:19 <balrog-k1n> in case someone changes their mind
May 11 21:23:55 <denkenz> Yeah, the other thing to consider is that _set functions generally can be used out-of-order
May 11 21:24:28 <denkenz> I'm not sure if the cases where set_length can be set before the tag, etc
May 11 21:24:33 <denkenz> are handled
May 11 21:25:01 <balrog-k1n> they're not handled, i should document it in the comments maybe
May 11 21:25:06 <balrog-k1n> i assumed there's no situation where the tag absolutely must be set after some data is already in the PDU
May 11 21:25:24 <denkenz> In that case you might want to cheat
May 11 21:25:33 <balrog-k1n> could as well add the tag parameter to the _next or _init functions,
May 11 21:25:37 <denkenz> e.g. have the user provide the tag information in next
May 11 21:25:42 <denkenz> Exactly
May 11 21:25:47 <balrog-k1n> but it looked nicer ot have the same calls as the decoder
May 11 21:26:23 <denkenz> I know, but this is the lesser evil
May 11 21:27:01 <denkenz> One tends to forget such trivialities like function order, and they come back to haunt you
May 11 21:27:53 <balrog-k1n> ok, let's change it
May 11 21:28:42 <denkenz> The other alternative is to write the tags and length during _optimize
May 11 21:28:52 <balrog-k1n> in that case i'll make the _next call obligatory after _init
May 11 21:29:03 <balrog-k1n> like in the decoder
May 11 21:29:30 <balrog-k1n> my thinking was that it was not logical to call _next after you just initialised the struct
May 11 21:29:54 <denkenz> yeah, the decoder api is actually a little wrong
May 11 21:30:29 <balrog-k1n> but it makes it easier to add new TLVs in loops
May 11 21:30:41 <balrog-k1n> if you look at the unit test, there's a check to not call _next after _init
May 11 21:31:49 <denkenz> hmm, I missed that one
May 11 21:31:50 <denkenz> paste?
May 11 21:33:03 <balrog-k1n> http://pastebin.com/cUMdtmgP
May 11 21:33:20 <denkenz> #
May 11 21:33:21 <denkenz>               if (i[0]++)
May 11 21:33:23 <denkenz> #
May 11 21:33:24 <denkenz>                         g_assert(ber_tlv_builder_next(&builder[1]));
May 11 21:33:26 <denkenz> that part?
May 11 21:33:29 <balrog-k1n> yes
May 11 21:33:50 <balrog-k1n> (and it's actually wrong, it should be &builder[0])
May 11 21:33:57 <balrog-k1n> (copy-pasted)
May 11 21:35:14 <denkenz> I really hate that i[0]++ thing btw
May 11 21:35:48 <balrog-k1n> it can be removed if _next needs to be called for every tlv
May 11 21:36:28 <denkenz> just talking about the int i[2]
May 11 21:36:35 <denkenz> Don't do that, just name them i and j or something ;)
May 11 21:37:06 <balrog-k1n> i don't know.. different names suggest they have different uses
May 11 21:38:17 <denkenz> Actually I'm totally lost in that unit test
May 11 21:38:28 <denkenz> wtf is builder[2] doing?
May 11 21:38:54 <balrog-k1n> it's two builders, one top level and one nested
May 11 21:39:19 <denkenz> Ok, so whats wrong with calling them top and recurse?
May 11 21:39:35 <balrog-k1n> kind of ugly :)
May 11 21:39:50 <denkenz> but readable
May 11 21:39:59 <denkenz> Right now I'm screaming 'My eyes, my eyes' 
May 11 21:40:14 <balrog-k1n> if you had three of them, you'd need top, recurse and recurse_recurse, and so on
May 11 21:40:39 <denkenz> That is actually fine too
May 11 21:41:13 <balrog-k1n> well, ok, i'll stop doing the arrays
May 11 21:41:54 <denkenz> so, I'm definitely leaning towards next() taking the tag stuff
May 11 21:42:56 <denkenz> maybe even rename it to open_container? :)
May 11 21:43:44 <balrog-k1n> how about start()?
May 11 21:45:52 <denkenz> open and close get my vote
May 11 21:46:07 <denkenz> and next isn't bad either
May 11 21:46:44 <denkenz> maybe even have optimize do its magic on every close
May 11 21:46:49 <denkenz> then you have symmetry
May 11 21:48:11 <balrog-k1n> but optimize is only done once for the entire pdu and returns the length of all the consecutive TLVs together
May 11 21:48:17 <balrog-k1n> so we don't need close
May 11 21:48:57 <denkenz> yeah I know
May 11 21:49:04 <denkenz> so leave it at next
May 11 21:49:08 <balrog-k1n> ok
May 11 21:49:15 <denkenz> and just have it take the tag info
May 11 21:49:28 <balrog-k1n> makes sense
May 11 21:53:00 <denkenz> and see if you want to combine the structs, maybe it looks ugly, maybe it doesn't
May 11 21:53:48 <denkenz> Also, please expand the unit tests for ber tlv builder with a few more examples, like maybe some of the EFopl crap
May 11 21:55:39 <balrog-k1n> okay
**** ENDING LOGGING AT Wed May 12 02:59:56 2010