[Marble-devel] Review Request: add APRS data display to marble

[Wes Hardaker]

> On 2010-04-29 09:32:03, Torsten Rahn wrote:
> > trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsFile.cpp, line 26
> > <http://reviewboard.kde.org/r/3843/diff/1/?file=24948#file24948line26>
> >
> >     What is "Direct"? :-)
> 
> Wes Hardaker wrote:
>     Not heard through a repeater.  IE, they're close enough that your antenna can pick them up directly.
> 
> Torsten Rahn wrote:
>     Well what I was hinting at was that maybe there is a more intuitive way to name this which would better get the point across. But that might not necessarily be the case. So it might be fine as is.

Ah.  I'm open to suggestions of course if someone wants to pick a different name.


> On 2010-04-29 09:32:03, Torsten Rahn wrote:
> > trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsGatherer.h, line 36
> > <http://reviewboard.kde.org/r/3843/diff/1/?file=24950#file24950line36>
> >
> >     Should be an enum.
> 
> Wes Hardaker wrote:
>     Why?  It's either supposed to dump or not.  That's very boolean to me.
> 
> Torsten Rahn wrote:
>     Well, this is a plugin and not public API, so this issue is not that important. However in general it's a matter of good API design to only use bools where the boolean meaning is obvious:
>     
>     joke->setEnabled(true); // the meaning of the boolean is obvious in this case
>     
>     Humour * joke = new Humour( "Programmer's joke", true ); // the meaning of the boolean in this case is not obvious 
>     
>     Humour * joke = new Humour( "Programmer's joke", RollingOnTheFloorLaughing ); // using an enum it's much more readable :-)

I think I understand your point now, though it still makes perfect sense as a bool to me.

That being said, your comment started me thinking and it may be that I want more than one dump state in the future.  Such as "dump everything" vs "dump only the packets I couldn't understand" which would indicate a multi-state and thus an enum would make more sense.


- Wes


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3843/#review5293
-----------------------------------------------------------


On 2010-04-30 03:27:20, Wes Hardaker wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3843/
> -----------------------------------------------------------
> 
> (Updated 2010-04-30 03:27:20)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> Adds support for displaying HAM radio aprs data to the map pulled from three different possible sources.  The TCP/IP source is the default that is enabled and usable by everyone.  The others would require special sources and default to off.  The icons, which are binary files, can be found at:  http://www.hardakers.net/code/marble/aprs-icons.tar.gz
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdeedu/marble/data/CMakeLists.txt 1119036 
>   trunk/KDE/kdeedu/marble/src/plugins/render/CMakeLists.txt 1119036 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsConfigWidget.ui PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsFile.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsFile.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsGatherGen.pl PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsGatherer.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsGatherer.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsGatherer_mic_e.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsObject.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsObject.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsPlugin.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsPlugin.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsSource.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsSource.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsTCPIP.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsTCPIP.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsTTY.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsTTY.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/CMakeLists.txt PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/GeoAprsCoordinates.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/GeoAprsCoordinates.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/posix_qextserialport.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/qextserialport.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/qextserialport.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/qextserialport_global.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/win_qextserialport.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/3843/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> screenshot of california with data
>   http://reviewboard.kde.org/r/3843/s/378/
> 
> 
> Thanks,
> 
> Wes
> 
>