Skip to content

Commit

Permalink
Adds traction node listener policy callback (#456)
Browse files Browse the repository at this point in the history 
Adds a policy function to the traction node which allows the node implementation to decide whether a function change request should be applied or not.
Adds additional unit tests for consisting with function forwarding.

====

* Adds a callback to evaluate a function policy in the traction handler.

* Fixes bug in implementation.
Adds unit tests.

* Add comments.
  • Loading branch information
@balazsracz
balazsracz authored Oct 26, 2020
1 parent d8d2283 commit 60dcdb0
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 15 deletions.
135 changes: 122 additions & 13 deletions src/openlcb/TractionConsist.cxxtest
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@ static constexpr NodeID nodeIdC3 = 0x060100000000 | 1373;
static constexpr NodeID nodeIdC4 = 0x060100000000 | 1374;
static constexpr NodeID nodeIdC5 = 0x060100000000 | 1375;

class TrainNodeWithMockPolicy : public TrainNodeForProxy
{
public:
INHERIT_CONSTRUCTOR(TrainNodeWithMockPolicy, TrainNodeForProxy);

MOCK_METHOD5(function_policy,
bool(NodeHandle src, uint8_t command_byte, uint32_t fnum,
uint16_t value, Notifiable *done));
};

class ConsistTest : public TractionTest
{
protected:
Expand All @@ -28,17 +38,22 @@ protected:
otherIf_.remote_aliases()->add(nodeIdC4, 0x774);
otherIf_.remote_aliases()->add(nodeIdC5, 0x775);
});
nodeLead_.reset(new TrainNodeForProxy(&trainService_, &trainLead_));
nodeC1_.reset(new TrainNodeForProxy(&trainService_, &trainC1_));
nodeC2_.reset(new TrainNodeForProxy(&trainService_, &trainC2_));
nodeC3_.reset(new TrainNodeForProxy(&trainService_, &trainC3_));
nodeLead_.reset(new StrictMock<TrainNodeWithMockPolicy>(
&trainService_, &trainLead_));
nodeC1_.reset(
new StrictMock<TrainNodeWithMockPolicy>(&trainService_, &trainC1_));
nodeC2_.reset(
new StrictMock<TrainNodeWithMockPolicy>(&trainService_, &trainC2_));
nodeC3_.reset(
new StrictMock<TrainNodeWithMockPolicy>(&trainService_, &trainC3_));
wait();
auto b = invoke_flow(&throttle_, TractionThrottleCommands::ASSIGN_TRAIN,
nodeIdLead, false);
EXPECT_EQ(0, b->data()->resultCode);
wait();
}

/// Sets up a star shaped consist.
void create_consist()
{
auto b = invoke_flow(
Expand All @@ -53,9 +68,29 @@ protected:
TractionDefs::CNSTFLAGS_REVERSE | TractionDefs::CNSTFLAGS_LINKF0 |
TractionDefs::CNSTFLAGS_LINKFN);
ASSERT_EQ(0, b->data()->resultCode);
// reverse link
nodeC3_->add_consist(nodeIdLead,
TractionDefs::CNSTFLAGS_REVERSE | TractionDefs::CNSTFLAGS_LINKF0 |
TractionDefs::CNSTFLAGS_LINKFN);
wait();
}

void inject_default_true_policy(TrainNodeWithMockPolicy *tn)
{
EXPECT_CALL(*tn, function_policy(_, _, _, _, _))
.WillRepeatedly(
DoAll(WithArg<4>(Invoke(&InvokeNotification)), Return(true)));
}

/// Sets up a default "true" function policy for all train nodes.
void inject_default_policy()
{
inject_default_true_policy(nodeLead_.get());
inject_default_true_policy(nodeC1_.get());
inject_default_true_policy(nodeC2_.get());
inject_default_true_policy(nodeC3_.get());
}

TractionThrottle throttle_ {node_};

