[Twisted-Python] positioning framework

[Laurens Van Houtven]

Hi Kasun!

I'm lvh, the primary author of the ticket.

On Wed, Feb 12, 2014 at 3:19 AM, Kasun Thennakoon <tmkasun at gmail.com> wrote:

> It took long time to go through the ticket[1] and understands,
> still i couldn't find any issue with it, instead its working perfectly for
> me. and thanks to the twisted positioning NMEA adapter, it become easy to
> me to write a adapter for the device(MVT380  device[2]) which i'm using in
> my project[3].
>

Awesome! Glad to hear it's working so well for you.

 IMHO,
>
> 1. isn't it good to use items() rather than iteritems() since python 3.x
> has deprecated iteritems(),
>

There are many more Python 3 issues, I suspect. Both of those would work;
I'm personally a bigger fan of using things like six' iteritems(d); which
does d.items() on 3.x and d.iteritems() on 2.x, since you guarantee that
you have identical semantics (modulo all the fancy things you can do with
views that you can't do with iterators). I think there's a few cases where
I don't really want *all* the items (although, of course, premature
optimization is the root of all evil :)).


> 2. In nmea._UNIT_CONVERTERS dictionary mapper, can't see any usage of key
> 'K' which convert Km/h unit to meters/s because it has a FIXER
> for speedInKnots but not for speedInKMh, i think in $GPVTG NMEA string type
> (it is not defined in _SENTENCE_CONTENTS) string they use text 'K' to
> indicates that speed over ground is in kilometers/hour.
>

That's there for forward compatibility as well as compatibility with really
bad GPSes that use illegal units.

The fix for that lives in:


 def _fixUnits

So it is independent from any particular unit. That just blindly indexes
into that dict; so if a GPS says something is in kph, it will convert to
m/s, even when according to the spec kph isn't a valid unit for that.

Also as you mentioned for some *other* sentences it *is* a valid unit, but
that's okay, we can just leave it there and then later add GPVTG support
and get units conversion for free!

 other than those suggestions,its works perfectly well. i wonder why this
> positioning module didn't merged with the trunk yet,and i'll be happy if i
> can help to get it merged.
>

The ticket is up for review, and has been for a long time. Since you have
apparently reviewed the ticket and used the code, would you mind giving it
a review so it can (finally!) get into Twisted itself? Review instructions
are here:

https://twistedmatrix.com/trac/wiki/ReviewProcess

Some of the review comments so far have asked for extensive changes. I am
reminded of this e-mail by Glyph:

https://twistedmatrix.com/pipermail/twisted-python/2014-January/027894.html

... which is quite long, but I think the bottom line is that you should
just trust the review process and do that :)

hth
lvh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://twistedmatrix.com/pipermail/twisted-python/attachments/20140212/6bcfd208/attachment.html>