BareMetal: Continue minimize dependency from GDB engine

The BareMetalDevice and the BareMetalDebugSupport classes should
not know anything about the specific implementation of the providers.

For this purpose, we just extend a providers by a new virtual methods
which are should be overridden for a concrete provider.

Change-Id: Ic5fef49f6b79e0ee7e8c4c386b4ed36cb5346dc1
Reviewed-by: hjk <hjk@qt.io>

src/plugins/baremetal/baremetaldebugsupport.cpp

@@ -66,78 +66,29 @@ BareMetalDebugSupport::BareMetalDebugSupport(RunControl *runControl)
     }
 
     const QString providerId = dev->debugServerProviderId();
-    const IDebugServerProvider *p = DebugServerProviderManager::findProvider(providerId);
+    IDebugServerProvider *p = DebugServerProviderManager::findProvider(providerId);
     if (!p) {
         reportFailure(tr("No debug server provider found for %1").arg(providerId));
         return;
     }
 
-    addTargetRunnerForProvider(p, runControl);
+    p->addTargetRunner(this, runControl);
 }
 
 void BareMetalDebugSupport::start()
 {
     const auto dev = qSharedPointerCast<const BareMetalDevice>(device());
     QTC_ASSERT(dev, reportFailure(); return);
-    const IDebugServerProvider *p = DebugServerProviderManager::findProvider(
+    IDebugServerProvider *p = DebugServerProviderManager::findProvider(
                 dev->debugServerProviderId());
     QTC_ASSERT(p, reportFailure(); return);
 
-    if (aboutToStart(p))
+    QString errorMessage;
+    if (!p->aboutToRun(this, errorMessage))
+        reportFailure(errorMessage);
+    else
         DebuggerRunTool::start();
 }
 
-bool BareMetalDebugSupport::aboutToStart(const IDebugServerProvider *provider)
-{
-    if (provider->engineType() != Debugger::GdbEngineType)
-        return true;
-
-    const auto exeAspect = runControl()->aspect<ExecutableAspect>();
-    QTC_ASSERT(exeAspect, reportFailure(); return false);
-
-    const FilePath bin = exeAspect->executable();
-    if (bin.isEmpty()) {
-        reportFailure(tr("Cannot debug: Local executable is not set."));
-        return false;
-    }
-    if (!bin.exists()) {
-        reportFailure(tr("Cannot debug: Could not find executable for \"%1\".")
-                      .arg(bin.toString()));
-        return false;
-    }
-
-    const auto gdbProvider = static_cast<const GdbServerProvider *>(provider);
-
-    Runnable inferior;
-    inferior.executable = bin;
-    if (const auto aspect = runControl()->aspect<ArgumentsAspect>())
-        inferior.commandLineArguments = aspect->arguments(runControl()->macroExpander());
-    setInferior(inferior);
-    setSymbolFile(bin);
-    setStartMode(AttachToRemoteServer);
-    setCommandsAfterConnect(gdbProvider->initCommands()); // .. and here?
-    setCommandsForReset(gdbProvider->resetCommands());
-    setRemoteChannel(gdbProvider->channelString());
-    setUseContinueInsteadOfRun(true);
-    setUseExtendedRemote(gdbProvider->useExtendedRemote());
-    return true;
-}
-
-void BareMetalDebugSupport::addTargetRunnerForProvider(const IDebugServerProvider *provider,
-                                                       ProjectExplorer::RunControl *runControl)
-{
-    if (provider->engineType() != Debugger::GdbEngineType)
-        return;
-    const auto gdbProvider = static_cast<const GdbServerProvider *>(provider);
-    if (gdbProvider->startupMode() == GdbServerProvider::StartupOnNetwork) {
-        Runnable r;
-        r.setCommandLine(gdbProvider->command());
-        // Command arguments are in host OS style as the bare metal's GDB servers are launched
-        // on the host, not on that target.
-        const auto gdbServer = new GdbServerProviderRunner(runControl, r);
-        addStartDependency(gdbServer);
-    }
-}
-
 } // namespace Internal
 } // namespace BareMetal

src/plugins/baremetal/baremetaldebugsupport.h

@@ -47,9 +47,6 @@ public:
 
 private:
     void start() final;
-    bool aboutToStart(const IDebugServerProvider *provider);
-    void addTargetRunnerForProvider(const IDebugServerProvider *provider,
-                                    ProjectExplorer::RunControl *runControl);
 };
 
 } // namespace Internal

src/plugins/baremetal/baremetaldevice.cpp

@@ -30,9 +30,8 @@
 #include "baremetaldeviceconfigurationwizard.h"
 #include "debugserverprovidermanager.h"
 
-// GDB server providers.
+// TODO: Remove mention about GDB server providers from this file!
 #include "debugservers/gdb/defaultgdbserverprovider.h"
-#include "debugservers/gdb/gdbserverproviderprocess.h"
 
 #include <coreplugin/id.h>
 
@@ -85,7 +84,7 @@ void BareMetalDevice::setDebugServerProviderId(const QString &id)
         currentProvider->unregisterDevice(this);
     m_debugServerProviderId = id;
     if (IDebugServerProvider *provider = DebugServerProviderManager::findProvider(id)) {
-        setChannelByServerProvider(provider);
+        provider->updateDevice(this);
         provider->registerDevice(this);
     }
 }
@@ -101,24 +100,7 @@ void BareMetalDevice::debugServerProviderUpdated(IDebugServerProvider *provider)
     IDebugServerProvider *myProvider = DebugServerProviderManager::findProvider(
                 m_debugServerProviderId);
     if (provider == myProvider)
-        setChannelByServerProvider(provider);
+        provider->updateDevice(this);
-}
-
-void BareMetalDevice::setChannelByServerProvider(IDebugServerProvider *provider)
-{
-    QTC_ASSERT(provider, return);
-    if (provider->engineType() != Debugger::GdbEngineType)
-        return;
-
-    const auto gdbProvider = static_cast<GdbServerProvider *>(provider);
-    const QString channel = gdbProvider->channelString();
-    const int colon = channel.indexOf(':');
-    if (colon < 0)
-        return;
-    QSsh::SshConnectionParameters sshParams = sshParameters();
-    sshParams.setHost(channel.left(colon));
-    sshParams.setPort(channel.midRef(colon + 1).toUShort());
-    setSshParameters(sshParams);
 }
 
 void BareMetalDevice::fromMap(const QVariantMap &map)
