Feature/rpc alignment #21

MelamudMichael · 2024-03-17T17:54:13Z

Using uattributes from protobuf
Aligning invokeMethod to up-core 1.5.7
Added invokeMethod with Cb
Added unit-tests for RPC client

Remove the init/term functions in favor of using the constructor and destructor for the sub classes, and use a shared_ptr to do the reference counting. Signed-off-by: Bill Pittman <william.1.pittman@gm.com>

Signed-off-by: Bill Pittman <william.1.pittman@gm.com>

gregmedd · 2024-03-20T20:34:09Z

lib/include/up-client-zenoh-cpp/rpc/zenohRpcClient.h

+            uprotocol::v1::UStatus invokeMethod(const uprotocol::v1::UUri &topic,
+                                                const uprotocol::utransport::UPayload &payload,
+                                                const uprotocol::v1::CallOptions &options,
+                                                const uprotocol::utransport::UListener &callback) noexcept;


This would need to be updated if UListener is changed to replace functors with lambdas. See my comment on the up-cpp PR

Not going to be handled part of this release

@MelamudMichael if something is not going to be resolved, add a link to the issue that will be used to track the change.

lib/src/zenohRpcClient.cpp

Signed-off-by: Bill Pittman <william.1.pittman@gm.com>

Feature/up client zenoh

MelamudMichael · 2024-04-07T07:28:47Z

lib/src/upZenohClient.cpp

