From d118ea402fec326815af9bcf3d3650762903a51a Mon Sep 17 00:00:00 2001 From: Klaus Basan Date: Mon, 22 Jan 2018 05:07:00 +0100 Subject: [PATCH] Ref T232 clear remote aircraft data once network disconnects * blackconfig in .pro * verify function to detect dangling states * clear remote data when disconnected from network * there was one problem that the data where not correctly cleaned up and hence new aircraft where not added again after a disconnect/reconnect from network * it is not yet clear why data happens (dangling data), that is what the debugVerify function is for --- src/blackcore/context/contextsimulator.h | 3 ++ .../context/contextsimulatorimpl.cpp | 12 ++++- src/blackcore/context/contextsimulatorimpl.h | 2 +- .../context/contextsimulatorproxy.cpp | 4 ++ src/blackcore/simulator.h | 10 ++++ src/blackcore/simulatorcommon.cpp | 26 ++++++++--- src/blackcore/simulatorcommon.h | 18 ++++---- .../simulatoremulatedmonitordialog.cpp | 2 +- src/plugins/simulator/fscommon/fscommon.pro | 2 +- src/plugins/simulator/fsx/fsx.pro | 2 +- .../fsxcommon/simulatorfsxcommon.cpp | 46 ++++++++++++++----- .../simulator/fsxcommon/simulatorfsxcommon.h | 3 +- src/plugins/simulator/p3d/p3d.pro | 2 +- 13 files changed, 98 insertions(+), 34 deletions(-) diff --git a/src/blackcore/context/contextsimulator.h b/src/blackcore/context/contextsimulator.h index d5d909149..be1458dd6 100644 --- a/src/blackcore/context/contextsimulator.h +++ b/src/blackcore/context/contextsimulator.h @@ -115,6 +115,9 @@ namespace BlackCore //! A weather grid, requested with requestWeatherGrid(), is received void weatherGridReceived(const BlackMisc::Weather::CWeatherGrid &weatherGrid, const BlackMisc::CIdentifier &identifier); + //! Relevant simulator messages to be explicitly displayed + void driverMessages(const BlackMisc::CStatusMessageList &messages); + public slots: //! Simulator info, currently loaded plugin virtual BlackMisc::Simulation::CSimulatorPluginInfo getSimulatorPluginInfo() const = 0; diff --git a/src/blackcore/context/contextsimulatorimpl.cpp b/src/blackcore/context/contextsimulatorimpl.cpp index 7b789781e..fd22c3e24 100644 --- a/src/blackcore/context/contextsimulatorimpl.cpp +++ b/src/blackcore/context/contextsimulatorimpl.cpp @@ -299,6 +299,8 @@ namespace BlackCore Q_ASSERT(c); c = connect(simulator, &ISimulator::airspaceSnapshotHandled, this, &IContextSimulator::airspaceSnapshotHandled); Q_ASSERT(c); + c = connect(simulator, &ISimulator::driverMessages, this, &IContextSimulator::driverMessages); + Q_ASSERT(c); // log from context to simulator c = connect(CLogHandler::instance(), &CLogHandler::localMessageLogged, this, &CContextSimulator::relayStatusMessageToSimulator); @@ -504,11 +506,19 @@ namespace BlackCore { m_networkSessionId = this->getIContextNetwork()->getConnectedServer().getServerSessionId(); } - else if (!m_networkSessionId.isEmpty()) + else if (INetwork::isDisconnectedStatus(to)) { m_networkSessionId.clear(); m_aircraftMatcher.clearMatchingStatistics(); m_matchingMessages.clear(); + + // check in case the plugin has been unloaded + if (m_simulatorPlugin.second) + { + const CStatusMessageList verifyMessages = m_simulatorPlugin.second->debugVerifyStateAfterAllAircraftRemoved(); + m_simulatorPlugin.second->clearAllRemoteAircraftData(); + if (!verifyMessages.isEmpty()) { emit this->driverMessages(verifyMessages); } + } } } diff --git a/src/blackcore/context/contextsimulatorimpl.h b/src/blackcore/context/contextsimulatorimpl.h index f27d2610c..858246325 100644 --- a/src/blackcore/context/contextsimulatorimpl.h +++ b/src/blackcore/context/contextsimulatorimpl.h @@ -225,7 +225,7 @@ namespace BlackCore QMap m_matchingMessages; BlackMisc::CSettingReadOnly m_messageSettings { this }; //!< settings for messages BlackMisc::CSettingReadOnly m_enabledSimulators { this, &CContextSimulator::changeEnabledSimulators }; - QString m_networkSessionId; //! Network session of CServer, if not connected empty + QString m_networkSessionId; //! Network session of CServer::getServerSessionId, if not connected empty bool m_initallyAddAircrafts = false; bool m_enableMatchingMessages = true; bool m_isWeatherActivated = false; diff --git a/src/blackcore/context/contextsimulatorproxy.cpp b/src/blackcore/context/contextsimulatorproxy.cpp index 7e39b6b5e..5fbbadf4d 100644 --- a/src/blackcore/context/contextsimulatorproxy.cpp +++ b/src/blackcore/context/contextsimulatorproxy.cpp @@ -74,6 +74,10 @@ namespace BlackCore s = connection.connect(serviceName, IContextSimulator::ObjectPath(), IContextSimulator::InterfaceName(), "addingRemoteModelFailed", this, SIGNAL(addingRemoteModelFailed(BlackMisc::Simulation::CSimulatedAircraft, BlackMisc::CStatusMessage))); Q_ASSERT(s); + s = connection.connect(serviceName, IContextSimulator::ObjectPath(), IContextSimulator::InterfaceName(), + "driverMessages", this, SIGNAL(driverMessages(BlackMisc::CStatusMessageList))); + + Q_ASSERT(s); Q_UNUSED(s); this->relayBaseClassSignals(serviceName, connection, IContextSimulator::ObjectPath(), IContextSimulator::InterfaceName()); } diff --git a/src/blackcore/simulator.h b/src/blackcore/simulator.h index e18224ba6..e0b4f1055 100644 --- a/src/blackcore/simulator.h +++ b/src/blackcore/simulator.h @@ -160,6 +160,13 @@ namespace BlackCore //! Driver will be unloaded virtual void unload() = 0; + //! Clear all aircraft related data + virtual void clearAllRemoteAircraftData() = 0; + + //! Debug function to check state after all aircraft have been removed + //! \remarks only in local developer builds + virtual BlackMisc::CStatusMessageList debugVerifyStateAfterAllAircraftRemoved() const = 0; + //! Is overall (swift) application shutting down virtual bool isShuttingDown() const = 0; @@ -202,6 +209,9 @@ namespace BlackCore //! An airspace snapshot was handled void airspaceSnapshotHandled(); + //! Relevant simulator messages to be explicitly displayed + void driverMessages(const BlackMisc::CStatusMessageList &messages); + protected: //! Default constructor ISimulator(QObject *parent = nullptr); diff --git a/src/blackcore/simulatorcommon.cpp b/src/blackcore/simulatorcommon.cpp index 48c13131a..49815ee06 100644 --- a/src/blackcore/simulatorcommon.cpp +++ b/src/blackcore/simulatorcommon.cpp @@ -23,6 +23,7 @@ #include "blackmisc/logmessage.h" #include "blackmisc/statusmessage.h" #include "blackmisc/threadutils.h" +#include "blackconfig/buildconfig.h" #include #include @@ -32,6 +33,7 @@ #include #include +using namespace BlackConfig; using namespace BlackMisc; using namespace BlackMisc::Geo; using namespace BlackMisc::Aviation; @@ -121,7 +123,7 @@ namespace BlackCore this->callPhysicallyRemoveRemoteAircraft(callsign); return true; } - // will be added with next snapshot ps_recalculateRenderedAircraft + // will be added with next snapshot onRecalculatedRenderedAircraft return false; } @@ -384,7 +386,10 @@ namespace BlackCore { // a default implementation, but normally overridden by the sims const CCallsignSet callsigns = this->getAircraftInRangeCallsigns(); - return this->physicallyRemoveMultipleRemoteAircraft(callsigns); + const int r = this->physicallyRemoveMultipleRemoteAircraft(callsigns); + this->debugVerifyStateAfterAllAircraftRemoved(); + this->clearAllRemoteAircraftData(); + return r; } bool CSimulatorCommon::parseCommandLine(const QString &commandLine, const CIdentifier &originator) @@ -490,7 +495,7 @@ namespace BlackCore CSimpleCommandParser::registerCommand({".drv spline|linear ", "set spline/linear interpolator for one/all callsign(s)"}); } - void CSimulatorCommon::resetStatistics() + void CSimulatorCommon::resetAircraftStatistics() { m_statsPhysicallyAddedAircraft = 0; m_statsPhysicallyRemovedAircraft = 0; @@ -498,6 +503,15 @@ namespace BlackCore m_statsSituationAdded = 0; } + CStatusMessageList CSimulatorCommon::debugVerifyStateAfterAllAircraftRemoved() const + { + CStatusMessageList msgs; + if (!CBuildConfig::isLocalDeveloperDebugBuild()) { return msgs; } + if (!m_addAgainAircraftWhenRemoved.isEmpty()) { msgs.push_back(CStatusMessage(this).error("m_addAgainAircraftWhenRemoved not empty: '%1'") << m_addAgainAircraftWhenRemoved.getCallsignStrings(true).join(", ")); } + if (!m_hints.isEmpty()) { msgs.push_back(CStatusMessage(this).error("m_hints not empty: '%1'") << CCallsignSet(m_hints.keys()).getCallsignStrings(true).join(", ")); } + return msgs; + } + void CSimulatorCommon::oneSecondTimerTimeout() { this->blinkHighlightedAircraft(); @@ -578,16 +592,16 @@ namespace BlackCore m_statsUpdateAircraftTimeTotalMs = 0; m_blinkCycle = false; m_highlightEndTimeMsEpoch = false; - this->resetStatistics(); - this->clearAllAircraft(); + this->clearAllRemoteAircraftData(); } - void CSimulatorCommon::clearAllAircraft() + void CSimulatorCommon::clearAllRemoteAircraftData() { m_addAgainAircraftWhenRemoved.clear(); m_highlightedAircraft.clear(); m_callsignsToBeRendered.clear(); m_hints.clear(); + this->resetAircraftStatistics(); } CAirportList CSimulatorCommon::getWebServiceAirports() const diff --git a/src/blackcore/simulatorcommon.h b/src/blackcore/simulatorcommon.h index 849358d30..25f0e8cba 100644 --- a/src/blackcore/simulatorcommon.h +++ b/src/blackcore/simulatorcommon.h @@ -86,6 +86,8 @@ namespace BlackCore virtual bool isShuttingDown() const override; virtual int physicallyRemoveMultipleRemoteAircraft(const BlackMisc::Aviation::CCallsignSet &callsigns) override; virtual int physicallyRemoveAllRemoteAircraft() override; + virtual void clearAllRemoteAircraftData() override; + virtual BlackMisc::CStatusMessageList debugVerifyStateAfterAllAircraftRemoved() const override; //! \addtogroup swiftdotcommands //! @{ @@ -100,14 +102,13 @@ namespace BlackCore //! @} //! \copydoc ISimulator::parseCommandLine virtual bool parseCommandLine(const QString &commandLine, const BlackMisc::CIdentifier &originator) override; + // --------- ISimulator implementations ------------ //! Register help static void registerHelp(); - // --------- ISimulator implementations ------------ - //! Reset the statistics counters - void resetStatistics(); + void resetAircraftStatistics(); //! Counter added aircraft int getStatisticsPhysicallyAddedAircraft() const { return m_statsPhysicallyAddedAircraft; } @@ -161,12 +162,11 @@ namespace BlackCore //! Max.airports in range int maxAirportsInRange() const; - //! Reset state + //! Full reset of state + //! \remark reset as it was unloaded without unloading + //! \sa ISimulator::clearAllRemoteAircraftData virtual void reset(); - //! Clear all aircraft related data - virtual void clearAllAircraft(); - //! Inject weather grid to simulator virtual void injectWeatherGrid(const BlackMisc::Weather::CWeatherGrid &weatherGrid) { Q_UNUSED(weatherGrid); } @@ -223,9 +223,9 @@ namespace BlackCore // setup for debugging, logs .. BlackMisc::Simulation::CInterpolationAndRenderingSetup m_interpolationRenderingSetup; //!< logging, rendering etc. - // some optional functionality which can be used by the sims as needed + // some optional functionality which can be used by the simulators as needed BlackMisc::Simulation::CSimulatedAircraftList m_addAgainAircraftWhenRemoved; //!< add this model again when removed, normally used to change model - QHash m_hints; //!< last ground elevation fetched + QHash m_hints; //!< hints for callsign, contains last ground elevation fetched bool m_isWeatherActivated = false; //!< Is simulator weather activated? BlackMisc::Geo::CCoordinateGeodetic m_lastWeatherPosition; //!< Own aircraft position at which weather was fetched and injected last diff --git a/src/plugins/simulator/emulated/simulatoremulatedmonitordialog.cpp b/src/plugins/simulator/emulated/simulatoremulatedmonitordialog.cpp index 1bab5fc9e..b117f4e9b 100644 --- a/src/plugins/simulator/emulated/simulatoremulatedmonitordialog.cpp +++ b/src/plugins/simulator/emulated/simulatoremulatedmonitordialog.cpp @@ -213,7 +213,7 @@ namespace BlackSimPlugin void CSimulatorEmulatedMonitorDialog::resetStatistics() { - if (!m_simulator) { m_simulator->resetStatistics(); } + if (!m_simulator) { m_simulator->resetAircraftStatistics(); } ui->le_PhysicallyAddedAircraft->clear(); ui->le_PhysicallyRemovedAircraft->clear(); ui->le_SituationAdded->clear(); diff --git a/src/plugins/simulator/fscommon/fscommon.pro b/src/plugins/simulator/fscommon/fscommon.pro index fac1d891d..e0be7997a 100644 --- a/src/plugins/simulator/fscommon/fscommon.pro +++ b/src/plugins/simulator/fscommon/fscommon.pro @@ -6,7 +6,7 @@ TARGET = simulatorfscommon TEMPLATE = lib CONFIG += staticlib -CONFIG += blackmisc +CONFIG += blackconfig blackmisc DEPENDPATH += . $$SourceRoot/src INCLUDEPATH += . $$SourceRoot/src diff --git a/src/plugins/simulator/fsx/fsx.pro b/src/plugins/simulator/fsx/fsx.pro index 4ae5c0d3b..162bd35ba 100644 --- a/src/plugins/simulator/fsx/fsx.pro +++ b/src/plugins/simulator/fsx/fsx.pro @@ -8,7 +8,7 @@ TARGET = simulatorfsx TEMPLATE = lib CONFIG += plugin shared -CONFIG += blackmisc blackcore +CONFIG += blackconfig blackmisc blackcore DEPENDPATH += . $$SourceRoot/src INCLUDEPATH += . $$SourceRoot/src diff --git a/src/plugins/simulator/fsxcommon/simulatorfsxcommon.cpp b/src/plugins/simulator/fsxcommon/simulatorfsxcommon.cpp index 8ef7af42d..e51224388 100644 --- a/src/plugins/simulator/fsxcommon/simulatorfsxcommon.cpp +++ b/src/plugins/simulator/fsxcommon/simulatorfsxcommon.cpp @@ -20,12 +20,15 @@ #include "blackmisc/aviation/airportlist.h" #include "blackmisc/geo/elevationplane.h" #include "blackmisc/logmessage.h" +#include "blackmisc/statusmessagelist.h" #include "blackmisc/threadutils.h" #include "blackmisc/verify.h" +#include "blackconfig/buildconfig.h" #include #include +using namespace BlackConfig; using namespace BlackMisc; using namespace BlackMisc::Aviation; using namespace BlackMisc::PhysicalQuantities; @@ -254,6 +257,17 @@ namespace BlackSimPlugin } } + CStatusMessageList CSimulatorFsxCommon::debugVerifyStateAfterAllAircraftRemoved() const + { + CStatusMessageList msgs; + if (CBuildConfig::isLocalDeveloperDebugBuild()) { return msgs; } + msgs = CSimulatorFsCommon::debugVerifyStateAfterAllAircraftRemoved(); + if (!m_simConnectObjects.isEmpty()) { msgs.push_back(CStatusMessage(this).error("m_simConnectObjects not empty: '%1'") << m_simConnectObjects.getAllCallsignStringsAsString(true)); } + if (!m_simConnectObjectsPositionAndPartsTraces.isEmpty()) { msgs.push_back(CStatusMessage(this).error("m_simConnectObjectsPositionAndPartsTraces not empty: '%1'") << m_simConnectObjectsPositionAndPartsTraces.getAllCallsignStringsAsString(true)); } + if (!m_addAgainAircraftWhenRemoved.isEmpty()) { msgs.push_back(CStatusMessage(this).error("m_addAgainAircraftWhenRemoved not empty: '%1'") << m_addAgainAircraftWhenRemoved.getCallsignStrings(true).join(", ")); } + return msgs; + } + bool CSimulatorFsxCommon::stillDisplayReceiveExceptions() { m_receiveExceptionCount++; @@ -267,7 +281,7 @@ namespace BlackSimPlugin this->emitSimulatorCombinedStatus(); // Internals depends on sim data which take a while to be read - // this is a trich and I re-init again after a while (which is not really expensive) + // this is a trick and I re-init again after a while (which is not really expensive) QTimer::singleShot(1000, this, [this] { this->initSimulatorInternals(); }); } @@ -665,7 +679,7 @@ namespace BlackSimPlugin // good case, object has been removed // we can remove the sim object } - else + else if (!this->isShuttingDown()) { // object was removed, but removal was not requested by us // this means we are out of the reality bubble or something else went wrong @@ -673,17 +687,19 @@ namespace BlackSimPlugin // 1) out of reality bubble // 2) wrong position (in ground etc.) // 3) Simulator not running (ie in stopped mode) + CStatusMessage msg; if (!simObject.getAircraftModelString().isEmpty()) { - m_addPendingAircraft.push_back(simObject.getAircraft()); - CLogMessage(this).warning("Aircraft removed, '%1' '%2' object id '%3' out of reality bubble or other reason. Interpolator: '%4'") - << callsign.toQString() << simObject.getAircraftModelString() - << objectID << simObject.getInterpolatorInfo(); + m_addPendingAircraft.replaceOrAddByCallsign(simObject.getAircraft()); + msg = CLogMessage(this).warning("Aircraft removed, '%1' '%2' object id '%3' out of reality bubble or other reason. Interpolator: '%4'") + << callsign.toQString() << simObject.getAircraftModelString() + << objectID << simObject.getInterpolatorInfo(); } else { - CLogMessage(this).warning("Removed '%1' from simulator, but was not initiated by us: %1 '%2' object id %3") << callsign.toQString() << simObject.getAircraftModelString() << objectID; + msg = CLogMessage(this).warning("Removed '%1' from simulator, but was not initiated by us: %1 '%2' object id %3") << callsign.toQString() << simObject.getAircraftModelString() << objectID; } + emit this->driverMessages(msg); } // in all cases we remove the object @@ -890,9 +906,12 @@ namespace BlackSimPlugin Q_ASSERT_X(CThreadUtils::isCurrentThreadObjectThread(this), Q_FUNC_INFO, "wrong thread"); if (callsign.isEmpty()) { return false; } // can happen if an object is not an aircraft + // clean up anyway this->removeFromAddPendingAndAddAgainAircraft(callsign); - if (!m_simConnectObjects.contains(callsign)) { return false; } // already fully removed or not yet added + m_hints.remove(callsign); + // really remove from simulator + if (!m_simConnectObjects.contains(callsign)) { return false; } // already fully removed or not yet added CSimConnectObject &simObject = m_simConnectObjects[callsign]; if (simObject.isPendingRemoved()) { return true; } @@ -914,7 +933,6 @@ namespace BlackSimPlugin } simObject.setPendingRemoved(true); - m_hints.remove(simObject.getCallsign()); if (this->showDebugLogMessage()) { this->debugLogMessage(Q_FUNC_INFO, QString("Cs: '%1' request/object id: %2/%3").arg(callsign.toQString()).arg(simObject.getRequestId()).arg(simObject.getObjectId())); } // call in SIM @@ -959,7 +977,7 @@ namespace BlackSimPlugin { if (this->physicallyRemoveRemoteAircraft(cs)) { r++; } } - this->clearAllAircraft(); + this->clearAllRemoteAircraftData(); return r; } @@ -1513,15 +1531,19 @@ namespace BlackSimPlugin m_dispatchErrors = 0; m_receiveExceptionCount = 0; m_sendIdTraces.clear(); + // m_simConnectObjects cleared below + // m_simConnectObjectsPositionAndPartsTraces + // m_addPendingAircraft CSimulatorFsCommon::reset(); // clears all pending aircraft etc } - void CSimulatorFsxCommon::clearAllAircraft() + void CSimulatorFsxCommon::clearAllRemoteAircraftData() { m_simConnectObjects.clear(); m_addPendingAircraft.clear(); m_simConnectObjectsPositionAndPartsTraces.clear(); - CSimulatorFsCommon::clearAllAircraft(); + // m_addAgainAircraftWhenRemoved cleared below + CSimulatorFsCommon::clearAllRemoteAircraftData(); } void CSimulatorFsxCommon::onRemoteProviderAddedAircraftSituation(const CAircraftSituation &situation) diff --git a/src/plugins/simulator/fsxcommon/simulatorfsxcommon.h b/src/plugins/simulator/fsxcommon/simulatorfsxcommon.h index fd6acbfb8..2d48940a1 100644 --- a/src/plugins/simulator/fsxcommon/simulatorfsxcommon.h +++ b/src/plugins/simulator/fsxcommon/simulatorfsxcommon.h @@ -130,6 +130,8 @@ namespace BlackSimPlugin virtual bool isPhysicallyRenderedAircraft(const BlackMisc::Aviation::CCallsign &callsign) const override; virtual BlackMisc::Aviation::CCallsignSet physicallyRenderedAircraft() const override; virtual bool setInterpolatorMode(BlackMisc::Simulation::CInterpolatorMulti::Mode mode, const BlackMisc::Aviation::CCallsign &callsign) override; + virtual void clearAllRemoteAircraftData() override; + virtual BlackMisc::CStatusMessageList debugVerifyStateAfterAllAircraftRemoved() const override; //! @} protected: @@ -142,7 +144,6 @@ namespace BlackSimPlugin //! \name Base class overrides //! @{ virtual void reset() override; - virtual void clearAllAircraft() override; virtual void initSimulatorInternals() override; virtual void injectWeatherGrid(const BlackMisc::Weather::CWeatherGrid &weatherGrid) override; //! @} diff --git a/src/plugins/simulator/p3d/p3d.pro b/src/plugins/simulator/p3d/p3d.pro index 4941c93e3..eee8347e7 100644 --- a/src/plugins/simulator/p3d/p3d.pro +++ b/src/plugins/simulator/p3d/p3d.pro @@ -8,7 +8,7 @@ TARGET = simulatorp3d TEMPLATE = lib CONFIG += plugin shared -CONFIG += blackmisc blackcore +CONFIG += blackconfig blackmisc blackcore DEPENDPATH += . $$SourceRoot/src INCLUDEPATH += . $$SourceRoot/src