Skip to content

Commit 29c3ef4

Browse files
refactor(autoware_command_gate): extract ROS-free mode dispatch helper (#1145)
* refactor(autoware_command_gate): extract ROS-free mode dispatch helper Move the request-to-mode dispatch (switch on the requested operation mode and build the corresponding outputs) out of the node's service-callback lambda into a ROS-free static helper CommandGateModeBuilder::dispatch_mode, which returns std::optional<ModeOutputs> (std::nullopt for unknown/unsupported modes). This makes the dispatch and its only error path (unknown mode -> PARAMETER_ERROR) unit-testable with plain gtest, without spinning up rclcpp. Behavior is preserved: the node still publishes outputs and sets status.success/code/message on the happy path, and emits success=false with PARAMETER_ERROR on an unknown mode. Add unit tests for dispatch_mode covering all four valid modes and the previously untested unknown-mode nullopt branch. The public node API is unchanged; dispatch_mode is an additive static method. Refs: #1096 Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp> * refactor(autoware_command_gate): address review feedback Move autoware_system_msgs/srv/change_operation_mode include from the builder header to the .cpp where ChangeOperationMode::Request is used, decoupling includers from the service definition. Assert stamp propagation in DispatchModeAutonomous/Local/Remote tests by passing non-zero stamps and checking outputs->state.stamp. Refs: #1096 Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp> * refactor(autoware_command_gate): split mode-output and response builders (#80) Make the ChangeOperationMode service callback route through two pure message-in -> message-out builders: create_mode_output() builds the messages to publish (std::nullopt for unknown modes) and create_response() builds the service response. Both are ROS-node-free static functions that take the request, so they are unit-testable in isolation. Behavior is preserved; the publish-only ModeOutputs no longer carries response status. Refs: #1096 Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp> --------- Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp> Co-authored-by: Junya Sasaki <j2sasaki1990@gmail.com>
1 parent 45af260 commit 29c3ef4

5 files changed

Lines changed: 252 additions & 68 deletions

File tree

control/autoware_command_gate/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ if(BUILD_TESTING)
3434
)
3535
ament_target_dependencies(test_command_gate_mode_builder
3636
autoware_adapi_v1_msgs
37+
autoware_common_msgs
38+
autoware_system_msgs
3739
autoware_vehicle_msgs
3840
builtin_interfaces
3941
)

control/autoware_command_gate/src/autoware_command_gate.cpp

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
#include <rclcpp/rclcpp.hpp>
2121
#include <rclcpp_components/register_node_macro.hpp>
2222

23-
#include <autoware_adapi_v1_msgs/msg/response_status.hpp>
24-
#include <autoware_common_msgs/msg/response_status.hpp>
2523
#include <autoware_vehicle_msgs/msg/gear_command.hpp>
2624

2725
#include <rmw/types.h>
@@ -77,32 +75,11 @@ class AutowareCommandGateNode : public rclcpp::Node
7775
const SystemChangeOperationMode::Service::Request::SharedPtr req,
7876
const SystemChangeOperationMode::Service::Response::SharedPtr res) {
7977
const builtin_interfaces::msg::Time stamp = now();
80-
ModeOutputs outputs;
81-
82-
switch (req->mode) {
83-
case SystemChangeOperationMode::Service::Request::STOP:
84-
outputs = mode_builder_.make_stop(stamp);
85-
break;
86-
case SystemChangeOperationMode::Service::Request::AUTONOMOUS:
87-
outputs = mode_builder_.make_autonomous(stamp);
88-
break;
89-
case SystemChangeOperationMode::Service::Request::LOCAL:
90-
outputs = mode_builder_.make_local(stamp);
91-
break;
92-
case SystemChangeOperationMode::Service::Request::REMOTE:
93-
outputs = mode_builder_.make_remote(stamp);
94-
break;
95-
default:
96-
res->status.success = false;
97-
res->status.code = autoware_common_msgs::msg::ResponseStatus::PARAMETER_ERROR;
98-
res->status.message = "Unknown operation mode requested.";
99-
return;
78+
const auto outputs = CommandGateModeBuilder::create_mode_output(*req, stamp);
79+
if (outputs) {
80+
publish(*outputs);
10081
}
101-
102-
publish(outputs);
103-
res->status.success = true;
104-
res->status.code = 0;
105-
res->status.message = outputs.status.message;
82+
*res = CommandGateModeBuilder::create_response(*req, stamp);
10683
});
10784
}
10885

