Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print the state of all configurable settings after parsing motoros2_config.yaml #81

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 36 additions & 23 deletions src/ConfigFile.c
Original file line number Diff line number Diff line change
Expand Up @@ -241,12 +241,10 @@ void Ros_ConfigFile_CheckYamlEvent(yaml_event_t* event)
{
case Value_String:
strcpy((char*)activeItem->valueToSet, (char*)event->data.scalar.value);
Ros_Debug_BroadcastMsg("Config: %s = %s", (char*)activeItem->yamlKey, (char*)activeItem->valueToSet);
break;

case Value_Int:
*(int*)activeItem->valueToSet = atoi((char*)event->data.scalar.value);
Ros_Debug_BroadcastMsg("Config: %s = %d", (char*)activeItem->yamlKey, *(int*)activeItem->valueToSet);
break;

case Value_Bool:
Expand Down Expand Up @@ -275,16 +273,11 @@ void Ros_ConfigFile_CheckYamlEvent(yaml_event_t* event)
mpSetAlarm(ALARM_CONFIGURATION_FAIL, "Invalid BOOL in motoros2_config", SUBCODE_CONFIGURATION_INVALID_BOOLEAN_VALUE);

Ros_Debug_BroadcastMsg("'%s' is not a valid boolean specifier", (char*)event->data.scalar.value);
Ros_Debug_BroadcastMsg("Config: %s left at default value", (char*)activeItem->yamlKey);
}
else
Ros_Debug_BroadcastMsg("Config: %s = %d", (char*)activeItem->yamlKey, *(BOOL*)activeItem->valueToSet);

break;

case Value_JointNameArray:
strncpy(*(char**)activeItem->valueToSet, (char*)event->data.scalar.value, MAX_YAML_STRING_LEN);
Ros_Debug_BroadcastMsg("Config: %s = %s", (char*)activeItem->yamlKey, *(char**)activeItem->valueToSet);
//Ros_Debug_BroadcastMsg(" - joint name saved at 0x%X (iter = 0x%X)", (int)*(char**)activeItem->valueToSet, (int)joint_names_iterator);
joint_names_iterator += MAX_JOINT_NAME_LENGTH;
jointIteratorCountdownPerGroup -= 1;
Expand All @@ -306,8 +299,6 @@ void Ros_ConfigFile_CheckYamlEvent(yaml_event_t* event)
(char*)activeItem->yamlKey,
(char*)event->data.scalar.value);
}

Ros_Debug_BroadcastMsg("Config: %s = %s", (char*)activeItem->yamlKey, (char*)event->data.scalar.value);
break;

case Value_UserLanPort:
Expand Down Expand Up @@ -376,20 +367,6 @@ void Ros_ConfigFile_CheckYamlEvent(yaml_event_t* event)
if (nestedListCounter == 0) //all sub-lists in [joint_names] have been processed
{
activeItem = NULL;

Ros_Debug_BroadcastMsg("List of configured joint names:");
for (int i = 0; i < MAX_CONTROLLABLE_GROUPS; i += 1)
{
Ros_Debug_BroadcastMsg("---");
for (int j = 0; j < MP_GRP_AXES_NUM; j += 1)
{
if (strlen(g_nodeConfigSettings.joint_names[(i * MP_GRP_AXES_NUM) + j]) > 0)
Ros_Debug_BroadcastMsg(g_nodeConfigSettings.joint_names[(i * MP_GRP_AXES_NUM) + j]);
else
Ros_Debug_BroadcastMsg("x");
}
}
Ros_Debug_BroadcastMsg("---");
}
}
}
Expand Down Expand Up @@ -780,6 +757,42 @@ void Ros_ConfigFile_Parse()

Ros_ConfigFile_ValidateCriticalSettings();
Ros_ConfigFile_ValidateNonCriticalSettings();

