Bug report #4071
GPS Information (Live GPS tracking) fixes and improvements
Status: | Closed | ||
---|---|---|---|
Priority: | Normal | ||
Assignee: | Steven Mizuno | ||
Category: | C++ Plugins | ||
Affected QGIS version: | Regression?: | No | |
Operating System: | Easy fix?: | No | |
Pull Request or Patch supplied: | Yes | Resolution: | |
Crashes QGIS or corrupts data: | Copied to github as #: | 14054 |
Description
GPS Information widget
Problems found:- high CPU utilization, approximately 50% on a dual-core processor, which is not good for portable use (see #3826 - may be related)
- satellite graph
- azimuth is backwards and rotated 90 degrees
- number of elevation circles is not what is set in object set up
- elevation is backwards - runs from 90 at edge to 0 at center - signal graph and satellite graphs flash with data received
- satellite graph doesn't indicate which bar is which
- the Live GPS Tracking menu item on the View menu does not indicate whether the GPS info is active; the state is also confusing because of being a dock widget - once it is active the GPS window can be hidden or shown via the context menu of the main window as well as the View, Panels menu
- the Track color dialog sets the color to black if Cancelled
- the map centering when leaving extent (fixed at map extent) is not as useful as if it were 50% or user adjustable
From personal use of the GPS info tool in the field, I realized that the Add feature and Add vertex buttons should be visible at all times, not just the Position page; and that keyboard navigation and returning focus to the Add feature button, for example, is very important.
Also, it is very useful to have more GPS fix status/quality information visible in order to decide whether to record a point.
It is also important to be able to have added features automatically saved, rather than having to remember to do this. This also helps guard against data loss if something happens to QGIS.
I am taking ownership of this ticket as I have already fixed most of the problems and improved the tool in my working copy, but I am not quite ready to submit a patch.
Fixes:- satellite graph has proper orientation - azimuth and elevation (found some tricky set up issues with QwtPolar)
- process data for display only for page currently visible, as much as possible (cuts CPU utilization)
- process data about the satellites and draw graphs only after complete set of data is received (fixes flashing of graphs/ CPU util.)
- Color dialog return is tested for valid color
- disable undo/redo and rich text capabilities on text controls (cut CPU util.)
- replace various instances of object deletion/creation on each cycle of display processing with creating the objects if necessary and retaining them until no longer needed (map marker, the svg symbol, objects on both graphs)
- moved Add Vertex, Add Feature, Reset feature buttons to top of window outside of any display pages
- added a quick glance fix quality indicator, also always visible - gray, red, yellow, green states to show No data/no connection, No fix, 2D, 3D respectively
- give user more choice in tool configuration - % of map extent when centering (10-100% range seems reasonable)
- added 'Auto save added feature' checkbox and call to layer commit function
- grouped some related controls in Options
- indented additional settings for Connection and Map centering groups to eliminate some labels and explanatory text (see pix)
- removed 'GPS' prefixed on some control labels as this is redundant - these are in the GPS Information window
- cosmetic improvement of layout spacing to even things out and compact it somewhat
- added various display fields on Position page and placed all controls in a scroll area widget
- added 57600 bps setting for serial device connection to accomodate one of my GPS receivers; others probably should be added, but this may slow down GPS device detection
- remove cursor on disconnect GPS
- changed auto vertex adding to be dependent on a GPS fix is valid flag rather than not 'near zero'
I have tested these changes with 8- and 12-channel Garmin receivers at 4800 bps and a Geneq SXBlue II GPS at rates up to 57600 bps. CPU utilization is generally somewhat less at higher rates.
On a specific computer the CPU utilization is now ~2-5% avg. at most, just on data receive (not redrawing map)
I haven't yet figured out what graph to use for the signal graphing, so that is not fixed.
I am not able to test the gpsd connection at this time. I believe the changes should not have any serious effect.
I have also been experimenting with adding a data log file capability to save all raw data (NMEA sentences) as I sometimes need to look back at the GPS conditions and the point/linestring/polygon features don't have anything more than location.
All the fixes and improvements were developed on Microsoft Windows. I believe they will work on other platforms.
Attached are some screen shots showing the changed UI as it is currently.
History
#1 Updated by Tim Sutton over 13 years ago
Hi Steven
I have read through your notes and your changes look great. Please forward me your patch (or pull request) and I will test and commit your changes when you are ready.
Regards
Tim
#2 Updated by Tim Sutton over 13 years ago
Hi
One other thing I should add - you have marked the ticket for 1.7.1, but for the 1.7.x series we should only be applying bug fixes, no new features. We will need to decide how to proceed on this since your changes introduce both bug fixes and ui changes / new features.
Regards
Tim
#3 Updated by Werner Macho over 13 years ago
- Pull Request or Patch supplied set to No
I'd go with trying to apply it only for master ..
If there are small fixes for 1.7.x - ok - bugfix ..
but IMHO no UI Changes to a released version .. (probably others might think other)
regards
Werner
#4 Updated by Steven Mizuno over 13 years ago
- File doc_for_4071_GPSInformation-20110803.txt added
- Pull Request or Patch supplied changed from No to Yes
- File patch_for_4071.diff added
Hi Tim, and other interested parties,
Here is a patch file containing the fixes and improvements I have been working on. Most problems identified are fixed, but a few have not been as noted below. Perhaps round 2 will take care of them.
The patch was taken against my local copy of trunk, updated on 8/8/11.
As there are many changes to the UI, I agree that this should not be applied to version 1.7.x.
I am including a file that has some notes about these changes to the GPS tool for use in the User Guide.
The GPS tool works well visually as a floated window or docked by itself on the right side of the QGIS window (assuming no other widgets are docked there) if one wants to see all or most of the display fields.
Even though I don't have a gpsd data source available, I did check the operation of the edit boxes for the gpsd configuration and found a minor problem as noted below.
I would appreciate feedback on the modified GPS information tool, especially about whether the gpsd connection has any problems.
I have recently tested the modified GPS tool on a low-end AMD C-50 processor (dual-core, 1GHz clock) computer and am getting avg. CPU utilization <1%. It builds up to 5% or more if the track has a very large number of points (upwards of 5000 or so) to draw.
An Intel Atom processor (fake dual-core) based netbook runs around 3-4% indication.
There are a number of minor changes that I might not have noted. If you want more information on anything, please ask.
I have some observations on possible future improvements:
There is currently no interface to Python, so it isn't possible to do real-time GPS data processing with Python plugins, for example.
I believe that this tool should be split into three or more parts - a GPS connection module, a track log/feature addition module, and log file or other GPS data processing modules. The GPS connection module would establish the connection to a GPS device for other modules to hook into and display information like position and satellite graph.
It isn't practical to keep adding more ways to use the GPS data to this tool.
I believe this was the intent as there is a connection registry that allows other modules to use an established connection.
It is not a good idea to size or move the GPS Information window (or any QGIS window) while a GPS connection is active. This is due to the GPS connection handling being in the same event loop as the window (single-threaded application at present). This leads to loss of data while the event loop is suspended. May need to think about a separate thread for the GPS connection.
Additional problems found:- nmealib does not process fractional seconds of time properly - 0.70 becomes 0.07 for example
- the cursor show/hide state isn't saved/restored
- displayed track: if the map CRS is changed after some points are stored the track likely has incorrect coordinates. The user shouldn't have to think about this and probably doesn't even know that this could be a problem. Note that the list of points used to add a feature is not affected. - NOT FIXED - need to intercept the map CRS change and reproject the track points
- there is a situation where a serial port (USB attached device with RS-232 emulation) has been detected by QGIS, but becomes disconnected before you attempt to Connect to it. There is no connection (there can't be, obviously) and it appears that the GPS detector doesn't signal failure. The connect button is in the connected state (toggled on), but its label is 'Connect'. It takes a click to return to the normal state. This is confusing behavior.
- the gpsd Port edit box does not accept zero (0) in any position. Not a problem for the standard port, but for a non-standard port...
- it is possible to hit the Connect button a second time before a connection attempt completes or times out, which will be confusing to the user. The button will be in the off state but may show Disconnect (if connection was successful) and would need two presses to disconnect. NOT FIXED. The obvious thing to is disable the button for the duration of the connect attempt until success or failure, then enable it. However, this seems like it could be confusing as well. And it will cause a transfer of focus to the next control in the tabstop order, which is also confusing.
- I believe there should be a Help button somewhere (where ?) and associated help text. - nothing done yet
- in Connect handling, the case of Connection, Serial device, but no device listed is not very good. A message box is popped about no path to GPS port, then unchecks the Connect button which now provokes the GPS disconnected status message. This could be a bit confusing. Since I discovered this very late (I thought I had checked this situation) I haven't tried to do anything. There probably should be a statusbar message about it and no message box. minor issue
- removed the 'Live GPS tracking' action item on the View menu and reworked when and how the GPS Information dock widget is built. This allowed some objects and functions to be removed from QgisApp, so there is an API change (I don't believe that this really affects much). This also simplified the coding.
Now the widget is always built on QGIS start and is shown/hidden from Panels on the View menu or the main window context menu, like various other tools, such as Browser.
The original logic always built the GPS widget after the first use of it on a given installation. - nmealib fixed to parse fractional seconds correctly and to deal with thousandths of a second instead of hundredths
- problem of disconnected serial port not showing failure is due to the immediate failure of the serial port open, which occurs before some signal/slot connections are set up. Removed the call to advance() in the QgsGPSDetector constructor, then in the Connect button handling call detector->advance() after the signals/slots are set up. Now there is a Failed to connect GPS status message virtually immediately on pressing Connect.
- cursor show/hide state is now saved/restored
- changed gpsd Port input mask from "ddddd" which only allows digits 1-9 to "00000" which allows zeroes. The mask "D0000" which I expected would require digits 1-9 in the first position actually inserts a space if a zero is typed in the first position. I didn't spend any more time trying to find a different mask.
- removed the horizontal and vertical accuracy indicators - they had been hidden and are hard to interpret without a scale; on some platforms, were annoying due to animation; replaced with text value displays
- moved Reset feature button to right of Add vertex, and set its tooltip to 'Reset track' - all this button does is clear the track object and list of points captured. It does not reset an added feature as it appears to imply.
- relabeled the Add Vertex button to 'Add track point'; and the Auto-add vertices check box to 'Automatically add points' (this is in the Track group)
reasons:
a. vertex is a rather technical term and isn't used elsewhere, I believe, at least in the core QGIS.
b. it operates on the track object, which wasn't exactly obvious
c. to be consistent and distinguish track from features of a layer (the track may become a feature, of course) - further rearranged the Options page to place controls/grouping in what I believe to be a good order of usefulness
- the 'Add feature' button tracks layer geometry type and editability for enabling it and changing its label - 'Add point' for example. This allows removal of a number of tests and their associated message boxes as the button can't be clicked if the layer type isn't correct or isn't in editing mode.
- added data log file capability - this is very basic, with minimal error handling. The user must choose a file using a save as file dialog as there is no file name validity checking if the name were typed into a line edit box.
The NMEA sentences received from the GPS are appended to the chosen file and have a ==== marker at the beginning of each chunk.
The log file is opened and closed on GPS connect/disconnect. - added fractional seconds in date/time handling - this may be useful for some users if they have a GPS unit that provides fixes at >1 per second. fractions are not normally displayed, but see next item
- added a date/time format string capability. This was used for testing the fractional seconds handling. the registry setting: /gps/dateTimeFormat
this uses the QDateTime::toString() format coding. I have not provided any UI for setting this. - relabeled Elevation to Altitude on the Position page. I have looked at various GPS receiver manuals and find that Altitude appears to be more commonly used (and is actually the antenna altitude). Also, elevation can refer to satellite height in degrees above the horizon, which may be abiguous.
- indicate units for speed and altitude in the display fields to avoid confusion
- tooltips added to various controls, but not all. In particular, the page buttons and the position page fields have tooltips.
- the Connection and Log file groups on the Options page are disabled while a GPS device is connected. This prevents changes to these controls, which could be confusing as they wouldn't affect the current connection.
- added several members to the QgsGPSInformation struct for more detail; when cleared some members are given values that are to be considered invalid (the GPS unit would not normally send these values). These are used for determining whether a fix is valid.
- in qextserialenumerator.h - changed the logic for MinGW compilation to eliminate compiler warnings about redefinition.
- changed the settings location for track color and simplified color handling - from /qgis/gps/line_(color) to /gps/trackColour as a single value - so all GPS settings are together
- changed track width setting so the value isn't saved on every change; now, almost all settings are saved in the destructor and restored in the constructor only; the exceptions: last port, log file path
- I changed much of the Debug panel data handling while learning how this tool worked. Most of the changes are left in and most of the code is disabled by #if 0 blocks. I believe that the Debug part probably can be removed now as it is not shown to users and most of the information presented is shown elsewhere.
- removed (by #if 0 blocks) some functions that weren't used or are no longer used
Steve
#5 Updated by Jürgen Fischer over 13 years ago
- Status changed from Open to Closed
applied in ee215805d0a6a5871cbb072b28efa53c24f674e6 - with some tweaks, the QwtPolar update "broke" the patch.
Thanks again Steven.
#6 Updated by Tim Sutton over 13 years ago
Yes thanks for all the nice updates you have made from me too!
Regards
Tim
#7 Updated by Steven Mizuno over 13 years ago
You're welcome.
Now to see about knocking off some of the things noted that weren't done...