@@ -121,7 +98,6 @@ class AutowareCommandGateNode : public rclcpp::Node
12198
rclcpp::Publisher<OperationModeSystemStateMsg>::SharedPtr system_state_pub_;
12299
rclcpp::Publisher<GearCommand>::SharedPtr gear_pub_;
123100
rclcpp::Service<SystemChangeOperationMode::Service>::SharedPtr srv_system_mode_;
124-
CommandGateModeBuilder mode_builder_;
125101
};
126102

127103
} // namespace autoware::control::command_gate

control/autoware_command_gate/src/command_gate_mode_builder.cpp

Lines changed: 60 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,76 @@
1414

1515
#include "command_gate_mode_builder.hpp"
1616

17+
#include <autoware_common_msgs/msg/response_status.hpp>
18+
19+
#include <optional>
1720
#include <string>
1821

1922
namespace autoware::control::command_gate
2023
{
2124

25+
std::optional<ModeOutputs> CommandGateModeBuilder::create_mode_output(
26+
const Request & request, const builtin_interfaces::msg::Time & stamp)
27+
{
28+
return dispatch_mode(request.mode, stamp);
29+
}
30+
31+
CommandGateModeBuilder::Response CommandGateModeBuilder::create_response(
32+
const Request & request, const builtin_interfaces::msg::Time & /*stamp*/)
33+
{
34+
Response response;
35+
const auto message = success_message(request.mode);
36+
if (!message) {
37+
response.status.success = false;
38+
response.status.code = autoware_common_msgs::msg::ResponseStatus::PARAMETER_ERROR;
39+
response.status.message = "Unknown operation mode requested.";
40+
return response;
41+
}
42+
43+
response.status.success = true;
44+
response.status.code = 0; // 0 represents success (no specific success code defined)
45+
response.status.message = *message;
46+
return response;
47+
}
48+
49+
std::optional<std::string> CommandGateModeBuilder::success_message(uint16_t mode)
50+
{
51+
switch (mode) {
52+
case Request::STOP:
53+
return "Switched to STOP";
54+
case Request::AUTONOMOUS:
55+
return "Switched to AUTONOMOUS";
56+
case Request::LOCAL:
57+
return "Switched to LOCAL";
58+
case Request::REMOTE:
59+
return "Switched to REMOTE";
60+
default:
61+
return std::nullopt;
62+
}
63+
}
64+
65+
std::optional<ModeOutputs> CommandGateModeBuilder::dispatch_mode(
66+
uint16_t mode, const builtin_interfaces::msg::Time & stamp)
67+
{
68+
switch (mode) {
69+
case Request::STOP:
70+
return make_stop(stamp);
71+
case Request::AUTONOMOUS:
72+
return make_autonomous(stamp);
73+
case Request::LOCAL:
74+
return make_local(stamp);
75+
case Request::REMOTE:
76+
return make_remote(stamp);
77+
default:
78+
return std::nullopt;
79+
}
80+
}
81+
2282
ModeOutputs CommandGateModeBuilder::make_stop(const builtin_interfaces::msg::Time & stamp)
2383
{
2484
ModeOutputs outputs;
2585
fill_state(outputs.state, autoware_adapi_v1_msgs::msg::OperationModeState::STOP, stamp);
2686
fill_gear(outputs.gear, autoware_vehicle_msgs::msg::GearCommand::PARK, stamp);
27-
outputs.status = make_status("Switched to STOP");
2887
return outputs;
2988
}
3089

@@ -33,7 +92,6 @@ ModeOutputs CommandGateModeBuilder::make_autonomous(const builtin_interfaces::ms
3392
ModeOutputs outputs;
3493
fill_state(outputs.state, autoware_adapi_v1_msgs::msg::OperationModeState::AUTONOMOUS, stamp);
3594
fill_gear(outputs.gear, autoware_vehicle_msgs::msg::GearCommand::DRIVE, stamp);
36-
outputs.status = make_status("Switched to AUTONOMOUS");
3795
return outputs;
3896
}
3997

@@ -42,7 +100,6 @@ ModeOutputs CommandGateModeBuilder::make_local(const builtin_interfaces::msg::Ti
42100
ModeOutputs outputs;
43101
fill_state(outputs.state, autoware_adapi_v1_msgs::msg::OperationModeState::LOCAL, stamp);
44102
fill_gear(outputs.gear, autoware_vehicle_msgs::msg::GearCommand::NONE, stamp);
45-
outputs.status = make_status("Switched to LOCAL");
46103
return outputs;
47104
}
48105

@@ -51,7 +108,6 @@ ModeOutputs CommandGateModeBuilder::make_remote(const builtin_interfaces::msg::T
51108
ModeOutputs outputs;
52109
fill_state(outputs.state, autoware_adapi_v1_msgs::msg::OperationModeState::REMOTE, stamp);
53110
fill_gear(outputs.gear, autoware_vehicle_msgs::msg::GearCommand::NONE, stamp);
54-
outputs.status = make_status("Switched to REMOTE");
55111
return outputs;
56112
}
57113

@@ -78,14 +134,4 @@ void CommandGateModeBuilder::fill_gear(
78134
msg.command = command;
79135
}
80136

81-
autoware_adapi_v1_msgs::msg::ResponseStatus CommandGateModeBuilder::make_status(
82-
const std::string & message)
83-
{
84-
autoware_adapi_v1_msgs::msg::ResponseStatus status;
85-
status.success = true;
86-
status.code = 0; // 0 represents success (no specific success code defined)
87-
status.message = message;
88-
return status;
89-
}
90-
91137
} // namespace autoware::control::command_gate

