[Twisted-Python] positioning framework

[Kasun Thennakoon]

Hi lvh,
Yes sure i will do some more testing with the module and do  review,  thank
you for the links

On Wed, Feb 12, 2014 at 1:58 PM, Laurens Van Houtven <_ at lvh.io> wrote:

> 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
>
> _______________________________________________
> Twisted-Python mailing list
> Twisted-Python at twistedmatrix.com
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
>
>


-- 
~ Kasun Thennakoon <http://me.knnect.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20140213/721bda06/attachment-0002.html>