dev-threads Qt example fork changes
For our Solum integration, we've decided to just extend the Qt example application. The big feature of our fork will be OpenIGTLink support, but we're also going to add more generally useful changes such as increased feedback and user-friendliness for the GUI. That's why it makes sense to integrate them back into the official repository, and it will also simplify upgrades to new API versions for everyone.
I'm going to push all changes to https://github.com/dev-threads/solum/tree/master as they get developed and describe them in new comments to this PR until it gets closed, so feel free to cherry-pick just the features you want, if any.
The first feature replaces the single-line status bar with a multi-line textbox. Some API and GUI actions result in multiple lines of status text being shown in quick succession, but the current status bar can only show the last one of them. This makes it more difficult than necessary to debug issues, especially during connection to the probe where the generic "connection failed" message previously overwrote the more helpful ConnectionError message. A multiline textbox helps massively there, and can also double nicely as a log.
Added two smaller bug fixes:
- Removed a second call to
solumDestroy()that led to use-after-free crashes when debugging the app, - and worked around a label color bug with
QPushButtonin the default Qt Windows style by using the Fusion style instead, which is bundled with Qt as well:
Thanks we will take look!
Great! Fixed two more dangling pointer errors during shutdown that caused crashes in debug mode in the latest commits, and prepared improved UI feedback for long-running actions.
Redesigned the connection-related parts of the UI in a more self-documenting way, making them more straightforward and less error-prone to use:
- The progress bar is now hidden until the first actual progress event.
- The program now automatically starts a BLE device search on startup.
- Since all interesting features of a TCP connection require a downloaded certificate, I moved the previous contents of the Cloud tab to the TCP tab. I also changed all TCP-related UI to only be enabled after a certificate was downloaded.
- The functional difference between the previous Wi-Fi and AP connection options is now briefly explained directly within the BLE tab. All of these options now also remain disabled until a BLE connection has been established.
- The SSID and password fields are now properly labeled. Previously, their purpose was only described in their tooltips.
- The Probe model combo box now only receives the subset of models that are certified to be used with the Cloud API token.
solumLoadApplication()silently does nothing when providing a model string that does not match the connected probe, so a limited subset can reduce another potential user error. This could be even nicer if we remembered the Bluetooth-provided serial number of the TCP-connected probe and used it to automatically set the correct model, which would allow us to remove the combo box altogether, especially for organizations that can use more than one scanner. But that's a feature for another day. - Finally, I added more status box updates to provide additional reactivity. (And joy while using the program.)
This is probably the final set of commits before I add OpenIGTLink support.
Alright, added another bunch of UI redesigns after all, together with complete support for streaming the received images via the OpenIGTLink protocol. Looks like this is all we planned to do with Solum.
I've cleanly separated the UX-related commits from the OpenIGTLink support, so if you don't want to keep the latter, you can simply remove all commits after and including the git subrepo clone one.
An overview of the final set of improvements:
-
I've tried to repair the currently broken Solum API include files and their inclusions. After manually specifying the path to
solum.lib, the Qt GUI example now compiles and runs again. -
The TCP connection is now coupled to the BLE one. The IP and port are announced via BLE, can randomly change between connection attempts, and aren't valid before the message is published anyway, so it makes little to use editable input fields here.
-
This coupling also allows the probe model to be automatically derived from the serial number of the BLE-connected probe, by taking the model string from the certification data sent by the cloud API. It's still checked against the list of Solum-supported probes, returned by
solumProbes(). -
I've combined the previously distinct Load and Run steps into a single button. While this adds the 1-second workaround delay for
solumLoadApplication()lack of a callback to the Run step as well, having to only click one button instead of two more than makes up for it in usability. Especially since it's unclear if the loaded application is retained after losing connection to a probe and then reconnecting. -
I added a new Certificates tab with an overview of all downloaded certificates and their effective and expiry dates.
-
All downloaded certificates are now persisted in the QSettings file. This allows the app to be used in a more offline setting…
-
…and without requiring the certificate to be set manually. I've removed the respective button, as it's strictly worse than this automatic solution.
-
I've reordered all widgets on the Params tab, adding labels to the dials and a group box to the TGC sliders.
Thanks for making the PR; we're about to release a pilot in early September, so I'll recommend you merge after our updates to the repo (I've actually done some more significant changes to the Qt Desktop program as well). Afterwards, I'll review your changes for merging back into main. Thanks again for the contributions!
Alright, will do!
@ClaudioHoffmann we've made some of the more major changes to the UI with version 11.0, if you wanted to revisit your PR and update as necessary, would be happy to review again, thanks!
Oh, I already updated it one month ago, and rebased our code on top of v11.0.0 to preserve any relevant changes from that version.