Add CHECK for remote timestamp channel frequencies
This makes it so that it is much harder to accidentally create a remote
timestmap channel whose frequency is lower than that of the actual
channel.
Change-Id: I69911ec1cf7b6e717813f3cd50087c90455cc73f
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>
diff --git a/aos/network/BUILD b/aos/network/BUILD
index 96825e4..4794838 100644
--- a/aos/network/BUILD
+++ b/aos/network/BUILD
@@ -375,6 +375,21 @@
deps = ["//aos/events:aos_config"],
)
+aos_config(
+ name = "timestamp_channel_test_config",
+ src = "timestamp_channel_test.json",
+ flatbuffers = [
+ ":remote_message_fbs",
+ "//aos/events:ping_fbs",
+ "//aos/events:pong_fbs",
+ "//aos/network:message_bridge_client_fbs",
+ "//aos/network:message_bridge_server_fbs",
+ "//aos/network:timestamp_fbs",
+ ],
+ target_compatible_with = ["@platforms//os:linux"],
+ deps = ["//aos/events:aos_config"],
+)
+
cc_test(
name = "message_bridge_test",
srcs = [
@@ -616,3 +631,19 @@
"//aos/testing:googletest",
],
)
+
+cc_test(
+ name = "timestamp_channel_test",
+ srcs = ["timestamp_channel_test.cc"],
+ data = [":timestamp_channel_test_config"],
+ deps = [
+ ":timestamp_channel",
+ "//aos:configuration",
+ "//aos/events:ping_fbs",
+ "//aos/events:shm_event_loop",
+ "//aos/events:simulated_event_loop",
+ "//aos/testing:googletest",
+ "//aos/testing:path",
+ "//aos/testing:tmpdir",
+ ],
+)
diff --git a/aos/network/message_bridge_test_combined_timestamps_common.json b/aos/network/message_bridge_test_combined_timestamps_common.json
index be79014..5d82965 100644
--- a/aos/network/message_bridge_test_combined_timestamps_common.json
+++ b/aos/network/message_bridge_test_combined_timestamps_common.json
@@ -75,8 +75,7 @@
{
"name": "/pi1/aos/remote_timestamps/pi2",
"type": "aos.message_bridge.RemoteMessage",
- "source_node": "pi1",
- "frequency": 15
+ "source_node": "pi1"
},
{
"name": "/pi2/aos/remote_timestamps/pi1",
diff --git a/aos/network/message_bridge_test_common.json b/aos/network/message_bridge_test_common.json
index 4623edb..a30734d 100644
--- a/aos/network/message_bridge_test_common.json
+++ b/aos/network/message_bridge_test_common.json
@@ -87,20 +87,17 @@
{
"name": "/pi1/aos/remote_timestamps/pi2/test/aos-examples-Ping",
"type": "aos.message_bridge.RemoteMessage",
- "source_node": "pi1",
- "frequency": 15
+ "source_node": "pi1"
},
{
"name": "/pi2/aos/remote_timestamps/pi1/test/aos-examples-Pong",
"type": "aos.message_bridge.RemoteMessage",
- "source_node": "pi2",
- "frequency": 15
+ "source_node": "pi2"
},
{
"name": "/pi1/aos/remote_timestamps/pi2/unreliable/aos-examples-Ping",
"type": "aos.message_bridge.RemoteMessage",
- "source_node": "pi1",
- "frequency": 15
+ "source_node": "pi1"
},
{
"name": "/pi1/aos",
diff --git a/aos/network/timestamp_channel.cc b/aos/network/timestamp_channel.cc
index fdaa031..f8f525f 100644
--- a/aos/network/timestamp_channel.cc
+++ b/aos/network/timestamp_channel.cc
@@ -109,6 +109,13 @@
const Channel *timestamp_channel = finder.ForChannel(channel, connection);
+ // Sanity-check that the timestamp channel can actually support full-rate
+ // messages coming through on the source channel.
+ CHECK_GE(timestamp_channel->frequency(), channel->frequency())
+ << ": Timestamp channel "
+ << configuration::StrippedChannelToString(timestamp_channel)
+ << "'s rate is lower than the source channel.";
+
{
auto it = timestamp_loggers_.find(timestamp_channel);
if (it != timestamp_loggers_.end()) {
diff --git a/aos/network/timestamp_channel_test.cc b/aos/network/timestamp_channel_test.cc
new file mode 100644
index 0000000..e3850ca
--- /dev/null
+++ b/aos/network/timestamp_channel_test.cc
@@ -0,0 +1,71 @@
+#include "aos/network/timestamp_channel.h"
+
+#include "aos/configuration.h"
+#include "aos/events/ping_generated.h"
+#include "aos/events/shm_event_loop.h"
+#include "aos/events/simulated_event_loop.h"
+#include "aos/testing/path.h"
+#include "aos/testing/tmpdir.h"
+#include "gtest/gtest.h"
+
+DECLARE_string(override_hostname);
+
+namespace aos::message_bridge::testing {
+class TimestampChannelTest : public ::testing::Test {
+ protected:
+ TimestampChannelTest()
+ : config_(aos::configuration::ReadConfig(aos::testing::ArtifactPath(
+ "aos/network/timestamp_channel_test_config.json"))) {
+ FLAGS_shm_base = aos::testing::TestTmpDir();
+ FLAGS_override_hostname = "pi1";
+ }
+ aos::FlatbufferDetachedBuffer<aos::Configuration> config_;
+};
+
+// Tests that creating a SimulatedEventLoopFactory with invalid remote timestamp
+// channel frequencies fails.
+TEST_F(TimestampChannelTest, SimulatedNetworkBridgeFrequencyMismatch) {
+ SimulatedEventLoopFactory factory(&config_.message());
+ EXPECT_DEATH(factory.RunFor(std::chrono::seconds(1)),
+ "rate is lower than the source channel");
+}
+
+class TimestampChannelParamTest
+ : public TimestampChannelTest,
+ public ::testing::WithParamInterface<
+ std::tuple<std::string, std::optional<std::string>>> {};
+
+// Tests whether we can or can't retrieve a timestamp channel depending on
+// whether it has a valid max frequency configured.
+TEST_P(TimestampChannelParamTest, ChannelFrequency) {
+ aos::ShmEventLoop event_loop(&config_.message());
+ ChannelTimestampSender timestamp_sender(&event_loop);
+ const aos::Channel *channel =
+ event_loop.GetChannel<aos::examples::Ping>(std::get<0>(GetParam()));
+ const std::optional<std::string> error_message = std::get<1>(GetParam());
+ if (error_message.has_value()) {
+ EXPECT_DEATH(timestamp_sender.SenderForChannel(
+ channel, channel->destination_nodes()->Get(0)),
+ error_message.value());
+ } else {
+ aos::Sender<RemoteMessage> *sender = timestamp_sender.SenderForChannel(
+ channel, channel->destination_nodes()->Get(0));
+ ASSERT_TRUE(sender != nullptr);
+ EXPECT_EQ(absl::StrCat("/pi1/aos/remote_timestamps/pi2",
+ std::get<0>(GetParam()), "/aos-examples-Ping"),
+ sender->channel()->name()->string_view());
+ }
+}
+
+std::tuple<std::string, std::optional<std::string>> MakeParams(
+ std::string channel, std::optional<std::string> error) {
+ return std::make_tuple(channel, error);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ ChannelFrequencyTest, TimestampChannelParamTest,
+ ::testing::Values(MakeParams("/nominal", std::nullopt),
+ MakeParams("/timestamps_too_fast", std::nullopt),
+ MakeParams("/timestamps_too_slow",
+ "rate is lower than the source channel")));
+} // namespace aos::message_bridge::testing
diff --git a/aos/network/timestamp_channel_test.json b/aos/network/timestamp_channel_test.json
new file mode 100644
index 0000000..f60cae8
--- /dev/null
+++ b/aos/network/timestamp_channel_test.json
@@ -0,0 +1,195 @@
+{
+ "channels": [
+ {
+ "name": "/pi1/aos",
+ "type": "aos.logging.LogMessageFbs",
+ "source_node": "pi1",
+ "frequency": 200,
+ "num_senders": 20,
+ "max_size": 2048
+ },
+ {
+ "name": "/pi2/aos",
+ "type": "aos.logging.LogMessageFbs",
+ "source_node": "pi2",
+ "frequency": 200,
+ "num_senders": 20,
+ "max_size": 2048
+ },
+ {
+ "name": "/pi1/aos",
+ "type": "aos.message_bridge.Timestamp",
+ "source_node": "pi1",
+ "frequency": 15,
+ "max_size": 200,
+ "destination_nodes": [
+ {
+ "name": "pi2",
+ "priority": 1,
+ "timestamp_logger": "LOCAL_AND_REMOTE_LOGGER",
+ "timestamp_logger_nodes": ["pi1"],
+ "time_to_live": 5000000
+ }
+ ]
+ },
+ {
+ "name": "/pi2/aos",
+ "type": "aos.message_bridge.Timestamp",
+ "source_node": "pi2",
+ "frequency": 15,
+ "max_size": 200,
+ "destination_nodes": [
+ {
+ "name": "pi1",
+ "priority": 1,
+ "timestamp_logger": "LOCAL_AND_REMOTE_LOGGER",
+ "timestamp_logger_nodes": ["pi2"],
+ "time_to_live": 5000000
+ }
+ ]
+ },
+ {
+ "name": "/pi1/aos/remote_timestamps/pi2/pi1/aos/aos-message_bridge-Timestamp",
+ "type": "aos.message_bridge.RemoteMessage",
+ "source_node": "pi1",
+ "frequency": 15
+ },
+ {
+ "name": "/pi2/aos/remote_timestamps/pi1/pi2/aos/aos-message_bridge-Timestamp",
+ "type": "aos.message_bridge.RemoteMessage",
+ "source_node": "pi2",
+ "frequency": 15
+ },
+ {
+ "name": "/pi1/aos",
+ "type": "aos.message_bridge.ServerStatistics",
+ "source_node": "pi1",
+ "frequency": 2
+ },
+ {
+ "name": "/pi2/aos",
+ "type": "aos.message_bridge.ServerStatistics",
+ "source_node": "pi2",
+ "frequency": 2
+ },
+ {
+ "name": "/pi1/aos",
+ "type": "aos.message_bridge.ClientStatistics",
+ "source_node": "pi1",
+ "frequency": 15
+ },
+ {
+ "name": "/pi2/aos",
+ "type": "aos.message_bridge.ClientStatistics",
+ "source_node": "pi2",
+ "frequency": 15
+ },
+ {
+ "name": "/pi1/aos",
+ "type": "aos.timing.Report",
+ "source_node": "pi1",
+ "frequency": 50,
+ "num_senders": 20,
+ "max_size": 2048
+ },
+ {
+ "name": "/pi2/aos",
+ "type": "aos.timing.Report",
+ "source_node": "pi2",
+ "frequency": 50,
+ "num_senders": 20,
+ "max_size": 2048
+ },
+ {
+ "name": "/pi1/aos/remote_timestamps/pi2/nominal/aos-examples-Ping",
+ "type": "aos.message_bridge.RemoteMessage",
+ "source_node": "pi1",
+ "frequency": 15
+ },
+ {
+ "name": "/pi1/aos/remote_timestamps/pi2/timestamps_too_slow/aos-examples-Ping",
+ "type": "aos.message_bridge.RemoteMessage",
+ "source_node": "pi1",
+ /* This frequency is configured lower than it should be. This will cause errors. */
+ "frequency": 5
+ },
+ {
+ "name": "/pi1/aos/remote_timestamps/pi2/timestamps_too_fast/aos-examples-Ping",
+ "type": "aos.message_bridge.RemoteMessage",
+ "source_node": "pi1",
+ /* This frequency is configured higher than it should be. This should not cause errors. */
+ "frequency": 10000
+ },
+ {
+ "name": "/nominal",
+ "type": "aos.examples.Ping",
+ "source_node": "pi1",
+ "frequency": 15,
+ "destination_nodes": [
+ {
+ "name": "pi2",
+ "timestamp_logger": "REMOTE_LOGGER",
+ "timestamp_logger_nodes": ["pi1"]
+ }
+ ]
+ },
+ {
+ "name": "/timestamps_too_slow",
+ "type": "aos.examples.Ping",
+ "source_node": "pi1",
+ "frequency": 15,
+ "destination_nodes": [
+ {
+ "name": "pi2",
+ "timestamp_logger": "REMOTE_LOGGER",
+ "timestamp_logger_nodes": ["pi1"]
+ }
+ ]
+ },
+ {
+ "name": "/timestamps_too_fast",
+ "type": "aos.examples.Ping",
+ "source_node": "pi1",
+ "frequency": 15,
+ "destination_nodes": [
+ {
+ "name": "pi2",
+ "timestamp_logger": "REMOTE_LOGGER",
+ "timestamp_logger_nodes": ["pi1"]
+ }
+ ]
+ }
+ ],
+ "maps": [
+ {
+ "match": {
+ "name": "/aos*",
+ "source_node": "pi1"
+ },
+ "rename": {
+ "name": "/pi1/aos"
+ }
+ },
+ {
+ "match": {
+ "name": "/aos*",
+ "source_node": "pi2"
+ },
+ "rename": {
+ "name": "/pi2/aos"
+ }
+ }
+ ],
+ "nodes": [
+ {
+ "name": "pi1",
+ "hostname": "pi1",
+ "port": 9971
+ },
+ {
+ "name": "pi2",
+ "hostname": "pi2",
+ "port": 9972
+ }
+ ]
+}