dev-threads Qt example fork changes #37
Conversation
|
Added two smaller bug fixes:
|
|
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:
This is probably the final set of commits before I add OpenIGTLink support.
|
ClaudioHoffmann
force-pushed
the
master
branch
from
3e03ab5 to
e7065ca
Compare
|
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.
An overview of the final set of improvements:
|
ClaudioHoffmann
force-pushed
the
master
branch
from
e7065ca to
6e90836
Compare
|
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
force-pushed
the
master
branch
2 times, most recently
from
90211a0 to
fe08fef
Compare
ClaudioHoffmann
force-pushed
the
master
branch
from
fe08fef to
8dfa975
Compare
|
@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! |
This restores the layout of the v10.4.0 UI, since our fork will add more certificate-related widgets.
Some operations result in multiple lines of text being shown in quick succession, but a status bar shows only the last one of them. This makes it more difficult to debug any issues that might occur, 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.
A small bit of polish to differentiate it from a regular editable textbox.
main() already contains such a call. Since solumInit() is also called there, it's the only place where this call is appropriate. Calling this function within Qt causes use-after-free errors in the main window cleanup code.
…el colors Kind of unfortunate, but the `fusion` style looks nicer anyway.
…dow class Previously, the destructor for `Ble` instance of the main window class ran after the deletion of `ui_`. This caused a rather involved use-after-free bug if a probe is connected via Bluetooth, through the following sequence of method calls: • `Ble::~Ble()` • `Ble::disconnectFromProbe()` • `QLowEnergyController::disconnectFromDevice()` • `QLowEnergyController::disconnected()` signal • `QLowEnergyController::disconnected()` slot, connected in `Ble::connectToProbe()` • `Ble::powerReady()` signal • `Ble::powerReady()` slot, connected in `Solum::Solum()` This last slot then tries to access fields of the deleted `ui_` pointer. `Ui::Solum` doesn't track any object lifetimes itself. It only tracks raw pointers into the Qt widget tree, and doesn't even have a destructor or a base class with a destructor. Therefore, we don't even have to rely on C++'s well-defined destructor order here (https://isocpp.org/wiki/faq/dtors#order-dtors-for-members), and turning the instance from a manually `new`ed object into a plain class member will just do the right thing. Sure, it's a more sweeping change than heap-allocating `Ble` instead and explicitly deleting it before `ui_`, but less manual lifetime management is always preferable.
All higher states after `QLowEnergyController::ConnectedState` also represent an existing connection.
Relying on all 4 prefix characters is better than relying on just 3 of them.
We'd like to have a separate SUBDIRS project to combine the new OpenIGTLink library project with the Qt example code.
The very unnecessary files generated by the original CMake scripts.
A build system where the supposed way to consume a library from an application is so broken on Windows that it requires manual directory specifications to get working… qmake might just be worse than CMake. And yes, this codebase throws enough integer truncation warnings that we just disable them all.
No disconnect detection yet, though. It seems as if disconnects can only be detected by reading from the client and having that fail?
Also handling client reconnection, since this is where we have to detect disconnects.
A simple first attempt, calculating FPS simply based on the time it took between sending two images.
A small adjustment for less jumpy FPS numbers.
Setting the number of components makes the receiving node a vector volume node. To have correct values in the received data, the scalar type needed to be set to uint8.
ClaudioHoffmann
force-pushed
the
master
branch
from
8dfa975 to
96ad2ec
Compare
|
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. |
Add some folders and files to gitignore which contain user configs and build files.
The file is not within the source dir, but one layer above.
Use QTs permission feature for bluetooth. Only if permissions are granted, the BLE search button will be enabled.
The Clarius probe returns a quoted string for the IP address. In order to connect, the quotes needed to be removed.
On macos, the settings.ini file could not be read from the app directory. Fix by using a writable location path.
Mac adjustments and update to latest SDK version 11.3.0
Disable all energy saving options to avoid the scanner going to freeze mode during a demo.
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 helpfulConnectionErrormessage. A multiline textbox helps massively there, and can also double nicely as a log.