On Wed, Jul 29, 2009 at 6:08 PM, Laurens Van Houtven <span dir="ltr">&lt;<a href="mailto:lvh@laurensvh.be">lvh@laurensvh.be</a>&gt;</span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">


As you might remember, we (thanks to the adhoc code reviews from<br>
glyph, tazle, dash andexarkun) decided on an implementation slightly<br>
different from the original one, using:<br>
    - an NMEAReceiver class, which *only* does the receiving bit and<br>
is perfectly stateless: this does things like unpacking the sentence<br>
and checking the checksum<br>
    - an IPositioningReceiver (now renamed IPositioningProvider, but<br>
I&#39;m not sure that&#39;s the right name yet) interface that has a bunch of<br>
*sane* callbacks -- ones you actually want instead of ones that are<br>
directly a consequence of NMEA being a steaming pile of dung</blockquote><div><br>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.)<br>
 </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">   - an NMEAAdapter that would adapt the receiver and keep as much<br>
state as necessary internally to produce the meaningful callbacks as<br>
defined in the interface<br>
</blockquote><div><br>This all sounds good.<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">Here&#39;s the code right now: <a href="http://bit.ly/1azA5q" target="_blank">http://bit.ly/1azA5q</a><br>

The interface lives in this file: <a href="http://bit.ly/3wWL2c" target="_blank">http://bit.ly/3wWL2c</a><br>
</blockquote><div><br>I think you mean the interface lives in this file: <a href="http://bit.ly/5Jc7p">http://bit.ly/5Jc7p</a>   (Your second link was to a directory.)<br><br>A minor nit here, would you put the interfaces in their own module, &#39;twisted.position.ipositioning&#39;?  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.<br>
<br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">I originally thought that the NMEAAdapter (which adapts an<br>
NMEAReceiver to the  IPositioningReceiver interface) was really an<br>
adapter in the twisted.python.components sense of the word, so that<br>
you could do something like PositioningReceiver(NMEAReceiver()) and<br>
you&#39;d get and NMEAAdapter object, as described in the Twisted<br>
components howto.<br>
</blockquote><div><br>This is the relationship, but it&#39;s backwards :).<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">The problem is attaching the method stubs in NMEAReceiver (nmea_GPGGA,<br>

nmea_GPRMC, nmea_GPGSV) to the methods in the NMEAAdapter that<br>
actually interpret these sentences. Ordinarily, you&#39;d subclass that<br>
class to override those stubs. How do you feel this should be done<br>
while keeping the adapter? Should we keep the adapter at all?</blockquote><div><br>There are multiple layers here.  Let&#39;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.<br>
<br>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.<br>
<br>I&#39;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.<br>
<br>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.<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Of course, I could do ugly things in the adapters&#39; __init__ like:<br></blockquote><div><br>Let&#39;s please not do that; there&#39;s no reason to.<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
But that looks like a really bad idea: it&#39;s like I&#39;m subverting<br>
inheritance. Instead, I&#39;m currently doing this by subclassing the<br>
receiver (thanks to exarkun for this suggestion):</blockquote><div> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">class NMEAAdapter(NMEAReceiver):<br>

</blockquote><div> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">This fixes the method stubs being correctly assigned (I suppose the<br>

correct term would be overriding, through plain old inheritance). The<br>
problem is that neither the interface nor components/adapters are<br>
involved in any way, so I feel like I&#39;m missing something important.<br>
</blockquote><div><br>This is an OK approach, convenient, and less code than what I&#39;m proposing, but it means that users need to subclass something to get the functionality they want, and there&#39;s no nice, clean interface that just describes what your application needs to do at each level.<br>
<br>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&#39;d rather that it be nice and clean.  When we did twisted.words for example, I assumed that everyone would want the nice, &quot;high level&quot; 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&#39;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 :).<br>
</div></div>