[Marble-devel] Review Request: Clean gpsd position provider shutdown

[Dennis Nienhüser]

> On 2010-04-28 00:01:00, jmho wrote:
> > /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/gpsd/GpsdConnection.cpp, line 85
> > <http://reviewboard.kde.org/r/3794/diff/1/?file=24417#file24417line85>
> >
> >     should not the "#endif" be here? otherwise for gpsd before 2.90 the gpdsInfo signal will not be emitted anymore.

The patch adds a second emit statement. Both versions handle data retrieval and signal emission by themselves in the version dependent code sections. Gpsd 2.3x returns a null pointer when the connection is lost and valid data otherwise, while gpsd 2.9x can also return partial data that needs to be discarded (PACKET_SET bit). It's easier to handle this separately.


- Dennis


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


On 2010-04-23 19:38:29, Dennis Nienhüser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3794/
> -----------------------------------------------------------
> 
> (Updated 2010-04-23 19:38:29)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> When gpsd is running, but no data is available, the gpsd position provider is currently blocked indefinitely for libgsp >= 2.90 by the poll() call. In that case the destruction of the thread hangs in wait() after terminate(), which leads to marble freezing when turning gpsd off. It may also cause the crash reported in https://bugs.kde.org/show_bug.cgi?id=234311.
> 
> The patch changes the behavior to only call the blocking method when data is available (and it won't block in that case). Additionally it removes the possibly dangerous (according to qt docs) terminate() call.
> 
> 
> This addresses bug 234311.
>     https://bugs.kde.org/show_bug.cgi?id=234311
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/gpsd/GpsdConnection.cpp 1117994 
>   /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/gpsd/GpsdPositionProviderPlugin.cpp 1117994 
> 
> Diff: http://reviewboard.kde.org/r/3794/diff
> 
> 
> Testing
> -------
> 
> Using libgps-2.92-4 and some gps device supported by gpsd.
> 
> 
> Thanks,
> 
> Dennis
> 
>