Refactor printing of current configuration settings #84

SejalBehere · 2023-07-10T20:14:47Z

For Fixes #33 this PR inserts the function Ros_ConfigFile_PrintActiveConfiguration() and is used to print the current configuration settings once the parsing of the configuration file is complete

…ump of g_nodeConfigSettings

gavanderhoorn

One in-line comment.

Additionally, I would suggest to prefix all keys with the enclosing namespace name (ie: instead of tf, print publisher_qos.tf).

src/ConfigFile.c

gavanderhoorn

This looks better now.

I can't test this, but if you could post a copy of the debug log lines this prints at startup just to show it's correct, we can merge.

I'd also recommend removing the trailing whitespace that was introduced.

gavanderhoorn · 2023-07-18T04:44:02Z

src/ConfigFile.c

+    Ros_Debug_BroadcastMsg("Config: publisher_qos.robot_status = %d", g_nodeConfigSettings.qos_robot_status);
+    Ros_Debug_BroadcastMsg("Config: publisher_qos.joint_states = %d", g_nodeConfigSettings.qos_joint_states);
+    Ros_Debug_BroadcastMsg("Config: publisher_qos.tf = %d", g_nodeConfigSettings.qos_tf);


minor, but these lines will now only print a nr (ie: the value of the enum member). They used to print the name of the QoS profile.

It would be nice if we could add a mapping from the enum integer to their string description again, as that would be much more human readable, but I won't block merging this PR for that (seeing as this was already accepted as part of #81.

…rints-parsed-items

src/ConfigFile.c

gavanderhoorn · 2023-07-19T09:03:56Z

@SejalBehere wrote:

Also, I am looking for a way to add a mapping from the enum integer to their string description.

I would perhaps suggest a simple switch(..), like we do here:

motoros2/src/ErrorHandling.c

Lines 11 to 33 in ab02d18

    
           //------------------------------------------------------------------- 
        
           // Convert error code to string 
        
           //------------------------------------------------------------------- 
        
           void Ros_ErrorHandling_ErrNo_ToString(int errNo, char* const errMsg, int errMsgSize) 
        
           { 
        
               switch (errNo) 
        
               { 
        
               case 0x2010: strncpy(errMsg, "Robot is in operation", errMsgSize); break; 
        
               case 0x2030: strncpy(errMsg, "In HOLD status (PP)", errMsgSize); break; 
        
               case 0x2040: strncpy(errMsg, "In HOLD status (External)", errMsgSize); break; 
        
               case 0x2050: strncpy(errMsg, "In HOLD status (Command)", errMsgSize); break; 
        
               case 0x2060: strncpy(errMsg, "In ERROR/ALARM status", errMsgSize); break; 
        
               case 0x2070: strncpy(errMsg, "In SERVO OFF status", errMsgSize); break; 
        
               case 0x2080: strncpy(errMsg, "Wrong operation mode", errMsgSize); break; 
        
               case 0x3040: strncpy(errMsg, "The home position is not registered", errMsgSize); break; 
        
               case 0x3050: strncpy(errMsg, "Out of range (ABSO data)", errMsgSize); break; 
        
               case 0x3400: strncpy(errMsg, "Cannot operate MASTER JOB", errMsgSize); break; 
        
               case 0x3410: strncpy(errMsg, "The JOB name is already registered in another task", errMsgSize); break; 
        
               case 0x4040: strncpy(errMsg, "Specified JOB not found", errMsgSize); break; 
        
               case 0x5200: strncpy(errMsg, "Over data range", errMsgSize); break; 
        
               default: strncpy(errMsg, "Unspecified reason", errMsgSize); break; 
        
               } 
        
           }

you'd have fewer values to check (there are only a couple of values in this enum), but the idea could be the same.

I might suggest not using strncpy(..) though. Returning static char[] ... might be less involved in this case (@ted-miller should be able to show you the pattern).

There could be something in either RCL or RCLC that already implements this (ie: enum value to char[] for the Qos values). That might save you some work, but I wouldn't be able to point you to it.

SejalBehere · 2023-07-19T13:41:13Z

Thank you @gavanderhoorn for the guidance. I'm working on this and will put up the modified code for review.

…strings

…th strncmp

SejalBehere · 2023-07-20T20:19:35Z

I have updated ConfigFile.c to add a lookup table for mapping from integers to strings for publisher_qos namespace. I have also refactored Ros_ConfigFile_CheckYamlEvent and used the same lookup table instead of the if/else hardcoded string. This PR is ready for another review.

I tested this on the controller and the output log displayed is as follows:

Click to expand

