fix(sample_application): use dedicated callback groups

[bdm-k]

Description

minimal_server.cpp and minimal_client.cpp previously used the default callback group with CIE, which results in the "contains both ROS 2 callbacks and Agnocast callbacks" warning. This PR fixes the issue by using dedicated callback groups.

How was this PR tested?

• Autoware (required)
• bash scripts/test/e2e_test_1to1.bash (required)
• bash scripts/test/e2e_test_2to2.bash (required)
• kunit tests (required when modifying the kernel module)
• bash scripts/test/run_requires_kernel_module_tests.bash (required)
• sample application

Notes for reviewers

[Copilot]

Pull request overview

Updates the sample service/server and client to use dedicated ROS 2 callback groups for Agnocast service entities, avoiding the warning about mixing ROS 2 and Agnocast callbacks in the default callback group when using the callback-isolated executor.

Changes:

• Add a dedicated MutuallyExclusive callback group to minimal_server and assign the Agnocast service to it.
• Create a dedicated MutuallyExclusive callback group in minimal_client and assign the Agnocast client to it (with explicit ServicesQoS).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/agnocast_sample_application/src/minimal_server.cpp	Creates a dedicated callback group and binds the Agnocast service to it.
src/agnocast_sample_application/src/minimal_client.cpp	Creates a dedicated callback group and binds the Agnocast client to it.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Koichi98]

@bdm-k
Thanks for catching this warning case!

One thought though: the warning on minimal_client / minimal_server is actually triggered by the built-in parameter services that rclcpp::Node registers on the default callback group - not by a real user-side mix. Parameter services don't have tight timing requirements, so being blocked a bit by an Agnocast timed wait isn't really a problem in practice.

Rather than adding dedicated callback groups on the application side, would it make more sense to relax the warning condition to exclude the built-in parameter services?

[bdm-k]

@Koichi98
I wonder whether using the default callback group with the callback isolated executor is a good practice. If it is not, we should use a dedicated callback group either way. After all, the sample applications work as guidelines for how to use Agnocast API.

Aside from this, I patched SingleThreadedAgnocastExecutor::warn_if_mixed_callback_groups() to relax the warning condition in commit 274469e. Thanks in advance for the review.

[bdm-k]

Oh, it's not just the parameter services; we also need to exclude the parameter events and the logging publisher.

[bdm-k]

@Koichi98
I checked again after excluding /parameter_events, and there are still two Waitable objects in the default callback group. I think excluding any Waitable would be too broad and could lead to sort of false negatives.

If we consider using the default callback group with CallbackIsolatedAgnocastExecutor as not a recommended practice, the current warning condition may still be acceptable, since it helps developers notice when they forgot to configure a custom callback group.

[Koichi98]

@bdm-k
I looked into a couple of alternatives to "users must create a dedicated callback group," but both run into blockers:

Option A: Have Agnocast lazy-create a dedicated callback group internally when options.callback_group == nullptr. Zero user burden, but it silently breaks the MutuallyExclusive guarantee users expect from the default group — Agnocast and parameter/timer/etc. callbacks would run concurrently in CIE, so any shared state would need explicit locking without the user realizing it.

Option B: Extend the allow-list in warn_if_mixed_callback_groups() to cover all rclcpp built-ins. No semantics change, but beyond parameter services / parameter events / /rosout, the default group also picks up things like the /clock subscription (under use_sim_time) and QoS event handlers, which do have timing sensitivity.

So I agree — requiring users to set up a dedicated callback group explicitly seems to be the only sound option. Keeping the current warning strict and treating "default group + CIE" as discouraged in docs/samples is probably the right path.

@atsushi421
I would like you to have some comments here. What do you think?

[atsushi421]

@Koichi98 I agree that requiring users to set up a dedicated callback group explicitly is the right approach.

Option A carries a significant risk — silently breaking the MutuallyExclusive guarantee would introduce subtle concurrency bugs that are hard to diagnose. As for Option B, given that the purpose of CIE is to apply scheduling attributes only to intended callbacks, it's desirable for agnocast callbacks to have their own independent callback group.

[bdm-k]

@Koichi98
I've reverted the previous commit.