Implement register_listener and unregister_listener functionality by arakabCL · Pull Request #75 · eclipse-uprotocol/up-transport-iceoryx2-rust

arakabCL · 2025-07-25T13:10:41Z

Implemented register_listener and unregister_listener methods.
Removed wildcard support for exact URI matching.
Added background task for message processing.
Included compute_service_name function for URI conversion.

…that all the tests would work as expected

…ta test added and runs

Merging all of the edits into one shared repo

…nning

tests run and there are not issues anymore which prevent them from ru…

… compute service name fuction

… type that iceoryx2 expects

…rately - using availbale helper methods defined in uri.rs

…e (iceoryx2 definition for service name)

This commit introduces the and functionalities for the . Key changes include: - Implemented the method to allow clients to subscribe to uProtocol messages. - Implemented the method to allow clients to unsubscribe. - Added tests for both registration and unregistration.

…g- will fix this but i want the functional code to be commited so in case i can go back to it

Two terminals

…memory safe structures

… exact definition of the uri being sent

…ew genetic data

…DataGeneric Making transmission data generic

…cards Tests without wildcards

Removed all wildcard support from compute listener matching logic and implemented exact URI matching via HashMap lookups. Enhanced register and unregister listener functionality with proper error handling, subscriber creation, and cleanup. Added comprehensive test coverage including race condition fixes and edge cases. All 16 tests passing with robust exact matching behavior for production readiness.

- Added working register_listener and unregister_listener methods - Removed wildcard support for exact URI matching - Added background task for message processing

PLeVasseur

Recording a few things we chatted about in the meeting, no action needed on this PR regarding these comments.

PLeVasseur · 2025-07-29T15:48:39Z

