diff --git a/dc_measurements/src/measurement_server.cpp b/dc_measurements/src/measurement_server.cpp index 6babaf75f..3c04b1344 100644 --- a/dc_measurements/src/measurement_server.cpp +++ b/dc_measurements/src/measurement_server.cpp @@ -3,11 +3,74 @@ #include "dc_measurements/measurement_server.hpp" +#include + +#include +#include + namespace measurement_server { using namespace std::chrono_literals; // NOLINT +namespace +{ + +// robot_name is the one custom key whose identity a fleet can't hand-edit per robot (#442): +// literal value, then a file's contents, then the machine's hostname as the default. +constexpr const char* kRobotNameKey = "robot_name"; + +std::string resolveRobotNameFromHostname() +{ + char hostname_buf[256] = { 0 }; + if (gethostname(hostname_buf, sizeof(hostname_buf) - 1) != 0) + { + throw std::runtime_error(std::string("robot_name: failed to resolve hostname: ") + std::strerror(errno)); + } + return std::string(hostname_buf); +} + +std::string resolveRobotNameFromFile(const std::string& path) +{ + std::ifstream ifs(path); + if (!ifs.good()) + { + throw std::runtime_error("robot_name: could not read value_from_file '" + path + "'"); + } + return dc_util::get_file_content(path); +} + +std::string resolveRobotName(const std::string& value, const std::string& value_from_file) +{ + std::string resolved; + std::string source; + + if (!value.empty()) + { + resolved = value; + source = "the literal value"; + } + else if (!value_from_file.empty()) + { + resolved = resolveRobotNameFromFile(value_from_file); + source = "value_from_file '" + value_from_file + "'"; + } + else + { + resolved = resolveRobotNameFromHostname(); + source = "the hostname"; + } + + if (resolved.empty()) + { + throw std::runtime_error("robot_name: resolved to an empty value from " + source); + } + + return resolved; +} + +} // namespace + MeasurementServer::MeasurementServer(const rclcpp::NodeOptions& options, const std::vector& measurement_plugins) : nav2_util::LifecycleNode("measurement_server", "", options) @@ -56,7 +119,11 @@ void MeasurementServer::setCustomKeys() custom_keys_[i]["key"] = key; custom_keys_[i]["override"] = force_override; - if (!value.empty()) + if (custom_key == kRobotNameKey) + { + custom_keys_[i]["value"] = resolveRobotName(value, value_from_file); + } + else if (!value.empty()) { custom_keys_[i]["value"] = value; } diff --git a/dc_measurements/test/test_measurement_dummy.cpp b/dc_measurements/test/test_measurement_dummy.cpp index ef54cc93e..975ed3cc8 100644 --- a/dc_measurements/test/test_measurement_dummy.cpp +++ b/dc_measurements/test/test_measurement_dummy.cpp @@ -2,10 +2,15 @@ // SPDX-License-Identifier: MPL-2.0 #include +#include + +#include +#include #include "dc_interfaces/msg/string_stamped.hpp" #include "dc_measurements/measurement_server.hpp" #include "dc_util/json_utils.hpp" +#include "lifecycle_msgs/msg/state.hpp" class MeasurementDummyTest : public ::testing::Test { @@ -100,6 +105,94 @@ TEST_F(MeasurementDummyTest, CustomKeysAreDeclaredInTheRecord) EXPECT_EQ(dummy_record_["custom_keys"], nlohmann::json::array({ "site" })); } +// robot_name is the one custom key whose resolution is flexible: literal, hostname, or a +// file's contents, in that order, with hostname as the default when nothing is set (#442). + +TEST_F(MeasurementDummyTest, RobotNameLiteralValueIsUnchanged) +{ + ms_node_->declare_parameter("dummy.plugin", std::string("dc_measurements/Dummy")); + ms_node_->declare_parameter("dummy.topic_output", std::string("/dc/measurement/dummy")); + ms_node_->declare_parameter("dummy.record", std::string("{\"message\": \"My message\"}")); + ms_node_->declare_parameter("custom_key_str_list", std::vector{ "robot_name" }); + ms_node_->declare_parameter("custom_keys_str.robot_name.name", std::string("robot_name")); + ms_node_->declare_parameter("custom_keys_str.robot_name.value", std::string("C3PO")); + + startLifecycleNode(); + + while (!dummy_callback_) + { + rclcpp::spin_some(ms_node_->get_node_base_interface()); + } + + EXPECT_EQ(dummy_record_["robot_name"], "C3PO"); +} + +TEST_F(MeasurementDummyTest, RobotNameDefaultsToTheHostname) +{ + ms_node_->declare_parameter("dummy.plugin", std::string("dc_measurements/Dummy")); + ms_node_->declare_parameter("dummy.topic_output", std::string("/dc/measurement/dummy")); + ms_node_->declare_parameter("dummy.record", std::string("{\"message\": \"My message\"}")); + ms_node_->declare_parameter("custom_key_str_list", std::vector{ "robot_name" }); + ms_node_->declare_parameter("custom_keys_str.robot_name.name", std::string("robot_name")); + + startLifecycleNode(); + + while (!dummy_callback_) + { + rclcpp::spin_some(ms_node_->get_node_base_interface()); + } + + char hostname_buf[256] = { 0 }; + ASSERT_EQ(gethostname(hostname_buf, sizeof(hostname_buf) - 1), 0); + EXPECT_EQ(dummy_record_["robot_name"], std::string(hostname_buf)); +} + +TEST_F(MeasurementDummyTest, RobotNameResolvesFromFile) +{ + auto robot_name_file = (std::filesystem::temp_directory_path() / "dc_measurement_dummy_robot_name_file").u8string(); + std::ofstream(robot_name_file) << "TB-42"; + + ms_node_->declare_parameter("dummy.plugin", std::string("dc_measurements/Dummy")); + ms_node_->declare_parameter("dummy.topic_output", std::string("/dc/measurement/dummy")); + ms_node_->declare_parameter("dummy.record", std::string("{\"message\": \"My message\"}")); + ms_node_->declare_parameter("custom_key_str_list", std::vector{ "robot_name" }); + ms_node_->declare_parameter("custom_keys_str.robot_name.name", std::string("robot_name")); + ms_node_->declare_parameter("custom_keys_str.robot_name.value_from_file", robot_name_file); + + startLifecycleNode(); + + while (!dummy_callback_) + { + rclcpp::spin_some(ms_node_->get_node_base_interface()); + } + + EXPECT_EQ(dummy_record_["robot_name"], "TB-42"); + + std::filesystem::remove(robot_name_file); +} + +// rclcpp_lifecycle wraps every transition callback (on_configure here) in its own catch, +// converting an uncaught exception into a CallbackReturn::ERROR rather than letting it +// propagate to the caller of configure() (same behavior documented in +// test_measurement_bool_equal.cpp's MeasurementServerConfigureFailsToReachInactiveState). +// So the observable, end-to-end consequence of an unreadable value_from_file is that the +// whole MeasurementServer fails to reach the "inactive" state, with the resolveRobotName() +// error logged as the ERROR/FATAL "Original error" during the transition. +TEST_F(MeasurementDummyTest, RobotNameMissingFileFailsConfigureClearly) +{ + ms_node_->declare_parameter("dummy.plugin", std::string("dc_measurements/Dummy")); + ms_node_->declare_parameter("dummy.topic_output", std::string("/dc/measurement/dummy")); + ms_node_->declare_parameter("dummy.record", std::string("{\"message\": \"My message\"}")); + ms_node_->declare_parameter("custom_key_str_list", std::vector{ "robot_name" }); + ms_node_->declare_parameter("custom_keys_str.robot_name.name", std::string("robot_name")); + ms_node_->declare_parameter("custom_keys_str.robot_name.value_from_file", + std::string("/nonexistent/dc_robot_name_that_does_not_exist")); + + auto result_state = ms_node_->configure(); + + EXPECT_NE(result_state.id(), lifecycle_msgs::msg::State::PRIMARY_STATE_INACTIVE); +} + TEST_F(MeasurementDummyTest, NoCustomKeysLeavesTheRecordUntouched) { ms_node_->declare_parameter("dummy.plugin", std::string("dc_measurements/Dummy")); diff --git a/doc/src/dc/measurements.md b/doc/src/dc/measurements.md index 07602449e..a2762bb12 100644 --- a/doc/src/dc/measurements.md +++ b/doc/src/dc/measurements.md @@ -52,6 +52,21 @@ the Record as a `tags` field, but it has no routing effect — remove it. See th | run_id.counter_path | Path to store the last run. It is expanded with environment variables id | str | "$HOME/run_id" | | run_id.uuid | Generate a new run ID by using a random UUID | str | false | +### robot_name resolution + +`robot_name` is a custom key like any other, but when it appears in `custom_key_str_list` +its value resolves in a fixed order rather than always being a literal string, so a fleet +does not need one hand-edited params file per robot: + +1. `custom_keys_str.robot_name.value` — a literal string, unchanged from before. +2. `custom_keys_str.robot_name.value_from_file` — the contents of a file, e.g. one written + by the provisioning process. +3. The machine's hostname — the default when neither of the above is set. + +A `value_from_file` that names a file that cannot be read, or any source that resolves to +an empty string, fails node configuration with a clear error rather than shipping Records +with a missing or blank `robot_name`. + ### Custom keys on Files The keys listed in `custom_key_str_list` label a Measurement's **Files** as well as its