From c21fbc9a469ca0c6903b097f0fec53c7b94bd5d8 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 17 Nov 2015 18:53:23 -0800 Subject: [PATCH 1/2] fix for messages-mixer bail early on empty settings --- assignment-client/src/AssignmentClient.cpp | 12 +++--- assignment-client/src/AssignmentClient.h | 1 + .../src/messages/MessagesMixer.cpp | 43 ++++++++++--------- assignment-client/src/octree/OctreeServer.cpp | 11 +++-- 4 files changed, 36 insertions(+), 31 deletions(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index bf5f9c3b7f..2d11f4d289 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -198,7 +198,7 @@ void AssignmentClient::sendStatusPacketToACM() { } void AssignmentClient::sendAssignmentRequest() { - if (!_currentAssignment) { + if (!_currentAssignment && !_isAssigned) { auto nodeList = DependencyManager::get(); @@ -229,8 +229,9 @@ void AssignmentClient::handleCreateAssignmentPacket(QSharedPointer pac // construct the deployed assignment from the packet data _currentAssignment = AssignmentFactory::unpackAssignment(*packet); - if (_currentAssignment) { + if (_currentAssignment && !_isAssigned) { qDebug() << "Received an assignment -" << *_currentAssignment; + _isAssigned = true; auto nodeList = DependencyManager::get(); @@ -309,12 +310,11 @@ void AssignmentClient::handleAuthenticationRequest() { } void AssignmentClient::assignmentCompleted() { - // we expect that to be here the previous assignment has completely cleaned up assert(_currentAssignment.isNull()); - // reset our current assignment pointer to NULL now that it has been deleted - _currentAssignment = NULL; + // reset our current assignment pointer to null now that it has been deleted + _currentAssignment = nullptr; // reset the logging target to the the CHILD_TARGET_NAME LogHandler::getInstance().setTargetName(ASSIGNMENT_CLIENT_TARGET_NAME); @@ -330,4 +330,6 @@ void AssignmentClient::assignmentCompleted() { nodeList->setOwnerType(NodeType::Unassigned); nodeList->reset(); nodeList->resetNodeInterestSet(); + + _isAssigned = false; } diff --git a/assignment-client/src/AssignmentClient.h b/assignment-client/src/AssignmentClient.h index 9d2c816861..9d7591f931 100644 --- a/assignment-client/src/AssignmentClient.h +++ b/assignment-client/src/AssignmentClient.h @@ -46,6 +46,7 @@ private: Assignment _requestAssignment; QPointer _currentAssignment; + bool _isAssigned { false }; QString _assignmentServerHostname; HifiSockAddr _assignmentServerSocket; QTimer _requestTimer; // timer for requesting and assignment diff --git a/assignment-client/src/messages/MessagesMixer.cpp b/assignment-client/src/messages/MessagesMixer.cpp index 99798b2d4f..f91076c335 100644 --- a/assignment-client/src/messages/MessagesMixer.cpp +++ b/assignment-client/src/messages/MessagesMixer.cpp @@ -128,27 +128,30 @@ void MessagesMixer::run() { auto nodeList = DependencyManager::get(); nodeList->addNodeTypeToInterestSet(NodeType::Agent); - + + // The messages-mixer currently does not have any settings, so it would be kind of insane to bail on an empty settings + // object. The below can be uncommented once messages-mixer settings are enabled. + // wait until we have the domain-server settings, otherwise we bail - DomainHandler& domainHandler = nodeList->getDomainHandler(); - - qDebug() << "Waiting for domain settings from domain-server."; - - // block until we get the settingsRequestComplete signal - QEventLoop loop; - connect(&domainHandler, &DomainHandler::settingsReceived, &loop, &QEventLoop::quit); - connect(&domainHandler, &DomainHandler::settingsReceiveFail, &loop, &QEventLoop::quit); - domainHandler.requestDomainSettings(); - loop.exec(); - - if (domainHandler.getSettingsObject().isEmpty()) { - qDebug() << "Failed to retreive settings object from domain-server. Bailing on assignment."; - setFinished(true); - return; - } - - // parse the settings to pull out the values we need - parseDomainServerSettings(domainHandler.getSettingsObject()); +// DomainHandler& domainHandler = nodeList->getDomainHandler(); +// +// qDebug() << "Waiting for domain settings from domain-server."; +// +// // block until we get the settingsRequestComplete signal +// QEventLoop loop; +// connect(&domainHandler, &DomainHandler::settingsReceived, &loop, &QEventLoop::quit); +// connect(&domainHandler, &DomainHandler::settingsReceiveFail, &loop, &QEventLoop::quit); +// domainHandler.requestDomainSettings(); +// loop.exec(); +// +// if (domainHandler.getSettingsObject().isEmpty()) { +// qDebug() << "Failed to retreive settings object from domain-server. Bailing on assignment."; +// setFinished(true); +// return; +// } +// +// // parse the settings to pull out the values we need +// parseDomainServerSettings(domainHandler.getSettingsObject()); } void MessagesMixer::parseDomainServerSettings(const QJsonObject& domainSettings) { diff --git a/assignment-client/src/octree/OctreeServer.cpp b/assignment-client/src/octree/OctreeServer.cpp index ad3df11474..84749bd975 100644 --- a/assignment-client/src/octree/OctreeServer.cpp +++ b/assignment-client/src/octree/OctreeServer.cpp @@ -953,7 +953,6 @@ bool OctreeServer::readConfiguration() { if (domainHandler.getSettingsObject().isEmpty()) { qDebug() << "Failed to retreive settings object from domain-server. Bailing on assignment."; - setFinished(true); return false; } @@ -1086,12 +1085,16 @@ void OctreeServer::run() { auto nodeList = DependencyManager::get(); nodeList->setOwnerType(getMyNodeType()); - // use common init to setup common timers and logging commonInit(getMyLoggingServerTargetName(), getMyNodeType()); + + // we need to ask the DS about agents so we can ping/reply with them + nodeList->addNodeTypeToInterestSet(NodeType::Agent); // read the configuration from either the payload or the domain server configuration if (!readConfiguration()) { + qDebug() << "OctreeServer bailing on run since readConfiguration has failed."; + setFinished(true); return; // bailing on run, because readConfiguration failed } @@ -1100,10 +1103,6 @@ void OctreeServer::run() { connect(nodeList.data(), SIGNAL(nodeAdded(SharedNodePointer)), SLOT(nodeAdded(SharedNodePointer))); connect(nodeList.data(), SIGNAL(nodeKilled(SharedNodePointer)), SLOT(nodeKilled(SharedNodePointer))); - - // we need to ask the DS about agents so we can ping/reply with them - nodeList->addNodeTypeToInterestSet(NodeType::Agent); - #ifndef WIN32 setvbuf(stdout, NULL, _IOLBF, 0); #endif From 2355ba70caa81e460599651cfcc3116692d82e28 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 17 Nov 2015 18:55:09 -0800 Subject: [PATCH 2/2] just remove messages-mixer settings grabbing all together --- .../src/messages/MessagesMixer.cpp | 30 ++----------------- .../src/messages/MessagesMixer.h | 2 -- 2 files changed, 2 insertions(+), 30 deletions(-) diff --git a/assignment-client/src/messages/MessagesMixer.cpp b/assignment-client/src/messages/MessagesMixer.cpp index f91076c335..d3662f3fb5 100644 --- a/assignment-client/src/messages/MessagesMixer.cpp +++ b/assignment-client/src/messages/MessagesMixer.cpp @@ -129,32 +129,6 @@ void MessagesMixer::run() { auto nodeList = DependencyManager::get(); nodeList->addNodeTypeToInterestSet(NodeType::Agent); - // The messages-mixer currently does not have any settings, so it would be kind of insane to bail on an empty settings - // object. The below can be uncommented once messages-mixer settings are enabled. - - // wait until we have the domain-server settings, otherwise we bail -// DomainHandler& domainHandler = nodeList->getDomainHandler(); -// -// qDebug() << "Waiting for domain settings from domain-server."; -// -// // block until we get the settingsRequestComplete signal -// QEventLoop loop; -// connect(&domainHandler, &DomainHandler::settingsReceived, &loop, &QEventLoop::quit); -// connect(&domainHandler, &DomainHandler::settingsReceiveFail, &loop, &QEventLoop::quit); -// domainHandler.requestDomainSettings(); -// loop.exec(); -// -// if (domainHandler.getSettingsObject().isEmpty()) { -// qDebug() << "Failed to retreive settings object from domain-server. Bailing on assignment."; -// setFinished(true); -// return; -// } -// -// // parse the settings to pull out the values we need -// parseDomainServerSettings(domainHandler.getSettingsObject()); -} - -void MessagesMixer::parseDomainServerSettings(const QJsonObject& domainSettings) { - // TODO - if we want options, parse them here... - const QString MESSAGES_MIXER_SETTINGS_KEY = "messages_mixer"; + // The messages-mixer currently does currently have any domain settings. If it did, they would be + // synchronously grabbed here. } diff --git a/assignment-client/src/messages/MessagesMixer.h b/assignment-client/src/messages/MessagesMixer.h index 12667bcc1b..65419a8ca6 100644 --- a/assignment-client/src/messages/MessagesMixer.h +++ b/assignment-client/src/messages/MessagesMixer.h @@ -35,8 +35,6 @@ private slots: void handleMessagesUnsubscribe(QSharedPointer packetList, SharedNodePointer senderNode); private: - void parseDomainServerSettings(const QJsonObject& domainSettings); - QHash> _channelSubscribers; };