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

Better errors from codegen #127

Merged
merged 8 commits into from
Oct 3, 2023
Merged

Better errors from codegen #127

merged 8 commits into from
Oct 3, 2023

Conversation

Carter12s
Copy link
Collaborator

@Carter12s Carter12s commented Sep 28, 2023

Closes: #126
Closes: #128

Trying to catch some common user errors and make them more clear. Investigated automated tests for these kinds of errors, but it looks like it is actually pretty annoying to setup. Looks like this will do the job, but a little much to setup right now: https://crates.io/crates/compiletest_rs

These test checks were manually performed by me while setting this up:
Manually modifying calling_services to introduce a typo on this line:
roslibrust_codegen_macro::find_and_generate_ros_messages!("assets/ros1_common_interfacs",);

Now produces the following compilation error when invoking cargo run --example calling_service

error: Failed to find any services or messages while generating ROS message definitions, paths searched: ["assets/ros1_common_interfacs"]
 --> roslibrust/examples/calling_service.rs:4:1
  |
4 | roslibrust_codegen_macro::find_and_generate_ros_messages!("assets/ros1_common_interfacs",);
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in the macro `roslibrust_codegen_macro::find_and_generate_ros_messages` (in Nightly builds, run with -Z macro-backtrace for more info)

@Carter12s Carter12s requested a review from ssnover September 28, 2023 23:41
@Carter12s Carter12s changed the title Better errors from codegen Draft: Better errors from codegen Sep 28, 2023
Copy link
Collaborator

@ssnover ssnover left a comment

Choose a reason for hiding this comment

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

Rather than messages or services can you reference ROS packages? I think that could make it more clear that they can't be standalong message files on disk, they should be part of a package (which is what we search for).

@Carter12s
Copy link
Collaborator Author

Continuing to add some better messages:

Invoking this line:

roslibrust_codegen_macro::find_and_generate_ros_messages!("assets/ros1_test_msgs");

Without std_msgs in ROS_PACKAGE_PATH now produces:

error: Unable to resolve dependencies after reaching search limit.
       The following messages have unresolved dependencies: ["test_msgs/Float64Stamped"]
       These messages likely depend on packages not found in the provided search paths.
 --> roslibrust/examples/basic_publisher.rs:6:1
  |
6 | roslibrust_codegen_macro::find_and_generate_ros_messages!("assets/ros1_test_msgs");
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in the macro `roslibrust_codegen_macro::find_and_generate_ros_messages` (in Nightly builds, run with -Z macro-backtrace for more info)

image

@Carter12s Carter12s changed the title Draft: Better errors from codegen Better errors from codegen Sep 30, 2023
@Carter12s
Copy link
Collaborator Author

Alright @ssnover I think this one is really ready for review now.

I added a big'ol pile of error improvements, replaced all of the panic! and unwrap() I reasonably could with proper error bubbling, and got the whole thing to print much more understandable errors way more often!

@Carter12s
Copy link
Collaborator Author

@ssnover Ping again on re-view, I think you missed this one in last pass cause you had previously reviewed it before the changes, but there is now a lot more here

@ssnover ssnover merged commit 1284401 into master Oct 3, 2023
@Carter12s Carter12s deleted the better-errors-from-codegen branch October 5, 2023 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Codegen: dependency iteration limit Overhaul errors for codegen
2 participants