RE: [RFC 1/3] doc: addidng documentation for basic assisted gps

[Marcel Holtmann <marcel@holtmann.org>]

[-- Attachment #1: Type: text/plain, Size: 2750 bytes --]

Hi Waldo,

> > oFono is strict CamelCase, even for abbreviations.  You do this
> > correctly above (AgpsManager) but not here.  Please be consistent.  And
> > it might be a good idea to not have abbreviations at all.  What does LCS
> > stand for in this case?
> 
> It refers to terminology from 3GPP TS 04.31:
> RRLP -> Radio Resource LCS Protocol
> LCS -> Location Services

I am with Denis here. If you mean Location Services, then call it that
way. Even within the API naming we are trying to be consistent and avoid
abbreviations whenever possible.

> > > +			The raw frame data is formatted as the concatenated
> > > +			sequence of the two digit hexadecimal representation
> > > +			of each of its octets. Example: "00FC2345"
> >
> > Sending hex-encoded data screams as being non-portable and AT command
> > modem specific.  Is this really your intent here?
> 
> The intent is to send an array of octets. A string seemed a debug-friendly way of
> mapping it onto a DBUS API, but other suggestion for a suitable DBUS type are welcome.

That is an array of bytes what you are looking for. A string of hex
encoded octets is pretty much a bad idea for an API.

It might be convenient for an AT command based modem, but a bad idea for
an D-Bus based API.

> > > +		void RequestFineTimeInjection(string rat, uint16 pulselength)
> > It seems to me that making the upper layers pass in the rat as a
> > parameter to this function is a pretty bad idea.  You should either have
> > the atom figure this out if it can (e.g. by using netreg atom) or have
> > the driver take care of this directly.
> 
> I was asked to add it as the AGPS manager that makes the request presumes a certain RAT and there is no point in generating the pulse if the RAT has changed since.
> See also http://lists.ofono.org/pipermail/ofono/2010-May/002347.html

Seems still wrong to me. Why would an application have to deal with
this. This looks like an internal implementation detail to me. Pushing
such a thing out to the application is against the philosophy of our
D-Bus APIs to simplify things.

> > +		FineTimeInjectionNotification(dict radioframenumber)
> > +
> > +			Notification about fine time injection pulse
> > +			generated by modem. The radioframenumber dict
> > +			is defined as follow:
> > +
> >
> > Why is this returned in a separate signal as opposed to a reply to the
> > RequestFineTimeInjection method call?
> 
> The concern was that the reply would not be immediate, but I don't think it's that much delayed that it runs in any DBUS timeouts, so I think it can be turned into a reply instead.

And so is the signal. Both are bound to the scheduling of the D-Bus
daemon.

Regards

Marcel