Skip to content

Commit f846b9e

Browse files
committed
fix: RpcCall error not private and thread safe
1 parent bf18094 commit f846b9e

File tree

6 files changed

+56
-19
lines changed

6 files changed

+56
-19
lines changed

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,9 @@ void loop() {
5757
if (!async_rpc.result(sum)) {
5858
Monitor.println("Error calling method: add");
5959
Monitor.print("Error code: ");
60-
Monitor.println(async_rpc.error.code);
60+
Monitor.println(async_rpc.getErrorCode);
6161
Monitor.print("Error message: ");
62-
Monitor.println(async_rpc.error.traceback);
62+
Monitor.println(async_rpc.getErrorMessage);
6363
}
6464

6565
// Implicit boolean cast. Use with caution as in this case the call is indeed

examples/simple_bridge/simple_bridge.ino

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ void loop() {
5959
Serial.print("Result of the operation is: ");
6060
Serial.println(res);
6161
} else {
62-
Serial.println(outcome.error.code);
63-
Serial.println(outcome.error.traceback);
62+
Serial.println(outcome.getErrorCode());
63+
Serial.println(outcome.getErrorMessage());
6464
}
6565

6666
Bridge.notify("signal", 200);

examples/test/test.ino

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,34 +64,34 @@ void loop() {
6464
if (async_res.result(pow)) {
6565
Monitor.println("Result of assignment and then result: "+String(pow)); // returns true, so the right result
6666
} else {
67-
Monitor.println("Error code: "+String(async_res.error.code));
68-
Monitor.println("Error message: "+async_res.error.traceback);
67+
Monitor.println("Error code: "+String(async_res.getErrorCode()));
68+
Monitor.println("Error message: "+async_res.getErrorMessage());
6969
}
7070

7171
float div = 0;
7272
RpcCall async_res1 = Bridge.call("2_args_float_result", 2.0); // passing 1 arg when 2 are expected
7373
if (async_res1.result(div)) {
7474
Monitor.println("Result of assignment and then result: "+String(div)); // returns true, so the right result
7575
} else {
76-
Monitor.println("Error code: "+String(async_res1.error.code));
77-
Monitor.println("Error message: "+async_res1.error.traceback);
76+
Monitor.println("Error code: "+String(async_res1.getErrorCode()));
77+
Monitor.println("Error message: "+async_res1.getErrorMessage());
7878
}
7979

8080
div = 0;
8181
RpcCall async_res2 = Bridge.call("2_args_float_result", 2.0, "invalid"); // passing a wrong type arg
8282
if (async_res2.result(div)) {
8383
Monitor.println("Result of assignment and then result: "+String(div)); // returns true, so the right result
8484
} else {
85-
Monitor.println("Error code: "+String(async_res2.error.code));
86-
Monitor.println("Error message: "+async_res2.error.traceback);
85+
Monitor.println("Error code: "+String(async_res2.getErrorCode()));
86+
Monitor.println("Error message: "+async_res2.getErrorMessage());
8787
}
8888

8989
x = false;
9090
RpcCall async_res3 = Bridge.call("0_args_bool_result");
9191
if (async_res3.result(x)) {
9292
Monitor.println("Result of assignment and then result: "+String(x)); // returns true, so the right result
9393
} else {
94-
Monitor.println("Error expecting bool result: "+String(async_res3.error.code));
94+
Monitor.println("Error expecting bool result: "+String(async_res3.getErrorCode()));
9595
}
9696

9797
// Avoid the following:

src/bridge.h

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,56 @@
2727
#include <zephyr/sys/atomic.h>
2828
#include <Arduino_RPClite.h>
2929

30+
#include <utility>
31+
3032

3133
void updateEntryPoint(void *, void *, void *);
3234

3335
template<typename... Args>
3436
class RpcCall {
37+
38+
RpcError error;
39+
40+
void setError(int code, MsgPack::str_t text) {
41+
k_mutex_lock(&call_mutex, K_FOREVER);
42+
error.code = code;
43+
error.traceback = std::move(text);
44+
k_mutex_unlock(&call_mutex);
45+
}
46+
3547
public:
36-
RpcError error{GENERIC_ERR, "This call is not executed yet"};
3748

38-
RpcCall(const MsgPack::str_t& m, RPCClient* c, struct k_mutex* rm, struct k_mutex* wm, Args&&... args): method(m), client(c), read_mutex(rm), write_mutex(wm), callback_params(std::forward_as_tuple(std::forward<Args>(args)...)) {}
49+
RpcCall(const MsgPack::str_t& m, RPCClient* c, struct k_mutex* rm, struct k_mutex* wm, Args&&... args): method(m), client(c), read_mutex(rm), write_mutex(wm), callback_params(std::forward_as_tuple(std::forward<Args>(args)...)) {
50+
k_mutex_init(&call_mutex);
51+
setError(GENERIC_ERR, "This call is not yet executed");
52+
}
53+
54+
bool isError() {
55+
k_mutex_lock(&call_mutex, K_FOREVER);
56+
const bool out = error.code > NO_ERR;
57+
k_mutex_unlock(&call_mutex);
58+
return out;
59+
}
60+
61+
int getErrorCode() {
62+
k_mutex_lock(&call_mutex, K_FOREVER);
63+
const int out = error.code;
64+
k_mutex_unlock(&call_mutex);
65+
return out;
66+
}
67+
68+
MsgPack::str_t getErrorMessage() {
69+
k_mutex_lock(&call_mutex, K_FOREVER);
70+
MsgPack::str_t out = error.traceback;
71+
k_mutex_unlock(&call_mutex);
72+
return out;
73+
}
3974

4075
template<typename RType> bool result(RType& result) {
4176

4277
if (!atomic_cas(&_executed, 0, 1)){
4378
// this thread lost the race
44-
error.code = GENERIC_ERR;
45-
error.traceback = "This call result is no longer available";
79+
setError(GENERIC_ERR, "This call is no longer available");
4680
return false;
4781
}
4882

@@ -60,13 +94,15 @@ class RpcCall {
6094

6195
while(true) {
6296
if (k_mutex_lock(read_mutex, K_MSEC(10)) == 0 ) {
63-
if (client->get_response(msg_id_wait, result, error)) {
97+
RpcError temp_err;
98+
if (client->get_response(msg_id_wait, result, temp_err)) {
6499
k_mutex_unlock(read_mutex);
65100
// if (error.code == PARSING_ERR) {
66101
// k_mutex_lock(write_mutex, K_FOREVER);
67102
// client->notify(BRIDGE_ERROR, error.traceback);
68103
// k_mutex_unlock(write_mutex);
69104
// }
105+
setError(temp_err.code, temp_err.traceback);
70106
break;
71107
}
72108
k_mutex_unlock(read_mutex);
@@ -76,7 +112,7 @@ class RpcCall {
76112
}
77113
}
78114

79-
return error.code == NO_ERR;
115+
return !isError();
80116
}
81117

82118
bool result() {
@@ -100,6 +136,7 @@ class RpcCall {
100136
RPCClient* client;
101137
struct k_mutex* read_mutex;
102138
struct k_mutex* write_mutex;
139+
struct k_mutex call_mutex{};
103140
std::tuple<Args...> callback_params;
104141
};
105142

src/monitor.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ class BridgeMonitor: public Stream {
151151
}
152152
}
153153

154-
// if (async_rpc.error.code > NO_ERR) {
154+
// if (async_rpc.getErrorCode() > NO_ERR) {
155155
// _connected = false;
156156
// }
157157

src/tcp_client.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ class BridgeTCPClient : public Client {
202202
}
203203
}
204204

205-
if (async_rpc.error.code > NO_ERR) {
205+
if (async_rpc.getErrorCode() > NO_ERR) {
206206
_connected = false;
207207
}
208208

0 commit comments

Comments
 (0)