Qt: Interface Toolbar improvements

- Select one of the capturing interfaces when start capture
- Only send user changed control values when start capture
- Don't show hidden interfaces
- Allow a toolbar with no interfaces
- Renamed button role "reset" to "restore"
- Improved control number validation
- Updated documentation

Change-Id: Icc8d04043c95c1f3ef8d7cdc3b251be4471cba0a
Reviewed-on: https://code.wireshark.org/review/21445
Petri-Dish: Stig Bjørlykke <stig@bjorlykke.org>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Stig Bjørlykke <stig@bjorlykke.org>

5 files changed, 151 insertions, 52 deletions

diff --git a/ui/iface_toolbar.h b/ui/iface_toolbar.h
index 888c817230..5a5421cad7 100644
--- a/ui/iface_toolbar.h
+++ b/ui/iface_toolbar.h
@@ -41,7 +41,7 @@ typedef enum {
     INTERFACE_ROLE_CONTROL,
     INTERFACE_ROLE_HELP,
     INTERFACE_ROLE_LOGGER,
-    INTERFACE_ROLE_RESET
+    INTERFACE_ROLE_RESTORE
 } iface_toolbar_ctrl_role;
 
 typedef struct _iface_toolbar_value {
diff --git a/ui/qt/interface_toolbar.cpp b/ui/qt/interface_toolbar.cpp
index 56a1a53b34..dd178c9195 100644
--- a/ui/qt/interface_toolbar.cpp
+++ b/ui/qt/interface_toolbar.cpp
@@ -29,6 +29,8 @@
 #include "ui/main_statusbar.h"
 #include <ui_interface_toolbar.h>
 
+#include "capture_opts.h"
+#include "ui/capture_globals.h"
 #include "sync_pipe.h"
 #include "wsutil/file_util.h"
 
@@ -69,16 +71,13 @@ InterfaceToolbar::InterfaceToolbar(QWidget *parent, const iface_toolbar *toolbar
     ui->setupUi(this);
 
     // Fill inn interfaces list and initialize default interface values
-    ui->interfacesComboBox->blockSignals(true);
     for (GList *walker = toolbar->ifnames; walker; walker = walker->next)
     {
         QString ifname((gchar *)walker->data);
-        ui->interfacesComboBox->addItem(ifname);
         interface_[ifname].reader_thread = NULL;
         interface_[ifname].out_fd = -1;
         interface_[ifname].log_dialog = NULL;
     }
-    ui->interfacesComboBox->blockSignals(false);
 
     initializeControls(toolbar);
 
@@ -194,7 +193,7 @@ QWidget *InterfaceToolbar::createButton(iface_toolbar_control *control)
     {
         case INTERFACE_ROLE_CONTROL:
             setDefaultValue(control->num, (gchar *)control->display);
-            connect(button, SIGNAL(pressed()), this, SLOT(onButtonPressed()));
+            connect(button, SIGNAL(pressed()), this, SLOT(onControlButtonPressed()));
             break;
 
         case INTERFACE_ROLE_HELP:
@@ -210,13 +209,12 @@ QWidget *InterfaceToolbar::createButton(iface_toolbar_control *control)
             connect(button, SIGNAL(pressed()), this, SLOT(onLogButtonPressed()));
             break;
 
-        case INTERFACE_ROLE_RESET:
-            button->setText("Reset");
-            button->setToolTip("Restore default values");
-            connect(button, SIGNAL(pressed()), this, SLOT(onResetButtonPressed()));
+        case INTERFACE_ROLE_RESTORE:
+            connect(button, SIGNAL(pressed()), this, SLOT(onRestoreButtonPressed()));
             break;
 
         default:
+            // Not supported
             break;
     }
 
@@ -237,7 +235,7 @@ QWidget *InterfaceToolbar::createSelector(iface_toolbar_control *control)
     {
         iface_toolbar_value *val = (iface_toolbar_value *)walker->data;
         QString value = QString().fromUtf8((gchar *)val->value);
-        if (value.length() == 0)
+        if (value.isEmpty())
         {
             // Invalid value
             continue;
@@ -246,7 +244,7 @@ QWidget *InterfaceToolbar::createSelector(iface_toolbar_control *control)
         QByteArray interface_value;
 
         interface_value.append(value);
-        if (display.length() == 0)
+        if (display.isEmpty())
         {
             display = value;
         }
@@ -422,6 +420,10 @@ void InterfaceToolbar::setInterfaceValue(QString ifname, QWidget *widget, int nu
         switch (command)
         {
             case commandControlSet:
+                if (interface_[ifname].value[num] != payload)
+                {
+                    interface_[ifname].value_changed[num] = false;
+                }
                 foreach (QByteArray entry, interface_[ifname].list[num])
                 {
                     if (entry == payload || entry.startsWith(payload + '\0'))
@@ -462,6 +464,10 @@ void InterfaceToolbar::setInterfaceValue(QString ifname, QWidget *widget, int nu
         switch (command)
         {
             case commandControlSet:
+                if (interface_[ifname].value[num] != payload)
+                {
+                    interface_[ifname].value_changed[num] = false;
+                }
                 interface_[ifname].value[num] = payload;
                 break;
 
@@ -492,11 +498,22 @@ void InterfaceToolbar::setInterfaceValue(QString ifname, QWidget *widget, int nu
     else if (widget->property(interface_role_property).toInt() == INTERFACE_ROLE_CONTROL)
     {
         // QCheckBox or QPushButton
-        interface_[ifname].value[num] = payload;
+        switch (command)
+        {
+            case commandControlSet:
+                if (interface_[ifname].value[num] != payload)
+                {
+                    interface_[ifname].value_changed[num] = false;
+                }
+                interface_[ifname].value[num] = payload;
+                break;
+
+            default:
+                break;
+        }
     }
 }
 
-
 void InterfaceToolbar::controlReceived(QString ifname, int num, int command, QByteArray payload)
 {
     switch (command)
@@ -504,8 +521,9 @@ void InterfaceToolbar::controlReceived(QString ifname, int num, int command, QBy
         case commandControlSet:
         case commandControlAdd:
         case commandControlRemove:
-            if (QWidget *widget = control_widget_[num])
+            if (control_widget_.contains(num))
             {
+                QWidget *widget = control_widget_[num];
                 setInterfaceValue(ifname, widget, num, command, payload);
 
                 if (ifname.compare(ui->interfacesComboBox->currentText()) == 0)
@@ -517,8 +535,9 @@ void InterfaceToolbar::controlReceived(QString ifname, int num, int command, QBy
 
         case commandControlEnable:
         case commandControlDisable:
-            if (QWidget *widget = control_widget_[num])
+            if (control_widget_.contains(num))
             {
+                QWidget *widget = control_widget_[num];
                 if (widget->property(interface_role_property).toInt() == INTERFACE_ROLE_CONTROL)
                 {
                     bool enable = (command == commandControlEnable ? true : false);
@@ -566,7 +585,7 @@ void InterfaceToolbar::controlSend(QString ifname, int num, int command, const Q
         return;
     }
 
-    if (interface_[ifname].out_fd == -1)
+    if (ifname.isEmpty() || interface_[ifname].out_fd == -1)
     {
         // Does not have a control out channel
         return;
@@ -595,7 +614,7 @@ void InterfaceToolbar::controlSend(QString ifname, int num, int command, const Q
     }
 }
 
-void InterfaceToolbar::onButtonPressed()
+void InterfaceToolbar::onControlButtonPressed()
 {
     const QString &ifname = ui->interfacesComboBox->currentText();
     QPushButton *button = static_cast<QPushButton *>(sender());
@@ -613,6 +632,7 @@ void InterfaceToolbar::onCheckBoxChanged(int state)
     QByteArray payload(1, state == Qt::Unchecked ? 0 : 1);
     controlSend(ifname, num, commandControlSet, payload);
     interface_[ifname].value[num] = payload;
+    interface_[ifname].value_changed[num] = true;
 }
 
 void InterfaceToolbar::onComboBoxChanged(int idx)
@@ -625,6 +645,7 @@ void InterfaceToolbar::onComboBoxChanged(int idx)
     QByteArray payload(value.toUtf8());
     controlSend(ifname, num, commandControlSet, payload);
     interface_[ifname].value[num] = payload;
+    interface_[ifname].value_changed[num] = true;
 }
 
 void InterfaceToolbar::onLineEditChanged()
@@ -636,6 +657,7 @@ void InterfaceToolbar::onLineEditChanged()
     QByteArray payload(lineedit->text().toUtf8());
     controlSend(ifname, num, commandControlSet, payload);
     interface_[ifname].value[num] = payload;
+    interface_[ifname].value_changed[num] = true;
 }
 
 void InterfaceToolbar::onLogButtonPressed()
@@ -661,7 +683,8 @@ void InterfaceToolbar::onHelpButtonPressed()
 {
     QUrl help_url(help_link_);
 
-    if (help_url.scheme().compare("file") != 0) {
+    if (help_url.scheme().compare("file") != 0)
+    {
         QDesktopServices::openUrl(help_url);
     }
 }
@@ -697,24 +720,48 @@ void InterfaceToolbar::startReaderThread(QString ifname, QString control_in)
     thread->start();
 }
 
-void InterfaceToolbar::startCapture(QString ifname, QString control_in, QString control_out)
+void InterfaceToolbar::startCapture(GArray *ifaces)
 {
-    if (!interface_.contains(ifname) ||      // This interface is not for us
-        interface_[ifname].out_fd != -1)     // Already have control channels for this interface
-    {
+    if (!ifaces || ifaces->len == 0)
         return;
-    }
 
-    // The reader thread will open control in channel
-    startReaderThread(ifname, control_in);
+    const QString &selected_ifname = ui->interfacesComboBox->currentText();
+    QString first_capturing_ifname;
+    bool selected_found = false;
 
-    // Open control out channel
-    interface_[ifname].out_fd = ws_open(control_out.toUtf8(), O_WRONLY | O_BINARY, 0);
+    for (guint i = 0; i < ifaces->len; i++)
+    {
+        interface_options interface_opts = g_array_index(ifaces, interface_options, i);
+        QString ifname(interface_opts.name);
 
-    sendChangedValues(ifname);
-    controlSend(ifname, 0, commandControlInitialized);
+        if (!interface_.contains(ifname))
+            // This interface is not for us
+            continue;
 
-    updateWidgets();
+        if (first_capturing_ifname.isEmpty())
+            first_capturing_ifname = ifname;
+
+        if (ifname.compare(selected_ifname) == 0)
+            selected_found = true;
+
+        if (interface_[ifname].out_fd != -1)
+            // Already have control channels for this interface
+            continue;
+
+        // The reader thread will open control in channel
+        startReaderThread(ifname, interface_opts.extcap_control_in);
+
+        // Open control out channel
+        interface_[ifname].out_fd = ws_open(interface_opts.extcap_control_out, O_WRONLY | O_BINARY, 0);
+
+        sendChangedValues(ifname);
+        controlSend(ifname, 0, commandControlInitialized);
+    }
+
+    if (!selected_found && !first_capturing_ifname.isEmpty())
+        ui->interfacesComboBox->setCurrentText(first_capturing_ifname);
+    else
+        updateWidgets();
 }
 
 void InterfaceToolbar::stopCapture()
@@ -764,7 +811,7 @@ void InterfaceToolbar::sendChangedValues(QString ifname)
     foreach (int num, control_widget_.keys())
     {
         QWidget *widget = control_widget_[num];
-        if ((interface_[ifname].value[num] != default_value_[num]) &&
+        if ((interface_[ifname].value_changed[num]) &&
             (widget->property(interface_type_property).toInt() != INTERFACE_TYPE_BUTTON) &&
             (widget->property(interface_role_property).toInt() == INTERFACE_ROLE_CONTROL))
         {
@@ -773,7 +820,7 @@ void InterfaceToolbar::sendChangedValues(QString ifname)
     }
 }
 
-void InterfaceToolbar::onResetButtonPressed()
+void InterfaceToolbar::onRestoreButtonPressed()
 {
     const QString &ifname = ui->interfacesComboBox->currentText();
 
@@ -799,6 +846,7 @@ void InterfaceToolbar::onResetButtonPressed()
             case INTERFACE_ROLE_CONTROL:
                 setWidgetValue(widget, commandControlSet, default_value_[num]);
                 interface_[ifname].value[num] = default_value_[num];
+                interface_[ifname].value_changed[num] = false;
                 break;
 
             case INTERFACE_ROLE_LOGGER:
@@ -828,20 +876,29 @@ void InterfaceToolbar::updateWidgets()
     foreach (int num, control_widget_.keys())
     {
         QWidget *widget = control_widget_[num];
-        if (!is_capturing &&
-            (widget->property(interface_type_property).toInt() == INTERFACE_TYPE_BUTTON) &&
-            (widget->property(interface_role_property).toInt() == INTERFACE_ROLE_CONTROL))
+        bool widget_enabled = true;
+
+        if (ifname.isEmpty() &&
+            (widget->property(interface_role_property).toInt() != INTERFACE_ROLE_HELP))
+        {
+            // No interface selected, disable all but Help button
+            widget_enabled = false;
+        }
+        else if (!is_capturing &&
+                 (widget->property(interface_type_property).toInt() == INTERFACE_TYPE_BUTTON) &&
+                 (widget->property(interface_role_property).toInt() == INTERFACE_ROLE_CONTROL))
         {
-            widget->setEnabled(false);
+            widget_enabled = false;
         }
         else if (widget->property(interface_role_property).toInt() == INTERFACE_ROLE_CONTROL)
         {
-            bool widget_enabled = !interface_[ifname].widget_disabled[num];
-            widget->setEnabled(widget_enabled);
-            if (label_widget_.contains(num))
-            {
-                label_widget_[num]->setEnabled(widget_enabled);
-            }
+            widget_enabled = !interface_[ifname].widget_disabled[num];
+        }
+
+        widget->setEnabled(widget_enabled);
+        if (label_widget_.contains(num))
+        {
+            label_widget_[num]->setEnabled(widget_enabled);
         }
     }
 
@@ -849,13 +906,50 @@ void InterfaceToolbar::updateWidgets()
     {
         QWidget *widget = control_widget_[num];
         if ((widget->property(interface_type_property).toInt() == INTERFACE_TYPE_BUTTON) &&
-            (widget->property(interface_role_property).toInt() == INTERFACE_ROLE_RESET))
+            (widget->property(interface_role_property).toInt() == INTERFACE_ROLE_RESTORE))
         {
             widget->setEnabled(!is_capturing);
         }
     }
 }
 
+void InterfaceToolbar::interfaceListChanged()
+{
+    const QString &selected_ifname = ui->interfacesComboBox->currentText();
+    bool keep_selected = false;
+
+    ui->interfacesComboBox->blockSignals(true);
+    ui->interfacesComboBox->clear();
+
+    for (guint i = 0; i < global_capture_opts.all_ifaces->len; i++)
+    {
+        interface_t device = g_array_index(global_capture_opts.all_ifaces, interface_t, i);
+        if (device.hidden)
+            continue;
+
+        if (interface_.keys().contains(device.name))
+        {
+            ui->interfacesComboBox->addItem(device.name);
+            if (selected_ifname.compare(device.name) == 0)
+            {
+                // Keep selected interface
+                ui->interfacesComboBox->setCurrentText(device.name);
+                keep_selected = true;
+            }
+        }
+    }
+
+    ui->interfacesComboBox->blockSignals(false);
+
+    if (!keep_selected)
+    {
+        // Select the first interface
+        on_interfacesComboBox_currentIndexChanged(ui->interfacesComboBox->currentText());
+    }
+
+    updateWidgets();
+}
+
 void InterfaceToolbar::on_interfacesComboBox_currentIndexChanged(const QString &ifname)
 {
     foreach (int num, control_widget_.keys())
diff --git a/ui/qt/interface_toolbar.h b/ui/qt/interface_toolbar.h
index ebde37db4d..cdbfb57906 100644
--- a/ui/qt/interface_toolbar.h
+++ b/ui/qt/interface_toolbar.h
@@ -29,6 +29,7 @@
 #include "interface_toolbar_reader.h"
 
 #include <QFrame>
+#include <QList>
 #include <QMap>
 #include <QString>
 
@@ -42,6 +43,7 @@ struct interface_values
     QThread *reader_thread;
     int out_fd;
     QMap<int, QByteArray> value;
+    QMap<int, bool> value_changed;
     QMap<int, QList<QByteArray> > list;
     FunnelTextDialog *log_dialog;
     QString log_text;
@@ -56,11 +58,12 @@ public:
     explicit InterfaceToolbar(QWidget *parent = 0, const iface_toolbar *toolbar = NULL);
     ~InterfaceToolbar();
 
-    void startCapture(QString ifname, QString control_in, QString control_out);
+    void startCapture(GArray *ifaces);
     void stopCapture();
     bool hasInterface(QString ifname);
 
 public slots:
+    void interfaceListChanged();
     void controlReceived(QString ifname, int num, int command, QByteArray message);
 
 signals:
@@ -70,13 +73,13 @@ private slots:
     void startReaderThread(QString ifname, QString control_in);
     void updateWidgets();
 
-    void onButtonPressed();
+    void onControlButtonPressed();
+    void onLogButtonPressed();
+    void onHelpButtonPressed();
+    void onRestoreButtonPressed();
     void onCheckBoxChanged(int state);
     void onComboBoxChanged(int idx);
     void onLineEditChanged();
-    void onLogButtonPressed();
-    void onHelpButtonPressed();
-    void onResetButtonPressed();
 
     void closeLog();
 
diff --git a/ui/qt/interface_toolbar.ui b/ui/qt/interface_toolbar.ui
index 20faae5a57..3c4f47c07d 100644
--- a/ui/qt/interface_toolbar.ui
+++ b/ui/qt/interface_toolbar.ui
@@ -38,6 +38,9 @@
    </item>
    <item>
     <widget class="QComboBox" name="interfacesComboBox">
+     <property name="sizeAdjustPolicy">
+      <enum>QComboBox::AdjustToContents</enum>
+     </property>
      <property name="toolTip">
       <string>Select interface</string>
      </property>
diff --git a/ui/qt/main_window.cpp b/ui/qt/main_window.cpp
index 500dee87e3..b47b83e34c 100644
--- a/ui/qt/main_window.cpp
+++ b/ui/qt/main_window.cpp
@@ -855,6 +855,8 @@ void MainWindow::addInterfaceToolbar(const iface_toolbar *toolbar_entry)
     menu->insertAction(before, action);
 
     InterfaceToolbar *interface_toolbar = new InterfaceToolbar(this, toolbar_entry);
+    connect(wsApp, SIGNAL(appInitialized()), interface_toolbar, SLOT(interfaceListChanged()));
+    connect(wsApp, SIGNAL(localInterfaceListChanged()), interface_toolbar, SLOT(interfaceListChanged()));
 
     QToolBar *toolbar = new QToolBar(this);
     toolbar->addWidget(interface_toolbar);
@@ -2483,10 +2485,7 @@ void MainWindow::setForCaptureInProgress(bool capture_in_progress, GArray *iface
     QList<InterfaceToolbar *> toolbars = findChildren<InterfaceToolbar *>();
     foreach (InterfaceToolbar *toolbar, toolbars) {
         if (capture_in_progress && ifaces) {
-            for (guint i = 0; i < ifaces->len; i++) {
-                interface_options interface_opts = g_array_index(ifaces, interface_options, i);
-                toolbar->startCapture(interface_opts.name, interface_opts.extcap_control_in, interface_opts.extcap_control_out);
-            }
+            toolbar->startCapture(ifaces);
         } else {
             toolbar->stopCapture();
         }