IfCan otherIf_ {&g_executor, &can_hub0, 5, 5, 5};
Expand All @@ -65,10 +100,10 @@ protected:
LoggingTrain trainC1_ {1371};
LoggingTrain trainC2_ {1372};
LoggingTrain trainC3_ {1373};
std::unique_ptr<TrainNode> nodeLead_;
std::unique_ptr<TrainNode> nodeC1_;
std::unique_ptr<TrainNode> nodeC2_;
std::unique_ptr<TrainNode> nodeC3_;
std::unique_ptr<TrainNodeWithMockPolicy> nodeLead_;
std::unique_ptr<TrainNodeWithMockPolicy> nodeC1_;
std::unique_ptr<TrainNodeWithMockPolicy> nodeC2_;
std::unique_ptr<TrainNodeWithMockPolicy> nodeC3_;
};

TEST_F(ConsistTest, CreateDestroy)
Expand All @@ -78,6 +113,7 @@ TEST_F(ConsistTest, CreateDestroy)
TEST_F(ConsistTest, CreateAndRunConsist)
{
create_consist();
inject_default_policy();
Velocity v;
v.set_mph(37.5);
throttle_.set_speed(v);
Expand Down Expand Up @@ -123,15 +159,16 @@ TEST_F(ConsistTest, CreateAndRunConsist)
throttle_.set_fn(2, 1);
wait();

// F2 forwarded to C2 and C3, not to C0.
EXPECT_TRUE(trainLead_.get_fn(0));
EXPECT_FALSE(trainC1_.get_fn(0));
EXPECT_TRUE(trainC2_.get_fn(0));
EXPECT_TRUE(trainC3_.get_fn(0));
// F2 forwarded to C3, not to C1 and C2.
EXPECT_TRUE(trainLead_.get_fn(2));
EXPECT_FALSE(trainC1_.get_fn(2));
EXPECT_FALSE(trainC2_.get_fn(2));
EXPECT_TRUE(trainC3_.get_fn(2));
}

TEST_F(ConsistTest, ListenerExpectations)
{
inject_default_policy();
auto b =
invoke_flow(&throttle_, TractionThrottleCommands::CONSIST_ADD, nodeIdC4,
TractionDefs::CNSTFLAGS_REVERSE | TractionDefs::CNSTFLAGS_LINKF0 |
Expand Down Expand Up @@ -164,4 +201,76 @@ TEST_F(ConsistTest, ListenerExpectations)
wait();
}

TEST_F(ConsistTest, FunctionPolicy)
{
create_consist();
inject_default_true_policy(nodeLead_.get());
inject_default_true_policy(nodeC1_.get());
inject_default_true_policy(nodeC2_.get());

Notifiable *done = nullptr;
EXPECT_CALL(*nodeC3_,
function_policy(Field(&NodeHandle::id, nodeIdLead), 0x81, 3, 1, _))
.WillOnce(DoAll(SaveArg<4>(&done), Return(true)));

throttle_.set_fn(3, 1);
wait();
EXPECT_TRUE(trainLead_.get_fn(3));
EXPECT_FALSE(trainC1_.get_fn(3));
EXPECT_FALSE(trainC2_.get_fn(3));
EXPECT_FALSE(trainC3_.get_fn(3));
ASSERT_TRUE(done);

EXPECT_CALL(*nodeC3_,
function_policy(Field(&NodeHandle::id, nodeIdLead), 0x81, 3, 1, _))
.WillOnce(DoAll(WithArg<4>(Invoke(&InvokeNotification)), Return(true)));

done->notify();
wait();
EXPECT_TRUE(trainC3_.get_fn(3));

// rejected by policy
EXPECT_CALL(*nodeC3_,
function_policy(Field(&NodeHandle::id, nodeIdLead), 0x81, 4, 1, _))
.WillOnce(
DoAll(WithArg<4>(Invoke(&InvokeNotification)), Return(false)));

throttle_.set_fn(4, 1);
wait();
EXPECT_TRUE(trainLead_.get_fn(4));
EXPECT_FALSE(trainC1_.get_fn(4));
EXPECT_FALSE(trainC2_.get_fn(4));
EXPECT_FALSE(trainC3_.get_fn(4));

// Switch cab to C3.
auto b = invoke_flow(
&throttle_, TractionThrottleCommands::ASSIGN_TRAIN, nodeIdC3, false);
EXPECT_EQ(0, b->data()->resultCode);

// Different policy now
EXPECT_CALL(*nodeC3_,
function_policy(Field(&NodeHandle::alias, 0x22A), 0x01, 5, 1, _))
.WillOnce(DoAll(WithArg<4>(Invoke(&InvokeNotification)), Return(true)));

throttle_.set_fn(5, 1);
wait();
EXPECT_TRUE(trainLead_.get_fn(5));
EXPECT_FALSE(trainC1_.get_fn(5));
EXPECT_FALSE(trainC2_.get_fn(5));
EXPECT_TRUE(trainC3_.get_fn(5));

// Local policy false should not prevent remote propagation.
EXPECT_CALL(*nodeC3_,
function_policy(Field(&NodeHandle::alias, 0x22A), 0x01, 0, 1, _))
.WillOnce(
DoAll(WithArg<4>(Invoke(&InvokeNotification)), Return(false)));

throttle_.set_fn(0, 1);
wait();
EXPECT_TRUE(trainLead_.get_fn(0));
EXPECT_FALSE(trainC1_.get_fn(0)); // no link F0
EXPECT_TRUE(trainC2_.get_fn(0));
EXPECT_FALSE(trainC3_.get_fn(0)); // no policy
}

} // namespace openlcb
20 changes: 19 additions & 1 deletion src/openlcb/TractionTrain.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,24 @@ struct TrainService::Impl
uint16_t value = payload()[4];
value <<= 8;
value |= payload()[5];
train_node()->train()->set_fn(address, value);
bn_.reset(this);
bool should_apply =
train_node()->function_policy(nmsg()->src, payload()[0],
address, value, bn_.new_child());
// The function_policy call may have completed inline. We
// can inquire from the barrier. If it was not completed
// inline, we have to wait for the notification and re-try
// the call.
if (!bn_.abort_if_almost_done())
{
// Not notified inline.
bn_.notify(); // consumes our share
return wait();
}
if (should_apply)
{
train_node()->train()->set_fn(address, value);
}
nextConsistIndex_ = 0;
return call_immediately(STATE(maybe_forward_consist));
}
Expand Down Expand Up @@ -627,6 +644,7 @@ struct TrainService::Impl
unsigned reserved_ : 1;
TrainService *trainService_;
Buffer<GenMessage> *response_;
BarrierNotifiable bn_;
};