control/autoware_command_gate/src/command_gate_mode_builder.hpp

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,11 @@
1818
#include <builtin_interfaces/msg/time.hpp>
1919

2020
#include <autoware_adapi_v1_msgs/msg/operation_mode_state.hpp>
21-
#include <autoware_adapi_v1_msgs/msg/response_status.hpp>
21+
#include <autoware_system_msgs/srv/change_operation_mode.hpp>
2222
#include <autoware_vehicle_msgs/msg/gear_command.hpp>
2323

24+
#include <cstdint>
25+
#include <optional>
2426
#include <string>
2527

2628
namespace autoware::control::command_gate
@@ -30,25 +32,49 @@ struct ModeOutputs
3032
{
3133
autoware_adapi_v1_msgs::msg::OperationModeState state;
3234
autoware_vehicle_msgs::msg::GearCommand gear;
33-
autoware_adapi_v1_msgs::msg::ResponseStatus status;
3435
};
3536

37+
/// Pure (ROS-node-free) builders that convert a ChangeOperationMode request message into the
38+
/// messages the command gate emits. Every entry point is a plain message-in -> message-out
39+
/// function so it can be unit-tested without spinning up a ROS node, publisher or service.
3640
class CommandGateModeBuilder
3741
{
3842
public:
43+
using Request = autoware_system_msgs::srv::ChangeOperationMode::Request;
44+
using Response = autoware_system_msgs::srv::ChangeOperationMode::Response;
45+
46+
/// Build the messages to publish for the requested operation mode.
47+
/// Returns std::nullopt for unknown/unsupported modes so the caller publishes nothing.
48+
/// The accepted values mirror the constants of
49+
/// autoware_system_msgs::srv::ChangeOperationMode::Request (STOP/AUTONOMOUS/LOCAL/REMOTE).
50+
static std::optional<ModeOutputs> create_mode_output(
51+
const Request & request, const builtin_interfaces::msg::Time & stamp);
52+
53+
/// Build the service response for the requested operation mode.
54+
/// Reports success for known modes and a PARAMETER_ERROR for unknown ones, mirroring the
55+
/// publish decision made by create_mode_output().
56+
static Response create_response(
57+
const Request & request, const builtin_interfaces::msg::Time & stamp);
58+
3959
static ModeOutputs make_stop(const builtin_interfaces::msg::Time & stamp);
4060
static ModeOutputs make_autonomous(const builtin_interfaces::msg::Time & stamp);
4161
static ModeOutputs make_local(const builtin_interfaces::msg::Time & stamp);
4262
static ModeOutputs make_remote(const builtin_interfaces::msg::Time & stamp);
4363

4464
private:
65+
/// Map an operation-mode request value to its outputs, or std::nullopt for unknown modes.
66+
static std::optional<ModeOutputs> dispatch_mode(
67+
uint16_t mode, const builtin_interfaces::msg::Time & stamp);
68+
69+
/// Human-readable success message for a known mode, or std::nullopt for unknown modes.
70+
static std::optional<std::string> success_message(uint16_t mode);
71+
4572
static void fill_state(
4673
autoware_adapi_v1_msgs::msg::OperationModeState & msg, uint8_t mode,
4774
const builtin_interfaces::msg::Time & stamp);
4875
static void fill_gear(
4976
autoware_vehicle_msgs::msg::GearCommand & msg, uint8_t command,
5077
const builtin_interfaces::msg::Time & stamp);
51-
static autoware_adapi_v1_msgs::msg::ResponseStatus make_status(const std::string & message);
5278
};
5379

5480
} // namespace autoware::control::command_gate

0 commit comments

Comments
 (0)