Rpc client u subscription #324

MaximilianToe · 2025-06-09T10:45:28Z

This pull request provides the following:

An interface for a USubscriptionClient in USubscription.h
An Implementation of that interface in RpcClientUSubscription.cpp/RpcClientUSubscription.h
A template method InvokeProtoMethod to send and receive requests with a specific payload type in RpcClient.h similar to the one present in up-rust
A helper request builder and uuri builder
Extracting part of Consumer.cpp that was also needed for USubscription to avoid code duplication

github-actions · 2025-06-09T10:47:40Z

Code coverage report is ready! 📈

Code Coverage Percentage: 90.60%
Code Coverage Report: View Coverage Report

PLeVasseur

Hey @MaximilianToe -- thanks for contributing!

I noted more up-spec-related items and have tagged @sophokles73 and @stevenhartley onto the PR as there may be gaps in the spec (or in my understanding of it!)

include/up-cpp/client/usubscription/v3/RpcClientUSubscription.h

include/up-cpp/communication/RpcClient.h

include/up-cpp/client/usubscription/v3/RequestBuilder.h

include/up-cpp/utils/ProtoConverter.h

src/client/usubscription/v3/USubscriptionUUriBuilder.cpp

github-actions · 2025-06-30T17:32:05Z

Code coverage report is ready! 📈

Code Coverage Percentage: 90.50%
Code Coverage Report: View Coverage Report

sophokles73 · 2025-07-02T11:07:47Z

@PLeVasseur would you mind taking another look?

PLeVasseur · 2025-07-02T11:11:27Z

Yup, will do. Try to make it fit this week 👍

PLeVasseur

Thank you for the updates @MaximilianToe! Can you take a look at the comments I left?

It feels to me there's still a bit of alignment work to ensure that the C++ RpcClient API mimics the Rust equivalent. Happy to hear your thoughts.

include/up-cpp/communication/RpcClient.h

include/up-cpp/client/usubscription/v3/RequestBuilder.h

include/up-cpp/communication/RpcClient.h

github-actions · 2025-08-11T19:47:20Z

Code coverage report is ready! 📈

Code Coverage Percentage: 90.50%
Code Coverage Report: View Coverage Report

github-actions · 2025-08-24T16:32:39Z

Code coverage report is ready! 📈

Code Coverage Percentage: 90.50%
Code Coverage Report: View Coverage Report

github-actions · 2025-08-24T16:39:40Z

Code coverage report is ready! 📈

Code Coverage Percentage: 90.50%
Code Coverage Report: View Coverage Report

github-actions · 2025-08-24T16:47:32Z

Code coverage report is ready! 📈

Code Coverage Percentage: 90.50%
Code Coverage Report: View Coverage Report

github-actions · 2025-08-24T17:26:17Z

Code coverage report is ready! 📈

Code Coverage Percentage: 90.60%
Code Coverage Report: View Coverage Report

github-actions · 2025-08-24T17:47:58Z

Code coverage report is ready! 📈

Code Coverage Percentage: 90.60%
Code Coverage Report: View Coverage Report

github-actions · 2025-08-24T18:02:55Z

Code coverage report is ready! 📈

Code Coverage Percentage: 90.60%
Code Coverage Report: View Coverage Report

github-actions · 2025-08-24T19:03:28Z

Code coverage report is ready! 📈

Code Coverage Percentage: 90.60%
Code Coverage Report: View Coverage Report

# Conflicts: # include/up-cpp/communication/RpcClient.h

github-actions · 2025-08-24T19:08:55Z

Code coverage report is ready! 📈

Code Coverage Percentage: 90.60%
Code Coverage Report: View Coverage Report

PLeVasseur

Hi @MaximilianToe -- thanks for troubleshooting the thread-safety issue. Let's merge this then

PLeVasseur · 2025-08-25T01:18:25Z

@sophokles73 -- hmm, have we made a change recently to how the repos are set up?

As far as I can see from the .eclipsefdn repo's up-cpp portion I don't see two approvals as needed. 🤔

sophokles73 · 2025-08-25T08:26:44Z

@sophokles73 -- hmm, have we made a change recently to how the repos are set up?

As far as I can see from the .eclipsefdn repo's up-cpp portion I don't see two approvals as needed. 🤔

I did indeed make a change to the org configuration but that should only have resulted in merge commits no longer being allowed ... I'll try to see if I can find out anything. Maybe the EF has made some corresponding general config change to all its orgs?

PLeVasseur · 2025-08-25T20:33:55Z

@sophokles73 -- hmm, have we made a change recently to how the repos are set up?
As far as I can see from the .eclipsefdn repo's up-cpp portion I don't see two approvals as needed. 🤔

I did indeed make a change to the org configuration but that should only have resulted in merge commits no longer being allowed ... I'll try to see if I can find out anything. Maybe the EF has made some corresponding general config change to all its orgs?

Okay -- I'll ask about this on Slack 🫡

