Skip to content

Conversation

@PLeVasseur
Copy link
Contributor

@PLeVasseur PLeVasseur commented Mar 16, 2024

Hey there 👋

Based on #65, I took my (second) shot at aligning our UTransport trait closer to the spec.

In particular, I have removed the requirement for the implementation to explicitly manage listeners using String identifiers, instead leaning on concrete implementations of the UListener trait.

This change is to align with registerListener() and unregisterListener() by bringing in a UListener trait.

As you read the code, you can see that there was a need for a new wrapper type called ListenerWrapper around UListener in order to allow us to disambiguate between concrete impls of it. The wrapper is also hashable to make it easy to use within containers that require that such as HashSet and HashMap. The ListenerWrapper need only be used in the up-client-foo-rust implementation. uEs just impl UListener.

Largely resolves the issues I had with the first attempt over in #67 thanks to that wrapper type.

Note: Over in a PR onto the up-client-zenoh-rust repo, I'm working on fleshing out how the changes will effect a up-client-foo-rust implementation, to get a feel for if this is a reasonable approach. This is now out of date, see #69 for details

I'm looking to see how this would impact current and future implementations of UTransport for UPClients. Feedback would be great from @AnotherDaniel, @sophokles73, @evshary in particular 🙂

@PLeVasseur
Copy link
Contributor Author

Closing in favor of #69, where we use the same semantics as up-client-android-java of using reference equality of UListener instances instead of type equality as we were in this approach.

@PLeVasseur PLeVasseur closed this Mar 17, 2024
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.

1 participant