Fix bug #285 by handling device display names through DeviceManager

Currently, DeviceManager::device_description() is used to determine
how a device should appear in the device selector combobox. This is
problematic because bug #285 requests that a device should show
the extra information only when multiple such devices are present.
Without having access to all devices attached to a particular driver,
this cannot be solved.

This patch makes it possible by creating each device's display name
in the DeviceManager class and storing it there. This way, they
can be constructed while having access to all devices and
transparently queried by anyone who needs them.

With this, bug #285 can easily be solved.

diff --git a/pv/devicemanager.cpp b/pv/devicemanager.cpp
index 4c9f768a909a07e1585c84d9c0785c2c7b92d6e8..ee6a01232e73e365a1dc399da73f3ddc04a99f14 100644 (file)
--- a/pv/devicemanager.cpp
+++ b/pv/devicemanager.cpp
@@ -88,12 +88,26 @@ list< shared_ptr<HardwareDevice> > DeviceManager::driver_scan(
        // Do the scan
        auto devices = driver->scan(drvopts);
        driver_devices.insert(driver_devices.end(), devices.begin(), devices.end());
-       driver_devices.sort(compare_devices);
 
-       // Add the scanned devices to the main list
+       // Add the scanned devices to the main list, set display names and sort.
        _devices.insert(_devices.end(), driver_devices.begin(),
                driver_devices.end());
-       _devices.sort(compare_devices);
+
+       for (shared_ptr<Device> device : _devices)
+               _display_names[device] = build_display_name(device);
+
+       _devices.sort([&](shared_ptr<Device> a, shared_ptr<Device> b)
+               { return compare_devices(a, b); });
+
+       // As the display names depend on the complete _devices list,
+       // we need to recompute them. However, there is no need to
+       // recomute all names of the _devices list since only the
+       // devices that use the given driver can be affected.
+       for (shared_ptr<Device> device : driver_devices)
+               _display_names[device] = build_display_name(device);
+
+       driver_devices.sort([&](shared_ptr<Device> a, shared_ptr<Device> b)
+               { return compare_devices(a, b); });
 
        return driver_devices;
 }
@@ -170,9 +184,10 @@ const shared_ptr<HardwareDevice> DeviceManager::find_device_from_info(
        return last_resort_dev;
 }
 
-string DeviceManager::device_description(shared_ptr<Device> device)
+const string DeviceManager::build_display_name(shared_ptr<Device> device)
 {
        auto session_device = dynamic_pointer_cast<SessionDevice>(device);
+       auto hardware_device = dynamic_pointer_cast<HardwareDevice>(device);
 
        if (session_device)
                return boost::filesystem::path(
@@ -180,8 +195,28 @@ string DeviceManager::device_description(shared_ptr<Device> device)
 
        ostringstream s;
 
-       vector<string> parts = {device->vendor(), device->model(),
-               device->version(), device->serial_number()};
+       bool multiple_dev = false;
+
+       // If we can find another device with the same model/vendor then
+       // we have at least two such devices and need to distinguish them.
+       if (hardware_device)
+               multiple_dev = any_of(_devices.begin(), _devices.end(),
+                       [&](shared_ptr<HardwareDevice> dev) {
+                       return (dev->vendor() == hardware_device->vendor() &&
+                       dev->model() == hardware_device->model()) &&
+                       dev != hardware_device;
+                       } );
+
+       vector<string> parts = {device->vendor(), device->model()};
+
+       if (multiple_dev) {
+               parts.push_back(device->version());
+               parts.push_back(device->serial_number());
+
+               if ((device->serial_number().length() == 0) &&
+                       (device->connection_id().length() > 0))
+                       parts.push_back("("+device->connection_id()+")");
+       }
 
        for (size_t i = 0; i < parts.size(); i++)
        {
@@ -193,20 +228,26 @@ string DeviceManager::device_description(shared_ptr<Device> device)
                }
        }
 
-       if (device->serial_number().length() == 0 &&
-                       device->connection_id().length() > 0)
-               s << " " << device->connection_id();
-
        return s.str();
 }
 
