search

Yaskawa-Global / motoros2

ROS 2 (rcl, rclc & micro-ROS) node for MotoPlus-compatible Yaskawa Motoman robot controllers
89 stars 14 forks source link

Check all alarms & subcodes are documented as part of CI #158

Closed gavanderhoorn closed 9 months ago

gavanderhoorn commented 9 months ago

Edit: update in https://github.com/Yaskawa-Global/motoros2/pull/158#issuecomment-1735239051.

As per subject.

This uses a custom Python script (added in f1df523d) to parse both the ErrorHandling.h and Troubleshooting.md files, extract the relevant information and then compare the two to make sure all alarms & subcodes defined in ErrorHandling.h are documented by sections in Troubleshooting.md.

A new workflow is added to our CI configuration (in 3c9b7b6d) which then runs this script for every PR opened against Yaskawa-Global/motoros2. Assuming we start from a situation where all subcodes are documented, any PRs which add new alarms and/or subcodes but do not also add documentation for those alarms and/or subcodes will fail the new workflow.

I've had to make some minor changes to ErrorHandling.h (e65b4eec) and Troubleshooting.md (727a2468) as we apparently haven't been entirely consistent with how we name and document things so far and to make parsing things slightly easier.

Example run of this workflow: here. This run failed.

Example output (verbose variant):

