Skip to content

Commit

Permalink
renamed to internal state for better semantic meaning
Browse files Browse the repository at this point in the history
  • Loading branch information
saikishor committed Aug 2, 2023
1 parent ca344f1 commit 7f7619b
Show file tree
Hide file tree
Showing 14 changed files with 157 additions and 153 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class ChainableControllerInterface : public ControllerInterfaceBase
bool is_chainable() const final;

CONTROLLER_INTERFACE_PUBLIC
std::vector<hardware_interface::StateInterface> export_state_interfaces() final;
std::vector<hardware_interface::StateInterface> export_internal_state_interfaces() final;

CONTROLLER_INTERFACE_PUBLIC
std::vector<hardware_interface::CommandInterface> export_reference_interfaces() final;
Expand All @@ -75,12 +75,12 @@ class ChainableControllerInterface : public ControllerInterfaceBase
/// chainable interfaces.
/**
* Each chainable controller implements this methods where all its state(read only) interfaces are
* exported. The method has the same meaning as `export_state_interfaces` method from
* exported. The method has the same meaning as `export_internal_state_interfaces` method from
* hardware_interface::SystemInterface or hardware_interface::ActuatorInterface.
*
* \returns list of StateInterfaces that other controller can use as their outputs.
*/
virtual std::vector<hardware_interface::StateInterface> on_export_state_interfaces() = 0;
virtual std::vector<hardware_interface::StateInterface> on_export_internal_state_interfaces() = 0;

/// Virtual method that each chainable controller should implement to export its read/write
/// chainable interfaces.
Expand Down Expand Up @@ -131,8 +131,8 @@ class ChainableControllerInterface : public ControllerInterfaceBase
virtual return_type update_and_write_commands(
const rclcpp::Time & time, const rclcpp::Duration & period) = 0;

/// Storage of values for exported state interfaces
std::vector<double> exported_state_interfaces_data_;
/// Storage of values for internal_state interfaces
std::vector<double> internal_state_interfaces_data_;

/// Storage of values for reference interfaces
std::vector<double> reference_interfaces_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class ControllerInterface : public controller_interface::ControllerInterfaceBase
* \returns empty list.
*/
CONTROLLER_INTERFACE_PUBLIC
std::vector<hardware_interface::StateInterface> export_state_interfaces() final;
std::vector<hardware_interface::StateInterface> export_internal_state_interfaces() final;

/**
* Controller has no reference interfaces.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,12 @@ class ControllerInterfaceBase : public rclcpp_lifecycle::node_interfaces::Lifecy
* \returns list of state interfaces for preceding controllers.
*/
CONTROLLER_INTERFACE_PUBLIC
virtual std::vector<hardware_interface::StateInterface> export_state_interfaces() = 0;
virtual std::vector<hardware_interface::StateInterface> export_internal_state_interfaces() = 0;

