Skip to content

Commit

Permalink
feat: Improve ChangeState API
Browse files Browse the repository at this point in the history 
* Add `[[nodiscard]]` to `bool Device::ChangeState()`
* Introduce throwing variant `void Device::ChangeStateOrThrow()`

resolves #441
  • Loading branch information
@dennisklein
dennisklein committed Mar 1, 2023
1 parent 5ef17fd commit 435d07e
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 24 deletions.
40 changes: 20 additions & 20 deletions fairmq/Device.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,30 +164,30 @@ void Device::TransitionTo(State s)
while (s != currentState) {
switch (currentState) {
case State::Idle:
if (s == State::Exiting) { ChangeState(Transition::End); }
else { ChangeState(Transition::InitDevice); }
if (s == State::Exiting) { ChangeStateOrThrow(Transition::End); }
else { ChangeStateOrThrow(Transition::InitDevice); }
break;
case State::InitializingDevice:
ChangeState(Transition::CompleteInit);
ChangeStateOrThrow(Transition::CompleteInit);
break;
case State::Initialized:
if (s == State::Exiting || s == State::Idle) { ChangeState(Transition::ResetDevice); }
else { ChangeState(Transition::Bind); }
if (s == State::Exiting || s == State::Idle) { ChangeStateOrThrow(Transition::ResetDevice); }
else { ChangeStateOrThrow(Transition::Bind); }
break;
case State::Bound:
if (s == State::DeviceReady || s == State::Ready || s == State::Running) { ChangeState(Transition::Connect); }
else { ChangeState(Transition::ResetDevice); }
if (s == State::DeviceReady || s == State::Ready || s == State::Running) { ChangeStateOrThrow(Transition::Connect); }
else { ChangeStateOrThrow(Transition::ResetDevice); }
break;
case State::DeviceReady:
if (s == State::Running || s == State::Ready) { ChangeState(Transition::InitTask); }
else { ChangeState(Transition::ResetDevice); }
if (s == State::Running || s == State::Ready) { ChangeStateOrThrow(Transition::InitTask); }
else { ChangeStateOrThrow(Transition::ResetDevice); }
break;
case State::Ready:
if (s == State::Running) { ChangeState(Transition::Run); }
else { ChangeState(Transition::ResetTask); }
if (s == State::Running) { ChangeStateOrThrow(Transition::Run); }
else { ChangeStateOrThrow(Transition::ResetTask); }
break;
case State::Running:
ChangeState(Transition::Stop);
ChangeStateOrThrow(Transition::Stop);
break;
case State::Binding:
case State::Connecting:
Expand Down Expand Up @@ -281,7 +281,7 @@ void Device::InitWrapper()
}
}

// ChangeState(Transition::Auto);
// ChangeStateOrThrow(Transition::Auto);
}

void Device::BindWrapper()
Expand All @@ -298,7 +298,7 @@ void Device::BindWrapper()
Bind();

if (!NewStatePending()) {
ChangeState(Transition::Auto);
ChangeStateOrThrow(Transition::Auto);
}
}

Expand Down Expand Up @@ -341,7 +341,7 @@ void Device::ConnectWrapper()
Connect();

if (!NewStatePending()) {
ChangeState(Transition::Auto);
ChangeStateOrThrow(Transition::Auto);
}
}

Expand Down Expand Up @@ -443,7 +443,7 @@ void Device::InitTaskWrapper()
InitTask();

if (!NewStatePending()) {
ChangeState(Transition::Auto);
ChangeStateOrThrow(Transition::Auto);
}
}

Expand All @@ -466,7 +466,7 @@ void Device::RunWrapper()

// change to Error state in case of an exception, to release LogSocketRates
tools::CallOnDestruction cod([&](){
ChangeState(Transition::ErrorFound);
ChangeStateOrThrow(Transition::ErrorFound);
});

PreRun();
Expand All @@ -493,7 +493,7 @@ void Device::RunWrapper()

// if Run() exited and the state is still RUNNING, transition to READY.
if (!NewStatePending()) {
ChangeState(Transition::Stop);
ChangeStateOrThrow(Transition::Stop);
}

PostRun();
Expand Down Expand Up @@ -794,7 +794,7 @@ void Device::ResetTaskWrapper()
ResetTask();

