Print error code texts - #564
Conversation
Instead of logging C100A3 we can log "Robot mode changed to: POWER_OFF" It generates a C++ header from the online json documentation for error codes. Local overwrites can be specified for each error_code/argument combination either as json or in a C++ file. Currently, that contains replacing the robot mode argument in C100 with the robot mode string. This adds two custom CMake target - generate_error_codes re-generates the header based on the online json - check_error_codes checks whether the generated header was generated on the version noted by the online json
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #564 +/- ##
==========================================
- Coverage 81.19% 80.97% -0.23%
==========================================
Files 117 119 +2
Lines 7146 7173 +27
Branches 3201 3215 +14
==========================================
+ Hits 5802 5808 +6
- Misses 961 975 +14
- Partials 383 390 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
There was a problem hiding this comment.
🟡 Changes recommended
The build references a missing source file, and generated text is passed unsafely as a printf format string.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds human-readable error-code logging using generated upstream data and runtime overrides.
Changes:
- Adds error-text generation, lookup, and override infrastructure.
- Enhances
ErrorCodeMessage::toString()and logging. - Adds tests, CMake targets, and nightly version monitoring.
File summaries
| File | Description |
|---|---|
CMakeLists.txt |
Registers override source and generator targets. |
.github/workflows/check_error_codes.yml |
Checks upstream error-code versions nightly. |
include/ur_client_library/primary/primary_consumer.h |
Logs formatted error-code descriptions. |
include/ur_client_library/ur/error_code_overrides.h |
Declares dynamic override API. |
include/ur_client_library/ur/error_code_texts.h |
Provides generated error-text lookup data. |
scripts/error_code_overrides.json |
Defines static local overrides. |
scripts/generate_error_codes.py |
Generates and verifies the lookup header. |
src/primary/robot_message/error_code_message.cpp |
Implements prioritized text lookup. |
tests/CMakeLists.txt |
Registers the new tests. |
tests/test_error_code_message.cpp |
Tests lookup, overrides, and fallback behavior. |
Review details
Suppressed comments (1)
include/ur_client_library/primary/primary_consumer.h:139
toString()can now contain text from the JSON/overlay, including%. Passing that text directly as the printf-style format string makesvsnprintfinterpret format specifiers without matching arguments, which can corrupt output or invoke undefined behavior. Pass the message through a constant%sformat; the already-computedcode.to_stringcan also avoid invoking the override twice.
const auto log_contents = pkg.toString();
switch (code.report_level)
{
case ReportLevel::DEBUG:
- Files reviewed: 9/10 changed files
- Comments generated: 7
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Dynamic text is used as a format string, and the update workflow cannot recover reliably from closed unmerged PRs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
include/ur_client_library/ur/error_code_overrides.h:64
- This second implementation reference also points to the nonexistent
src/primary/robot_message/error_code_overrides.cpprather than the new source file.
* Implement additional cases in
* src/primary/robot_message/error_code_overrides.cpp.
- Files reviewed: 10/11 changed files
- Comments generated: 4
- Review effort level: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Out-of-range modes can be misreported, generator escaping is incomplete, and generated documentation needs correction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
scripts/generate_error_codes.py:91
- This escaping routine leaves NUL, backspace, form-feed, and other controls embedded raw in the generated C++ source. An escaped JSON
\u0000additionally truncates the displayedconst char*text at runtime. Escape source controls explicitly and render NUL visibly rather than generating an embedded terminator.
include/ur_client_library/ur/error_code_overrides.h:64
- This points contributors to a file that does not exist; the implementation added by this PR is
src/ur/error_code_overrides.cpp.
* Implement additional cases in
* src/primary/robot_message/error_code_overrides.cpp.
- Files reviewed: 10/11 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Valid JSON control characters can currently produce malformed generated C++ source.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
include/ur_client_library/ur/error_code_overrides.h:64
- The implementation is actually in
src/ur/error_code_overrides.cpp; the documented path points contributors to a nonexistent file.
* Implement additional cases in
* src/primary/robot_message/error_code_overrides.cpp.
- Files reviewed: 10/11 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Python compatibility and stale-branch handling can break the maintenance targets and nightly update workflow.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
CMakeLists.txt:138
- This accepts any Python 3 interpreter, but
generate_error_codes.py:106uses PEP 585 built-in generic annotations (dict[...]andtuple[...]), which require Python 3.9+. With Python 3.8 or older, CMake exposes these targets but the script fails during startup. Require 3.9 here, or rewrite the annotations usingtyping.Dict/typing.Tuple.
include/ur_client_library/ur/error_code_overrides.h:64
- This points contributors to a nonexistent source path; the implementation added by this PR is
src/ur/error_code_overrides.cpp. Following the public API documentation currently leads to the wrong location.
* Implement additional cases in
* src/primary/robot_message/error_code_overrides.cpp.
.github/workflows/check_error_codes.yml:90
- If an update PR is closed without merging, its remote branch remains. A later run sees no open PR, creates a fresh local branch from the default branch, and this plain push is rejected as non-fast-forward, preventing the automation from recreating the update PR. Handle an existing remote branch or closed PR explicitly by reopening/updating it or safely deleting and recreating the branch.
git push origin "$BRANCH"
- Files reviewed: 10/11 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The nightly automation cannot reliably notify or recreate updates across successive upstream releases.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/check_error_codes.yml:126
- The duplicate check is global to the label, but this workflow never closes the notification issue after an update is merged. Consequently, after the first update, that old open issue makes every later upstream version skip its notification. Match the issue title/version (as below), or add a success path that closes resolved notification issues.
- Files reviewed: 10/11 changed files
- Comments generated: 0 new
- Review effort level: Balanced
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Reviewed by Cursor Bugbot for commit ab233af. Configure here.
There was a problem hiding this comment.
🟡 Changes recommended
The valid C100A-1 case loses its argument identifier, and the update workflow still cannot recover from a stale remote branch.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 10/11 changed files
- Comments generated: 1
- Review effort level: Balanced
Since C100 can also have -1 as an argument, we cannot ignore it
There was a problem hiding this comment.
🟡 Changes recommended
Remote version data can inject generated C++, and transient network failures are incorrectly handled as version mismatches.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/check_error_codes.yml:60
- This condition treats every failure of the first network-backed check as a version mismatch. If that request fails transiently but the retry in “Extract versions” succeeds and reports equal versions, regeneration produces no change and
git commitfails, so the workflow neither opens a PR nor accurately reports an outdated header. Gate this step on the extracted versions actually differing (and ideally apply the same guard to label creation).
- Files reviewed: 10/11 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Bot-generated pull requests will not trigger required workflows when created using GITHUB_TOKEN.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/ur/error_code_overrides.cpp:76
- The new exception fallback is not exercised: the existing unknown-mode test uses
127, which is rejected by the range check beforerobotModeString()is called. Add a case such asarg == -2(inside the numeric range but not a definedRobotMode) to verify this catch path still returns the intendedUNKNOWNtext.
- Files reviewed: 10/11 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Version validation permits malformed input, and stale update branches can break the nightly automation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
scripts/generate_error_codes.py:94
- The generator's network request has no timeout, so an unresponsive upstream can block the CMake target or nightly workflow until the runner's job limit. Set a finite timeout on
urlopenso this maintenance path fails promptly and can be retried.
.github/workflows/check_error_codes.yml:96
- A closed update PR can leave this remote branch behind. Because only open PRs are detected, the next run creates the same branch locally and the push is rejected as non-fast-forward, so automation cannot recreate an update PR for that version. Explicitly handle an existing remote branch/closed PR (for example, reopen and update it or safely delete/recreate the branch).
git checkout -b "$BRANCH"
- Files reviewed: 11/12 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The acknowledged stale update-branch failure remains unresolved, and one lookup comment is inaccurate.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/primary/robot_message/error_code_message.cpp:79
- This note points to the wrong lookup step: an argument of
-1produces the sentinel key in the exact-match lookup above, so execution never reaches this fallback block. Update it to avoid contradicting the test and actual control flow.
- Files reviewed: 11/12 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Add string replacement of logged error codes
Instead of logging a plain "C100A3" we can log "C100A3: Robot mode changed to: POWER_OFF"
It generates a C++ header from the online json documentation for error codes.
Local overwrites can be specified for each error_code/argument combination
either as json or in a C++ file. Currently, that contains replacing the
robot mode argument in C100 with the robot mode string.
This PR implements:
ErrorCodeMessage::toString()method thaterror_code_overrides.cppErrorCodeMessage::toString()methodNote
Medium Risk
User-visible log and callback strings change for all error-code messages, and a large generated header must stay in sync with upstream JSON; risk is operational rather than security-critical.
Overview
Error code messages now include UR documentation text instead of only
C<code>A<arg>.ErrorCodeMessage::toString()appends a description from a runtime override, an exact(code, arg)lookup, or a code-only fallback in the generated map.A codegen and sync pipeline adds
scripts/generate_error_codes.py, commits auto-generatederror_code_texts.h(UR ErrorCodes JSON v40.121.0), and supports static tweaks viascripts/error_code_overrides.jsonplus dynamic cases insrc/ur/error_code_overrides.cpp(e.g. C100 → robot mode name). CMake exposesgenerate_error_codes/check_error_codestargets when Python is available.Automation and tests: a nightly GitHub workflow compares the committed header version to upstream JSON and can open a labeled PR to regenerate it; codecov ignores the generator script. Unit tests cover the lookup chain and overrides. Primary-interface logging now prints
toString()with%sformat (no extra prefix wrapper).Reviewed by Cursor Bugbot for commit 37841f3. Bugbot is set up for automated code reviews on this repo. Configure here.