/**
* Set chained mode of a chainable controller. This method triggers internal processes to switch
* a chainable controller to "chained" mode and vice-versa. Setting controller to "chained" mode
* usually involves the usage of the controller's reference/state interfaces by the other
* usually involves the usage of the controller's reference/internal state interfaces by the other
* controllers
*
* \returns true if mode is switched successfully and false if not.
Expand Down
29 changes: 15 additions & 14 deletions controller_interface/src/chainable_controller_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,41 +45,42 @@ return_type ChainableControllerInterface::update(
}

std::vector<hardware_interface::StateInterface>
ChainableControllerInterface::export_state_interfaces()
ChainableControllerInterface::export_internal_state_interfaces()
{
auto state_interfaces = on_export_state_interfaces();
// check if the "state_interfaces_data_" variable is resized to number of interfaces
if (exported_state_interfaces_data_.size() != state_interfaces.size())
auto internal_state_interfaces = on_export_internal_state_interfaces();
// check if the "internal_state_interfaces_data_" variable is resized to number of interfaces
if (internal_state_interfaces_data_.size() != internal_state_interfaces.size())
{
// TODO(destogl): Should here be "FATAL"? It is fatal in terms of controller but not for the
// framework
RCLCPP_FATAL(
get_node()->get_logger(),
"The internal storage for exported state values 'state_interfaces_data_' variable has size "
"'%zu', but it is expected to have the size '%zu' equal to the number of exported state "
"interfaces. No state interface will be exported. Please correct and recompile "
"The internal storage for internal state values 'internal_state_interfaces_data_' variable "
"has size '%zu', but it is expected to have the size '%zu' equal to the number of internal "
"state interfaces. No state interface will be exported. Please correct and recompile "
"the controller with name '%s' and try again.",
exported_state_interfaces_data_.size(), state_interfaces.size(), get_node()->get_name());
state_interfaces.clear();
internal_state_interfaces_data_.size(), internal_state_interfaces.size(),
get_node()->get_name());
internal_state_interfaces.clear();
}

// check if the names of the controller state interfaces begin with the controller's name
for (const auto & interface : state_interfaces)
for (const auto & interface : internal_state_interfaces)
{
if (interface.get_prefix_name() != get_node()->get_name())
{
RCLCPP_FATAL(
get_node()->get_logger(),
"The name of the interface '%s' does not begin with the controller's name. This is "
"mandatory for state interfaces. No state interface will be exported. Please "
"correct and recompile the controller with name '%s' and try again.",
"mandatory for internal state interfaces. No internal state interface will be exported. "
"Please correct and recompile the controller with name '%s' and try again.",
interface.get_name().c_str(), get_node()->get_name());
state_interfaces.clear();
internal_state_interfaces.clear();
break;
}
}

return state_interfaces;
return internal_state_interfaces;
}

std::vector<hardware_interface::CommandInterface>
Expand Down
3 changes: 2 additions & 1 deletion controller_interface/src/controller_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ ControllerInterface::ControllerInterface() : ControllerInterfaceBase() {}

bool ControllerInterface::is_chainable() const { return false; }

std::vector<hardware_interface::StateInterface> ControllerInterface::export_state_interfaces()
std::vector<hardware_interface::StateInterface>
ControllerInterface::export_internal_state_interfaces()
{
return {};
}
Expand Down
40 changes: 20 additions & 20 deletions controller_interface/test/test_chainable_controller_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,21 +28,21 @@ TEST_F(ChainableControllerInterfaceTest, default_returns)
EXPECT_FALSE(controller.is_in_chained_mode());
}

TEST_F(ChainableControllerInterfaceTest, export_state_interfaces)
TEST_F(ChainableControllerInterfaceTest, export_internal_state_interfaces)
{
TestableChainableControllerInterface controller;

// initialize, create node
ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME), controller_interface::return_type::OK);
ASSERT_NO_THROW(controller.get_node());

auto exported_state_interfaces = controller.export_state_interfaces();
auto internal_state_interfaces = controller.export_internal_state_interfaces();

ASSERT_EQ(exported_state_interfaces.size(), 1u);
EXPECT_EQ(exported_state_interfaces[0].get_prefix_name(), TEST_CONTROLLER_NAME);
EXPECT_EQ(exported_state_interfaces[0].get_interface_name(), "test_state");
ASSERT_EQ(internal_state_interfaces.size(), 1u);
EXPECT_EQ(internal_state_interfaces[0].get_prefix_name(), TEST_CONTROLLER_NAME);
EXPECT_EQ(internal_state_interfaces[0].get_interface_name(), "test_state");

EXPECT_EQ(exported_state_interfaces[0].get_value(), EXPORTED_STATE_INTERFACE_VALUE);
EXPECT_EQ(internal_state_interfaces[0].get_value(), INTERNAL_STATE_INTERFACE_VALUE);
}

TEST_F(ChainableControllerInterfaceTest, export_reference_interfaces)
Expand Down Expand Up @@ -75,9 +75,9 @@ TEST_F(ChainableControllerInterfaceTest, interfaces_storage_not_correct_size)
auto reference_interfaces = controller.export_reference_interfaces();
ASSERT_TRUE(reference_interfaces.empty());
// expect empty return because storage is not resized
controller.exported_state_interfaces_data_.clear();
auto exported_state_interfaces = controller.export_state_interfaces();
ASSERT_TRUE(exported_state_interfaces.empty());
controller.internal_state_interfaces_data_.clear();
auto internal_state_interfaces = controller.export_internal_state_interfaces();
ASSERT_TRUE(internal_state_interfaces.empty());
}

TEST_F(ChainableControllerInterfaceTest, interfaces_prefix_is_not_node_name)
Expand All @@ -94,8 +94,8 @@ TEST_F(ChainableControllerInterfaceTest, interfaces_prefix_is_not_node_name)
auto reference_interfaces = controller.export_reference_interfaces();
ASSERT_TRUE(reference_interfaces.empty());
// expect empty return because interface prefix is not equal to the node name
auto exported_state_interfaces = controller.export_state_interfaces();
ASSERT_TRUE(exported_state_interfaces.empty());
auto internal_state_interfaces = controller.export_internal_state_interfaces();
ASSERT_TRUE(internal_state_interfaces.empty());
}

TEST_F(ChainableControllerInterfaceTest, setting_chained_mode)
Expand All @@ -108,14 +108,14 @@ TEST_F(ChainableControllerInterfaceTest, setting_chained_mode)

auto reference_interfaces = controller.export_reference_interfaces();
ASSERT_EQ(reference_interfaces.size(), 1u);
auto exported_state_interfaces = controller.export_state_interfaces();
ASSERT_EQ(exported_state_interfaces.size(), 1u);
auto internal_state_interfaces = controller.export_internal_state_interfaces();
ASSERT_EQ(internal_state_interfaces.size(), 1u);

EXPECT_FALSE(controller.is_in_chained_mode());

// Fail setting chained mode
EXPECT_EQ(reference_interfaces[0].get_value(), INTERFACE_VALUE);
EXPECT_EQ(exported_state_interfaces[0].get_value(), EXPORTED_STATE_INTERFACE_VALUE);
EXPECT_EQ(internal_state_interfaces[0].get_value(), INTERNAL_STATE_INTERFACE_VALUE);

EXPECT_FALSE(controller.set_chained_mode(true));
EXPECT_FALSE(controller.is_in_chained_mode());
Expand All @@ -128,7 +128,7 @@ TEST_F(ChainableControllerInterfaceTest, setting_chained_mode)

EXPECT_TRUE(controller.set_chained_mode(true));
EXPECT_TRUE(controller.is_in_chained_mode());
EXPECT_EQ(exported_state_interfaces[0].get_value(), EXPORTED_STATE_INTERFACE_VALUE_IN_CHAINMODE);
EXPECT_EQ(internal_state_interfaces[0].get_value(), INTERNAL_STATE_INTERFACE_VALUE_IN_CHAINMODE);

controller.configure();
EXPECT_TRUE(controller.set_chained_mode(false));
Expand Down Expand Up @@ -165,23 +165,23 @@ TEST_F(ChainableControllerInterfaceTest, test_update_logic)
controller.update(rclcpp::Time(0), rclcpp::Duration::from_seconds(0.01)),
controller_interface::return_type::OK);
ASSERT_EQ(controller.reference_interfaces_[0], INTERFACE_VALUE_INITIAL_REF - 1);
ASSERT_EQ(controller.exported_state_interfaces_data_[0], EXPORTED_STATE_INTERFACE_VALUE + 1);
ASSERT_EQ(controller.internal_state_interfaces_data_[0], INTERNAL_STATE_INTERFACE_VALUE + 1);

// Provoke error in update from subscribers - return ERROR and update_and_write_commands not exec.
controller.set_new_reference_interface_value(INTERFACE_VALUE_SUBSCRIBER_ERROR);
ASSERT_EQ(
controller.update(rclcpp::Time(0), rclcpp::Duration::from_seconds(0.01)),
controller_interface::return_type::ERROR);
ASSERT_EQ(controller.reference_interfaces_[0], INTERFACE_VALUE_INITIAL_REF - 1);
ASSERT_EQ(controller.exported_state_interfaces_data_[0], EXPORTED_STATE_INTERFACE_VALUE + 1);
ASSERT_EQ(controller.internal_state_interfaces_data_[0], INTERNAL_STATE_INTERFACE_VALUE + 1);

// Provoke error from update - return ERROR, but reference interface is updated and not reduced
controller.set_new_reference_interface_value(INTERFACE_VALUE_UPDATE_ERROR);
ASSERT_EQ(
controller.update(rclcpp::Time(0), rclcpp::Duration::from_seconds(0.01)),
controller_interface::return_type::ERROR);
ASSERT_EQ(controller.reference_interfaces_[0], INTERFACE_VALUE_UPDATE_ERROR);
ASSERT_EQ(controller.exported_state_interfaces_data_[0], EXPORTED_STATE_INTERFACE_VALUE + 1);
ASSERT_EQ(controller.internal_state_interfaces_data_[0], INTERNAL_STATE_INTERFACE_VALUE + 1);

controller.reference_interfaces_[0] = 0.0;

Expand All @@ -197,7 +197,7 @@ TEST_F(ChainableControllerInterfaceTest, test_update_logic)
controller_interface::return_type::OK);
ASSERT_EQ(controller.reference_interfaces_[0], -1.0);
ASSERT_EQ(
controller.exported_state_interfaces_data_[0], EXPORTED_STATE_INTERFACE_VALUE_IN_CHAINMODE + 1);
controller.internal_state_interfaces_data_[0], INTERNAL_STATE_INTERFACE_VALUE_IN_CHAINMODE + 1);

// Provoke error from update - return ERROR, but reference interface is updated directly
controller.set_new_reference_interface_value(INTERFACE_VALUE_SUBSCRIBER_ERROR);
Expand All @@ -207,5 +207,5 @@ TEST_F(ChainableControllerInterfaceTest, test_update_logic)
controller_interface::return_type::ERROR);
ASSERT_EQ(controller.reference_interfaces_[0], INTERFACE_VALUE_UPDATE_ERROR);
ASSERT_EQ(
controller.exported_state_interfaces_data_[0], EXPORTED_STATE_INTERFACE_VALUE_IN_CHAINMODE + 1);
controller.internal_state_interfaces_data_[0], INTERNAL_STATE_INTERFACE_VALUE_IN_CHAINMODE + 1);
}
22 changes: 11 additions & 11 deletions controller_interface/test/test_chainable_controller_interface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ constexpr double INTERFACE_VALUE = 1989.0;
constexpr double INTERFACE_VALUE_SUBSCRIBER_ERROR = 12345.0;
constexpr double INTERFACE_VALUE_UPDATE_ERROR = 67890.0;
constexpr double INTERFACE_VALUE_INITIAL_REF = 1984.0;
constexpr double EXPORTED_STATE_INTERFACE_VALUE = 21833.0;
constexpr double EXPORTED_STATE_INTERFACE_VALUE_IN_CHAINMODE = 82802.0;
constexpr double INTERNAL_STATE_INTERFACE_VALUE = 21833.0;
constexpr double INTERNAL_STATE_INTERFACE_VALUE_IN_CHAINMODE = 82802.0;

class TestableChainableControllerInterface
: public controller_interface::ChainableControllerInterface
Expand All @@ -43,8 +43,8 @@ class TestableChainableControllerInterface
{
reference_interfaces_.reserve(1);
reference_interfaces_.push_back(INTERFACE_VALUE);
exported_state_interfaces_data_.reserve(1);
exported_state_interfaces_data_.push_back(EXPORTED_STATE_INTERFACE_VALUE);
internal_state_interfaces_data_.reserve(1);
internal_state_interfaces_data_.push_back(INTERNAL_STATE_INTERFACE_VALUE);
}

controller_interface::CallbackReturn on_init() override
Expand All @@ -68,14 +68,14 @@ class TestableChainableControllerInterface
}

// Implementation of ChainableController virtual methods
std::vector<hardware_interface::StateInterface> on_export_state_interfaces() override
std::vector<hardware_interface::StateInterface> on_export_internal_state_interfaces() override
{
std::vector<hardware_interface::StateInterface> state_interfaces;
std::vector<hardware_interface::StateInterface> internal_state_interfaces;

state_interfaces.push_back(hardware_interface::StateInterface(
name_prefix_of_interfaces_, "test_state", &exported_state_interfaces_data_[0]));
internal_state_interfaces.push_back(hardware_interface::StateInterface(
name_prefix_of_interfaces_, "test_state", &internal_state_interfaces_data_[0]));

return state_interfaces;
return internal_state_interfaces;
}

// Implementation of ChainableController virtual methods
Expand All @@ -93,7 +93,7 @@ class TestableChainableControllerInterface
{
if (reference_interfaces_[0] == 0.0)
{
exported_state_interfaces_data_[0] = EXPORTED_STATE_INTERFACE_VALUE_IN_CHAINMODE;
internal_state_interfaces_data_[0] = INTERNAL_STATE_INTERFACE_VALUE_IN_CHAINMODE;
return true;
}
else
Expand Down Expand Up @@ -123,7 +123,7 @@ class TestableChainableControllerInterface
}

reference_interfaces_[0] -= 1;
exported_state_interfaces_data_[0] += 1;
internal_state_interfaces_data_[0] += 1;

return controller_interface::return_type::OK;
}
Expand Down
Loading

0 comments on commit 7f7619b

Please sign in to comment.