[Marble-devel] Review Request: add APRS data display to marble
Dennis Nienhüser
earthwings at gentoo.org
Thu Apr 29 22:09:11 CEST 2010
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3843/#review5302
-----------------------------------------------------------
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsObject.h
<http://reviewboard.kde.org/r/3843/#comment4981>
Different to addSeenFrom?
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsObject.cpp
<http://reviewboard.kde.org/r/3843/#comment4941>
Move to the bottom of the file, if possible
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsObject.cpp
<http://reviewboard.kde.org/r/3843/#comment4942>
Intentionally initializing the QString with 0 here? Looks odd.
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsObject.cpp
<http://reviewboard.kde.org/r/3843/#comment4943>
Intentionally initializing the QString with 0 here? Looks odd.
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsObject.cpp
<http://reviewboard.kde.org/r/3843/#comment4946>
The method name indicates a setter, but it is more (calculates a suitable color, sets it and returns it). I'd find it easier to understand if the method was
QColor calculatePaintColor(int from, QTime time, int fadetime)
and the caller would be responsible to modify the painter.
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsObject.cpp
<http://reviewboard.kde.org/r/3843/#comment4953>
Method can be const (another reason not to prefix it with set)
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsPlugin.cpp
<http://reviewboard.kde.org/r/3843/#comment4965>
use an unnamed namespace to avoid naming conflicts.
namespace {
const int timeBetweenDownloads = 30000;
}
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsPlugin.cpp
<http://reviewboard.kde.org/r/3843/#comment4967>
Leaking m_mutex
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsPlugin.cpp
<http://reviewboard.kde.org/r/3843/#comment4969>
What's that used for?
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsPlugin.cpp
<http://reviewboard.kde.org/r/3843/#comment4971>
See above. Debug leftovers?
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsSource.h
<http://reviewboard.kde.org/r/3843/#comment4974>
const?
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsSource.h
<http://reviewboard.kde.org/r/3843/#comment4975>
const?
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsTCPIP.cpp
<http://reviewboard.kde.org/r/3843/#comment4977>
Local variables should not be prefixed m_
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsTCPIP.cpp
<http://reviewboard.kde.org/r/3843/#comment4978>
Hardcoded hostname / port vs. members?
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/GeoAprsCoordinates.h
<http://reviewboard.kde.org/r/3843/#comment4979>
const?
trunk/KDE/kdeedu/marble/src/plugins/render/aprs/GeoAprsCoordinates.h
<http://reviewboard.kde.org/r/3843/#comment4980>
const?
- Dennis
On 2010-04-29 05:30:51, Wes Hardaker wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3843/
> -----------------------------------------------------------
>
> (Updated 2010-04-29 05:30:51)
>
>
> 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 1089346
> trunk/KDE/kdeedu/marble/src/plugins/render/CMakeLists.txt 1089346
> 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
>
>
More information about the Marble-devel
mailing list