+            return handle;
+        } else {
+            return nullptr;


@billpittman add print

MelamudMichael · 2024-04-07T07:29:10Z

lib/src/upZenohClient.cpp

+
+        handle = std::make_shared<upZenohClient>(ConstructToken());
+        if (handle->rpcSuccess_.code() == UCode::OK && handle->uSuccess_.code() == UCode::OK) {


@billpittman please rename uSucess to uTransportSuccess

@billpittman please add brackets around each condition

MelamudMichael · 2024-04-07T07:31:34Z

lib/src/zenohRpcClient.cpp

-    UStatus status;
+    if (UCode::OK != ZenohSessionManager::instance().init(config)) {
+       spdlog::error("zenohSessionManager::instance().init() failed");
+       rpcSuccess_.set_code(UCode::UNAVAILABLE);


@billpittman instead of setting the code in each failure, set it to failure by default in the beginning of the function

MelamudMichael · 2024-04-07T07:32:03Z

lib/src/zenohRpcClient.cpp

-            }
+ZenohRpcClient::~ZenohRpcClient() noexcept {
+    if (UCode::OK != ZenohSessionManager::instance().term()) {
+        spdlog::error("zenohSessionManager::instance().term() failed");


@billpittman do return in case of failure

lib/src/zenohRpcClient.cpp

MelamudMichael · 2024-04-09T08:45:16Z

lib/src/zenohRpcClient.cpp


    if (0 == refCount_) {
        spdlog::error("ZenohRpcClient is not initialized");
        return future;
    }

-    if (false == isRPCMethod(uri.resource())) {
+    if (false == isRPCMethod(topic.resource())) {
        spdlog::error("URI is not of RPC type");


@stevenhartley

MelamudMichael · 2024-04-09T08:45:24Z

lib/include/up-client-zenoh-cpp/rpc/zenohRpcClient.h

+             * get queue size
+             * @return queue size
+            */
+            size_t getQueueSize() {


@stevenhartley

MelamudMichael · 2024-04-09T08:45:52Z

lib/include/up-client-zenoh-cpp/rpc/zenohRpcClient.h

+             * get the number of max concurrent request 
+             * @return number of concurrent requests
+            */
+            size_t getMaxConcurrentRequests() {


@stevenhartley

stevenhartley

Please see comments

stevenhartley · 2024-04-09T14:52:35Z

lib/include/up-client-zenoh-cpp/rpc/zenohRpcClient.h

+            uprotocol::v1::UStatus invokeMethod(const uprotocol::v1::UUri &topic,
+                                                const uprotocol::utransport::UPayload &payload,
+                                                const uprotocol::v1::CallOptions &options,
+                                                const uprotocol::utransport::UListener &callback) noexcept;


@MelamudMichael if something is not going to be resolved, add a link to the issue that will be used to track the change.

stevenhartley · 2024-04-09T14:53:42Z

lib/include/up-client-zenoh-cpp/rpc/zenohRpcClient.h

+             * get the number of max concurrent request 
+             * @return number of concurrent requests
+            */
+            size_t getMaxConcurrentRequests() {


@MelamudMichael, could we not just fail if the max concurrent requests fail? I'm worried this is adding complexity to the app developer that they do not need.

stevenhartley · 2024-04-09T14:54:34Z

lib/include/up-client-zenoh-cpp/rpc/zenohRpcClient.h

+             * get queue size
+             * @return queue size
+            */
+            size_t getQueueSize() {


@MelamudMichael if you expose this logic to the app developer, they need to maintain a list/queue of requests as well, I don't think we should do this.

stevenhartley

Merging for now, open items will be addressed later.

monierabdullah and others added 30 commits February 20, 2024 17:52

Resolving Code Review Comments

5d7b3df

Post-Code-Review minor changes

0565041

Merge remote-tracking branch 'main_stream/main'

97e4088

Adjusting code to use protobuf files

84fd49f

Update README.md

40ef4bd

Delete samples directory

22a9728

Minor changes based on code review

0cf2ecb

Merge branch 'eclipse-uprotocol:main' into main

dd0e27d

Fixing merge conflicts

08ba408

Merge branch 'eclipse-uprotocol:main' into compilation_issues_fix

0122863

Attachment handling in handleReply

3406bcc

cleanup

a0b24be

removing file

f0c47db

Merge branch 'eclipse-uprotocol:main' into main

c4fbf18

enabling full build

783f9f0

Update CMakeLists.txt

39fecbe

fixing compilation

8ab3cf3

fixing compilation

c5aa5c8

updating version

9c08214

Update conanfile.py

94779f7

Update ci.yml

238b3e3

Delete conaninfo directory

8d7a232

Update ci.yml

6e24e1f

Update conanfile.py

fd385a6

Update ci.yml

72ce57b

Update conanfile.py

bc234f0

Update ci.yml

67e535e

Update CMakeLists.txt

a76d23c

Update CMakeLists.txt

2eb7752

Update CMakeLists.txt

2950ae7

billpittman added 2 commits March 20, 2024 11:40

upZenohClient: Remove init/term functions

7226534

Remove the init/term functions in favor of using the constructor and destructor for the sub classes, and use a shared_ptr to do the reference counting. Signed-off-by: Bill Pittman <william.1.pittman@gm.com>

main_test: Update to use shared_ptr

44374d6

Signed-off-by: Bill Pittman <william.1.pittman@gm.com>

gregmedd reviewed Mar 20, 2024

View reviewed changes

MelamudMichael and others added 16 commits March 25, 2024 06:20

test cleanup

919b56c

cleanup

d3cfa07

merge

b619a95

adding unit-tests and fixing

0e7b053

cleanup

3f66341

upZenohClient: force use of Instance function

8bf96e3

Signed-off-by: Bill Pittman <william.1.pittman@gm.com>

aligning uproto

bddc2a5

aligning

2225676

Merge pull request #7 from billpittman/feature/upClientZenoh

de68cf5

Feature/up client zenoh

Merge branch 'main' into feature/rpc_alignment

2ec8d11

fixing compilation

0d15bd3

cleanup

f3b2f06

cleanup

df63083

Merge branch 'feature/rpc_alignment_latency' into feature/rpc_alignment

6883244

fixing runtime and compilation issues

81c300a

code review fixes and compilation

459bb7b

gregmedd approved these changes Apr 8, 2024

View reviewed changes

MelamudMichael added 5 commits April 9, 2024 14:31

Update CMakeLists.txt

abc8a0c

Update conanfile.py

4b10027

Update CMakeLists.txt

fcf94fd

Update CMakeLists.txt

a1fae21

Update ci.yml

9d0c790

MelamudMichael commented Apr 9, 2024

View reviewed changes

stevenhartley requested changes Apr 9, 2024

View reviewed changes

stevenhartley approved these changes Apr 9, 2024

View reviewed changes

stevenhartley merged commit 8c03c2a into eclipse-uprotocol:main Apr 9, 2024
3 checks passed

gregmedd mentioned this pull request Apr 10, 2024

Feature/1 5 7 alignment #22

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/rpc alignment #21

Feature/rpc alignment #21

MelamudMichael commented Mar 17, 2024 •

edited

Loading

gregmedd Mar 20, 2024

MelamudMichael Apr 4, 2024 •

edited

Loading

stevenhartley Apr 9, 2024

MelamudMichael Apr 7, 2024

MelamudMichael Apr 7, 2024

MelamudMichael Apr 7, 2024

MelamudMichael Apr 7, 2024

MelamudMichael Apr 7, 2024

MelamudMichael Apr 9, 2024

MelamudMichael Apr 9, 2024

MelamudMichael Apr 9, 2024

stevenhartley left a comment

stevenhartley Apr 9, 2024

stevenhartley Apr 9, 2024

stevenhartley Apr 9, 2024

stevenhartley left a comment


		handle = std::make_shared<upZenohClient>(ConstructToken());
		if (handle->rpcSuccess_.code() == UCode::OK && handle->uSuccess_.code() == UCode::OK) {

Feature/rpc alignment #21

Feature/rpc alignment #21

Conversation

MelamudMichael commented Mar 17, 2024 • edited Loading

Choose a reason for hiding this comment

MelamudMichael Apr 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenhartley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenhartley left a comment

Choose a reason for hiding this comment

MelamudMichael commented Mar 17, 2024 •

edited

Loading

MelamudMichael Apr 4, 2024 •

edited

Loading