[Twisted-Python] positioning framework
Hi, 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]. IMHO, 1. isn't it good to use items() rather than iteritems() since python 3.x has deprecated iteritems(), 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. 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. [1] https://twistedmatrix.com/trac/ticket/3926 [2] http://www.mvt380.com/ [3] https://github.com/tmkasun/port/blob/kasun/server_Program/server_twisting/sy... -- ~ Kasun Thennakoon <http://me.knnect.com>
Hi Kasun! I'm lvh, the primary author of the ticket. On Wed, Feb 12, 2014 at 3:19 AM, Kasun Thennakoon <tmkasun@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
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 <_@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@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@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
-- ~ Kasun Thennakoon <http://me.knnect.com>
participants (2)
-
Kasun Thennakoon -
Laurens Van Houtven