-bool DeviceManager::compare_devices(shared_ptr<HardwareDevice> a,
-       shared_ptr<HardwareDevice> b)
+const std::string DeviceManager::get_display_name(std::shared_ptr<sigrok::Device> dev)
+{
+       return _display_names[dev];
+}
+
+void DeviceManager::update_display_name(std::shared_ptr<sigrok::Device> dev)
+{
+       _display_names[dev] = build_display_name(dev);
+}
+
+bool DeviceManager::compare_devices(shared_ptr<Device> a,
+       shared_ptr<Device> b)
 {
        assert(a);
        assert(b);
 
-       return device_description(a).compare(device_description(b)) < 0;
+       return _display_names[a].compare(_display_names[b]) < 0;
 }
 
 } // namespace pv

diff --git a/pv/devicemanager.h b/pv/devicemanager.h
index 0a47a35a004659b09da56ce7545135159a448939..511ba76091083679bdf96b021a6c4f312e12b5a2 100644 (file)
--- a/pv/devicemanager.h
+++ b/pv/devicemanager.h
@@ -64,15 +64,20 @@ public:
        const std::shared_ptr<sigrok::HardwareDevice> find_device_from_info(
                const std::map<std::string, std::string> search_info);
 
-       static std::string device_description(std::shared_ptr<sigrok::Device> device);
+       const std::string build_display_name(std::shared_ptr<sigrok::Device> device);
+
+       const std::string get_display_name(std::shared_ptr<sigrok::Device> dev);
+
+       void update_display_name(std::shared_ptr<sigrok::Device> dev);
 
 private:
-       static bool compare_devices(std::shared_ptr<sigrok::HardwareDevice> a,
-               std::shared_ptr<sigrok::HardwareDevice> b);
+       bool compare_devices(std::shared_ptr<sigrok::Device> a,
+               std::shared_ptr<sigrok::Device> b);
 
 protected:
        std::shared_ptr<sigrok::Context> _context;
        std::list< std::shared_ptr<sigrok::HardwareDevice> > _devices;
+       std::map< std::shared_ptr<sigrok::Device>, std::string > _display_names;
 };
 
 } // namespace pv

diff --git a/pv/dialogs/connect.cpp b/pv/dialogs/connect.cpp
index 19dfde8d1a3f227065e39cd1499b2300212f3d8a..e4a22bd3e38f74ad06425775f844e23707a6d081 100644 (file)
--- a/pv/dialogs/connect.cpp
+++ b/pv/dialogs/connect.cpp
@@ -154,7 +154,7 @@ void Connect::scan_pressed()
                assert(device);
 
                QString text = QString::fromStdString(
-                       _device_manager.device_description(device));
+                       _device_manager.get_display_name(device));
                text += QString(" with %1 channels").arg(device->channels().size());
 
                QListWidgetItem *const item = new QListWidgetItem(text,

diff --git a/pv/mainwindow.cpp b/pv/mainwindow.cpp
index 598976eb77298b83d4aa9b123259722c82964e85..42ae957640d491c19a7c740b9204c55224d7f852 100644 (file)
--- a/pv/mainwindow.cpp
+++ b/pv/mainwindow.cpp
@@ -384,7 +384,7 @@ void MainWindow::update_device_list()
        map<shared_ptr<Device>, string> device_names;
 
        for (auto device : devices)
-               device_names[device] = _device_manager.device_description(device);
+               device_names[device] = _device_manager.get_display_name(device);
 
        _sampling_bar->set_device_list(device_names, selected_device);
 }

diff --git a/pv/sigsession.cpp b/pv/sigsession.cpp
index 2e49422fb9a60746246c1d858e66234940674426..1d424c000bf3972a8c697534af02522a05d99591 100644 (file)
--- a/pv/sigsession.cpp
+++ b/pv/sigsession.cpp
@@ -151,6 +151,7 @@ void SigSession::set_file(const string &name)
                (shared_ptr<Device> device, shared_ptr<Packet> packet) {
                        data_feed_in(device, packet);
                });
+       _device_manager.update_display_name(_device);
        update_signals(_device);
 }