diff --git a/Framework/LiveData/src/SNSLiveEventDataListener.cpp b/Framework/LiveData/src/SNSLiveEventDataListener.cpp
index 5fa90939a83df42ef7a2c6615f5ce7088a8aa64c..faedc8f77adbc3318aedce77ca5d4942f5469381 100644
--- a/Framework/LiveData/src/SNSLiveEventDataListener.cpp
+++ b/Framework/LiveData/src/SNSLiveEventDataListener.cpp
@@ -63,9 +63,6 @@ Mantid::Kernel::DateAndTime timeFromPacket(const ADARA::PacketHeader &hdr) {
namespace Mantid {
namespace LiveData {
DECLARE_LISTENER(SNSLiveEventDataListener)
-// The DECLARE_LISTENER macro seems to confuse some editors' syntax checking.
-// The semi-colon limits the complaints to one line. It has no actual effect
-// on the code.
namespace {
/// static logger
@@ -183,8 +180,7 @@ bool SNSLiveEventDataListener::isConnected() { return m_isConnected; }
void SNSLiveEventDataListener::start(const Kernel::DateAndTime startTime) {
// Save the startTime and kick off the background thread
// (Can't really do anything else until we send the hello packet and the SMS
- // sends us
- // back the various metadata packets
+ // sends us back the various metadata packets
m_startTime = startTime;
if (m_startTime.totalNanoseconds() == 1000000000) {
@@ -379,8 +375,7 @@ bool SNSLiveEventDataListener::rxPacket(const ADARA::BankedEventPkt &pkt) {
// Might want to think about rate limiting this somehow...
return false; // We still return false (ie: "no error") because there's no
- // reason to stop
- // parsing the data stream
+ // reason to stop parsing the data stream
}
}
@@ -467,8 +462,7 @@ bool SNSLiveEventDataListener::rxPacket(const ADARA::BeamMonitorPkt &pkt) {
}
// We'll likely be modifying m_eventBuffer (specifically,
- // m_eventBuffer->m_monitorWorkspace),
- // so lock the mutex
+ // m_eventBuffer->m_monitorWorkspace), so lock the mutex
std::lock_guard<std::mutex> scopedLock(m_mutex);
auto monitorBuffer = boost::static_pointer_cast<DataObjects::EventWorkspace>(
@@ -492,8 +486,7 @@ bool SNSLiveEventDataListener::rxPacket(const ADARA::BeamMonitorPkt &pkt) {
monitorID + 48); // The +48 converts to the ASCII character
monName += "_counts";
// Note: The monitor name must exactly match one of the entries in the
- // ADDABLE
- // list at the top of Run.cpp!
+ // ADDABLE list at the top of Run.cpp!
int events = pkt.getSectionEventCount();
if (m_eventBuffer->run().hasProperty(monName)) {
@@ -666,8 +659,7 @@ bool SNSLiveEventDataListener::rxPacket(const ADARA::RunStatusPkt &pkt) {
// Pay close attention here - this gets complicated!
//
// Setting m_status to "Running" is something of a little white lie. We
- // are
- // in fact at the beginning of a run. However, since we haven't yet
+ // are in fact at the beginning of a run. However, since we haven't yet
// initialized the workspace, this must be one of the first packets we've
// actually received. (Probably, the user selected the option to replay
// history starting from the start of the current run.) Normally, when
@@ -675,55 +667,41 @@ bool SNSLiveEventDataListener::rxPacket(const ADARA::RunStatusPkt &pkt) {
// (see below). That would cause us to halt reading packets until the
// flag was reset down in runStatus(). Having m_status set to BeginRun
// would also cause runStatus() to reset all the data we need to
- // initialize
- // the workspace in preparation for a new run. In most cases, this is
- // exactly
- // what we want.
+ // initialize the workspace in preparation for a new run. In most cases,
+ // this is exactly what we want.
//
// HOWEVER, in this particular case, we can't set m_pauseNetRead. If we
- // do, we
- // will not read the Geometry and BeamMonitor packets that have the data
- // we
- // need to complete the workspace initialization. Until we complete the
- // initialization, the extractData() function won't complete successfully
- // and
- // the runStatus() function will thus never be called. Since
- // m_pauseNetRead
- // is reset down in runStatus(), the whole live listener subsystem
- // basically
- // deadlocks.
+ // do, we will not read the Geometry and BeamMonitor packets that have
+ // the data we need to complete the workspace initialization. Until we
+ // complete the initialization, the extractData() function won't complete
+ // successfully and the runStatus() function will thus never be called.
+ // Since m_pauseNetRead is reset down in runStatus(), the whole live
+ // listener subsystem basically deadlocks.
//
// So, we can't set m_pauseNetRead. That's OK, because we don't actually
- // have
- // any data from a previous run that we need to keep separate from this
- // run
- // (which was the whole purpose of m_pauseNetRead). However, when the
- // runStatus() function sees m_status == BeginRun (or EndRun), it sets
- // m_workspaceInitialized to false and clears all the old data we used to
- // initialize the workspace. It does this because it thinks a run
- // transition
- // has happened and new initialization data will be arriving shortly. As
- // such, it implicitly assumes that m_pauseNetRead was set and we stopped
- // reading packets. In this particular case, we can't set m_pauseNetRead,
- // and we're guaranteed to have initialized the workspace before
- // runStatus()
- // would ever be called. (See the previous paragraph.) As such, the
- // initialization data that runStatus() would clear is actually the data
- // that we need.
-
+ // have any data from a previous run that we need to keep separate from
+ // this run (which was the whole purpose of m_pauseNetRead). However,
+ // when the runStatus() function sees m_status == BeginRun (or EndRun),
+ // it sets m_workspaceInitialized to false and clears all the old data we
+ // used to initialize the workspace. It does this because it thinks a
+ // run transition has happened and new initialization data will be
+ // arriving shortly. As such, it implicitly assumes that m_pauseNetRead
+ // was set and we stopped reading packets. In this particular case, we
+ // can't set m_pauseNetRead, and we're guaranteed to have initialized the
+ // workspace before runStatus() would ever be called. (See the previous
+ // paragraph.) As such, the initialization data that runStatus() would
+ // clear is actually the data that we need.
+ //
// So, by setting m_status to Running, we avoid runStatus() wiping out our
// workspace initialization. We then call setRunDetails() (which would
// normally happen down in runStatus(), except that we've just gone out
// of our way to make sure that part of runStatus() *DOESN'T* get
// executed) and everything runs as it should.
-
+ //
// It's debatable whether runStatus() should retain that implicit
- // asumption of
- // m_pauseNetRead being true, or should explicitly check its state in
- // addition
- // to m_status. Either way, you're still going to need several paragraphs
- // of
- // comments to explain what the heck is going on.
+ // asumption of m_pauseNetRead being true, or should explicitly check its
+ // state in addition to m_status. Either way, you're still going to need
+ // several paragraphs of comments to explain what the heck is going on.
m_status = Running;
setRunDetails(pkt);
}
@@ -738,17 +716,15 @@ bool SNSLiveEventDataListener::rxPacket(const ADARA::RunStatusPkt &pkt) {
"ignored.\n"
"(This should never happen. Talk to the Mantid developers.)");
} else {
- // Save a copy of the packet so we can call setRunDetails() later (after
- // extractData() has been called to fetch any data remaining from before
- // this run start.
- // Note: need to actually copy the contents (not just a pointer) because
- // pkt will go away when this function returns. And since packets don't
- // have
- // default constructors, we can only keep a pointer as a member, and
- // thus
- // have to actually allocate our deferred packet with new.
- // Fortunately, this doesn't happen to often, so performance isn't an
- // issue.
+ // Save a copy of the packet so we can call setRunDetails() later
+ // (after extractData() has been called to fetch any data remaining
+ // from before this run start.
+ // Note: need to actually copy the contents (not just a pointer)
+ // because pkt will go away when this function returns. And since
+ // packets don't have default constructors, we can only keep a pointer
+ // as a member, and thus have to actually allocate our deferred packet
+ // with new. Fortunately, this doesn't happen to often, so performance
+ // isn't an issue.
m_deferredRunDetailsPkt = boost::shared_ptr<ADARA::RunStatusPkt>(
new ADARA::RunStatusPkt(pkt));
}
@@ -762,12 +738,11 @@ bool SNSLiveEventDataListener::rxPacket(const ADARA::RunStatusPkt &pkt) {
} else if (pkt.status() == ADARA::RunStatus::END_RUN) {
// Run has ended: update m_status and set the flag to stop parsing network
- // packets.
- // (see comments below for why)
+ // packets. (see comments below for why)
if ((m_status != Running) && (m_status != BeginRun)) {
// Previous status should have been Running or BeginRun. Spit out a
- // warning if it's not. (If it's BeginRun, that's fine. Itjust means
+ // warning if it's not. (If it's BeginRun, that's fine. It just means
// that the run ended before extractData() was called.)
g_log.warning() << "Unexpected end of run. Run status should have been "
<< Running << " (Running), but was " << m_status << '\n';
@@ -784,16 +759,13 @@ bool SNSLiveEventDataListener::rxPacket(const ADARA::RunStatusPkt &pkt) {
// 3) We don't have to worry about the case where more than one run has
// started and finished between calls to extractData() (ie: 5 second runs
// and a 10 second update interval. Yes, that's an operator error, but I
- // still
- // don't want to worry about it.)
+ // still don't want to worry about it.)
//
- // Because of this, however, if extractData() isn't called at least once per
- // run,
- // the network packets may start to back up and SMS may eventually
+ // Because of this, however, if extractData() isn't called at least once
+ // per run, the network packets may start to back up and SMS may eventually
// disconnect us.
// This flag will be cleared down in runStatus(), which is guaranteed to be
- // called
- // after extractData().
+ // called after extractData().
m_pauseNetRead = true;
// Set the run number & start time if we don't already have it
@@ -801,7 +773,15 @@ bool SNSLiveEventDataListener::rxPacket(const ADARA::RunStatusPkt &pkt) {
setRunDetails(pkt);
}
} else if (pkt.status() == ADARA::RunStatus::STATE && !haveRunNumber) {
- setRunDetails(pkt);
+ // A packet status of STATE and no run number means we've just connected
+ // to the SMS. Specifically, this is the RunStatus packet that SMS
+ // initially sends out when a client hasn't set the flag to request
+ // historical data. We may or may not actually be in a run right now.
+ // If we are, then we need to set the run details. If not, there's
+ // nothing we need to do with this packet.
+ if (pkt.runNumber() != 0) {
+ setRunDetails(pkt);
+ }
}
// Note: all other possibilities for pkt.status() can be ignored
@@ -816,8 +796,7 @@ bool SNSLiveEventDataListener::rxPacket(const ADARA::RunStatusPkt &pkt) {
return m_pauseNetRead;
// If we've set m_pauseNetRead, it means we want to stop processing packets.
// In that case, we need to return true so that we'll break out of the read()
- // loop
- // in the packet parser.
+ // loop in the packet parser.
}
void SNSLiveEventDataListener::setRunDetails(const ADARA::RunStatusPkt &pkt) {
@@ -1428,7 +1407,7 @@ boost::shared_ptr<Workspace> SNSLiveEventDataListener::extractData() {
/// Check the status of the current run
-/// Called by the foreground thread check the status of the current run
+/// Called by the foreground thread to check the status of the current run
/// @returns Returns an enum indicating beginning of a run, in the middle
/// of a run, ending a run or not in a run.
ILiveListener::RunStatus SNSLiveEventDataListener::runStatus() {