diff --git a/fairmq/plugins/DDS/DDS.cxx b/fairmq/plugins/DDS/DDS.cxx index 88922abc..a3ee39e2 100644 --- a/fairmq/plugins/DDS/DDS.cxx +++ b/fairmq/plugins/DDS/DDS.cxx @@ -344,10 +344,10 @@ auto DDS::HandleCmd(const string& id, sdk::cmd::Cmd& cmd, const string& cond, ui case Type::change_state: { Transition transition = static_cast(cmd).GetTransition(); if (ChangeDeviceState(transition)) { - Cmds outCmds(make(id, fDDSTaskId, Result::Ok, transition)); + Cmds outCmds(make(id, fDDSTaskId, Result::Ok, transition, GetCurrentDeviceState())); fDDS.Send(outCmds.Serialize(), to_string(senderId)); } else { - Cmds outCmds(make(id, fDDSTaskId, Result::Failure, transition)); + Cmds outCmds(make(id, fDDSTaskId, Result::Failure, transition, GetCurrentDeviceState())); fDDS.Send(outCmds.Serialize(), to_string(senderId)); } { diff --git a/fairmq/sdk/Topology.h b/fairmq/sdk/Topology.h index c6260b21..7f3ab9ac 100644 --- a/fairmq/sdk/Topology.h +++ b/fairmq/sdk/Topology.h @@ -421,13 +421,16 @@ class BasicTopology : public AsioBase auto HandleCmd(cmd::TransitionStatus const& cmd) -> void { if (cmd.GetResult() != cmd::Result::Ok) { - FAIR_LOG(error) << cmd.GetTransition() << " transition failed for " << cmd.GetDeviceId(); DDSTask::Id taskId(cmd.GetTaskId()); std::lock_guard lk(fMtx); for (auto& op : fChangeStateOps) { - if (!op.second.IsCompleted() && op.second.ContainsTask(taskId) && - fStateData.at(fStateIndex.at(taskId)).state != op.second.GetTargetState()) { - op.second.Complete(MakeErrorCode(ErrorCode::DeviceChangeStateFailed)); + if (!op.second.IsCompleted() && op.second.ContainsTask(taskId)) { + if (fStateData.at(fStateIndex.at(taskId)).state != op.second.GetTargetState()) { + FAIR_LOG(error) << cmd.GetTransition() << " transition failed for " << cmd.GetDeviceId() << ", device is in " << cmd.GetCurrentState() << " state."; + op.second.Complete(MakeErrorCode(ErrorCode::DeviceChangeStateFailed)); + } else { + FAIR_LOG(debug) << cmd.GetTransition() << " transition failed for " << cmd.GetDeviceId() << ", device is already in " << cmd.GetCurrentState() << " state."; + } } } } diff --git a/fairmq/sdk/commands/Commands.cxx b/fairmq/sdk/commands/Commands.cxx index ece53af2..1a442333 100644 --- a/fairmq/sdk/commands/Commands.cxx +++ b/fairmq/sdk/commands/Commands.cxx @@ -290,6 +290,7 @@ string Cmds::Serialize(const Format type) const cmdBuilder->add_task_id(_cmd.GetTaskId()); cmdBuilder->add_result(GetFBResult(_cmd.GetResult())); cmdBuilder->add_transition(GetFBTransition(_cmd.GetTransition())); + cmdBuilder->add_current_state(GetFBState(_cmd.GetCurrentState())); } break; case Type::config: { @@ -445,7 +446,7 @@ void Cmds::Deserialize(const string& str, const Format type) fCmds.emplace_back(make(cmdPtr.device_id()->str(), GetMQState(cmdPtr.current_state()))); break; case FBCmd_transition_status: - fCmds.emplace_back(make(cmdPtr.device_id()->str(), cmdPtr.task_id(), GetResult(cmdPtr.result()), GetMQTransition(cmdPtr.transition()))); + fCmds.emplace_back(make(cmdPtr.device_id()->str(), cmdPtr.task_id(), GetResult(cmdPtr.result()), GetMQTransition(cmdPtr.transition()), GetMQState(cmdPtr.current_state()))); break; case FBCmd_config: fCmds.emplace_back(make(cmdPtr.device_id()->str(), cmdPtr.config_string()->str())); diff --git a/fairmq/sdk/commands/Commands.h b/fairmq/sdk/commands/Commands.h index f00f4ee7..8e2b146b 100644 --- a/fairmq/sdk/commands/Commands.h +++ b/fairmq/sdk/commands/Commands.h @@ -50,7 +50,7 @@ enum class Type : int subscription_heartbeat, // args: { interval } current_state, // args: { device_id, current_state } - transition_status, // args: { device_id, task_id, Result, transition } + transition_status, // args: { device_id, task_id, Result, transition, current_state } config, // args: { device_id, config_string } state_change_subscription, // args: { device_id, task_id, Result } state_change_unsubscription, // args: { device_id, task_id, Result } @@ -188,12 +188,13 @@ struct CurrentState : Cmd struct TransitionStatus : Cmd { - explicit TransitionStatus(const std::string& deviceId, const uint64_t taskId, const Result result, const Transition transition) + explicit TransitionStatus(const std::string& deviceId, const uint64_t taskId, const Result result, const Transition transition, State currentState) : Cmd(Type::transition_status) , fDeviceId(deviceId) , fTaskId(taskId) , fResult(result) , fTransition(transition) + , fCurrentState(currentState) {} std::string GetDeviceId() const { return fDeviceId; } @@ -204,12 +205,15 @@ struct TransitionStatus : Cmd void SetResult(const Result result) { fResult = result; } Transition GetTransition() const { return fTransition; } void SetTransition(const Transition transition) { fTransition = transition; } + fair::mq::State GetCurrentState() const { return fCurrentState; } + void SetCurrentState(fair::mq::State state) { fCurrentState = state; } private: std::string fDeviceId; uint64_t fTaskId; Result fResult; Transition fTransition; + fair::mq::State fCurrentState; }; struct Config : Cmd diff --git a/fairmq/sdk/commands/CommandsFormat.fbs b/fairmq/sdk/commands/CommandsFormat.fbs index 0a011553..59fe752b 100644 --- a/fairmq/sdk/commands/CommandsFormat.fbs +++ b/fairmq/sdk/commands/CommandsFormat.fbs @@ -56,7 +56,7 @@ enum FBCmd:byte { subscription_heartbeat, // args: { interval } current_state, // args: { device_id, current_state } - transition_status, // args: { device_id, task_id, Result, transition } + transition_status, // args: { device_id, task_id, Result, transition, current_state } config, // args: { device_id, config_string } state_change_subscription, // args: { device_id, task_id, Result } state_change_unsubscription, // args: { device_id, task_id, Result } diff --git a/fairmq/sdk/runDDSCommandUI.cxx b/fairmq/sdk/runDDSCommandUI.cxx index 4a7a954d..6d43232d 100644 --- a/fairmq/sdk/runDDSCommandUI.cxx +++ b/fairmq/sdk/runDDSCommandUI.cxx @@ -58,12 +58,15 @@ void printControlsHelp() void handleCommand(const string& command, const string& path, unsigned int timeout, Topology& topo, const string& pKey, const string& pVal) { + std::pair changeStateResult; + if (command == "c") { cout << "> checking state of the devices" << endl; auto const result = topo.GetCurrentState(); for (const auto& d : result) { cout << d.taskId << " : " << d.state << endl; } + return; } else if (command == "o") { cout << "> dumping config of " << (path == "" ? "all" : path) << endl; // TODO: extend this regex to return all properties, once command size limitation is removed. @@ -73,6 +76,7 @@ void handleCommand(const string& command, const string& path, unsigned int timeo cout << d.first << ": " << p.first << " : " << p.second << endl; } } + return; } else if (command == "p") { if (pKey == "" || pVal == "") { cout << "cannot send property with empty key and/or value! given key: '" << pKey << "', value: '" << pVal << "'." << endl; @@ -83,42 +87,48 @@ void handleCommand(const string& command, const string& path, unsigned int timeo topo.SetProperties(props, path); // give dds time to complete request this_thread::sleep_for(chrono::milliseconds(100)); + return; } else if (command == "i") { cout << "> initiating InitDevice transition --> " << (path == "" ? "all" : path) << endl; - topo.ChangeState(TopologyTransition::InitDevice, path, std::chrono::milliseconds(timeout)); + changeStateResult = topo.ChangeState(TopologyTransition::InitDevice, path, std::chrono::milliseconds(timeout)); } else if (command == "k") { cout << "> initiating CompleteInit transition --> " << (path == "" ? "all" : path) << endl; - topo.ChangeState(TopologyTransition::CompleteInit, path, std::chrono::milliseconds(timeout)); + changeStateResult = topo.ChangeState(TopologyTransition::CompleteInit, path, std::chrono::milliseconds(timeout)); } else if (command == "b") { cout << "> initiating Bind transition --> " << (path == "" ? "all" : path) << endl; - topo.ChangeState(TopologyTransition::Bind, path, std::chrono::milliseconds(timeout)); + changeStateResult = topo.ChangeState(TopologyTransition::Bind, path, std::chrono::milliseconds(timeout)); } else if (command == "x") { cout << "> initiating Connect transition --> " << (path == "" ? "all" : path) << endl; - topo.ChangeState(TopologyTransition::Connect, path, std::chrono::milliseconds(timeout)); + changeStateResult = topo.ChangeState(TopologyTransition::Connect, path, std::chrono::milliseconds(timeout)); } else if (command == "j") { cout << "> initiating InitTask transition --> " << (path == "" ? "all" : path) << endl; - topo.ChangeState(TopologyTransition::InitTask, path, std::chrono::milliseconds(timeout)); + changeStateResult = topo.ChangeState(TopologyTransition::InitTask, path, std::chrono::milliseconds(timeout)); } else if (command == "r") { cout << "> initiating Run transition --> " << (path == "" ? "all" : path) << endl; - topo.ChangeState(TopologyTransition::Run, path, std::chrono::milliseconds(timeout)); + changeStateResult = topo.ChangeState(TopologyTransition::Run, path, std::chrono::milliseconds(timeout)); } else if (command == "s") { cout << "> initiating Stop transition --> " << (path == "" ? "all" : path) << endl; - topo.ChangeState(TopologyTransition::Stop, path, std::chrono::milliseconds(timeout)); + changeStateResult = topo.ChangeState(TopologyTransition::Stop, path, std::chrono::milliseconds(timeout)); } else if (command == "t") { cout << "> initiating ResetTask transition --> " << (path == "" ? "all" : path) << endl; - topo.ChangeState(TopologyTransition::ResetTask, path, std::chrono::milliseconds(timeout)); + changeStateResult = topo.ChangeState(TopologyTransition::ResetTask, path, std::chrono::milliseconds(timeout)); } else if (command == "d") { cout << "> initiating ResetDevice transition --> " << (path == "" ? "all" : path) << endl; - topo.ChangeState(TopologyTransition::ResetDevice, path, std::chrono::milliseconds(timeout)); + changeStateResult = topo.ChangeState(TopologyTransition::ResetDevice, path, std::chrono::milliseconds(timeout)); } else if (command == "q") { cout << "> initiating End transition --> " << (path == "" ? "all" : path) << endl; - topo.ChangeState(TopologyTransition::End, path, std::chrono::milliseconds(timeout)); + changeStateResult = topo.ChangeState(TopologyTransition::End, path, std::chrono::milliseconds(timeout)); } else if (command == "h") { cout << "> help" << endl; printControlsHelp(); + return; } else { cout << "\033[01;32mInvalid input: [" << command << "]\033[0m" << endl; printControlsHelp(); + return; + } + if (changeStateResult.first != std::error_code()) { + cout << "ERROR: ChangeState failed for '" << path << "': " << changeStateResult.first.message() << endl; } } diff --git a/test/commands/_commands.cxx b/test/commands/_commands.cxx index b0e68cc6..d762ec23 100644 --- a/test/commands/_commands.cxx +++ b/test/commands/_commands.cxx @@ -31,7 +31,7 @@ TEST(Format, Construction) Cmds setPropertiesCmds(make(42, props)); Cmds subscriptionHeartbeatCmds(make(60000)); Cmds currentStateCmds(make("somedeviceid", State::Running)); - Cmds transitionStatusCmds(make("somedeviceid", 123456, Result::Ok, Transition::Stop)); + Cmds transitionStatusCmds(make("somedeviceid", 123456, Result::Ok, Transition::Stop, State::Running)); Cmds configCmds(make("somedeviceid", "someconfig")); Cmds stateChangeSubscriptionCmds(make("somedeviceid", 123456, Result::Ok)); Cmds stateChangeUnsubscriptionCmds(make("somedeviceid", 123456, Result::Ok)); @@ -63,6 +63,7 @@ TEST(Format, Construction) ASSERT_EQ(static_cast(transitionStatusCmds.At(0)).GetTaskId(), 123456); ASSERT_EQ(static_cast(transitionStatusCmds.At(0)).GetResult(), Result::Ok); ASSERT_EQ(static_cast(transitionStatusCmds.At(0)).GetTransition(), Transition::Stop); + ASSERT_EQ(static_cast(transitionStatusCmds.At(0)).GetCurrentState(), State::Running); ASSERT_EQ(configCmds.At(0).GetType(), Type::config); ASSERT_EQ(static_cast(configCmds.At(0)).GetDeviceId(), "somedeviceid"); ASSERT_EQ(static_cast(configCmds.At(0)).GetConfig(), "someconfig"); @@ -104,7 +105,7 @@ void fillCommands(Cmds& cmds) cmds.Add(42, props); cmds.Add(60000); cmds.Add("somedeviceid", State::Running); - cmds.Add("somedeviceid", 123456, Result::Ok, Transition::Stop); + cmds.Add("somedeviceid", 123456, Result::Ok, Transition::Stop, State::Running); cmds.Add("somedeviceid", "someconfig"); cmds.Add("somedeviceid", 123456, Result::Ok); cmds.Add("somedeviceid", 123456, Result::Ok); @@ -167,6 +168,7 @@ void checkCommands(Cmds& cmds) ASSERT_EQ(static_cast(*cmd).GetTaskId(), 123456); ASSERT_EQ(static_cast(*cmd).GetResult(), Result::Ok); ASSERT_EQ(static_cast(*cmd).GetTransition(), Transition::Stop); + ASSERT_EQ(static_cast(*cmd).GetCurrentState(), State::Running); break; case Type::config: ++count;