TractionRequestFlow traction_;
Expand Down
27 changes: 26 additions & 1 deletion src/openlcb/TractionTrain.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,22 @@ public:
/// this train.
virtual TrainImpl *train() = 0;

/// Applies a policy to function change requests coming in from the OpenLCB
/// bus. If the policy returns false, the change will not be applied to the
/// TrainImpl. This is used to implement consist function behavior.
/// @param src source node where the request came from.
/// @param command_byte is the first byte of the payload (usually 0x01 or
/// 0x81 depending on the REQ_LISTENER bit)
/// @param fnum which function to set
/// @param value what value to set this function to
/// @param done must be notified inline if the policy application is
/// successful. If not notified inline, then the returned value is ignored
/// and the call is repeated after done has been invoked by the callee.
/// @return true if the function should be applied to the TrainImpl, false
/// if it should not be applied.
virtual bool function_policy(NodeHandle src, uint8_t command_byte,
uint32_t fnum, uint16_t value, Notifiable *done) = 0;

/// @return the last stored controller node.
virtual NodeHandle get_controller() = 0;

Expand Down Expand Up @@ -146,7 +162,16 @@ private:
class TrainNodeWithConsist : public TrainNode {
public:
~TrainNodeWithConsist();


/// @copydoc TrainNode::function_policy()
/// The default function policy applies everything.
bool function_policy(NodeHandle src, uint8_t command_byte, uint32_t fnum,
uint16_t value, Notifiable *done) override
{
AutoNotify an(done);
return true;
}

/** Adds a node ID to the consist targets. @return false if the node was
* already in the target list, true if it was newly added. */
bool add_consist(NodeID tgt, uint8_t flags) override
Expand Down

0 comments on commit 60dcdb0

Please sign in to comment.