[1689882942.78701329] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.312 Config: ros_domain_id = 0 
[1689882942.78710938] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.312 Config: node_name = 'motoman'
[1689882942.78720188] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.312 Config: node_namespace = ''
[1689882942.78730178] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.312 Config: remap_rules = ''
[1689882942.78738475] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.312 Config: agent_ip_address = '192.168.1.50'
[1689882942.78741455] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.312 Config: agent_port_number = '8888'
[1689882942.78746367] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.312 Config: sync_timeclock_with_agent = 1
[1689882942.78754473] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 Config: namespace_tf = 1
[1689882942.78761578] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 Config: publish_tf = 1
[1689882942.78767252] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 List of configured joint names:
[1689882942.78775954] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 ---
[1689882942.78783751] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78788447] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78791237] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78794026] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78796816] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78799605] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78804374] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78808808] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78811598] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 ---
[1689882942.78816962] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78819799] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78822613] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78825402] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78829455] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78835988] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78838897] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78844595] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78847361] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 ---
[1689882942.78852773] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78855562] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78858328] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.313 x
[1689882942.78861070] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.314 x
[1689882942.78867054] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.314 x
[1689882942.78871441] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.314 x
[1689882942.78875566] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.314 x
[1689882942.78881073] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.314 x
[1689882942.78883862] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.314 ---
[1689882942.79081631] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.314 x
[1689882942.79106975] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.316 x
[1689882942.79112935] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.316 x
[1689882942.79119349] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.316 x
[1689882942.79127884] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79133821] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79136801] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79143405] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79148269] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 ---
[1689882942.79152489] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79158998] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79163289] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79167581] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79171872] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79174709] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79180717] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79183578] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79187775] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 ---
[1689882942.79193926] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79198217] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79201078] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79205251] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79209399] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79213572] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79216433] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79219246] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79223967] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 ---
[1689882942.79229856] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79234076] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.317 x
[1689882942.79238224] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79242396] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79245210] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79263949] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79268360] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79273057] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79276061] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 ---
[1689882942.79281092] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79286242] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79290748] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79293609] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79296470] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79299283] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79303336] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79306197] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 x
[1689882942.79308987] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.318 ---
[1689882942.79315782] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.319 Config: logging.log_to_stdout = 0
[1689882942.79321742] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.319 Config: update_periods.executor_sleep_period = 10
[1689882942.79326105] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.319 Config: update_periods.action_feedback_publisher_period = 20
[1689882942.79328990] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.319 Config: update_periods.controller_status_monitor_period = 10
[1689882942.79331851] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.319 Config: publisher_qos.robot_status = 'sensor_data'
[1689882942.79334688] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.319 Config: publisher_qos.joint_states = 'sensor_data'
[1689882942.79337502] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.319 Config: publisher_qos.tf = 'default'
[1689882942.79341698] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.319 Config: tf_frame_prefix = ''
[1689882942.79359031] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.319 Config: stop_motion_on_disconnect = 1
[1689882942.79366231] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.319 Config: inform_job_name = 'INIT_ROS'
[1689882942.79369569] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.319 Config: allow_custom_inform_job = 0
[1689882942.79372621] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.319 Config: userlan_monitor_enabled = 1
[1689882942.79375529] [192.168.1.31:59091]: THU 2023-07-20 15:55:43.319 Config: userlan_monitor_port = 1

ted-miller · 2023-07-28T14:48:35Z

This looks ok to me. I'll defer to @gavanderhoorn though since it was his initial request. (At least I think it was. My memory is fuzzy.)

gavanderhoorn · 2023-08-16T13:51:30Z

Closing in favour of #127.

Thanks for all the initial work @SejalBehere 👍

Insert Ros_ConfigFile_PrintActiveConfiguration() to obtain complete d…

fb43013

…ump of g_nodeConfigSettings

gavanderhoorn mentioned this pull request Jul 11, 2023

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

Merged

gavanderhoorn added this to the 0.1.1 milestone Jul 12, 2023

gavanderhoorn requested changes Jul 14, 2023

View reviewed changes

src/ConfigFile.c Outdated Show resolved Hide resolved

gavanderhoorn reviewed Jul 14, 2023

View reviewed changes

src/ConfigFile.c Show resolved Hide resolved

Prefixing keys with enclosing namespaces

9a49f6f

gavanderhoorn reviewed Jul 18, 2023

View reviewed changes

src/ConfigFile.c Outdated Show resolved Hide resolved

gavanderhoorn previously approved these changes Jul 18, 2023

View reviewed changes

gavanderhoorn reviewed Jul 18, 2023

View reviewed changes

SejalBehere added 2 commits July 18, 2023 08:34

Merge branch 'Yaskawa-Global:main' into Issue-33-Config-Loader-only-p…

56dff4e

…rints-parsed-items

Removal of trailing whitespaces

2cc92a9

SejalBehere dismissed gavanderhoorn’s stale review via 2cc92a9 July 18, 2023 12:49

gavanderhoorn reviewed Jul 18, 2023

View reviewed changes

src/ConfigFile.c Outdated Show resolved Hide resolved

Removal of whitespaces

acc865f

SejalBehere added 3 commits July 19, 2023 16:09

Mapping enum integers to strings

ba26c3e

Refactoring Ros_ConfigFile_CheckYamlEvent and Mapping enum values to …

bd676f7

…strings

Prefixing "Ros_" before QosString and QosMappings, Replaced strcmp wi…

a2d2b29

…th strncmp

gavanderhoorn mentioned this pull request Aug 16, 2023

Dump complete cfg after parsing it #127

Merged

gavanderhoorn closed this Aug 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor printing of current configuration settings #84

Refactor printing of current configuration settings #84

SejalBehere commented Jul 10, 2023

gavanderhoorn left a comment

gavanderhoorn left a comment

gavanderhoorn Jul 18, 2023

gavanderhoorn commented Jul 19, 2023

SejalBehere commented Jul 19, 2023

SejalBehere commented Jul 20, 2023 •

edited by gavanderhoorn

Loading

ted-miller commented Jul 28, 2023

gavanderhoorn commented Aug 16, 2023

Refactor printing of current configuration settings #84

Refactor printing of current configuration settings #84

Conversation

SejalBehere commented Jul 10, 2023

gavanderhoorn left a comment

Choose a reason for hiding this comment

gavanderhoorn left a comment

Choose a reason for hiding this comment

gavanderhoorn Jul 18, 2023

Choose a reason for hiding this comment

gavanderhoorn commented Jul 19, 2023

SejalBehere commented Jul 19, 2023

SejalBehere commented Jul 20, 2023 • edited by gavanderhoorn Loading

ted-miller commented Jul 28, 2023

gavanderhoorn commented Aug 16, 2023

SejalBehere commented Jul 20, 2023 •

edited by gavanderhoorn

Loading