Add and test TryMakeFetcher, TryMakeSender, and NodeHasTag helpers
Using a Fetcher for a channel on nodes where it exists and triggering
logic to handle other nodes based on whether the fetcher is valid works
well for some use cases. Add a method to EventLoop to support that
directly, instead of forcing the caller to redo the logic around whether
a channel exists and is readable. Basing similar logic off whether the
node has a given tag is also fairly common.
In the process, fill out the EventLoop tests around channels that don't
exist or aren't readable/writable a bit better.
Change-Id: I168e3c1e98adc4461a01d98360f0f0a78d24bbb5
Signed-off-by: Brian Silverman <brian.silverman@bluerivertech.com>
diff --git a/aos/configuration.cc b/aos/configuration.cc
index e9c7160..d6394bb 100644
--- a/aos/configuration.cc
+++ b/aos/configuration.cc
@@ -1133,6 +1133,18 @@
return nodes;
}
+bool NodeHasTag(const Node *node, std::string_view tag) {
+ if (node == nullptr) {
+ return true;
+ }
+
+ const auto *const tags = node->tags();
+ return std::find_if(tags->begin(), tags->end(),
+ [tag](const flatbuffers::String *candidate) {
+ return candidate->string_view() == tag;
+ }) != tags->end();
+}
+
bool MultiNode(const Configuration *config) { return config->has_nodes(); }
bool ChannelIsSendableOnNode(const Channel *channel, const Node *node) {
diff --git a/aos/configuration.h b/aos/configuration.h
index e8722d6..9cb1132 100644
--- a/aos/configuration.h
+++ b/aos/configuration.h
@@ -113,6 +113,10 @@
std::vector<const Node *> GetNodesWithTag(const Configuration *config,
std::string_view tag);
+// Returns whether the given node has the provided tag. If this is a single-node
+// world, we assume that all tags match.
+bool NodeHasTag(const Node *node, std::string_view tag);
+
// Returns the node index for a node. Note: will be faster if node is a pointer
// to a node in config, but is not required.
int GetNodeIndex(const Configuration *config, const Node *node);
diff --git a/aos/configuration_test.cc b/aos/configuration_test.cc
index a73a34a..f5081d2 100644
--- a/aos/configuration_test.cc
+++ b/aos/configuration_test.cc
@@ -767,6 +767,27 @@
}
}
+// Tests that we can check if a node has a tag.
+TEST_F(ConfigurationTest, NodeHasTag) {
+ {
+ FlatbufferDetachedBuffer<Configuration> config =
+ ReadConfig(ArtifactPath("aos/testdata/good_multinode.json"));
+ const Node *pi1 = GetNode(&config.message(), "pi1");
+ const Node *pi2 = GetNode(&config.message(), "pi2");
+
+ EXPECT_TRUE(NodeHasTag(pi1, "a"));
+ EXPECT_FALSE(NodeHasTag(pi2, "a"));
+ EXPECT_FALSE(NodeHasTag(pi1, "b"));
+ EXPECT_TRUE(NodeHasTag(pi2, "b"));
+ EXPECT_TRUE(NodeHasTag(pi1, "c"));
+ EXPECT_TRUE(NodeHasTag(pi2, "c"));
+ EXPECT_FALSE(NodeHasTag(pi1, "nope"));
+ EXPECT_FALSE(NodeHasTag(pi2, "nope"));
+ }
+
+ EXPECT_TRUE(NodeHasTag(nullptr, "arglfish"));
+}
+
// Tests that we can extract a node index from a config.
TEST_F(ConfigurationTest, GetNodeIndex) {
FlatbufferDetachedBuffer<Configuration> config =
diff --git a/aos/events/event_loop.h b/aos/events/event_loop.h
index 6f51fb0..42489e5 100644
--- a/aos/events/event_loop.h
+++ b/aos/events/event_loop.h
@@ -517,31 +517,62 @@
return GetChannel<T>(channel_name) != nullptr;
}
+ // Like MakeFetcher, but returns an invalid fetcher if the given channel is
+ // not readable on this node or does not exist.
+ template <typename T>
+ Fetcher<T> TryMakeFetcher(const std::string_view channel_name) {
+ const Channel *const channel = GetChannel<T>(channel_name);
+ if (channel == nullptr) {
+ return Fetcher<T>();
+ }
+
+ if (!configuration::ChannelIsReadableOnNode(channel, node())) {
+ return Fetcher<T>();
+ }
+
+ return Fetcher<T>(MakeRawFetcher(channel));
+ }
+
// Makes a class that will always fetch the most recent value
// sent to the provided channel.
template <typename T>
Fetcher<T> MakeFetcher(const std::string_view channel_name) {
- const Channel *channel = GetChannel<T>(channel_name);
- CHECK(channel != nullptr)
+ CHECK(HasChannel<T>(channel_name))
<< ": Channel { \"name\": \"" << channel_name << "\", \"type\": \""
<< T::GetFullyQualifiedName() << "\" } not found in config.";
- if (!configuration::ChannelIsReadableOnNode(channel, node())) {
+ Fetcher<T> result = TryMakeFetcher<T>(channel_name);
+ if (!result.valid()) {
LOG(FATAL) << "Channel { \"name\": \"" << channel_name
<< "\", \"type\": \"" << T::GetFullyQualifiedName()
<< "\" } is not able to be fetched on this node. Check your "
"configuration.";
}
- return Fetcher<T>(MakeRawFetcher(channel));
+ return result;
+ }
+
+ // Like MakeSender, but returns an invalid sender if the given channel is
+ // not readable on this node or does not exist.
+ template <typename T>
+ Sender<T> TryMakeSender(const std::string_view channel_name) {
+ const Channel *channel = GetChannel<T>(channel_name);
+ if (channel == nullptr) {
+ return Sender<T>();
+ }
+
+ if (!configuration::ChannelIsSendableOnNode(channel, node())) {
+ return Sender<T>();
+ }
+
+ return Sender<T>(MakeRawSender(channel));
}
// Makes class that allows constructing and sending messages to
// the provided channel.
template <typename T>
Sender<T> MakeSender(const std::string_view channel_name) {
- const Channel *channel = GetChannel<T>(channel_name);
- CHECK(channel != nullptr)
+ CHECK(HasChannel<T>(channel_name))
<< ": Channel { \"name\": \"" << channel_name << "\", \"type\": \""
<< T::GetFullyQualifiedName() << "\" } not found in config for "
<< name()
@@ -549,14 +580,15 @@
? absl::StrCat(" on node ", node()->name()->string_view())
: ".");
- if (!configuration::ChannelIsSendableOnNode(channel, node())) {
+ Sender<T> result = TryMakeSender<T>(channel_name);
+ if (!result) {
LOG(FATAL) << "Channel { \"name\": \"" << channel_name
<< "\", \"type\": \"" << T::GetFullyQualifiedName()
<< "\" } is not able to be sent on this node. Check your "
"configuration.";
}
- return Sender<T>(MakeRawSender(channel));
+ return result;
}
// This will watch messages sent to the provided channel.
diff --git a/aos/events/event_loop_param_test.cc b/aos/events/event_loop_param_test.cc
index b976d8f..956693a 100644
--- a/aos/events/event_loop_param_test.cc
+++ b/aos/events/event_loop_param_test.cc
@@ -1191,7 +1191,8 @@
}
// Verifies that the event loop implementations detect when Channel is not a
-// pointer into configuration()
+// pointer into configuration(), or a name doesn't map to a channel in
+// configuration().
TEST_P(AbstractEventLoopDeathTest, InvalidChannel) {
auto loop = MakePrimary();
@@ -1201,19 +1202,85 @@
FlatbufferDetachedBuffer<Channel> channel_copy = CopyFlatBuffer(channel);
EXPECT_DEATH(
- { loop->MakeRawSender(&channel_copy.message()); },
+ loop->MakeRawSender(&channel_copy.message()),
"Channel pointer not found in configuration\\(\\)->channels\\(\\)");
EXPECT_DEATH(
- { loop->MakeRawFetcher(&channel_copy.message()); },
+ loop->MakeSender<TestMessage>("/testbad"),
+ "Channel \\{ \"name\": \"/testbad\", \"type\": \"aos.TestMessage\" \\}"
+ " not found in config");
+
+ EXPECT_FALSE(loop->TryMakeSender<TestMessage>("/testbad"));
+
+ EXPECT_DEATH(
+ loop->MakeRawFetcher(&channel_copy.message()),
"Channel pointer not found in configuration\\(\\)->channels\\(\\)");
EXPECT_DEATH(
+ loop->MakeFetcher<TestMessage>("/testbad"),
+ "Channel \\{ \"name\": \"/testbad\", \"type\": \"aos.TestMessage\" \\}"
+ " not found in config");
+
+ EXPECT_FALSE(loop->TryMakeFetcher<TestMessage>("/testbad").valid());
+
+ EXPECT_DEATH(
{
loop->MakeRawWatcher(&channel_copy.message(),
[](const Context, const void *) {});
},
"Channel pointer not found in configuration\\(\\)->channels\\(\\)");
+
+ EXPECT_DEATH(
+ { loop->MakeWatcher("/testbad", [](const TestMessage &) {}); },
+ "Channel \\{ \"name\": \"/testbad\", \"type\": \"aos.TestMessage\" \\}"
+ " not found in config");
+}
+
+// Verifies that the event loop handles a channel which is not readable or
+// writable on the current node nicely.
+TEST_P(AbstractEventLoopDeathTest, InaccessibleChannel) {
+ EnableNodes("me");
+ auto loop = MakePrimary("me");
+ auto loop2 = Make("them");
+
+ const Channel *channel = configuration::GetChannel(
+ loop->configuration(), "/test_noforward", "aos.TestMessage", "", nullptr);
+
+ FlatbufferDetachedBuffer<Channel> channel_copy = CopyFlatBuffer(channel);
+
+ EXPECT_DEATH(
+ loop2->MakeSender<TestMessage>("/test_forward"),
+ "Channel"
+ " \\{ \"name\": \"/test_forward\", \"type\": \"aos.TestMessage\" \\}"
+ " is not able to be sent on this node");
+
+ EXPECT_FALSE(loop2->TryMakeSender<TestMessage>("/test_forward"));
+
+ EXPECT_DEATH(
+ loop2->MakeRawFetcher(channel),
+ "Channel"
+ " \\{ \"name\": \"/test_noforward\", \"type\": \"aos.TestMessage\" \\}"
+ " is not able to be fetched on this node");
+
+ EXPECT_DEATH(
+ loop2->MakeFetcher<TestMessage>("/test_noforward"),
+ "Channel"
+ " \\{ \"name\": \"/test_noforward\", \"type\": \"aos.TestMessage\" \\}"
+ " is not able to be fetched on this node");
+
+ EXPECT_FALSE(loop2->TryMakeFetcher<TestMessage>("/test_noforward").valid());
+
+ EXPECT_DEATH(
+ { loop2->MakeRawWatcher(channel, [](const Context, const void *) {}); },
+ "\\{ \"name\": \"/test_noforward\", \"type\": \"aos.TestMessage\", "
+ "\"source_node\": \"them\" \\}"
+ " is not able to be watched on this node");
+
+ EXPECT_DEATH(
+ { loop2->MakeWatcher("/test_noforward", [](const TestMessage &) {}); },
+ "\\{ \"name\": \"/test_noforward\", \"type\": \"aos.TestMessage\", "
+ "\"source_node\": \"them\" \\}"
+ " is not able to be watched on this node");
}
// Verifies that the event loop implementations detect when Channel has an
diff --git a/aos/events/event_loop_param_test.h b/aos/events/event_loop_param_test.h
index 543bb21..a9d280e 100644
--- a/aos/events/event_loop_param_test.h
+++ b/aos/events/event_loop_param_test.h
@@ -225,6 +225,21 @@
"name": "/test2",
"type": "aos.TestMessage",
"source_node": "me"
+ },
+ {
+ "name": "/test_forward",
+ "type": "aos.TestMessage",
+ "source_node": "them",
+ "destination_nodes": [
+ {
+ "name": "me"
+ }
+ ]
+ },
+ {
+ "name": "/test_noforward",
+ "type": "aos.TestMessage",
+ "source_node": "them"
}
],
"nodes": [