+#[repr(C)]
+pub struct CustomHeader {
+    pub version: i32,
+    pub timestamp: u64,


According to @catappreciationhours2 -- the serialized UAttributes is planned for inclusion in the send PR

PLeVasseur · 2025-07-29T15:49:21Z

+    fn from(header: &CustomHeader) -> UAttributes {
+        let mut attrs = UAttributes::default();
+
+        // Map CustomHeader fields back to UAttributes using available fields


All of this mapping is planned the upcoming Send PR according to @catappreciationhours2

Trying to resolve the build error

sophokles73 · 2025-07-30T12:09:42Z

+        run: cargo build --verbose
+
+      - name: Run tests
        run: cargo test --verbose


missing EOL

sophokles73 · 2025-07-30T12:12:13Z

please move this file to the examples folder

sophokles73 · 2025-07-30T12:12:22Z

please move this file to the examples folder

sophokles73 · 2025-07-30T12:13:10Z

are you having the same issues with line endings as @catappreciationhours2 ?

sophokles73 · 2025-07-30T13:05:25Z

+enum TransportCommand {
+    Send {
+        message: UMessage,
+        response: std::sync::mpsc::Sender<Result<(), UStatus>>,


given that we are depending on tokio anyway, you might want to consider using a one-shot channel for the response instead ...

Agree with this feedback. It's a nice approach and signals intent.

sophokles73 · 2025-07-30T13:42:16Z

+
+impl Iceoryx2Transport {
+    pub fn new() -> Result<Self, UStatus> {
+        let (tx, rx) = std::sync::mpsc::channel();


you might want to consider using the async tokio channel pendant instead

sophokles73 · 2025-07-30T13:47:27Z

+                            let publisher =
+                                publishers.entry(service_name.clone()).or_insert_with(|| {
+                                    let service_name_res: Result<ServiceName, _> =
+                                        service_name.as_str().try_into();
+                                    let service = node
+                                        .service_builder(&service_name_res.unwrap())
+                                        .publish_subscribe::<RawBytes>()
+                                        .user_header::<CustomHeader>()
+                                        .open_or_create()
+                                        .expect("Failed to create service");
+
+                                    service
+                                        .publisher_builder()
+                                        .create()
+                                        .expect("Failed to create publisher")
+                                });


it feels a little odd that the lookup and potential creation of the publisher is done here instead of handle_send (as for the register/unregister listener commands) ...

sophokles73 · 2025-07-30T13:54:36Z

+
+                // Integrate dispatch: In polling/receive, extract attributes and reconstruct UMessage
+                // Only process subscribers that have active listeners
+                let active_services: Vec<(String, Vec<Arc<dyn UListener>>)> = listeners


it looks like we never poll for incoming messages unless there a no more commands to process, or am I mistaken? Meaning: if we continuously send messages, we never get a response ...
You might want to replace the two separate while loops (for processing commands and for polling receivers) with a single loop around tokio's select! macro

Agree with this feedback.

Bump for response

Co-authored-by: Kai Hudalla <sophokles.kh@gmail.com>

PLeVasseur

I wasn't able to get to the tests yet, but submitting a partial review for your consideration.

PLeVasseur · 2025-07-31T22:34:55Z

+log = "0.4.27"
+env_logger = "0.11.8"
+once_cell = "1.18.0"
+uuid = { version = "1", features = ["v4"] }


I don't see the uuid crate being used, could you point me to it?
I think up-rust should have abstracted this away.

PLeVasseur · 2025-07-31T22:36:27Z

+protobuf = "3.7.2"
+bytes = "1.10.1"
+tokio = { version = "1", features = ["full"] }
+test-case = "3"


Any deps which are used only for tests should be moved under:

[dev-dependencies]

instead.

reference

PLeVasseur · 2025-07-31T22:41:43Z

@@ -0,0 +1,229 @@
+use super::*;


You can place tests either at the bottom of the file which implements some functionality and then wrap it in correctly in a module for testing.

It's also possible if you'd like you can have a tests/ folder at the root of the project in which to place them (this is often done for integration tests).

I suppose the easiest change in this case is to move this file to tests/receiver.rs.

reference to read more

PLeVasseur · 2025-07-31T22:42:42Z

+tokio = { version = "1", features = ["full"] }
+test-case = "3"
+log = "0.4.27"
+env_logger = "0.11.8"


Any deps which are used only for tests should be moved under:

[dev-dependencies]

instead.

reference

PLeVasseur · 2025-07-31T23:48:06Z

+                    if let Some(subscriber) = subscribers.get(&service_name) {
+                        while let Some(sample) = subscriber.receive().ok().flatten() {
+                            for listener in &listeners_to_notify {
+                                // Extract payload bytes


You can remove these line-by-line comments. Should be understandable what's going on.
Reserve comments for where something non-obvious and/or complicated is happening.

PLeVasseur · 2025-07-31T23:49:09Z

+
+    fn handle_register_listener(
+        node: &Node<ipc::Service>,
+        subscribers: &mut HashMap<


Another good reason to use a type alias. You can keep this tidy here too.

Bump for response

PLeVasseur · 2025-07-31T23:51:06Z

+        };
+
+        // Create subscriber if it doesn't exist for this service_name
+        if !subscribers.contains_key(&service_name) {


I think you'll need to do a bit more error handling around here to e.g. return a UStatus which indicates said set of source and sink filters are already registered.

Bump for response

PLeVasseur · 2025-07-31T23:52:08Z

+        };
+
+        // Remove from hashmap: listeners.get_mut(&service_name).and_then(|vec| vec.remove(&listener));
+        if let Some(listener_vec) = listeners.get_mut(&service_name) {


I think you'll need to do a bit more error handling around here to return a UStatus indicating that said set of source and sink filters are not registered.

PLeVasseur

Hey @arakabCL -- thanks for making some updates.
I recently found out how to correctly use a byte slice [u8] for the underlying buffer.

As I wrote on Slack I'm going to get support from Kenny to implement this in time for the Eclipse SDV Hackathon in ~4 weeks.

Could you commit to getting this done in a week or two?
Otherwise I think I'll have to ask Kenny to pick up where you left off here a bit and revise based on the suggestions I left.

PLeVasseur · 2025-08-23T18:30:01Z

+mod raw_bytes;
+use raw_bytes::RawBytes;
+
+use std::collections::HashMap;


Bump to resolve

PLeVasseur · 2025-08-23T18:30:56Z

+                }
+            };
+
+            let mut publishers: HashMap<


Bump to resolve

PLeVasseur · 2025-08-23T18:57:19Z

+
+                // Integrate dispatch: In polling/receive, extract attributes and reconstruct UMessage
+                // Only process subscribers that have active listeners
+                let active_services: Vec<(String, Vec<Arc<dyn UListener>>)> = listeners


Bump for response

PLeVasseur · 2025-08-23T18:57:36Z

+
+    fn handle_register_listener(
+        node: &Node<ipc::Service>,
+        subscribers: &mut HashMap<


Bump for response

PLeVasseur · 2025-08-23T18:57:47Z

+        };
+
+        // Create subscriber if it doesn't exist for this service_name
+        if !subscribers.contains_key(&service_name) {


Bump for response

PLeVasseur · 2025-08-23T19:00:53Z

    }

-    fn compute_service_name(message: &UMessage) -> Result<String, UStatus> {
+    fn compute_service_name_from_message(message: &UMessage) -> Result<String, UStatus> {


I'd recommend removing this and simply having compute_service_name_from_uris() from below, since it will be a larger maintenance burden to have two different ways of doing the same thing, when it's possible to use compute_service_name_from_uris() even with a UMessage's source and sink UUris.

PLeVasseur · 2025-10-29T18:28:04Z

Functionality was implemented in #78

kwarraich and others added 30 commits June 12, 2025 13:58

looking through noellas work

c76c856

more of what noella has done

aec4d04

fixed all the errors reguarding the cargo test

3ec7d93

seperated out the test cases from lib.rs and then made adjustment so …

c83ca8d

…that all the tests would work as expected

added custom header case & muliple messages in seq case

aa81d20

checks that the umessage is correctly parsed into the transmission da…

89596de

…ta test added and runs

tested sending w empty payload, no payload, and max size payload

fff54b7

included the multiple concurent messages

d97041c

adding all files so can merge into one shared fork

ee5116f

Merge pull request #1 from kwarraich/testing_send_reg_unreg

7ee3b66

Merging all of the edits into one shared repo

tests run and there are not issues anymore which prevent them from ru…

a525b97

…nning

Merge pull request #2 from uProtocol-cmu-team-Luna/fixingInitalErrors

b78e66f

tests run and there are not issues anymore which prevent them from ru…

skeleton code for receiver testing

761c7d7

receiver test with compilation errors

2a4e050

modifed send test cases for muli sends, implemented send(), written a…

2e2ba85

… compute service name fuction

modified compute service name to work correctly with the service name…

7c43ba4

… type that iceoryx2 expects

updated encode uuri segments to handle type and instance from id sepa…

b83872b

…rately - using availbale helper methods defined in uri.rs

receiver.rs detected by cargo test

a77699f

test cases to verify conversion from uri to iceoryx2 addressing schem…

94ce300

…e (iceoryx2 definition for service name)

fixed logger error

32afab3

Merge remote-tracking branch 'origin/main' into receiver

2c75b13

mostly fixing the failing tests, only one test the wildcard is failin…

33f3303

…g- will fix this but i want the functional code to be commited so in case i can go back to it

creating it so that it sends and recives on seperate terminals

8b8b170

Merge pull request #3 from uProtocol-cmu-team-Luna/twoTerminals

5e79a23

Two terminals

made the transmission data completely genetic

418af5b

adding a test case to make sure mockUListener works

6e8524c

testing unregister in receiver

1a0d554

added a test case

ed4cb86

adding some more tests and pushing the mockUListener into the test block

73456ff

catappreciationhours2 and others added 15 commits July 10, 2025 08:44

fixed uuri wildcard logic and made RawBytes compatible with iceoryx2 …

3aa4582

…memory safe structures

modified cargo.toml to make test-utils availale in dependencies

e36dc08

these tests pass without wildcard URIs so replaced Uuri::any with the…

bf2f608

… exact definition of the uri being sent

adjusted so the ternimals would send the data and properly with the n…

2f2e4ee

…ew genetic data

adjusted teh reciving end so it would print in decimal

65768ab

Merge pull request #4 from uProtocol-cmu-team-Luna/makingTransmission…

fc0e8e4

…DataGeneric Making transmission data generic

Merge branch 'main' into tests_without_wildcards

c3fa305

Merge pull request #5 from uProtocol-cmu-team-Luna/tests_without_wild…

49fe222

…cards Tests without wildcards

Removed wildcard support from compute listener matching logic.

6aa08f6

Completed wildcard removal from compute listener with tests.

f74cc6f

Updated register with deserialized attributes handling

e367b19

Implement register_listener and unregister_listener functionality

d815582

- Added working register_listener and unregister_listener methods - Removed wildcard support for exact URI matching - Added background task for message processing

fixing workflow error

8e9b311

Merge branch 'main' into listener-wildcard-removal-pr

4a491ca

PLeVasseur reviewed Jul 29, 2025

View reviewed changes

vidishac2004 and others added 2 commits July 30, 2025 03:26

Update lib.rs

d33aab8

Trying to resolve the build error

Update lib.rs

84c6d60

sophokles73 reviewed Jul 30, 2025

View reviewed changes

Update src/lib.rs

265fba1

Co-authored-by: Kai Hudalla <sophokles.kh@gmail.com>

PLeVasseur requested changes Jul 31, 2025

View reviewed changes

vidishac2004 and others added 2 commits August 2, 2025 11:30

Remove .DS_Store and add it to .gitignore

d453dec

removed rawbytes

9376043

PLeVasseur requested changes Aug 23, 2025

View reviewed changes

PLeVasseur mentioned this pull request Aug 23, 2025

Implement send #76

Closed

PLeVasseur closed this Oct 29, 2025

Conversation

arakabCL commented Jul 25, 2025

Uh oh!

PLeVasseur left a comment

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!

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!

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!

PLeVasseur left a comment

Choose a reason for hiding this comment

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!

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!

Uh oh!

PLeVasseur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PLeVasseur commented Oct 29, 2025