From: Soeren Apel <redacted>
Date: Fri, 7 Nov 2014 16:11:30 +0000 (+0100)
Subject: Fix bug #285 by handling device display names through DeviceManager
X-Git-Tag: pulseview-0.3.0~521
X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=a4cf020a253c8c874ed8652f38a1c61c9a4754a4;p=pulseview.git

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 4c9f768a..ee6a0123 100644
--- 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 0a47a35a..511ba760 100644
--- 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 19dfde8d..e4a22bd3 100644
--- 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 598976eb..42ae9576 100644
--- 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 2e49422f..1d424c00 100644
--- 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);
 }