Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GPS information panel UI and code rework #50753

Merged
merged 31 commits into from Nov 4, 2022

Conversation

nyalldawson
Copy link
Collaborator

@nyalldawson nyalldawson commented Nov 1, 2022

This PR implements a substantial rework of the GPS information panel. The intention here is to simplify the user operation of GPS within QGIS and provide a more streamlined GPS experience.

Previously, ALL app level GPS functionality was exposed through the GPS "Information" panel. This included GPS connection settings, digitizing from GPS, and lastly... actually showing GPS information. It left us with a horrible dumping ground of all GPS settings and widgets and knobs and switches, giving a very disorganised UI which was confusing for users.

Now, the GPS functionality is split up:

  1. Application level, rarely changed GPS settings are present in the Settings - Options - GPS tab (done in Move a bunch of infrequently changed GPS settings to Options dialog #50678, but a few more settings migrated there in this PR)
  2. A new floating "GPS Toolbar" has been added, which contains the most heavily used, common GPS related operations. The screenshot below shows this toolbar.

image

  1. The final action in the toolbar opens a "GPS settings" popup, containing settings which are expected to be modified mid-session, and accordingly need to be readily accessible:

image

(The last action in this menu opens the Settings - Options - GPS Pane)

  1. The GPS information widget has been stripped right back so that it contains ONLY GPS information. Specifically, only the "information" and "signal" tabs remain. The GPS information widget still contains a shortcut to the connect/disconnect action, and also a settings button which popups up the same GPS settings menu as is used in the GPS toolbar: (This was done to ease users into the new UI)

image

image

The secondary intention was to clean up the app level GPS code, which until now was all tied heavily into the GPS information widget class. Now, that GPS information widget class is responsible ONLY for showing the GPS information in the panel, with new dedicated classes being created for handling digitizing from GPS, the new GPS toolbar, and GPS settings.

This PR will be followed up by several smaller changes which further improve GPS handling within QGIS app.

Sponsored by NIWA

@github-actions github-actions bot added this to the 3.30.0 milestone Nov 1, 2022
@nyalldawson nyalldawson added GUI/UX Related to QGIS application GUI or User Experience GPS Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. Feature labels Nov 1, 2022
@github-actions
Copy link

github-actions bot commented Nov 1, 2022

@nyalldawson
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

images/images.qrc Show resolved Hide resolved
images/images.qrc Show resolved Hide resolved
src/app/gps/qgsappgpsdigitizing.cpp Outdated Show resolved Hide resolved
src/app/gps/qgsappgpsdigitizing.cpp Outdated Show resolved Hide resolved
src/app/gps/qgsappgpssettingsmenu.cpp Show resolved Hide resolved
src/app/gps/qgsgpscanvasbridge.cpp Show resolved Hide resolved
src/app/gps/qgsgpsinformationwidget.h Show resolved Hide resolved
src/app/gps/qgsgpstoolbar.cpp Show resolved Hide resolved
src/app/gps/qgsgpstoolbar.cpp Outdated Show resolved Hide resolved
src/app/gps/qgsgpstoolbar.cpp Show resolved Hide resolved
"Configure Device" button which directly opens the GPS options
dialog

Makes it easier for users to find the correct place to fix
these settings
…ting

"failed to connect" message bar warnings

And show success message in message bar, instead of status bar
@github-actions
Copy link

github-actions bot commented Nov 4, 2022

@nyalldawson
A documentation ticket has been opened at qgis/QGIS-Documentation#7862
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!

@nyalldawson nyalldawson mentioned this pull request Nov 4, 2022
@jef-n
Copy link
Member

jef-n commented Nov 7, 2022

I think this breaks the QGIS start on windows (see https://trac.osgeo.org/osgeo4w/ticket/765). Back trace:

 	Qt5Core.dll!QMetaObject::indexOfEnumerator(const char * name) Line 982	C++
 	qgis_app.dll!QMetaEnum::fromType<enum QgsGpsCanvasBridge::MapCenteringMode>() Line 236	C++
 	qgis_app.dll!QgsSettingsEntryEnumFlag<enum QgsGpsCanvasBridge::MapCenteringMode>::QgsSettingsEntryEnumFlag<enum QgsGpsCanvasBridge::MapCenteringMode>(const QString & key, const QString & section, QgsGpsCanvasBridge::MapCenteringMode defaultValue, const QString & description, QFlags<enum Qgis::SettingsOption> options) Line 50	C++
	qgis_app.dll!`dynamic initializer for 'QgsGpsCanvasBridge::settingMapCenteringMode''() Line 55	C++
 	ucrtbase.dll!_initterm()	Unknown
 	qgis_app.dll!dllmain_crt_process_attach(HINSTANCE__ * const instance, void * const reserved) Line 66	C++
 	qgis_app.dll!dllmain_dispatch(HINSTANCE__ * const instance, const unsigned long reason, void * const reserved) Line 276	C++
 	ntdll.dll!LdrpCallInitRoutine()	Unknown
 	ntdll.dll!LdrpInitializeNode()	Unknown
 	ntdll.dll!LdrpInitializeGraphRecurse()	Unknown
 	ntdll.dll!LdrpPrepareModuleForExecution()	Unknown
 	ntdll.dll!LdrpLoadDllInternal()	Unknown
 	ntdll.dll!LdrpLoadDll()	Unknown
 	ntdll.dll!LdrLoadDll()	Unknown
 	KernelBase.dll!LoadLibraryExW()	Unknown
 	KernelBase.dll!LoadLibraryExA()	Unknown
 	KernelBase.dll!LoadLibraryA()	Unknown
 	qgis-dev-bin.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 161	C++
 	[Inline Frame] qgis-dev-bin.exe!invoke_main() Line 102	C++
 	qgis-dev-bin.exe!__scrt_common_main_seh() Line 288	C++
 	kernel32.dll!BaseThreadInitThunk()	Unknown
 	ntdll.dll!RtlUserThreadStart()	Unknown

Apparently the dynamic initialization happens before the QMetaObject is initialized properly:

int QMetaObject::indexOfEnumerator(const char *name) const
{
    const QMetaObject *m = this;
    while (m) {
        const QMetaObjectPrivate *d = priv(m->d.data); 
        const int intsPerEnum = d->revision >= 8 ? 5 : 4; // crashes here: d == nullptr
…

Findings:

@nyalldawson
Copy link
Collaborator Author

@jef-n

Let's see if #50813 fixes it...

@agiudiceandrea
Copy link
Contributor

agiudiceandrea commented Nov 10, 2022

It seems the GPS Toolbar is enabled by default (it happens with new and old user profiles). @nyalldawson is this intended? I think it is not a toolbar to be enabled by default.

@nyalldawson
Copy link
Collaborator Author

@agiudiceandrea

It seems the GPS Toolbar is enabled by default (it happens with new and old user profiles). @nyalldawson is this intended? I think it is not a toolbar to be enabled by default.

Fixed in #50947 (applies to new profiles only unfortunately)

@zacharlie zacharlie added Changelog Items that are queued to appear in the visual changelog - remove after harvesting ChangelogHarvested This PR description has been harvested in the Changelog already. and removed Changelog Items that are queued to appear in the visual changelog - remove after harvesting labels Feb 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ChangelogHarvested This PR description has been harvested in the Changelog already. Feature GPS GUI/UX Related to QGIS application GUI or User Experience Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants