Fix #805 by resetting selected device on failure

It can happen that devices can be selected but not used
(permissions problems, connection issues, driver issues, etc.),
so in those cases we want to fail gracefully instead of
segfaulting.
The reason for the segfault is the device selector button
isn't reset in case the device couldn't be opened, causing
the rest of the application to try and work with a device
instance that is actually invalid.

Resetting the device selector when the device failed to
open not only fixes this but also makes the UI more
consistent with the internal state.

diff --git a/pv/mainwindow.cpp b/pv/mainwindow.cpp
index 4f6bc41d551c4182919539cb3810882f9e10e60d..2558cb07692fe497e87d262fad574bc81ad70bfd 100644 (file)
--- a/pv/mainwindow.cpp
+++ b/pv/mainwindow.cpp
@@ -882,8 +882,11 @@ void MainWindow::device_selected()
 {
        // Set the title to include the device/file name
        const shared_ptr<devices::Device> device = session_.device();
-       if (!device)
+
+       if (!device) {
+               main_bar_->reset_device_selector();
                return;
+       }
 
        const string display_name = device->display_name(device_manager_);
        setWindowTitle(tr("%1 - PulseView").arg(display_name.c_str()));

diff --git a/pv/session.cpp b/pv/session.cpp
index 73dd339b60fea95259d6fddc7aace7ec9f3ad0e1..b8e98ece9a5dce42ce5e6e774e689bd34abed5a0 100644 (file)
--- a/pv/session.cpp
+++ b/pv/session.cpp
@@ -161,7 +161,15 @@ void Session::set_device(shared_ptr<devices::Device> device)
        signals_changed();
 
        device_ = std::move(device);
-       device_->open();
+
+       try {
+               device_->open();
+       } catch (const QString &e) {
+               device_.reset();
+               device_selected();
+               throw;
+       }
+
        device_->session()->add_datafeed_callback([=]
                (shared_ptr<sigrok::Device> device, shared_ptr<Packet> packet) {
                        data_feed_in(device, packet);

diff --git a/pv/toolbars/mainbar.cpp b/pv/toolbars/mainbar.cpp
index 8c0b06391db310f8e1dad603cf38ad7de6c81864..cca4d9d5bab2d38cae0af4eb65de58718ef7d38f 100644 (file)
--- a/pv/toolbars/mainbar.cpp
+++ b/pv/toolbars/mainbar.cpp
@@ -248,6 +248,11 @@ void MainBar::set_capture_state(pv::Session::capture_state state)
        sample_rate_.setEnabled(ui_enabled);
 }
 
+void MainBar::reset_device_selector()
+{
+       device_selector_.reset();
+}
+
 void MainBar::update_sample_rate_selector()
 {
        Glib::VariantContainerBase gvar_dict;

diff --git a/pv/toolbars/mainbar.hpp b/pv/toolbars/mainbar.hpp
index 6e2a706833bc566c687e3d20a6bc7da51919de04..9c4b1cd4b0e4a9f9484d00736109b75a8f8cf8ee 100644 (file)
--- a/pv/toolbars/mainbar.hpp
+++ b/pv/toolbars/mainbar.hpp
@@ -69,6 +69,8 @@ public:
 
        void set_capture_state(pv::Session::capture_state state);
 
+       void reset_device_selector();
+
 private:
        void update_sample_rate_selector();
        void update_sample_rate_selector_value();

diff --git a/pv/widgets/devicetoolbutton.cpp b/pv/widgets/devicetoolbutton.cpp
index a0fc10e6f5a36de217cc2008cb1201e2f6bf0a02..d553aba32987d7bded07a7be5767d63501fb69c0 100644 (file)
--- a/pv/widgets/devicetoolbutton.cpp
+++ b/pv/widgets/devicetoolbutton.cpp
@@ -73,11 +73,18 @@ void DeviceToolButton::set_device_list(
 {
        selected_device_ = selected;
        setText(selected ? QString::fromStdString(
-               selected->display_name(device_manager_)) : "<No Device>");
+               selected->display_name(device_manager_)) : tr("<No Device>"));
        devices_ = vector< weak_ptr<Device> >(devices.begin(), devices.end());
        update_device_list();
 }
 
+void DeviceToolButton::reset()
+{
+       setText(tr("<No Device>"));
+       selected_device_.reset();
+       update_device_list();
+}
+
 void DeviceToolButton::update_device_list()
 {
        menu_.clear();
@@ -108,6 +115,8 @@ void DeviceToolButton::on_action(QObject *action)
 {
        assert(action);
 
+       selected_device_.reset();
+
        Device *const dev = (Device*)((QAction*)action)->data().value<void*>();
        for (weak_ptr<Device> dev_weak_ptr : devices_) {
                shared_ptr<Device> dev_ptr(dev_weak_ptr);

diff --git a/pv/widgets/devicetoolbutton.hpp b/pv/widgets/devicetoolbutton.hpp
index ed69a4b64a4ef1b0209daf004f2192117a9af8c6..589fbf2338016138be21050dd250032d54a04050 100644 (file)
--- a/pv/widgets/devicetoolbutton.hpp
+++ b/pv/widgets/devicetoolbutton.hpp
@@ -70,6 +70,12 @@ public:
                const std::list< std::shared_ptr<devices::Device> > &devices,
                std::shared_ptr<devices::Device> selected);
 
+       /**
+        * Sets the current device to "no device". Useful for when a selected
+        * device fails to open.
+        */
+       void reset();
+
 private:
        /**
         * Repopulates the menu from the device list.