sophokles73 · 2025-08-26T06:46:24Z

Okay -- I'll ask about this on Slack 🫡

I have already created a PR to reduce the number of required reviews to 1 again ...

PLeVasseur · 2025-08-26T13:12:31Z

Okay -- I'll ask about this on Slack 🫡

I have already created a PR to reduce the number of required reviews to 1 again ...

Thanks! It does still seem to show as needing two reviews on this one tho 🤔

Is there some lingering effect on already-existing PRs or something like that?

sophokles73 · 2025-08-27T06:33:15Z

Okay -- I'll ask about this on Slack 🫡

I have already created a PR to reduce the number of required reviews to 1 again ...

Thanks! It does still seem to show as needing two reviews on this one tho 🤔
Is there some lingering effect on already-existing PRs or something like that?

Maybe, I don't know. Anyway, I will simply approve this PR now and then you can squash and merge it ...

sophokles73

LGTM

PLeVasseur reviewed Jun 16, 2025

View reviewed changes

PLeVasseur reviewed Jul 4, 2025

View reviewed changes

include/up-cpp/communication/RpcClient.h Outdated Show resolved Hide resolved

include/up-cpp/client/usubscription/v3/RequestBuilder.h Outdated Show resolved Hide resolved

include/up-cpp/communication/RpcClient.h Outdated Show resolved Hide resolved

MaximilianToe force-pushed the RpcClientUSubscription branch from 701bfdf to 6aae8ea Compare August 11, 2025 19:45

MaximilianToe force-pushed the RpcClientUSubscription branch from 2d22bb9 to 3b8a7ca Compare August 24, 2025 16:37

MaximilianToe force-pushed the RpcClientUSubscription branch from 3b8a7ca to 4375d15 Compare August 24, 2025 16:45

MaximilianToe force-pushed the RpcClientUSubscription branch from ccb349b to 15e8bcb Compare August 24, 2025 17:44

MaximilianToe force-pushed the RpcClientUSubscription branch from 15e8bcb to 0c41572 Compare August 24, 2025 17:59

MaximilianToe force-pushed the RpcClientUSubscription branch from 0c41572 to c8fdce4 Compare August 24, 2025 19:01

MaximilianToe added 8 commits August 24, 2025 21:05

first implementation of usubscription

b557c55

Added test and refactored some parts of usubscription

7354136

# Conflicts: # include/up-cpp/communication/RpcClient.h

made USubscription async with InvokeProtoFuture return type

2bd40b1

implemented cr comments

19fcfd1

rebase and use method in subscription calls

17ef0cf

renamed invokeProtoMethod

f665f7d

added local copies of builder in invokeMethod for thread safety

ce56279

new overloads for build method of UMessageBuilder to avoid copy

f899ee8

MaximilianToe force-pushed the RpcClientUSubscription branch from c8fdce4 to f899ee8 Compare August 24, 2025 19:05

PLeVasseur approved these changes Aug 25, 2025

View reviewed changes

mbarbero mentioned this pull request Aug 26, 2025

Remove repository level merge commit settings eclipse-uprotocol/.eclipsefdn#87

Merged

sophokles73 approved these changes Aug 27, 2025

View reviewed changes

PLeVasseur merged commit 91b6eec into eclipse-uprotocol:main Sep 9, 2025
12 checks passed

pkleymonov-qnx mentioned this pull request Oct 10, 2025

issue with non standard types: u_int32_t, u_int16_t #327

Open

Rpc client u subscription #324

Rpc client u subscription #324

Uh oh!

Conversation

MaximilianToe commented Jun 9, 2025

Uh oh!

github-actions bot commented Jun 9, 2025

Uh oh!

PLeVasseur 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!

github-actions bot commented Jun 30, 2025

Uh oh!

sophokles73 commented Jul 2, 2025

Uh oh!

PLeVasseur commented Jul 2, 2025

Uh oh!

PLeVasseur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Aug 11, 2025

Uh oh!

github-actions bot commented Aug 24, 2025

Uh oh!

github-actions bot commented Aug 24, 2025

Uh oh!

github-actions bot commented Aug 24, 2025

Uh oh!

github-actions bot commented Aug 24, 2025

Uh oh!

github-actions bot commented Aug 24, 2025

Uh oh!

github-actions bot commented Aug 24, 2025

Uh oh!

github-actions bot commented Aug 24, 2025

Uh oh!

github-actions bot commented Aug 24, 2025

Uh oh!

PLeVasseur left a comment

Choose a reason for hiding this comment

Uh oh!

PLeVasseur commented Aug 25, 2025

Uh oh!

sophokles73 commented Aug 25, 2025

Uh oh!

PLeVasseur commented Aug 25, 2025

Uh oh!

sophokles73 commented Aug 26, 2025

Uh oh!

PLeVasseur commented Aug 26, 2025

Uh oh!

sophokles73 commented Aug 27, 2025

Uh oh!

sophokles73 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants