From 85ef2c57e1a195851ab85d5a029890dded021dbe Mon Sep 17 00:00:00 2001 From: Nav Date: Sat, 4 Sep 2021 18:03:45 +0100 Subject: [PATCH] Removed tight coupling of target pin widgets with Insight window - moved target pin state toggling into an InsightWorker task. --- src/Insight/Insight.cpp | 1 - src/Insight/InsightWorker/InsightWorker.cpp | 16 ------ src/Insight/InsightWorker/InsightWorker.hpp | 7 --- .../InsightWorker/Tasks/SetTargetPinState.cpp | 7 +++ .../InsightWorker/Tasks/SetTargetPinState.hpp | 22 ++++++++ .../InsightWindow/InsightWindow.cpp | 20 +++----- .../InsightWindow/InsightWindow.hpp | 6 --- .../DIP/DualInlinePackageWidget.cpp | 4 +- .../Widgets/TargetWidgets/DIP/PinWidget.cpp | 8 ++- .../Widgets/TargetWidgets/DIP/PinWidget.hpp | 5 +- .../Widgets/TargetWidgets/QFP/PinWidget.cpp | 8 ++- .../Widgets/TargetWidgets/QFP/PinWidget.hpp | 5 +- .../QFP/QuadFlatPackageWidget.cpp | 4 +- .../Widgets/TargetWidgets/TargetPinWidget.cpp | 50 +++++++++++++++++++ .../Widgets/TargetWidgets/TargetPinWidget.hpp | 36 +++---------- 15 files changed, 114 insertions(+), 85 deletions(-) create mode 100644 src/Insight/InsightWorker/Tasks/SetTargetPinState.cpp create mode 100644 src/Insight/InsightWorker/Tasks/SetTargetPinState.hpp create mode 100644 src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPinWidget.cpp diff --git a/src/Insight/Insight.cpp b/src/Insight/Insight.cpp index d0f9195b..1a41e539 100644 --- a/src/Insight/Insight.cpp +++ b/src/Insight/Insight.cpp @@ -77,7 +77,6 @@ void Insight::startup() { this->connect(this->insightWorker, &InsightWorker::targetProgramCounterUpdated, this->mainWindow, &InsightWindow::onTargetProgramCounterUpdate); this->connect(this->insightWorker, &InsightWorker::targetIoPortsUpdated, this->mainWindow, &InsightWindow::onTargetIoPortsUpdate); this->connect(this->mainWindow, &InsightWindow::refreshTargetPinStates, this->insightWorker, &InsightWorker::requestPinStates); - this->connect(this->mainWindow, &InsightWindow::setTargetPinState, this->insightWorker, &InsightWorker::requestPinStateUpdate); this->mainWindow->setInsightConfig(this->insightConfig); this->mainWindow->setEnvironmentConfig(this->environmentConfig); diff --git a/src/Insight/InsightWorker/InsightWorker.cpp b/src/Insight/InsightWorker/InsightWorker.cpp index 9aebd2db..f88b6d38 100644 --- a/src/Insight/InsightWorker/InsightWorker.cpp +++ b/src/Insight/InsightWorker/InsightWorker.cpp @@ -28,10 +28,6 @@ void InsightWorker::startup() { std::bind(&InsightWorker::onTargetResumedEvent, this, std::placeholders::_1) ); - this->eventListener->registerCallbackForEventType( - std::bind(&InsightWorker::onTargetPinStatesRetrievedEvent, this, std::placeholders::_1) - ); - this->eventListener->registerCallbackForEventType( std::bind(&InsightWorker::onTargetIoPortsUpdatedEvent, this, std::placeholders::_1) ); @@ -81,14 +77,6 @@ void InsightWorker::requestPinStates(int variantId) { this->targetControllerConsole.requestPinStates(variantId); } -void InsightWorker::requestPinStateUpdate( - int variantId, - Bloom::Targets::TargetPinDescriptor pinDescriptor, - Bloom::Targets::TargetPinState pinState -) { - this->targetControllerConsole.setPinState(variantId, pinDescriptor, pinState); -} - void InsightWorker::onTargetStoppedEvent(const Events::TargetExecutionStopped& event) { /* * When we report a target halt to Insight, Insight will immediately seek more data from the target (such as GPIO @@ -125,10 +113,6 @@ void InsightWorker::onTargetResumedEvent(const Events::TargetExecutionResumed& e emit this->targetStateUpdated(TargetState::RUNNING); } -void InsightWorker::onTargetPinStatesRetrievedEvent(const Events::TargetPinStatesRetrieved& event) { - emit this->targetPinStatesUpdated(event.variantId, event.pinSatesByNumber); -} - void InsightWorker::onTargetIoPortsUpdatedEvent(const Events::TargetIoPortsUpdated& event) { emit this->targetIoPortsUpdated(); } diff --git a/src/Insight/InsightWorker/InsightWorker.hpp b/src/Insight/InsightWorker/InsightWorker.hpp index a92e3ed5..40dcc45a 100644 --- a/src/Insight/InsightWorker/InsightWorker.hpp +++ b/src/Insight/InsightWorker/InsightWorker.hpp @@ -39,7 +39,6 @@ namespace Bloom void onTargetStoppedEvent(const Events::TargetExecutionStopped& event); void onTargetResumedEvent(const Events::TargetExecutionResumed& event); - void onTargetPinStatesRetrievedEvent(const Events::TargetPinStatesRetrieved& event); void onTargetIoPortsUpdatedEvent(const Events::TargetIoPortsUpdated& event); void onTargetControllerStateReported(const Events::TargetControllerStateReported& event); @@ -58,17 +57,11 @@ namespace Bloom public slots: void startup(); void requestPinStates(int variantId); - void requestPinStateUpdate( - int variantId, - Bloom::Targets::TargetPinDescriptor pinDescriptor, - Bloom::Targets::TargetPinState pinState - ); signals: void taskQueued(); void targetStateUpdated(Bloom::Targets::TargetState newState); void targetProgramCounterUpdated(quint32 programCounter); - void targetPinStatesUpdated(int variantId, Bloom::Targets::TargetPinStateMappingType pinStatesByNumber); void targetIoPortsUpdated(); void targetControllerSuspended(); void targetControllerResumed(const Bloom::Targets::TargetDescriptor& targetDescriptor); diff --git a/src/Insight/InsightWorker/Tasks/SetTargetPinState.cpp b/src/Insight/InsightWorker/Tasks/SetTargetPinState.cpp new file mode 100644 index 00000000..d53d452b --- /dev/null +++ b/src/Insight/InsightWorker/Tasks/SetTargetPinState.cpp @@ -0,0 +1,7 @@ +#include "SetTargetPinState.hpp" + +using namespace Bloom; + +void SetTargetPinState::run(TargetControllerConsole& targetControllerConsole) { + targetControllerConsole.setPinState(this->pinDescriptor, this->pinState); +} diff --git a/src/Insight/InsightWorker/Tasks/SetTargetPinState.hpp b/src/Insight/InsightWorker/Tasks/SetTargetPinState.hpp new file mode 100644 index 00000000..f4d027c0 --- /dev/null +++ b/src/Insight/InsightWorker/Tasks/SetTargetPinState.hpp @@ -0,0 +1,22 @@ +#pragma once + +#include "InsightWorkerTask.hpp" +#include "src/Targets/TargetPinDescriptor.hpp" + +namespace Bloom +{ + class SetTargetPinState: public InsightWorkerTask + { + Q_OBJECT + private: + Targets::TargetPinDescriptor pinDescriptor; + Targets::TargetPinState pinState; + + protected: + void run(TargetControllerConsole& targetControllerConsole) override; + + public: + SetTargetPinState(const Targets::TargetPinDescriptor& pinDescriptor, const Targets::TargetPinState& pinState): + InsightWorkerTask(), pinDescriptor(pinDescriptor), pinState(pinState) {} + }; +} diff --git a/src/Insight/UserInterfaces/InsightWindow/InsightWindow.cpp b/src/Insight/UserInterfaces/InsightWindow/InsightWindow.cpp index 6958a9b1..ba0d9805 100644 --- a/src/Insight/UserInterfaces/InsightWindow/InsightWindow.cpp +++ b/src/Insight/UserInterfaces/InsightWindow/InsightWindow.cpp @@ -333,9 +333,14 @@ void InsightWindow::selectVariant(const TargetVariant* variant) { } if (this->targetPackageWidget != nullptr) { + this->targetPackageWidget->setTargetState(this->targetState); + if (this->targetState == TargetState::STOPPED) { - this->toggleUi(true); - emit this->refreshTargetPinStates(variant->id); + this->targetPackageWidget->refreshPinStates([this] { + if (this->targetState == TargetState::STOPPED) { + this->targetPackageWidget->setDisabled(false); + } + }); } this->targetPackageWidget->show(); @@ -441,17 +446,6 @@ void InsightWindow::onTargetIoPortsUpdate() { } } -void InsightWindow::togglePinIoState(InsightTargetWidgets::TargetPinWidget* pinWidget) { - auto pinState = pinWidget->getPinState(); - // Currently, we only allow users to toggle the IO state of output pins - if (pinState.has_value() - && pinState.value().ioDirection == TargetPinState::IoDirection::OUTPUT - && this->selectedVariant != nullptr - ) { - auto& pinDescriptor = pinWidget->getPinDescriptor(); - pinState.value().ioState = (pinState.value().ioState == TargetPinState::IoState::HIGH) ? - TargetPinState::IoState::LOW : TargetPinState::IoState::HIGH; - emit this->setTargetPinState(this->selectedVariant->id, pinDescriptor, pinState.value()); } } diff --git a/src/Insight/UserInterfaces/InsightWindow/InsightWindow.hpp b/src/Insight/UserInterfaces/InsightWindow/InsightWindow.hpp index 384360e1..51bdf91a 100644 --- a/src/Insight/UserInterfaces/InsightWindow/InsightWindow.hpp +++ b/src/Insight/UserInterfaces/InsightWindow/InsightWindow.hpp @@ -87,14 +87,8 @@ namespace Bloom void openReportIssuesUrl(); static void openGettingStartedUrl(); void openAboutWindow(); - void togglePinIoState(Widgets::InsightTargetWidgets::TargetPinWidget* pinWidget); signals: void refreshTargetPinStates(int variantId); - void setTargetPinState( - int variantId, - Bloom::Targets::TargetPinDescriptor pinDescriptor, - Bloom::Targets::TargetPinState pinState - ); }; } diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/DIP/DualInlinePackageWidget.cpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/DIP/DualInlinePackageWidget.cpp index ed1b12a2..6c20381f 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/DIP/DualInlinePackageWidget.cpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/DIP/DualInlinePackageWidget.cpp @@ -48,7 +48,7 @@ DualInlinePackageWidget::DualInlinePackageWidget( assert(insightWindow != nullptr); for (const auto& [targetPinNumber, targetPinDescriptor]: targetVariant.pinDescriptorsByNumber) { - auto pinWidget = new PinWidget(this, targetPinDescriptor, targetVariant); + auto pinWidget = new PinWidget(targetPinDescriptor, targetVariant, insightWorker, this); this->pinWidgets.push_back(pinWidget); if (targetPinNumber <= (targetVariant.pinDescriptorsByNumber.size() / 2)) { @@ -56,8 +56,6 @@ DualInlinePackageWidget::DualInlinePackageWidget( } else { this->topPinLayout->addWidget(pinWidget, 0, Qt::AlignmentFlag::AlignRight); } - - connect(pinWidget, &TargetPinWidget::toggleIoState, insightWindow, &InsightWindow::togglePinIoState); } this->layout->addLayout(this->topPinLayout); diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/DIP/PinWidget.cpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/DIP/PinWidget.cpp index 06bf6c50..88076742 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/DIP/PinWidget.cpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/DIP/PinWidget.cpp @@ -11,8 +11,12 @@ using namespace Bloom::Widgets::InsightTargetWidgets::Dip; using namespace Bloom::Targets; -PinWidget::PinWidget(QWidget* parent, const TargetPinDescriptor& pinDescriptor, const TargetVariant& targetVariant): - TargetPinWidget(parent, pinDescriptor, targetVariant) { +PinWidget::PinWidget( + const TargetPinDescriptor& pinDescriptor, + const TargetVariant& targetVariant, + InsightWorker& insightWorker, + QWidget* parent +): TargetPinWidget(pinDescriptor, targetVariant, insightWorker, parent) { this->layout = new QVBoxLayout(); this->layout->setContentsMargins(0, 0, 0, 0); this->layout->setSpacing(0); diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/DIP/PinWidget.hpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/DIP/PinWidget.hpp index cf39222f..4d78ba32 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/DIP/PinWidget.hpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/DIP/PinWidget.hpp @@ -34,9 +34,10 @@ namespace Bloom::Widgets::InsightTargetWidgets::Dip + (PinWidget::LABEL_HEIGHT * PinWidget::MAXIMUM_LABEL_COUNT); PinWidget( - QWidget* parent, const Targets::TargetPinDescriptor& pinDescriptor, - const Targets::TargetVariant& targetVariant + const Targets::TargetVariant& targetVariant, + InsightWorker& insightWorker, + QWidget* parent ); void updatePinState(const Targets::TargetPinState& pinState) override { diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/QFP/PinWidget.cpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/QFP/PinWidget.cpp index d50d7efa..e0b62aeb 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/QFP/PinWidget.cpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/QFP/PinWidget.cpp @@ -11,8 +11,12 @@ using namespace Bloom::Widgets::InsightTargetWidgets::Qfp; using namespace Bloom::Targets; -PinWidget::PinWidget(QWidget* parent, const TargetPinDescriptor& pinDescriptor, const TargetVariant& targetVariant): - TargetPinWidget(parent, pinDescriptor, targetVariant) { +PinWidget::PinWidget( + const TargetPinDescriptor& pinDescriptor, + const TargetVariant& targetVariant, + InsightWorker& insightWorker, + QWidget* parent +): TargetPinWidget(pinDescriptor, targetVariant, insightWorker, parent) { this->layout = new QBoxLayout(QBoxLayout::TopToBottom); this->layout->setContentsMargins(0, 0, 0, 0); this->layout->setSpacing(0); diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/QFP/PinWidget.hpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/QFP/PinWidget.hpp index a8b962eb..252a437f 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/QFP/PinWidget.hpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/QFP/PinWidget.hpp @@ -42,9 +42,10 @@ namespace Bloom::Widgets::InsightTargetWidgets::Qfp static const int MAXIMUM_VERTICAL_WIDTH = PinBodyWidget::WIDTH; PinWidget( - QWidget* parent, const Targets::TargetPinDescriptor& pinDescriptor, - const Targets::TargetVariant& targetVariant + const Targets::TargetVariant& targetVariant, + InsightWorker& insightWorker, + QWidget* parent ); void updatePinState(const Targets::TargetPinState& pinState) override; diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/QFP/QuadFlatPackageWidget.cpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/QFP/QuadFlatPackageWidget.cpp index 5d4e72e3..f4e1fffa 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/QFP/QuadFlatPackageWidget.cpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/QFP/QuadFlatPackageWidget.cpp @@ -61,7 +61,7 @@ QuadFlatPackageWidget::QuadFlatPackageWidget( auto pinCountPerLayout = (targetVariant.pinDescriptorsByNumber.size() / 4); for (const auto& [targetPinNumber, targetPinDescriptor]: targetVariant.pinDescriptorsByNumber) { - auto pinWidget = new PinWidget(this, targetPinDescriptor, targetVariant); + auto pinWidget = new PinWidget(targetPinDescriptor, targetVariant, insightWorker, this); this->pinWidgets.push_back(pinWidget); if (targetPinNumber <= pinCountPerLayout) { @@ -76,8 +76,6 @@ QuadFlatPackageWidget::QuadFlatPackageWidget( } else if (targetPinNumber > (pinCountPerLayout * 3) && targetPinNumber <= (pinCountPerLayout * 4)) { this->topPinLayout->addWidget(pinWidget, 0, Qt::AlignmentFlag::AlignBottom); } - - connect(pinWidget, &TargetPinWidget::toggleIoState, insightWindow, &InsightWindow::togglePinIoState); } this->bodyWidget = new BodyWidget(this); diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPinWidget.cpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPinWidget.cpp new file mode 100644 index 00000000..ac909dda --- /dev/null +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPinWidget.cpp @@ -0,0 +1,50 @@ +#include "TargetPinWidget.hpp" + +#include "src/Insight/InsightWorker/Tasks/SetTargetPinState.hpp" + +using namespace Bloom; +using namespace Bloom::Widgets::InsightTargetWidgets; + +using Bloom::Targets::TargetVariant; +using Bloom::Targets::TargetPinDescriptor; +using Bloom::Targets::TargetPinType; +using Bloom::Targets::TargetPinState; + +TargetPinWidget::TargetPinWidget( + Targets::TargetPinDescriptor pinDescriptor, + Targets::TargetVariant targetVariant, + InsightWorker& insightWorker, + QWidget* parent +): QWidget(parent), +insightWorker(insightWorker), +targetVariant(std::move(targetVariant)), +pinDescriptor(std::move(pinDescriptor)) { + if (this->pinDescriptor.type == TargetPinType::UNKNOWN) { + this->setDisabled(true); + } +} + +void TargetPinWidget::onWidgetBodyClicked() { + // Currently, we only allow users to toggle the IO state of output pins + if (this->pinState.has_value() + && this->pinState.value().ioDirection == TargetPinState::IoDirection::OUTPUT + ) { + this->setDisabled(true); + + auto pinState = this->pinState.value(); + pinState.ioState = (pinState.ioState == TargetPinState::IoState::HIGH) ? + TargetPinState::IoState::LOW : TargetPinState::IoState::HIGH; + + auto setPinStateTask = new SetTargetPinState(this->pinDescriptor, pinState); + this->connect(setPinStateTask, &InsightWorkerTask::completed, this, [this, pinState] { + this->updatePinState(pinState); + this->setDisabled(false); + }); + + this->connect(setPinStateTask, &InsightWorkerTask::failed, this, [this] { + this->setDisabled(false); + }); + + this->insightWorker.queueTask(setPinStateTask); + } +} diff --git a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPinWidget.hpp b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPinWidget.hpp index 0f12138b..3a21c33b 100644 --- a/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPinWidget.hpp +++ b/src/Insight/UserInterfaces/InsightWindow/Widgets/TargetWidgets/TargetPinWidget.hpp @@ -3,6 +3,7 @@ #include #include +#include "src/Insight/InsightWorker/InsightWorker.hpp" #include "src/Targets/TargetVariant.hpp" #include "src/Targets/TargetPinDescriptor.hpp" @@ -12,6 +13,8 @@ namespace Bloom::Widgets::InsightTargetWidgets { Q_OBJECT protected: + InsightWorker& insightWorker; + Targets::TargetVariant targetVariant; Targets::TargetPinDescriptor pinDescriptor; std::optional pinState; @@ -19,25 +22,16 @@ namespace Bloom::Widgets::InsightTargetWidgets public: TargetPinWidget( - QWidget* parent, Targets::TargetPinDescriptor pinDescriptor, - Targets::TargetVariant targetVariant - ): QWidget(parent), targetVariant(std::move(targetVariant)), pinDescriptor(std::move(pinDescriptor)) { - this->setDisabled(false); - }; + Targets::TargetVariant targetVariant, + InsightWorker& insightWorker, + QWidget* parent + ); int getPinNumber() const { return this->pinDescriptor.number; } - const Targets::TargetPinDescriptor& getPinDescriptor() const { - return this->pinDescriptor; - } - - std::optional getPinState() const { - return this->pinState; - } - virtual void updatePinState(const Targets::TargetPinState& pinState) { this->pinStateChanged = !this->pinState.has_value() || this->pinState->ioState != pinState.ioState @@ -46,21 +40,7 @@ namespace Bloom::Widgets::InsightTargetWidgets this->pinState = pinState; } - void setDisabled(bool disabled) { - if (pinDescriptor.type != Targets::TargetPinType::UNKNOWN) { - QWidget::setDisabled(disabled); - - } else { - QWidget::setDisabled(true); - } - } - public slots: - void onWidgetBodyClicked() { - emit this->toggleIoState(this); - } - - signals: - void toggleIoState(TargetPinWidget* pinWidget); + virtual void onWidgetBodyClicked(); }; }