@dirk-thomas I believe you're the best person to review this. Not sure how this plays with (or if it's already fixed in) the refactored IDL code.

The patch looks fine to me. Please run CI for the change and maybe add a test to check what happens when passing non-ASCII strings to it.

Running CI (again, including rclpy test):

• Linux
• Linux-aarch64
• macOS
• Windows

@dirk-thomas I think that writing a test for this is going to be somewhat involved. The conversions only take place within rclpy C extension code. I'd have to call publish on a message with a non-ascii string field to trigger a check when going from Python to C, and then take a message with a non-ascii string as sent by code that doesn't have such validations (e.g. a C++ node) to trigger a check when going from C to Python. Maybe a candidate for a test_rclpy package in system_tests? An easier alternative might be to write the test in C, using Python's C API.

I'd have to call publish on a message with a non-ascii string field to trigger a check when going from Python to C

That part sounds fairly straight forward?

and then take a message with a non-ascii string as sent by code that doesn't have such validations (e.g. a C++ node) to trigger a check when going from C to Python.

If the test only covers the Python-to-C case but not this one that would be sufficient for me.

That part sounds fairly straight forward?

That's the easy part, yes.

If you're OK with just testing Python-to-C, so am I. But I think we could add the test in rclpy, to avoid adding it as a dependency here.

CI's green now! We're good to go.