Skip to content

Commit

Permalink
Improve error messages in rcl_lifecycle (ros2#742)
Browse files Browse the repository at this point in the history 
* Improve error messages in rcl_lifecycle

Signed-off-by: Lei Liu <[email protected]>

* Add the detail code.

Signed-off-by: Lei Liu <[email protected]>

* Update code with RCL_CHECK_FOR_NULL_WITH_MSG.

Signed-off-by: Lei Liu <[email protected]>

* Update code as suggested.

Signed-off-by: Lei Liu <[email protected]>

* Update code as suggested.

Signed-off-by: Lei Liu <[email protected]>

* remove no need returned values.

Signed-off-by: Lei Liu <[email protected]>
  • Loading branch information
@llapx @Barry-Xu-2018
llapx authored and Barry-Xu-2018 committed Oct 29, 2020
1 parent e4e8a82 commit ece87b1
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 149 deletions.
6 changes: 6 additions & 0 deletions rcl/include/rcl/types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,12 @@ typedef rmw_ret_t rcl_ret_t;
/// Failed to take an event from the event handle
#define RCL_RET_EVENT_TAKE_FAILED 2001

/// rcl_lifecycle state register ret codes in 30XX
/// rcl_lifecycle state registered
#define RCL_RET_LIFECYCLE_STATE_REGISTERED 3000
/// rcl_lifecycle state not registered
#define RCL_RET_LIFECYCLE_STATE_NOT_REGISTERED 3001

/// typedef for rmw_serialized_message_t;
typedef rmw_serialized_message_t rcl_serialized_message_t;

Expand Down
11 changes: 9 additions & 2 deletions rcl_lifecycle/include/rcl_lifecycle/rcl_lifecycle.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ rcl_lifecycle_get_zero_initialized_state();
* \param[in] label label of the state
* \param[in] allocator a valid allocator used to initialized the lifecycle state
* \return `RCL_RET_OK` if state was initialized successfully, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
*/
RCL_LIFECYCLE_PUBLIC
Expand Down Expand Up @@ -106,7 +107,7 @@ rcl_lifecycle_state_init(
* \param[inout] state struct to be finalized
* \param[in] allocator a valid allocator used to finalize the lifecycle state
* \return `RCL_RET_OK` if the state was finalized successfully, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid.
*/
RCL_LIFECYCLE_PUBLIC
RCL_WARN_UNUSED
Expand Down Expand Up @@ -152,6 +153,7 @@ rcl_lifecycle_get_zero_initialized_transition();
* \param[in] goal the objetive of the transition
* \param[in] allocator a valid allocator used to finalize the lifecycle state
* \return `RCL_RET_OK` if the transition is initialized successfully, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
*/
RCL_LIFECYCLE_PUBLIC
Expand Down Expand Up @@ -182,6 +184,7 @@ rcl_lifecycle_transition_init(
* \param[inout] transition struct to be finalized
* \param[in] allocator a valid allocator used to finalize the transition
* \return `RCL_RET_OK` if the state was finalized successfully, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
*/
RCL_LIFECYCLE_PUBLIC
Expand Down Expand Up @@ -229,6 +232,7 @@ rcl_lifecycle_get_zero_initialized_state_machine();
* the state_machine pointer is only used to initialize the interfaces
* \param[in] allocator a valid allocator used to initialized the state machine
* \return `RCL_RET_OK` if the state machine was initialized successfully, or
* \return `RCL_RET_INVALID_ARGUMENT` if input params is NULL, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
*/
RCL_LIFECYCLE_PUBLIC
Expand Down Expand Up @@ -264,6 +268,7 @@ rcl_lifecycle_state_machine_init(
* \param[in] node_handle valid (not finalized) handle to the node
* \param[in] allocator a valid allocator used to finalize the state machine
* \return `RCL_RET_OK` if the state was finalized successfully, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
*/
RCL_LIFECYCLE_PUBLIC
Expand All @@ -290,7 +295,7 @@ rcl_lifecycle_state_machine_fini(
*
* \param[in] state_machine pointer to the state machine struct
* \return `RCL_RET_OK` if the state is initialized, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid.
*/
RCL_LIFECYCLE_PUBLIC
RCL_WARN_UNUSED
Expand Down Expand Up @@ -365,6 +370,7 @@ rcl_lifecycle_get_transition_by_label(
* \param[in] publish_notification if the value is `true` a message will be published
* notifying the transition, otherwise no message will be published
* \return `RCL_RET_OK` if the transition was triggered successfully, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
*/
RCL_LIFECYCLE_PUBLIC
Expand Down Expand Up @@ -394,6 +400,7 @@ rcl_lifecycle_trigger_transition_by_id(
* \param[in] publish_notification if the value is `true` a message will be published
* notifying the transition, otherwise no message will be published
* \return `RCL_RET_OK` if the transition was triggered successfully, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
*/
RCL_LIFECYCLE_PUBLIC
Expand Down
10 changes: 7 additions & 3 deletions rcl_lifecycle/include/rcl_lifecycle/transition_map.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ rcl_lifecycle_get_zero_initialized_transition_map();
*
* \param[in] transition_map pointer to the transition map struct to check
* \return `RCL_RET_OK` if the transition map is initialized successfully, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_ERROR` if the transition map is not initialized.
*/
RCL_LIFECYCLE_PUBLIC
Expand All @@ -78,6 +79,7 @@ rcl_lifecycle_transition_map_is_initialized(
* \param[inout] transition_map struct to be deinitialized
* \param[in] allocator a valid allocator used to deinitialized the state machine
* \return `RCL_RET_OK` if the state was deinitialized successfully, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
*/
RCL_LIFECYCLE_PUBLIC
Expand All @@ -99,18 +101,18 @@ rcl_lifecycle_transition_map_fini(
* Uses Atomics | No
* Lock-Free | Yes
*
* \param[in] map to be modified
* \param[in] transition_map to be modified
* \param[in] state the state to register
* \param[in] allocator a valid allocator used to register the state machine
* \return `RCL_RET_OK` if the state was registered successfully, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
* \return `RCL_RET_LIFECYCLE_STATE_REGISTERED` if state is already registered.
*/
RCL_LIFECYCLE_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
rcl_lifecycle_register_state(
rcl_lifecycle_transition_map_t * map,
rcl_lifecycle_transition_map_t * transition_map,
rcl_lifecycle_state_t state,
const rcl_allocator_t * allocator);

Expand All @@ -131,6 +133,8 @@ rcl_lifecycle_register_state(
* \param[in] allocator a valid allocator used to register the state machine
* \return `RCL_RET_OK` if the state was deinitialized successfully, or
* \return `RCL_RET_BAD_ALLOC` if allocating memory failed, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_LIFECYCLE_STATE_NOT_REGISTERED` if state is not registered, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
*/
RCL_LIFECYCLE_PUBLIC
Expand Down
136 changes: 53 additions & 83 deletions rcl_lifecycle/src/rcl_lifecycle.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,25 +53,19 @@ rcl_lifecycle_state_init(
const char * label,
const rcl_allocator_t * allocator)
{
if (!allocator) {
RCL_SET_ERROR_MSG("can't initialize state, no allocator given\n");
return RCL_RET_ERROR;
}
if (!state) {
RCL_SET_ERROR_MSG("state pointer is null\n");
return RCL_RET_ERROR;
}
if (!label) {
RCL_SET_ERROR_MSG("State label is null\n");
return RCL_RET_ERROR;
}
RCL_CHECK_ALLOCATOR_WITH_MSG(
allocator, "can't initialize state, no allocator given\n", return RCL_RET_INVALID_ARGUMENT);

RCL_CHECK_FOR_NULL_WITH_MSG(
state, "state pointer is null\n", return RCL_RET_INVALID_ARGUMENT);

RCL_CHECK_FOR_NULL_WITH_MSG(
label, "State label is null\n", return RCL_RET_INVALID_ARGUMENT);

state->id = id;
state->label = rcutils_strndup(label, strlen(label), *allocator);
if (!state->label) {
RCL_SET_ERROR_MSG("failed to duplicate label for rcl_lifecycle_state_t\n");
return RCL_RET_ERROR;
}
RCL_CHECK_FOR_NULL_WITH_MSG(
state->label, "failed to duplicate label for rcl_lifecycle_state_t\n", return RCL_RET_ERROR);

return RCL_RET_OK;
}
Expand All @@ -81,12 +75,10 @@ rcl_lifecycle_state_fini(
rcl_lifecycle_state_t * state,
const rcl_allocator_t * allocator)
{
RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCL_RET_ERROR);
RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);

if (!allocator) {
RCL_SET_ERROR_MSG("can't free state, no allocator given\n");
return RCL_RET_ERROR;
}
RCL_CHECK_ALLOCATOR_WITH_MSG(
allocator, "can't free state, no allocator given\n", return RCL_RET_INVALID_ARGUMENT);
// it is already NULL
if (!state) {
return RCL_RET_OK;
Expand Down Expand Up @@ -120,30 +112,24 @@ rcl_lifecycle_transition_init(
rcl_lifecycle_state_t * goal,
const rcl_allocator_t * allocator)
{
if (!allocator) {
RCL_SET_ERROR_MSG("can't initialize transition, no allocator given\n");
return RCL_RET_ERROR;
}
RCL_CHECK_ALLOCATOR_WITH_MSG(
allocator, "can't initialize transition, no allocator given\n",
return RCL_RET_INVALID_ARGUMENT);

if (!transition) {
RCL_SET_ERROR_MSG("transition pointer is null\n");
return RCL_RET_ERROR;
}
RCL_CHECK_FOR_NULL_WITH_MSG(
transition, "transition pointer is null\n", return RCL_RET_INVALID_ARGUMENT);

if (!label) {
RCL_SET_ERROR_MSG("label pointer is null\n");
return RCL_RET_ERROR;
}
RCL_CHECK_FOR_NULL_WITH_MSG(
label, "label pointer is null\n", return RCL_RET_INVALID_ARGUMENT);

transition->start = start;
transition->goal = goal;

transition->id = id;
transition->label = rcutils_strndup(label, strlen(label), *allocator);
if (!transition->label) {
RCL_SET_ERROR_MSG("failed to duplicate label for rcl_lifecycle_transition_t\n");
return RCL_RET_ERROR;
}
RCL_CHECK_FOR_NULL_WITH_MSG(
transition->label, "failed to duplicate label for rcl_lifecycle_transition_t\n",
return RCL_RET_ERROR);

return RCL_RET_OK;
}
Expand All @@ -153,10 +139,8 @@ rcl_lifecycle_transition_fini(
rcl_lifecycle_transition_t * transition,
const rcl_allocator_t * allocator)
{
if (!allocator) {
RCL_SET_ERROR_MSG("can't finalize transition, no allocator given\n");
return RCL_RET_ERROR;
}
RCL_CHECK_ALLOCATOR_WITH_MSG(
allocator, "can't finalize transition, no allocator given\n", return RCL_RET_INVALID_ARGUMENT);
// it is already NULL
if (!transition) {
return RCL_RET_OK;
Expand Down Expand Up @@ -206,18 +190,15 @@ rcl_lifecycle_state_machine_init(
bool default_states,
const rcl_allocator_t * allocator)
{
if (!state_machine) {
RCL_SET_ERROR_MSG("State machine is null\n");
return RCL_RET_ERROR;
}
if (!node_handle) {
RCL_SET_ERROR_MSG("Node handle is null\n");
return RCL_RET_ERROR;
}
if (!allocator) {
RCL_SET_ERROR_MSG("can't initialize state machine, no allocator given\n");
return RCL_RET_ERROR;
}
RCL_CHECK_FOR_NULL_WITH_MSG(
state_machine, "State machine is null\n", return RCL_RET_INVALID_ARGUMENT);

RCL_CHECK_FOR_NULL_WITH_MSG(
node_handle, "Node handle is null\n", return RCL_RET_INVALID_ARGUMENT);

RCL_CHECK_ALLOCATOR_WITH_MSG(
allocator, "can't initialize state machine, no allocator given\n",
return RCL_RET_INVALID_ARGUMENT);

rcl_ret_t ret = rcl_lifecycle_com_interface_init(
&state_machine->com_interface, node_handle,
Expand Down Expand Up @@ -247,10 +228,8 @@ rcl_lifecycle_state_machine_fini(
rcl_node_t * node_handle,
const rcl_allocator_t * allocator)
{
if (!allocator) {
RCL_SET_ERROR_MSG("can't free state machine, no allocator given\n");
return RCL_RET_ERROR;
}
RCL_CHECK_ALLOCATOR_WITH_MSG(
allocator, "can't free state machine, no allocator given\n", return RCL_RET_INVALID_ARGUMENT);

rcl_ret_t fcn_ret = RCL_RET_OK;

Expand Down Expand Up @@ -278,17 +257,17 @@ rcl_lifecycle_state_machine_fini(
rcl_ret_t
rcl_lifecycle_state_machine_is_initialized(const rcl_lifecycle_state_machine_t * state_machine)
{
if (!state_machine->com_interface.srv_get_state.impl) {
RCL_SET_ERROR_MSG("get_state service is null");
return RCL_RET_ERROR;
}
if (!state_machine->com_interface.srv_change_state.impl) {
RCL_SET_ERROR_MSG("change_state service is null");
return RCL_RET_ERROR;
}
RCL_CHECK_FOR_NULL_WITH_MSG(
state_machine->com_interface.srv_get_state.impl, "get_state service is null\n",
return RCL_RET_INVALID_ARGUMENT);

RCL_CHECK_FOR_NULL_WITH_MSG(
state_machine->com_interface.srv_change_state.impl, "change_state service is null\n",
return RCL_RET_INVALID_ARGUMENT);

if (rcl_lifecycle_transition_map_is_initialized(&state_machine->transition_map) != RCL_RET_OK) {
RCL_SET_ERROR_MSG("transition map is null");
return RCL_RET_ERROR;
return RCL_RET_INVALID_ARGUMENT;
}
return RCL_RET_OK;
}
Expand Down Expand Up @@ -342,16 +321,11 @@ _trigger_transition(
bool publish_notification)
{
// If we have a faulty transition pointer
if (!transition) {
RCL_SET_ERROR_MSG("Transition is not registered.");
return RCL_RET_ERROR;
}
RCL_CHECK_FOR_NULL_WITH_MSG(
transition, "Transition is not registered.", return RCL_RET_INVALID_ARGUMENT);

if (!transition->goal) {
RCUTILS_LOG_ERROR_NAMED(
ROS_PACKAGE_NAME, "No valid goal is set");
return RCL_RET_ERROR;
}
RCL_CHECK_FOR_NULL_WITH_MSG(
transition->goal, "No valid goal is set.", return RCL_RET_INVALID_ARGUMENT);
state_machine->current_state = transition->goal;

if (publish_notification) {
Expand All @@ -374,10 +348,8 @@ rcl_lifecycle_trigger_transition_by_id(
uint8_t id,
bool publish_notification)
{
if (!state_machine) {
RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "state machine pointer is null");
return RCL_RET_ERROR;
}
RCL_CHECK_FOR_NULL_WITH_MSG(
state_machine, "state machine pointer is null.", return RCL_RET_INVALID_ARGUMENT);

const rcl_lifecycle_transition_t * transition =
rcl_lifecycle_get_transition_by_id(state_machine->current_state, id);
Expand All @@ -391,10 +363,8 @@ rcl_lifecycle_trigger_transition_by_label(
const char * label,
bool publish_notification)
{
if (!state_machine) {
RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "state machine pointer is null");
return RCL_RET_ERROR;
}
RCL_CHECK_FOR_NULL_WITH_MSG(
state_machine, "state machine pointer is null.", return RCL_RET_INVALID_ARGUMENT);

const rcl_lifecycle_transition_t * transition =
rcl_lifecycle_get_transition_by_label(state_machine->current_state, label);
Expand Down
Loading

0 comments on commit ece87b1

Please sign in to comment.