Rust::com Rust APIs Example app Optimize by bharatGoswami8 · Pull Request #556 · eclipse-score/communication

bharatGoswami8 · 2026-06-18T03:36:04Z

Created Example app which can run based on CLI argument like only Producer / Consumer or Both and different Async APIs of Consumer.
Independent a test unit kept which use tokio runtime for async testing.

* Sync APIs added on this

* Updated example binary with proper structure * Kept only async test cases using tokio

rpreddyhv

few comments & queries to clarify.

darkwisebear · 2026-06-25T19:17:14Z

+- **Consumer Thread**: Receives and processes data independently
+
+### Async Operations
+- Uses `futures::executor::block_on()` for async operations


Sloppy - block_on is synchronous. AI should elaborate more about the execution model instead of making obvious but inaccurate statements like here.

Rephrased the statement.

darkwisebear · 2026-06-25T19:18:08Z

+
+A separate test target (`tests_using_tokio_runtime.rs`) provides comprehensive integration tests using the **Tokio async runtime**. These tests are independent of the main application and use the `com-api-gen` generated types together with the `com_api` runtime, service discovery, and subscription APIs directly.
+
+**Why Separate?**


I don't get how the points below answer this question.

It was added to give impression why we have iteration test separately with tokio, so the simple reason for that is we do not want to use tokio in application and as we have already existing test case with tokio which can be used for quick and async runtime related testing.

darkwisebear · 2026-06-25T19:19:34Z

+/// All execution modes supported by the example application.
+/// Used by clap for CLI argument parsing and by the consumer/producer to select behaviour.
+#[derive(clap::ValueEnum, Clone, Debug)]
+pub enum ExampleType {


Why is this here? Imo that's clearly something that does not belong in a library but to the main CLI crate.

Yes in first iteration of implementation, it was in main file and we were having different functions for each operation, but as it was calling separate function for each APIs so i thought to generalize and keep minimum function call from main and parsing can be done on library,
But I agree in that case sync execution is done with async function which is not proper way.

I have moved to the first iteration design.

darkwisebear · 2026-06-25T19:21:25Z

-load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_test")
+load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library", "rust_test")
+
+rust_library(


Please choose the 2024 edition.

darkwisebear · 2026-06-25T19:37:15Z

+                Err(e) => eprintln!("[SENDER] Failed to send sample: {:?}", e),
+            }
+            println!("[SENDER] Sent sample with pressure: {}", 1.0 + i as f32);
+            tokio::time::sleep(tokio::time::Duration::from_millis(1000)).await;


Magic number.

created const

darkwisebear · 2026-06-25T19:38:30Z

+        let mut buffer = SampleContainer::new(5);
+        for _ in 0..3 {
+            let (returned_buf, result) = if is_timeout {
+                let timeout = tokio::time::sleep(Duration::from_millis(1000));


Magic number.

created const

darkwisebear · 2026-06-30T18:14:12Z

+/// Polling interval between iterations in synchronous receive loops.
+const SYNC_RECEIVE_POLL_INTERVAL_MS: u64 = 1000;
+
+impl<R: Runtime> VehicleMonitorConsumer<R> {


Is there a reason why the impl block is separate from the type definition. Imo this only makes it harder to comprehend the logic.

I was thinking to keep vehicleMonitor on separate module for better modularity but agreed to your point moved type into same file and removed vehicle_monitor file.

darkwisebear · 2026-06-30T18:20:17Z

+        let consumer_discovery =
+            runtime.find_service::<VehicleInterface>(FindServiceSpecifier::Specific(service_id));
+        let instances = match mode {
+            ExampleType::Sync => consumer_discovery


Using a parameter to distinguish between sync and async doesn't make sense imo, as the call is enclosed in an async function in any case. I think you should split this into a sync and an async create function instead.

Now thinking about it, the definition of get_available_instances_async is odd: What's the async part of this method actually waiting for? Presence of at least one instance? If yes, then this behavior should be documented.

Moved async and sync APIs in separate function and behavior is documented.

darkwisebear · 2026-06-30T18:21:32Z

+    ///
+    /// Takes `&self` to support all receive modes including streaming,
+    /// which requires exclusive access to the subscriber.
+    pub async fn read_tire_data(&self, mode: &ExampleType) -> Result<String> {


Same comment as in the new case: Having a sync code path in an async function isn't wrong (as log as it's nonblocking, which is the case), but kinda pointless: Why have the burden of an async function without any benefit?

Agreed, it may give wrong impression to user as well, so moved async and sync function separately.

darkwisebear · 2026-06-30T18:22:57Z

+                    Err(e) => Err(e),
+                }
+            }
+            _ => unreachable!(


Another code smell that reveals the non-ideal design here: This method isn't defined for all modes, which leads to mixing static and dynamic properties in this regard.

As enum moved to main so now design is purely based on each APIs function call.

* Optimize the application * Remove VehicleMonitor and keep only lib * Keep enum parsing on main * Updated integration test

rpreddyhv

Looks okay to me!

bharatGoswami8 requested review from LittleHuba, bemerybmw, castler, crimson11, hoe-jo and limdor as code owners June 18, 2026 03:36

github-project-automation Bot added this to COM - Communication FT Jun 18, 2026

github-project-automation Bot moved this to Backlog in COM - Communication FT Jun 18, 2026

bharatGoswami8 self-assigned this Jun 18, 2026

bharatGoswami8 added the rust-api label Jun 18, 2026

bharatGoswami8 requested a review from darkwisebear June 18, 2026 07:02

bharatGoswami8 force-pushed the rust_example_update branch 2 times, most recently from 8484cc6 to 91acf6a Compare June 24, 2026 08:51

Rust::com Rust Example app Updated version

20a3949

* Sync APIs added on this

bharatGoswami8 force-pushed the rust_example_update branch 2 times, most recently from 9e5470a to c4a5d19 Compare June 25, 2026 07:19

Rust::com Added Asyncronous APIs And Test for example

7d60e31

* Updated example binary with proper structure * Kept only async test cases using tokio

bharatGoswami8 force-pushed the rust_example_update branch from c4a5d19 to 7d60e31 Compare June 25, 2026 09:29

rpreddyhv reviewed Jun 30, 2026

View reviewed changes

darkwisebear suggested changes Jun 30, 2026

View reviewed changes

bharatGoswami8 force-pushed the rust_example_update branch from 9a86f50 to 27c5215 Compare July 1, 2026 07:20

bharatGoswami8 requested review from darkwisebear and rpreddyhv July 1, 2026 08:19

bharatGoswami8 force-pushed the rust_example_update branch 2 times, most recently from ec2bd21 to 3fbbbd6 Compare July 1, 2026 08:26

Rust::com Update the example application

820b85b

* Optimize the application * Remove VehicleMonitor and keep only lib * Keep enum parsing on main * Updated integration test

bharatGoswami8 force-pushed the rust_example_update branch from 3fbbbd6 to 820b85b Compare July 1, 2026 09:45

rpreddyhv approved these changes Jul 1, 2026

View reviewed changes

bharatGoswami8 requested a review from rpreddyhv July 1, 2026 10:18


		A separate test target (`tests_using_tokio_runtime.rs`) provides comprehensive integration tests using the Tokio async runtime. These tests are independent of the main application and use the `com-api-gen` generated types together with the `com_api` runtime, service discovery, and subscription APIs directly.

		Why Separate?

Uh oh!

Conversation

bharatGoswami8 commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rpreddyhv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bharatGoswami8 Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rpreddyhv left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bharatGoswami8 commented Jun 18, 2026 •

edited

Loading

bharatGoswami8 Jul 1, 2026 •

edited

Loading