Click to expand ``` user@host:~: python \ .ci/alarm_subcode_doc_linter.py \ --verbose \ --check-all \ --ignore-enum Failed_Trajectory_Status \ --ignore-enum Init_Trajectory_Status \ --ignore-enum MotionNotReadyCode \ src/ErrorHandling.h \ doc/troubleshooting.md D | Parsing C header: 'src/ErrorHandling.h' D | Parsing Markdown document: 'doc/troubleshooting.md' D | Found 10 enums D | Ignoring: Failed_Trajectory_Status, Init_Trajectory_Status, MotionNotReadyCode D | Will check: ALARM_MAIN_CODE, ALARM_TASK_CREATE_FAIL_SUBCODE, ALARM_ASSERTION_FAIL_SUBCODE, ALARM_ALLOCATION_FAIL_SUBCODE, ALARM_CONFIGURATION_FAIL_SUBCODE, ALARM_INFORM_JOB_FAIL_SUBCODE, ALARM_DAT_FILE_PARSE_FAIL_SUBCODE D | Parsed 56 headings from Markdown file D | Extracted 54 alarm headings D | Alarms documented (54): 1020[5], 1020[6], 1050[1], 4207[1101], 4430[6], 4997[4], 8001[10], 8003[100-111], 8003[1], 8003[2], 8003[3], 8003[4], 8003[5], 8003[6], 8003[7], 8003[8], 8003[9], 8003[11], 8010[2], 8010[xx], 8011[xx], 8011[15], 8011[16], 8011[17], 8011[19], 8011[20], 8011[21], 8011[22], 8011[23-54], 8011[55], 8011[56-58], 8011[59], 8011[60-62], 8012[xx], 8013[0], 8013[1], 8013[2], 8013[3], 8013[4], 8013[5], 8013[6], 8013[7], 8013[8], 8013[10], 8013[12], 8013[13], 8013[14], 8013[15], 8014[0], 8014[1], 8014[2], 8014[3], 8015[0], 8015[1-4] D | Checking subcodes for documentation .. D | D | ALARM_TASK_CREATE_FAIL (8010): D | 8010[ 0] SUBCODE_INITIALIZATION : ✓ [xx] D | 8010[ 1] SUBCODE_EXECUTOR : ✓ [xx] D | 8010[ 2] SUBCODE_INCREMENTAL_MOTION : ✓ D | 8010[ 3] SUBCODE_ADD_TO_INC_Q : ✓ [xx] D | D | ALARM_ASSERTION_FAIL (8011): D | 8011[ 0] SUBCODE_FAIL_ROS_CONTROLLER_INIT : ✓ [xx] D | 8011[ 1] SUBCODE_INVALID_AXIS_TYPE : ✓ [xx] D | 8011[ 2] SUBCODE_FAIL_OPTIONS_INIT : ✓ [xx] D | 8011[ 3] SUBCODE_FAIL_RMW_OPTIONS_INIT : ✓ [xx] D | 8011[ 4] SUBCODE_FAIL_SUPPORT_INIT : ✓ [xx] D | 8011[ 5] SUBCODE_FAIL_NODE_INIT : ✓ [xx] D | 8011[ 6] SUBCODE_FAIL_MEM_ALLOC_CFG : ✓ [xx] D | 8011[ 7] SUBCODE_FAIL_CREATE_PUBLISHER_JOINT_STATE : ✓ [xx] D | 8011[ 8] SUBCODE_FAIL_CREATE_PUBLISHER_JOINT_STATE_ALL : ✓ [xx] D | 8011[ 9] SUBCODE_FAIL_CREATE_PUBLISHER_TRANSFORM : ✓ [xx] D | 8011[ 10] SUBCODE_FAIL_CREATE_PUBLISHER_ROBOT_STATUS : ✓ [xx] D | 8011[ 11] SUBCODE_FAIL_CREATE_TIMER : ✓ [xx] D | 8011[ 12] SUBCODE_FAIL_ALLOCATE_TRANSFORM : ✓ [xx] D | 8011[ 13] SUBCODE_FAIL_IO_STATUS_UPDATE : ✓ [xx] D | 8011[ 14] SUBCODE_FAIL_OPTIONS_INIT_DOMAIN_ID : ✓ [xx] D | 8011[ 15] SUBCODE_CONFIGURATION_INVALID_DOMAIN_ID : ✓ [xx] D | 8011[ 16] SUBCODE_CONFIGURATION_MISSING_AGENT_IP : ✓ [xx] D | 8011[ 17] SUBCODE_CONFIGURATION_INVALID_AGENT_SUBNET : ✓ [xx] D | 8011[ 18] SUBCODE_FAIL_ROS_CONTROLLER_INIT_TOO_MANY_GROUPS : ✓ [xx] D | 8011[ 19] SUBCODE_FAIL_MP_NICDATA_INIT0 : ✓ [xx] D | 8011[ 20] SUBCODE_MULTIPLE_INSTANCES_DETECTED : ✓ [xx] D | 8011[ 21] SUBCODE_CONFIGURATION_INVALID_NODE_NAME : ✓ [xx] D | 8011[ 22] SUBCODE_CONFIGURATION_INVALID_JOB_NAME : ✓ [xx] D | 8011[ 23] SUBCODE_FAIL_CREATE_SERVICE_QUEUE_POINT : ✓ [xx] D | 8011[ 24] SUBCODE_FAIL_TIMER_INIT_PING : ✓ [xx] D | 8011[ 25] SUBCODE_FAIL_TIMER_INIT_ROBOT_FB : ✓ [xx] D | 8011[ 26] SUBCODE_FAIL_TIMER_INIT_ACTION_FB : ✓ [xx] D | 8011[ 27] SUBCODE_FAIL_CREATE_MOTION_EXECUTOR : ✓ [xx] D | 8011[ 28] SUBCODE_FAIL_TIMER_ADD_PING : ✓ [xx] D | 8011[ 29] SUBCODE_FAIL_TIMER_ADD_ROBOT_FB : ✓ [xx] D | 8011[ 30] SUBCODE_FAIL_TIMER_ADD_ACTION_FB : ✓ [xx] D | 8011[ 31] SUBCODE_FAIL_ADD_FJT_SERVER : ✓ [xx] D | 8011[ 32] SUBCODE_FAIL_ADD_SERVICE_STOP_TRAJ : ✓ [xx] D | 8011[ 33] SUBCODE_FAIL_ADD_SERVICE_READ_SINGLE_IO : ✓ [xx] D | 8011[ 34] SUBCODE_FAIL_ADD_SERVICE_READ_GROUP_IO : ✓ [xx] D | 8011[ 35] SUBCODE_FAIL_ADD_SERVICE_WRITE_SINGLE_IO : ✓ [xx] D | 8011[ 36] SUBCODE_FAIL_ADD_SERVICE_WRITE_GROUP_IO : ✓ [xx] D | 8011[ 37] SUBCODE_FAIL_ADD_SERVICE_READ_M_REG : ✓ [xx] D | 8011[ 38] SUBCODE_FAIL_ADD_SERVICE_WRITE_M_REG : ✓ [xx] D | 8011[ 39] SUBCODE_FAIL_ADD_SERVICE_RESET_ERROR : ✓ [xx] D | 8011[ 40] SUBCODE_FAIL_ADD_SERVICE_START_TRAJ_MODE : ✓ [xx] D | 8011[ 41] SUBCODE_FAIL_ADD_SERVICE_START_QUEUE_MODE : ✓ [xx] D | 8011[ 42] SUBCODE_FAIL_ADD_SERVICE_QUEUE_POINT : ✓ [xx] D | 8011[ 43] SUBCODE_FAIL_INIT_SERVICE_READ_SINGLE_IO : ✓ [xx] D | 8011[ 44] SUBCODE_FAIL_INIT_SERVICE_READ_GROUP_IO : ✓ [xx] D | 8011[ 45] SUBCODE_FAIL_INIT_SERVICE_WRITE_SINGLE_IO : ✓ [xx] D | 8011[ 46] SUBCODE_FAIL_INIT_SERVICE_WRITE_GROUP_IO : ✓ [xx] D | 8011[ 47] SUBCODE_FAIL_INIT_SERVICE_READ_M_REG : ✓ [xx] D | 8011[ 48] SUBCODE_FAIL_INIT_SERVICE_WRITE_M_REG : ✓ [xx] D | 8011[ 49] SUBCODE_FAIL_INIT_SERVICE_RESET_ERROR : ✓ [xx] D | 8011[ 50] SUBCODE_FAIL_INIT_SERVICE_START_TRAJ_MODE : ✓ [xx] D | 8011[ 51] SUBCODE_FAIL_INIT_SERVICE_START_QUEUE_MODE : ✓ [xx] D | 8011[ 52] SUBCODE_FAIL_INIT_SERVICE_STOP_TRAJ_MODE : ✓ [xx] D | 8011[ 53] SUBCODE_FAIL_ADD_SERVICE_SELECT_MOTION_TOOL : ✓ [xx] D | 8011[ 54] SUBCODE_FAIL_INIT_SERVICE_SELECT_MOTION_TOOL : ✓ [xx] D | 8011[ 55] SUBCODE_CONFIGURATION_EMPTY_JOINT_NAME : ✓ [xx] D | 8011[ 56] SUBCODE_FAIL_CREATE_IO_EXECUTOR : ✓ [xx] D | 8011[ 57] SUBCODE_FAIL_TIMER_INIT_USERLAN_MONITOR : ✓ [xx] D | 8011[ 58] SUBCODE_FAIL_TIMER_ADD_USERLAN_MONITOR : ✓ [xx] D | 8011[ 59] SUBCODE_CONFIGURATION_AGENT_ON_NET_CHECK : ✓ [xx] D | 8011[ 60] SUBCODE_CONFIGURATION_FAIL_MP_NICDATA0 : ✓ [xx] D | 8011[ 61] SUBCODE_CONFIGURATION_FAIL_MP_NICDATA1 : ✓ [xx] D | 8011[ 62] SUBCODE_FAIL_MP_NICDATA_INIT1 : ✓ [xx] D | D | ALARM_ALLOCATION_FAIL (8012): D | 8012[ 0] SUBCODE_ALLOCATION_MALLOC : ✓ [xx] D | 8012[ 1] SUBCODE_ALLOCATION_CALLOC : ✓ [xx] D | 8012[ 2] SUBCODE_ALLOCATION_REALLOC : ✓ [xx] D | D | ALARM_CONFIGURATION_FAIL (8013): D | 8013[ 0] SUBCODE_CONFIGURATION_MISSINGFILE : ✓ D | 8013[ 1] SUBCODE_CONFIGURATION_SRAM_ACCESS_FAILURE : ✓ D | 8013[ 2] SUBCODE_CONFIGURATION_SRAM_WRITE_FAILURE : ✓ D | 8013[ 3] SUBCODE_CONFIGURATION_INVALID_BOOLEAN_VALUE : ✓ D | 8013[ 4] SUBCODE_CONFIGURATION_INVALID_QOS_VALUE : ✓ D | 8013[ 5] SUBCODE_CONFIGURATION_INVALID_EXECUTOR_PERIOD : ✓ D | 8013[ 6] SUBCODE_CONFIGURATION_INVALID_FEEDBACK_PERIOD : ✓ D | 8013[ 7] SUBCODE_CONFIGURATION_INVALID_IO_PERIOD : ✓ D | 8013[ 8] SUBCODE_CONFIGURATION_TOO_MANY_REMAP_RULES1 : ✓ E | 8013[ 9] SUBCODE_CONFIGURATION_TOO_MANY_REMAP_RULES2 : ✘ D | 8013[ 10] SUBCODE_CONFIGURATION_INVALID_REMAP_RULE_FORMAT : ✓ E | 8013[ 11] SUBCODE_CONFIGURATION_FAIL_NODE_INIT_ARG_PARSE : ✘ D | 8013[ 12] SUBCODE_CONFIGURATION_INVALID_CUSTOM_JOINT_NAME : ✓ D | 8013[ 13] SUBCODE_CONFIGURATION_INVALID_USERLAN_MONITOR_PORT : ✓ D | 8013[ 14] SUBCODE_CONFIGURATION_USERLAN_MONITOR_AUTO_DETECT_FAILED : ✓ D | 8013[ 15] SUBCODE_CONFIGURATION_RUNTIME_USERLAN_LINKUP_ERR : ✓ D | D | ALARM_INFORM_JOB_FAIL (8014): D | 8014[ 0] SUBCODE_INFORM_FAIL_TO_OPEN_DRAM : ✓ D | 8014[ 1] SUBCODE_INFORM_INVALID_JOB : ✓ D | 8014[ 2] SUBCODE_INFORM_FAIL_TO_CREATE_JOB : ✓ D | 8014[ 3] SUBCODE_INFORM_FAIL_TO_LOAD_JOB : ✓ D | D | ALARM_DAT_FILE_PARSE_FAIL (8015): D | 8015[ 0] SUBCODE_DAT_FAIL_OPEN : ✓ D | 8015[ 1] SUBCODE_DAT_FAIL_PARSE_RBCALIB : ✓ D | 8015[ 2] SUBCODE_DAT_FAIL_PARSE_MGROUP : ✓ D | 8015[ 3] SUBCODE_DAT_FAIL_PARSE_SGROUP : ✓ D | 8015[ 4] SUBCODE_DAT_FAIL_PARSE_SRANG : ✓ ```

Example output (non-verbose variant):

Click to expand ``` E | 8013[ 9] SUBCODE_CONFIGURATION_TOO_MANY_REMAP_RULES2 : ✘ E | 8013[ 11] SUBCODE_CONFIGURATION_FAIL_NODE_INIT_ARG_PARSE : ✘ ```

Note that we currently apparently don't document these two subcodes anywhere.

gavanderhoorn commented 9 months ago

Note btw: this obviously doesn't actually read the documentation.

It just checks whether there is a heading somewhere in Troubleshooting.md with an alarm and subcode specification which corresponds to what is defined in ErrorHandling.h.

But seeing as we have a rather standardised approach to documenting alarms and subcodes, this should be enough to catch at least the cases where we completely forget to document something.

ted-miller commented 9 months ago

Merging. will open a new issue for the missing subcodes

gavanderhoorn commented 9 months ago

I have a couple ideas on how to make this a little nicer.

I'll change to draft for now so I can implement those.

gavanderhoorn commented 9 months ago

I've reimplemented the linter to output GCC style errors and warnings, and it now also links all output to lines in ErrorHandling.h:

src/ErrorHandling.h:188:5: error: no documentation for '8013[9]' in 'doc/troubleshooting.md'
src/ErrorHandling.h:191:5: error: no documentation for '8013[11]' in 'doc/troubleshooting.md'

With a GCC-style problem matcher registered, this results in annotations like this on the PR:

image

which is a nice reminder when reviewing PRs that something is missing.

I've also added an option (--warn-catch-all) to the linter to make it emit warnings for subcodes which are (only) documented by catch all range documentation sections (such as 8010[xx]).

Having more specific documentation is always better I believe, but I've not enabled those warnings in this PR, as it would result in quite some output from the linter of the form:

src/ErrorHandling.h:101:5: warning: documented by catch-all '8011[xx]'

we currently have 69 subcodes only documented by such catch all ranges.

ted-miller commented 9 months ago

I'm confused. I thought I already merged this... Did you somehow un-merge? Or did I just forget to click a button?

gavanderhoorn commented 9 months ago

The latter :)

gavanderhoorn commented 9 months ago

Friendly ping @ted-miller