@@ -165,8 +147,7 @@ bool BareMetalDevice::canCreateProcess() const
 {
     if (IDebugServerProvider *provider = DebugServerProviderManager::findProvider(
                 m_debugServerProviderId)) {
-        if (provider->engineType() == Debugger::GdbEngineType)
+        return provider->canCreateProcess();
-            return true;
     }
 
     return false;
@@ -176,8 +157,7 @@ DeviceProcess *BareMetalDevice::createProcess(QObject *parent) const
 {
     if (IDebugServerProvider *provider = DebugServerProviderManager::findProvider(
                 m_debugServerProviderId)) {
-        if (provider->engineType() == Debugger::GdbEngineType)
+        return provider->createProcess(sharedFromThis(), parent);
-            return new GdbServerProviderProcess(sharedFromThis(), parent);
     }
 
     return nullptr;

src/plugins/baremetal/baremetaldevice.h

@@ -64,10 +64,6 @@ public:
 
 private:
     BareMetalDevice();
-
-    // Only for GDB servers!
-    void setChannelByServerProvider(IDebugServerProvider *provider);
-
     QString m_debugServerProviderId;
 };
 

src/plugins/baremetal/debugservers/gdb/gdbserverprovider.cpp

@@ -25,17 +25,26 @@
 
 #include "gdbserverprovider.h"
 
+#include <baremetal/baremetaldebugsupport.h>
 #include <baremetal/baremetaldevice.h>
 #include <baremetal/debugserverprovidermanager.h>
 
+#include <projectexplorer/runconfigurationaspects.h>
+
 #include <utils/environment.h>
 #include <utils/qtcassert.h>
 
+#include <ssh/sshconnection.h>
+
 #include <QComboBox>
 #include <QFormLayout>
 #include <QLineEdit>
 #include <QSpinBox>
 
+using namespace Debugger;
+using namespace ProjectExplorer;
+using namespace Utils;
+
 namespace BareMetal {
 namespace Internal {
 
@@ -164,6 +173,69 @@ bool GdbServerProvider::isValid() const
     return !channelString().isEmpty();
 }
 
+bool GdbServerProvider::aboutToRun(DebuggerRunTool *runTool,
+                                   QString &errorMessage) const
+{
+    QTC_ASSERT(runTool, return false);
+    const RunControl *runControl = runTool->runControl();
+    const auto exeAspect = runControl->aspect<ExecutableAspect>();
+    QTC_ASSERT(exeAspect, return false);
+
+    const FilePath bin = exeAspect->executable();
+    if (bin.isEmpty()) {
+        errorMessage = BareMetalDebugSupport::tr(
+                    "Cannot debug: Local executable is not set.");
+        return false;
+    }
+    if (!bin.exists()) {
+        errorMessage = BareMetalDebugSupport::tr(
+                    "Cannot debug: Could not find executable for \"%1\".")
+                .arg(bin.toString());
+        return false;
+    }
+
+    Runnable inferior;
+    inferior.executable = bin;
+    if (const auto argAspect = runControl->aspect<ArgumentsAspect>())
+        inferior.commandLineArguments = argAspect->arguments(runControl->macroExpander());
+    runTool->setInferior(inferior);
+    runTool->setSymbolFile(bin);
+    runTool->setStartMode(AttachToRemoteServer);
+    runTool->setCommandsAfterConnect(initCommands()); // .. and here?
+    runTool->setCommandsForReset(resetCommands());
+    runTool->setRemoteChannel(channelString());
+    runTool->setUseContinueInsteadOfRun(true);
+    runTool->setUseExtendedRemote(useExtendedRemote());
+    return true;
+}
+
+void GdbServerProvider::addTargetRunner(Debugger::DebuggerRunTool *runTool,
+                                        ProjectExplorer::RunControl *runControl) const
+{
+    if (m_startupMode != GdbServerProvider::StartupOnNetwork)
+        return;
+
+    Runnable r;
+    r.setCommandLine(command());
+    // Command arguments are in host OS style as the bare metal's GDB servers are launched
+    // on the host, not on that target.
+    const auto runner = new GdbServerProviderRunner(runControl, r);
+    runTool->addStartDependency(runner);
+}
+
+void GdbServerProvider::updateDevice(ProjectExplorer::IDevice *dev) const
+{
+    QTC_ASSERT(dev, return);
+    const QString channel = channelString();
+    const int colon = channel.indexOf(':');
+    if (colon < 0)
+        return;
+    QSsh::SshConnectionParameters sshParams = dev->sshParameters();
+    sshParams.setHost(channel.left(colon));
+    sshParams.setPort(channel.midRef(colon + 1).toUShort());
+    dev->setSshParameters(sshParams);
+}
+
 bool GdbServerProvider::canStartupMode(StartupMode m) const
 {
     return m == NoStartup;

src/plugins/baremetal/debugservers/gdb/gdbserverprovider.h

@@ -64,6 +64,12 @@ public:
 
     virtual Utils::CommandLine command() const;
 
+    bool aboutToRun(Debugger::DebuggerRunTool *runTool,
+                    QString &errorMessage) const final;
+    void addTargetRunner(Debugger::DebuggerRunTool *runTool,
+                         ProjectExplorer::RunControl *runControl) const final;
+    void updateDevice(ProjectExplorer::IDevice *dev) const final;
+
     bool isValid() const override;
     virtual bool canStartupMode(StartupMode) const;
 

src/plugins/baremetal/debugservers/gdb/jlinkgdbserverprovider.cpp

@@ -23,6 +23,7 @@
 **
 ****************************************************************************/
 
+#include "gdbserverproviderprocess.h"
 #include "jlinkgdbserverprovider.h"
 
 #include <baremetal/baremetalconstants.h>
@@ -142,6 +143,13 @@ bool JLinkGdbServerProvider::isValid() const
     return true;
 }
 
+ProjectExplorer::DeviceProcess *JLinkGdbServerProvider::createProcess(
+        const QSharedPointer<const ProjectExplorer::IDevice> &device,
+        QObject *parent) const
+{
+    return new GdbServerProviderProcess(device, parent);
+}
+
 GdbServerProvider *JLinkGdbServerProvider::clone() const
 {
     return new JLinkGdbServerProvider(*this);

src/plugins/baremetal/debugservers/gdb/jlinkgdbserverprovider.h

@@ -54,7 +54,11 @@ public:
 
     bool canStartupMode(StartupMode mode) const final;
     bool isValid() const final;
-    bool hasProcess() const final { return true; }
+
+    bool canCreateProcess() const final { return true; }
+    ProjectExplorer::DeviceProcess *createProcess(
+            const QSharedPointer<const ProjectExplorer::IDevice> &device,
+            QObject *parent = nullptr) const final;
 
 private:
     explicit JLinkGdbServerProvider();

src/plugins/baremetal/debugservers/gdb/openocdgdbserverprovider.cpp

@@ -23,6 +23,7 @@
 **
 ****************************************************************************/
 
+#include "gdbserverproviderprocess.h"
 #include "openocdgdbserverprovider.h"
 
 #include <baremetal/baremetalconstants.h>
@@ -149,6 +150,13 @@ bool OpenOcdGdbServerProvider::isValid() const
     return true;
 }
 
+ProjectExplorer::DeviceProcess *OpenOcdGdbServerProvider::createProcess(
+        const QSharedPointer<const ProjectExplorer::IDevice> &device,
+        QObject *parent) const
+{
+    return new GdbServerProviderProcess(device, parent);
+}
+
 GdbServerProvider *OpenOcdGdbServerProvider::clone() const
 {
     return new OpenOcdGdbServerProvider(*this);

src/plugins/baremetal/debugservers/gdb/openocdgdbserverprovider.h

@@ -54,7 +54,11 @@ public:
 
     bool canStartupMode(StartupMode mode) const final;
     bool isValid() const final;
-    bool hasProcess() const final { return true; }
+
+    bool canCreateProcess() const final { return true; }
+    ProjectExplorer::DeviceProcess *createProcess(
+            const QSharedPointer<const ProjectExplorer::IDevice> &device,
+            QObject *parent = nullptr) const final;
 
 private:
     explicit OpenOcdGdbServerProvider();

src/plugins/baremetal/debugservers/gdb/stlinkutilgdbserverprovider.cpp

@@ -23,6 +23,7 @@
 **
 ****************************************************************************/
 
+#include "gdbserverproviderprocess.h"
 #include "stlinkutilgdbserverprovider.h"
 
 #include <baremetal/baremetalconstants.h>
@@ -142,6 +143,13 @@ bool StLinkUtilGdbServerProvider::isValid() const
     return true;
 }
 
+ProjectExplorer::DeviceProcess *StLinkUtilGdbServerProvider::createProcess(
+        const QSharedPointer<const ProjectExplorer::IDevice> &device,
+        QObject *parent) const
+{
+    return new GdbServerProviderProcess(device, parent);
+}
+
 GdbServerProvider *StLinkUtilGdbServerProvider::clone() const
 {
     return new StLinkUtilGdbServerProvider(*this);

src/plugins/baremetal/debugservers/gdb/stlinkutilgdbserverprovider.h

@@ -57,7 +57,11 @@ public:
 
     bool canStartupMode(StartupMode mode) const final;
     bool isValid() const final;
-    bool hasProcess() const final { return true; }
+
+    bool canCreateProcess() const final { return true; }
+    ProjectExplorer::DeviceProcess *createProcess(
+            const QSharedPointer<const ProjectExplorer::IDevice> &device,
+            QObject *parent = nullptr) const final;
 
 private:
     explicit StLinkUtilGdbServerProvider();

src/plugins/baremetal/idebugserverprovider.h

@@ -40,6 +40,17 @@ class QLabel;
 class QLineEdit;
 QT_END_NAMESPACE
 
+namespace Debugger {
+class DebuggerRunTool;
+}
+
+namespace ProjectExplorer {
+class IDevice;
+class RunControl;
+class DeviceProcess;
+}
+
+
 namespace BareMetal {
 namespace Internal {
 
@@ -68,8 +79,18 @@ public:
 
     virtual QVariantMap toMap() const;
 
+    virtual bool aboutToRun(Debugger::DebuggerRunTool *runTool,
+                            QString &errorMessage) const = 0;
+    virtual void addTargetRunner(Debugger::DebuggerRunTool *runTool,
+                                 ProjectExplorer::RunControl *runControl) const = 0;
+    virtual void updateDevice(ProjectExplorer::IDevice *dev) const = 0;
+
     virtual bool isValid() const = 0;
-    virtual bool hasProcess() const { return false; }
+
+    virtual bool canCreateProcess() const { return false; }
+    virtual ProjectExplorer::DeviceProcess *createProcess(
+            const QSharedPointer<const ProjectExplorer::IDevice> &,
+            QObject *) const { return nullptr; }
 
     void registerDevice(BareMetalDevice *device);
     void unregisterDevice(BareMetalDevice *device);