[Twisted-Python] twisted.positioning: status report and request for comments/feedback

[Glyph Lefkowitz]

On Wed, Jul 29, 2009 at 6:08 PM, Laurens Van Houtven <lvh at laurensvh.be>wrote:

> As you might remember, we (thanks to the adhoc code reviews from
> glyph, tazle, dash andexarkun) decided on an implementation slightly
> different from the original one, using:
>    - an NMEAReceiver class, which *only* does the receiving bit and
> is perfectly stateless: this does things like unpacking the sentence
> and checking the checksum
>    - an IPositioningReceiver (now renamed IPositioningProvider, but
> I'm not sure that's the right name yet) interface that has a bunch of
> *sane* callbacks -- ones you actually want instead of ones that are
> directly a consequence of NMEA being a steaming pile of dung


Presumably the future plan is to have IPositioningProvider that receives
data from something other than NMEA, as well?  (Other GPS protocols,
whatever cell phones give you for triangulation, skyhook, etc.)


>    - an NMEAAdapter that would adapt the receiver and keep as much
> state as necessary internally to produce the meaningful callbacks as
> defined in the interface
>

This all sounds good.

Here's the code right now: http://bit.ly/1azA5q
> The interface lives in this file: http://bit.ly/3wWL2c
>

I think you mean the interface lives in this file: http://bit.ly/5Jc7p
(Your second link was to a directory.)

A minor nit here, would you put the interfaces in their own module,
'twisted.position.ipositioning'?  There are use-cases where one may want to
import interfaces, but not actually load any of the code from a package, so
in new code we try to segregate them out into their own modules.

I originally thought that the NMEAAdapter (which adapts an
> NMEAReceiver to the  IPositioningReceiver interface) was really an
> adapter in the twisted.python.components sense of the word, so that
> you could do something like PositioningReceiver(NMEAReceiver()) and
> you'd get and NMEAAdapter object, as described in the Twisted
> components howto.
>

This is the relationship, but it's backwards :).


> The problem is attaching the method stubs in NMEAReceiver (nmea_GPGGA,
> nmea_GPRMC, nmea_GPGSV) to the methods in the NMEAAdapter that
> actually interpret these sentences. Ordinarily, you'd subclass that
> class to override those stubs. How do you feel this should be done
> while keeping the adapter? Should we keep the adapter at all?


There are multiple layers here.  Let's have an object for each layer.  The
NMEA protocol parser object should send NMEA-sentence events to a sentence
receiver object, then there should be an implementation of the
sentence-receiver interface which generates positioning events and sends
them on to a positioning-receiver interface.

The dispatch from the nmea-sentence-received method to the nmea_-prefixed
methods is an internal implementation detail of the specific implementation
of the NMEA-event-to-positioning-event object, which users can subclass if
they want to get the same dispatch.

I've attached a sketch, but it elides certain details like set-up and
tear-down; there should be methods which allow users to tell when sentences
and positions start and stop arriving.  The gist would be to have a
connectionMade and connectionLost invoke startReceivingNMEA,
stopReceivingNMEA, which in turn would invoke startReceivingPositions, and
stopReceivingPositions.

There should also be a factory which nicely hooks everything together and
takes only a factory for your IPositioningProvider, so that users can
quickly get started with a positioning provider.

Of course, I could do ugly things in the adapters' __init__ like:
>

Let's please not do that; there's no reason to.

But that looks like a really bad idea: it's like I'm subverting
> inheritance. Instead, I'm currently doing this by subclassing the
> receiver (thanks to exarkun for this suggestion):



> class NMEAAdapter(NMEAReceiver):
>


> This fixes the method stubs being correctly assigned (I suppose the
> correct term would be overriding, through plain old inheritance). The
> problem is that neither the interface nor components/adapters are
> involved in any way, so I feel like I'm missing something important.
>

This is an OK approach, convenient, and less code than what I'm proposing,
but it means that users need to subclass something to get the functionality
they want, and there's no nice, clean interface that just describes what
your application needs to do at each level.

That might be fine if you really want to punt on allowing anyone to deal
with NMEA data directly, but given that the object is going to be public and
subclassing is going to be possible anyway, I'd rather that it be nice and
clean.  When we did twisted.words for example, I assumed that everyone would
want the nice, "high level" interface that abstractly defined chat events,
but instead users want to write XMPP and IRC bots directly, dealing with
gross protocol implementation details in their applications.  I suspect that
will be less common in the world of positioning data, but I certainly don't
know for sure, and it would be nice to avoid having to suppot a grotty
subclassing interface in the future, since the whole point of this nice new
system is to get rid of our old grotty interfaces :).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20090729/7e690e48/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: positioning-sketch.py
Type: application/x-python
Size: 2323 bytes
Desc: not available
URL: </pipermail/twisted-python/attachments/20090729/7e690e48/attachment-0002.bin>