From 632004b52efab01b25126f9448a8577630cc8ec7 Mon Sep 17 00:00:00 2001 From: Gordon McCann Date: Sun, 6 Nov 2022 09:59:18 -0500 Subject: [PATCH] Fixed gross code for creating data sources. Now much cleaner, easier to understand. --- Specter/src/Specter/Editor/EditorLayer.cpp | 6 +- Specter/src/Specter/Editor/SourceDialog.cpp | 83 +++++++++---------- Specter/src/Specter/Editor/SourceDialog.h | 10 +-- Specter/src/Specter/Events/PhysicsEvent.h | 18 ++-- Specter/src/Specter/Physics/DataSource.cpp | 14 ++-- Specter/src/Specter/Physics/DataSource.h | 13 ++- Specter/src/Specter/Physics/PhysicsLayer.cpp | 48 ++++------- Specter/src/Specter/Physics/PhysicsLayer.h | 3 +- .../Specter/Physics/nscldaq/CharonClient.cpp | 3 +- 9 files changed, 90 insertions(+), 108 deletions(-) diff --git a/Specter/src/Specter/Editor/EditorLayer.cpp b/Specter/src/Specter/Editor/EditorLayer.cpp index acdb609..3c5b4af 100644 --- a/Specter/src/Specter/Editor/EditorLayer.cpp +++ b/Specter/src/Specter/Editor/EditorLayer.cpp @@ -226,7 +226,11 @@ namespace Specter { m_scalerPanel.OnImGuiRender(m_manager, m_scalerList, m_graphList); - m_sourceDialog.ImGuiRenderSourceDialog(); + if (m_sourceDialog.ImGuiRenderSourceDialog()) + { + PhysicsStartEvent event(m_sourceDialog.GetArgs()); + m_callbackFunc(event); + } RemoveHistogramDialog(); diff --git a/Specter/src/Specter/Editor/SourceDialog.cpp b/Specter/src/Specter/Editor/SourceDialog.cpp index 7f5a4d1..a0b446e 100644 --- a/Specter/src/Specter/Editor/SourceDialog.cpp +++ b/Specter/src/Specter/Editor/SourceDialog.cpp @@ -18,7 +18,7 @@ namespace Specter { SourceDialog::SourceDialog() : - m_openFlag(false), m_chosenPort("51489"), m_chosenWindow(2000000) + m_openFlag(false) { } @@ -26,63 +26,60 @@ namespace Specter { { } - void SourceDialog::ImGuiRenderSourceDialog() + bool SourceDialog::ImGuiRenderSourceDialog() { SPEC_PROFILE_FUNCTION(); - static bool onlineFlag = false; - static bool offlineFlag = false; + static bool result = false; static std::vector availTypes = { DataSource::SourceType::CompassOnline, DataSource::SourceType::CompassOffline, DataSource::SourceType::DaqromancyOnline, DataSource::SourceType::DaqromancyOffline, DataSource::SourceType::CharonOnline }; - + result = false; if (m_openFlag) { - onlineFlag = false; - offlineFlag = false; m_openFlag = false; - m_chosenType = DataSource::SourceType::None; - m_chosenLocation = ""; - m_chosenPort = "51489"; - m_chosenWindow = 3000000; - m_bitflags = 0; + m_args.type = DataSource::SourceType::None; + m_args.location = ""; + m_args.port = "52324"; + m_args.coincidenceWindow = 3000000; + m_args.bitflags = 0; ImGui::OpenPopup(ICON_FA_LINK " Attach Source"); } if (ImGui::BeginPopupModal(ICON_FA_LINK " Attach Source")) { - if (ImGui::BeginCombo("Source Type", ConvertDataSourceTypeToString(m_chosenType).c_str())) + if (ImGui::BeginCombo("Source Type", ConvertDataSourceTypeToString(m_args.type).c_str())) { for (auto& type : availTypes) { - if (ImGui::Selectable(ConvertDataSourceTypeToString(type).c_str(), type == m_chosenType, ImGuiSelectableFlags_DontClosePopups)) + if (ImGui::Selectable(ConvertDataSourceTypeToString(type).c_str(), type == m_args.type, ImGuiSelectableFlags_DontClosePopups)) { - m_chosenType = type; + m_args.type = type; } } ImGui::EndCombo(); } - if (m_chosenType == DataSource::SourceType::CompassOnline) + if (m_args.type == DataSource::SourceType::CompassOnline) { - ImGui::InputText("Hostname", &m_chosenLocation); - ImGui::InputText("Port", &m_chosenPort); - if (ImGui::RadioButton("Energy", (m_bitflags & CompassHeaders::Energy) != 0)) + ImGui::InputText("Hostname", &m_args.location); + ImGui::InputText("Port", &m_args.port); + if (ImGui::RadioButton("Energy", (m_args.bitflags & CompassHeaders::Energy) != 0)) { - m_bitflags = m_bitflags ^ CompassHeaders::Energy; + m_args.bitflags = m_args.bitflags ^ CompassHeaders::Energy; } ImGui::SameLine(); - if (ImGui::RadioButton("Energy Short", (m_bitflags & CompassHeaders::EnergyShort) != 0)) + if (ImGui::RadioButton("Energy Short", (m_args.bitflags & CompassHeaders::EnergyShort) != 0)) { - m_bitflags = m_bitflags ^ CompassHeaders::EnergyShort; + m_args.bitflags = m_args.bitflags ^ CompassHeaders::EnergyShort; } ImGui::SameLine(); - if (ImGui::RadioButton("Energy Calibrated", (m_bitflags & CompassHeaders::EnergyCalibrated) != 0)) + if (ImGui::RadioButton("Energy Calibrated", (m_args.bitflags & CompassHeaders::EnergyCalibrated) != 0)) { - m_bitflags = m_bitflags ^ CompassHeaders::EnergyCalibrated; + m_args.bitflags = m_args.bitflags ^ CompassHeaders::EnergyCalibrated; } - ImGui::InputInt("Coinc. Window (ps)", &m_chosenWindow); + ImGui::InputScalar("Coinc. Window (ps)", ImGuiDataType_U64, &m_args.coincidenceWindow); } - else if (m_chosenType == DataSource::SourceType::CompassOffline) + else if (m_args.type == DataSource::SourceType::CompassOffline) { - ImGui::InputText("Run Directory", &m_chosenLocation); + ImGui::InputText("Run Directory", &m_args.location); ImGui::SameLine(); if (ImGui::Button("Choose Location")) { @@ -90,18 +87,18 @@ namespace Specter { } auto temp = m_fileDialog.RenderFileDialog(); if (!temp.first.empty() && temp.second == FileDialog::Type::OpenDir) - m_chosenLocation = temp.first; - ImGui::InputInt("Coinc. Window (ps)", &m_chosenWindow); + m_args.location = temp.first; + ImGui::InputScalar("Coinc. Window (ps)", ImGuiDataType_U64, &m_args.coincidenceWindow); } - else if (m_chosenType == DataSource::SourceType::DaqromancyOnline) + else if (m_args.type == DataSource::SourceType::DaqromancyOnline) { - ImGui::InputText("Hostname", &m_chosenLocation); - ImGui::InputText("Port", &m_chosenPort); - ImGui::InputInt("Coinc. Window (ps)", &m_chosenWindow); + ImGui::InputText("Hostname", &m_args.location); + ImGui::InputText("Port", &m_args.port); + ImGui::InputScalar("Coinc. Window (ps)", ImGuiDataType_U64, &m_args.coincidenceWindow); } - else if (m_chosenType == DataSource::SourceType::DaqromancyOffline) + else if (m_args.type == DataSource::SourceType::DaqromancyOffline) { - ImGui::InputText("Run Directory", &m_chosenLocation); + ImGui::InputText("Run Directory", &m_args.location); ImGui::SameLine(); if (ImGui::Button("Choose Location")) { @@ -109,29 +106,29 @@ namespace Specter { } auto temp = m_fileDialog.RenderFileDialog(); if (!temp.first.empty() && temp.second == FileDialog::Type::OpenDir) - m_chosenLocation = temp.first; - ImGui::InputInt("Coinc. Window (ps)", &m_chosenWindow); + m_args.location = temp.first; + ImGui::InputScalar("Coinc. Window (ps)", ImGuiDataType_U64, &m_args.coincidenceWindow); } - else if (m_chosenType == DataSource::SourceType::CharonOnline) + else if (m_args.type == DataSource::SourceType::CharonOnline) { - ImGui::InputText("Hostname", &m_chosenLocation); - ImGui::InputText("Port", &m_chosenPort); + ImGui::InputText("Hostname", &m_args.location); + ImGui::InputText("Port", &m_args.port); } if (ImGui::Button("Ok")) { - PhysicsStartEvent event(m_chosenLocation, m_chosenType, m_chosenWindow, m_chosenPort, m_bitflags); - Application::Get().OnEvent(event); - + result = true; ImGui::CloseCurrentPopup(); } ImGui::SameLine(); if (ImGui::Button("Cancel")) { + result = false; ImGui::CloseCurrentPopup(); } ImGui::EndPopup(); } + return result; } } diff --git a/Specter/src/Specter/Editor/SourceDialog.h b/Specter/src/Specter/Editor/SourceDialog.h index b160543..96e7e14 100644 --- a/Specter/src/Specter/Editor/SourceDialog.h +++ b/Specter/src/Specter/Editor/SourceDialog.h @@ -19,17 +19,15 @@ namespace Specter { SourceDialog(); ~SourceDialog(); - void ImGuiRenderSourceDialog(); + bool ImGuiRenderSourceDialog(); + + inline const SourceArgs& GetArgs() const { return m_args; } inline void OpenSourceDialog() { m_openFlag = true; } private: bool m_openFlag; - DataSource::SourceType m_chosenType; - std::string m_chosenLocation; - std::string m_chosenPort; + SourceArgs m_args; FileDialog m_fileDialog; - uint16_t m_bitflags; - int m_chosenWindow; }; } diff --git a/Specter/src/Specter/Events/PhysicsEvent.h b/Specter/src/Specter/Events/PhysicsEvent.h index 01e9411..dd39010 100644 --- a/Specter/src/Specter/Events/PhysicsEvent.h +++ b/Specter/src/Specter/Events/PhysicsEvent.h @@ -21,30 +21,22 @@ namespace Specter { { public: //Bitflags is a final option for random crap needed for a source. Currently used for compass online to indicate header state. - PhysicsStartEvent(const std::string& loc, DataSource::SourceType type, uint64_t window, const std::string& port = "51489", uint16_t bitflags = 0) : - m_sourceLocation(loc), m_port(port), m_sourceType(type), m_coincidenceWindow(window), m_bitflags(bitflags) + PhysicsStartEvent(const SourceArgs& args) : + m_args(args) {} - inline const std::string GetSourceLocation() const { return m_sourceLocation; } - inline const std::string GetSourcePort() const { return m_port; } - inline const DataSource::SourceType GetSourceType() const { return m_sourceType; } - inline const uint64_t GetCoincidenceWindow() const { return m_coincidenceWindow; } - inline const uint16_t GetBitFlags() const { return m_bitflags; } + const SourceArgs& GetSourceArgs() const { return m_args; } std::string ToString() const override { - return "Starting PhysicsEventBuilder with DataSource of type {0} at location {1}" + m_sourceLocation + ConvertDataSourceTypeToString(m_sourceType); + return "Starting PhysicsEventBuilder with DataSource of type " + ConvertDataSourceTypeToString(m_args.type) + " at location " + m_args.location; } EVENT_CATEGORY_SETUP(EventCategoryPhysics); EVENT_TYPE_SETUP(PhysicsStart); private: - std::string m_sourceLocation; - std::string m_port; - DataSource::SourceType m_sourceType; - uint64_t m_coincidenceWindow; - uint16_t m_bitflags; + SourceArgs m_args; }; class PhysicsStopEvent : public Event diff --git a/Specter/src/Specter/Physics/DataSource.cpp b/Specter/src/Specter/Physics/DataSource.cpp index ff7b698..8c9dd69 100644 --- a/Specter/src/Specter/Physics/DataSource.cpp +++ b/Specter/src/Specter/Physics/DataSource.cpp @@ -16,15 +16,15 @@ namespace Specter { //loc=either an ip address or a file location, port=address port, or unused in case of file - DataSource* CreateDataSource(const std::string& location, const std::string& port, uint16_t header, DataSource::SourceType type, uint64_t coincidenceWindow) + DataSource* CreateDataSource(const SourceArgs& args) { - switch(type) + switch(args.type) { - case DataSource::SourceType::CompassOffline: return new CompassRun(location, coincidenceWindow); - case DataSource::SourceType::CompassOnline: return new CompassOnlineSource(location, port, header, coincidenceWindow); - case DataSource::SourceType::DaqromancyOffline: return new DYFileSource(location, coincidenceWindow); - case DataSource::SourceType::DaqromancyOnline: return new DYOnlineSource(location, port, coincidenceWindow); - case DataSource::SourceType::CharonOnline: return new CharonOnlineSource(location, port); + case DataSource::SourceType::CompassOffline: return new CompassRun(args.location, args.coincidenceWindow); + case DataSource::SourceType::CompassOnline: return new CompassOnlineSource(args.location, args.port, args.bitflags, args.coincidenceWindow); + case DataSource::SourceType::DaqromancyOffline: return new DYFileSource(args.location, args.coincidenceWindow); + case DataSource::SourceType::DaqromancyOnline: return new DYOnlineSource(args.location, args.port, args.coincidenceWindow); + case DataSource::SourceType::CharonOnline: return new CharonOnlineSource(args.location, args.port); case DataSource::SourceType::None: return nullptr; } SPEC_WARN("Invalid DataSourceType at CreateDataSource!"); diff --git a/Specter/src/Specter/Physics/DataSource.h b/Specter/src/Specter/Physics/DataSource.h index 3668e4c..c06f7df 100644 --- a/Specter/src/Specter/Physics/DataSource.h +++ b/Specter/src/Specter/Physics/DataSource.h @@ -14,7 +14,7 @@ #include "SpecData.h" namespace Specter { - + class DataSource { public: @@ -46,7 +46,16 @@ namespace Specter { PhysicsEventBuilder m_eventBuilder; }; - DataSource* CreateDataSource(const std::string& location, const std::string& port, uint16_t bitflags, DataSource::SourceType type, uint64_t coincidenceWindow); + struct SourceArgs + { + DataSource::SourceType type = DataSource::SourceType::None; + std::string location = ""; + std::string port = ""; + uint64_t coincidenceWindow = 0; + uint16_t bitflags = 0; + }; + + DataSource* CreateDataSource(const SourceArgs& args); std::string ConvertDataSourceTypeToString(DataSource::SourceType type); } diff --git a/Specter/src/Specter/Physics/PhysicsLayer.cpp b/Specter/src/Specter/Physics/PhysicsLayer.cpp index 8b59903..2d40db1 100644 --- a/Specter/src/Specter/Physics/PhysicsLayer.cpp +++ b/Specter/src/Specter/Physics/PhysicsLayer.cpp @@ -23,7 +23,6 @@ namespace Specter { if (m_activeFlag) { DetachDataSource(); - DestroyPhysThread(); } } @@ -50,17 +49,10 @@ namespace Specter { if(m_activeFlag) { DetachDataSource(); - DestroyPhysThread(); } - AttachDataSource(event); + AttachDataSource(event.GetSourceArgs()); - //If we succesfully attached, fire up a new phys thread - if(m_activeFlag) - { - SPEC_INFO("Starting new analysis thread..."); - m_physThread = new std::thread(&PhysicsLayer::RunSource, std::ref(*this)); - } return true; } @@ -70,7 +62,6 @@ namespace Specter { if (m_activeFlag) { DetachDataSource(); - DestroyPhysThread(); } return true; } @@ -85,34 +76,17 @@ namespace Specter { /*Threaded functions*/ - void PhysicsLayer::DestroyPhysThread() - { - SPEC_PROFILE_FUNCTION(); - SPEC_INFO("Destroying the analysis thread..."); - //Join the thread back to the parent (finish up the thread) - if(m_physThread != nullptr && m_physThread->joinable()) - { - m_physThread->join(); - } - - //Free the thread memory - if(m_physThread != nullptr) - { - delete m_physThread; - m_physThread = nullptr; - } - SPEC_INFO("Thread destroyed."); - } - - void PhysicsLayer::AttachDataSource(PhysicsStartEvent& event) + void PhysicsLayer::AttachDataSource(const SourceArgs& args) { SPEC_PROFILE_FUNCTION(); std::scoped_lock guard(m_sourceMutex); //Shouldn't matter for this, but safety first - m_source.reset(CreateDataSource(event.GetSourceLocation(), event.GetSourcePort(), event.GetBitFlags(), event.GetSourceType(), event.GetCoincidenceWindow())); + m_source.reset(CreateDataSource(args)); if (m_source->IsValid()) { - SPEC_INFO("Attach successful. Enabling data pull..."); + SPEC_INFO("Source attached... Starting new analysis thread..."); m_activeFlag = true; + + m_physThread = new std::thread(&PhysicsLayer::RunSource, std::ref(*this)); } else { @@ -124,10 +98,18 @@ namespace Specter { void PhysicsLayer::DetachDataSource() { SPEC_PROFILE_FUNCTION(); - std::scoped_lock guard(m_sourceMutex); SPEC_INFO("Detaching physics data source..."); + + std::scoped_lock guard(m_sourceMutex); m_activeFlag = false; m_source.reset(nullptr); + if (m_physThread != nullptr && m_physThread->joinable()) + { + m_physThread->join(); + } + delete m_physThread; + m_physThread = nullptr; + SPEC_INFO("Detach succesful."); } diff --git a/Specter/src/Specter/Physics/PhysicsLayer.h b/Specter/src/Specter/Physics/PhysicsLayer.h index 0bbdd5b..7ec2570 100644 --- a/Specter/src/Specter/Physics/PhysicsLayer.h +++ b/Specter/src/Specter/Physics/PhysicsLayer.h @@ -42,8 +42,7 @@ namespace Specter { void PushStage(AnalysisStage* stage); private: - void DestroyPhysThread(); - void AttachDataSource(PhysicsStartEvent& event); + void AttachDataSource(const SourceArgs& args); void DetachDataSource(); void RunSource(); diff --git a/Specter/src/Specter/Physics/nscldaq/CharonClient.cpp b/Specter/src/Specter/Physics/nscldaq/CharonClient.cpp index 1291780..d854fbc 100644 --- a/Specter/src/Specter/Physics/nscldaq/CharonClient.cpp +++ b/Specter/src/Specter/Physics/nscldaq/CharonClient.cpp @@ -30,7 +30,7 @@ namespace Specter { asio::ip::tcp::resolver resolver(m_context); auto end_points = resolver.resolve(hostname, port); - m_deadline.expires_after(std::chrono::seconds(60)); + m_deadline.expires_after(std::chrono::seconds(30)); asio::async_connect(m_socket, end_points, [this, hostname, port](std::error_code ec, asio::ip::tcp::endpoint endpoint) { @@ -44,6 +44,7 @@ namespace Specter { else { SPEC_WARN("Unable to connect to CharonClient {0}:{1}" , hostname, port); + m_socket.close(); } } );