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 <[email protected]> wrote: > Hi Kasun! > > I'm lvh, the primary author of the ticket. > > On Wed, Feb 12, 2014 at 3:19 AM, Kasun Thennakoon <[email protected]>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 > [email protected] > http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python > > -- ~ Kasun Thennakoon <http://me.knnect.com>
_______________________________________________ Twisted-Python mailing list [email protected] http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