if (!NewStatePending()) {
ChangeState(Transition::Auto);
ChangeStateOrThrow(Transition::Auto);
}
}

Expand All @@ -813,7 +813,7 @@ void Device::ResetWrapper()
GetChannels().clear();
fTransportFactory.reset();
if (!NewStatePending()) {
ChangeState(Transition::Auto);
ChangeStateOrThrow(Transition::Auto);
}
}

Expand Down
33 changes: 31 additions & 2 deletions fairmq/Device.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

// FairMQ
#include <fairmq/Channel.h>
#include <fairmq/Error.h>
#include <fairmq/Message.h>
#include <fairmq/Parts.h>
#include <fairmq/ProgOptions.h>
Expand Down Expand Up @@ -498,21 +499,49 @@ class Device
public:
/// @brief Request a device state transition
/// @param transition state transition
/// @return whether the transition was successfully scheduled
///
/// The state transition may not happen immediately, but when the current state evaluates the
/// pending transition event and terminates. In other words, the device states are scheduled
/// cooperatively.
bool ChangeState(const Transition transition) { return fStateMachine.ChangeState(transition); }
[[nodiscard]] bool ChangeState(const Transition transition)
{
return fStateMachine.ChangeState(transition);
}
/// @brief Request a device state transition
/// @param transition state transition
/// @return whether the transition was successfully scheduled
///
/// The state transition may not happen immediately, but when the current state evaluates the
/// pending transition event and terminates. In other words, the device states are scheduled
/// cooperatively.
bool ChangeState(const std::string& transition)
[[nodiscard]] bool ChangeState(const std::string& transition)
{
return fStateMachine.ChangeState(GetTransition(transition));
}
/// @brief Request a device state transition
/// @param transition state transition
/// @throws when the transition could not have been scheduled
///
/// Throwing version of Device::ChangeState().
void ChangeStateOrThrow(Transition transition)
{
if(!ChangeState(transition)) {
auto const err = MakeErrorCode(ErrorCode::DeviceChangeStateFailed);
throw std::system_error(err.value(),
err.category(),
tools::ToString("Invalid transition: ", transition));
}
}
/// @brief Request a device state transition
/// @param transition state transition
/// @throws when the transition could not have been scheduled
///
/// Throwing version of Device::ChangeState().
void ChangeStateOrThrow(std::string const& transition)
{
ChangeStateOrThrow(GetTransition(transition));
}

/// @brief waits for the next state (any) to occur
State WaitForNextState() { return fStateQueue.WaitForNext(); }
Expand Down
4 changes: 2 additions & 2 deletions fairmq/DeviceRunner.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,15 +152,15 @@ auto DeviceRunner::Run() -> int
fDevice->RegisterChannelEndpoints();
if (fConfig.Count("print-channels")) {
fDevice->PrintRegisteredChannels();
fDevice->ChangeState(fair::mq::Transition::End);
fDevice->ChangeStateOrThrow(fair::mq::Transition::End);
return 0;
}

// Handle --version
if (fConfig.Count("version")) {
LOGV(info, verylow) << "FairMQ version: " << FAIRMQ_GIT_VERSION;
LOGV(info, verylow) << "User device version: " << fDevice->GetVersion();
fDevice->ChangeState(fair::mq::Transition::End);
fDevice->ChangeStateOrThrow(fair::mq::Transition::End);
return 0;
}

Expand Down
22 changes: 22 additions & 0 deletions test/device/_transitions.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,26 @@ TEST(Transitions, ConcurrentTransitionTos)
}
}

TEST(Transitions, InvalidChangeState)
{
Device device;
thread t([&] { device.RunStateMachine(); });

ASSERT_FALSE(device.ChangeState(Transition::Connect));

ASSERT_TRUE(device.ChangeState(Transition::End));
if (t.joinable()) { t.join(); }
}

TEST(Transitions, InvalidChangeStateOrThrow)
{
Device device;
thread t([&] { device.RunStateMachine(); });

ASSERT_THROW(device.ChangeStateOrThrow(Transition::Connect), std::system_error);

ASSERT_NO_THROW(device.ChangeStateOrThrow(Transition::End));
if (t.joinable()) { t.join(); }
}

} // namespace

0 comments on commit 435d07e

Please sign in to comment.