Ros_Debug_BroadcastMsg("Config: ros_domain_id = %d", g_nodeConfigSettings.ros_domain_id);
Ros_Debug_BroadcastMsg("Config: node_name = %s", g_nodeConfigSettings.node_name);
Ros_Debug_BroadcastMsg("Config: node_namespace = %s", g_nodeConfigSettings.node_namespace);
Ros_Debug_BroadcastMsg("Config: remap_rules = %s", g_nodeConfigSettings.remap_rules);
Ros_Debug_BroadcastMsg("Config: agent_ip_address = %s", g_nodeConfigSettings.agent_ip_address);
Ros_Debug_BroadcastMsg("Config: agent_port_number = %s", g_nodeConfigSettings.agent_port_number);
Ros_Debug_BroadcastMsg("Config: sync_timeclock_with_agent = %d", g_nodeConfigSettings.sync_timeclock_with_agent);
Ros_Debug_BroadcastMsg("Config: namespace_tf = %d", g_nodeConfigSettings.namespace_tf);
Ros_Debug_BroadcastMsg("Config: publish_tf = %d", g_nodeConfigSettings.publish_tf);
Ros_Debug_BroadcastMsg("List of configured joint names:");
for (int i = 0; i < MAX_CONTROLLABLE_GROUPS; i += 1)
{
Ros_Debug_BroadcastMsg("---");
for (int j = 0; j < MP_GRP_AXES_NUM; j += 1)
{
if (strlen(g_nodeConfigSettings.joint_names[(i * MP_GRP_AXES_NUM) + j]) > 0)
Ros_Debug_BroadcastMsg(g_nodeConfigSettings.joint_names[(i * MP_GRP_AXES_NUM) + j]);
else
Ros_Debug_BroadcastMsg("x");
}
}
Ros_Debug_BroadcastMsg("---");
Ros_Debug_BroadcastMsg("Config: log_to_stdout = %d", g_nodeConfigSettings.log_to_stdout);
Ros_Debug_BroadcastMsg("Config: executor_sleep_period = %d", g_nodeConfigSettings.executor_sleep_period);
Ros_Debug_BroadcastMsg("Config: action_feedback_publisher_period = %d", g_nodeConfigSettings.action_feedback_publisher_period);
Ros_Debug_BroadcastMsg("Config: controller_status_monitor_period = %d", g_nodeConfigSettings.controller_status_monitor_period);
Ros_Debug_BroadcastMsg("Config: robot_status = %d", g_nodeConfigSettings.qos_robot_status);
Ros_Debug_BroadcastMsg("Config: joint_states = %d", g_nodeConfigSettings.qos_joint_states);
Ros_Debug_BroadcastMsg("Config: tf = %d", g_nodeConfigSettings.qos_tf);
Ros_Debug_BroadcastMsg("Config: tf_frame_prefix = %s", g_nodeConfigSettings.tf_frame_prefix);
Ros_Debug_BroadcastMsg("Config: stop_motion_on_disconnect = %d", g_nodeConfigSettings.stop_motion_on_disconnect);
Ros_Debug_BroadcastMsg("Config: inform_job_name = %s", g_nodeConfigSettings.inform_job_name);
Ros_Debug_BroadcastMsg("Config: allow_custom_inform_job = %d", g_nodeConfigSettings.allow_custom_inform_job);
Ros_Debug_BroadcastMsg("Config: userlan_monitor_enabled = %d", g_nodeConfigSettings.userlan_monitor_enabled);
Ros_Debug_BroadcastMsg("Config: userlan_monitor_port = %d", g_nodeConfigSettings.userlan_monitor_port);
Comment on lines +761 to +795
Copy link
Collaborator

@gavanderhoorn gavanderhoorn Jul 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move this to its own function instead?

One of the earlier commits in this PR seems to have it had structured that way, but that was changed to this version instead. Was there a particular reason for that?

Perhaps something like Ros_ConfigFile_DumpSettings(Ros_Configuration_Settings const* const settings)?

Additionally: is the idea we keep adding lines to this? It would have been ideal if somehow we could generalise the printing a bit and not rely on the developer to remember having to extend these prints.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did try to make this into a function and then call on it but it still wasn't printing out the values of all the elements of g_nodeConfigSettings so I tried this as an an alternate solution.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would have been ideal if somehow we could generalise the printing a bit and not rely on the developer to remember having to extend these prints.

I don't know how you would do that in C and still make it human-readable.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can think of some ways to do that, but none of them are necessarily easy, nor elegant.

We can revisit this later.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep things connected: #84.

}

rmw_qos_profile_t const* const Ros_ConfigFile_To_Rmw_Qos_Profile(Ros_QoS_Profile_Setting val)
Expand Down