diff --git a/include/thingset++/ThingSetClient.hpp b/include/thingset++/ThingSetClient.hpp index ff4c326..7c2d45f 100644 --- a/include/thingset++/ThingSetClient.hpp +++ b/include/thingset++/ThingSetClient.hpp @@ -8,7 +8,7 @@ #include "thingset++/ThingSetBinaryDecoder.hpp" #include "thingset++/ThingSetBinaryEncoder.hpp" #include "thingset++/ThingSetClientTransport.hpp" -#include "thingset++/ThingSetStatus.hpp" +#include "thingset++/ThingSetResult.hpp" #include "thingset++/internal/logging.hpp" namespace ThingSet { @@ -43,7 +43,7 @@ class ThingSetClient /// @param result A pointer to a variable which will hold the decoded return value. /// @param ...args The arguments to pass to the function. /// @return True if the function was successfully executed, otherwise false. - template bool exec(const uint16_t &id, Result *result, TArg... args) + template ThingSetResult exec(const uint16_t &id, Result *result, TArg... args) { return doRequest(id, ThingSetBinaryRequestType::exec, [&](auto encoder) { return encoder->encodeList(args...); }, result); } @@ -53,7 +53,7 @@ class ThingSetClient /// @param id The integer identifier of the function. /// @param ...args The arguments to pass to the function. /// @return True if the function was successfully executed, otherwise false. - template bool exec(const uint16_t &id, TArg... args) + template ThingSetResult exec(const uint16_t &id, TArg... args) { return doRequest(id, ThingSetBinaryRequestType::exec, [&](auto encoder) { return encoder->encodeList(args...); }); } @@ -63,7 +63,7 @@ class ThingSetClient /// @param id The integer identifier of the value. /// @param result A reference to a variable which will hold the retrieved value. /// @return True if retrieval succeeded, otherwise false. - template bool get(const uint16_t &id, T &result) + template ThingSetResult get(const uint16_t &id, T &result) { return doRequest(id, ThingSetBinaryRequestType::get, [](auto) { return true; }, &result); } @@ -73,20 +73,20 @@ class ThingSetClient /// @param id The string identifier of the value. /// @param result A reference to a variable which will hold the retrieved value. /// @return True if retrieval succeeded, otherwise false. - template bool get(const std::string &id, T &result) + template ThingSetResult get(const std::string &id, T &result) { return doRequest(id, ThingSetBinaryRequestType::get, [](auto) { return true; }, &result); } template requires std::is_integral_v or std::is_convertible_v - bool fetch(const Id &id, std::vector &result) + ThingSetResult fetch(const Id &id, std::vector &result) { return doRequest(id, ThingSetBinaryRequestType::fetch, [](auto encoder) { return encoder->encodeNull(); }, &result); } template - bool update(const uint16_t &id, const T &value) + ThingSetResult update(const uint16_t &id, const T &value) { // use root node as parent ID for ID-based updates return doRequest(0, ThingSetBinaryRequestType::update, [&](auto encoder) { @@ -97,7 +97,7 @@ class ThingSetClient }); } - template bool update(const std::string &fullyQualifiedName, const T &value) + template ThingSetResult update(const std::string &fullyQualifiedName, const T &value) { size_t index = fullyQualifiedName.find_last_of('/'); const std::string pathToParent = index != std::string::npos ? fullyQualifiedName.substr(0, index) : ""; @@ -116,25 +116,29 @@ class ThingSetClient /// @param id The integer identifier of the value to get or function to invoke, etc. /// @param type The request type. /// @param encode A function to encode any additional data in a request. - /// @param result A pointer to a variable which will contain the decoded result, if any. Pass null if no result is expected. + /// @param value A pointer to a variable which will contain the decoded result, if any. Pass null if no result is expected. /// @return True if invocation succeeded, otherwise false. template requires std::is_integral_v or std::is_convertible_v - bool doRequest(const Id &id, ThingSetBinaryRequestType type, std::function encode, T *result) + ThingSetResult doRequest(const Id &id, ThingSetBinaryRequestType type, std::function encode, T *value) { uint8_t *responseBuffer; size_t responseSize; - if (doRequestCore(id, type, encode, &responseBuffer, responseSize)) { + ThingSetResult result = doRequestCore(id, type, encode, &responseBuffer, responseSize); + if (result) { // decode result into pointer FixedDepthThingSetBinaryDecoder decoder(responseBuffer, responseSize); - return decoder.decode(result); + if (decoder.decode(value)) { + return result; + } + return ThingSetResult(ThingSetStatusCode::internalServerError); } - return false; + return result; } template requires std::is_integral_v or std::is_convertible_v - bool doRequest(const Id &id, ThingSetBinaryRequestType type, std::function encode) + ThingSetResult doRequest(const Id &id, ThingSetBinaryRequestType type, std::function encode) { uint8_t *responseBuffer; size_t responseSize; @@ -143,28 +147,28 @@ class ThingSetClient template requires std::is_integral_v or std::is_convertible_v - bool doRequestCore(const Id &id, ThingSetBinaryRequestType type, std::function encode, uint8_t **responseBuffer, size_t &responseSize) + ThingSetResult doRequestCore(const Id &id, ThingSetBinaryRequestType type, std::function encode, uint8_t **responseBuffer, size_t &responseSize) { _txBuffer[0] = (uint8_t)type; FixedDepthThingSetBinaryEncoder encoder(_txBuffer + 1, _txBufferSize - 1); if (!encoder.encode(id) || !encode(&encoder)) { - return false; + return ThingSetResult(ThingSetStatusCode::requestIncomplete); } if (!_transport.write(_txBuffer, 1 + encoder.getEncodedLength())) { LOG_ERROR("Failed to send request"); - return false; + return ThingSetResult(ThingSetStatusCode::requestIncomplete); } - if (!read(responseBuffer, responseSize)) { + ThingSetResult result = read(responseBuffer, responseSize); + if (!result) { LOG_ERROR("Failed to read response"); - return false; } - return true; + return result; } - bool read(uint8_t **responseBuffer, size_t &responseSize); + ThingSetResult read(uint8_t **responseBuffer, size_t &responseSize); }; } // namespace ThingSet diff --git a/include/thingset++/ThingSetResult.hpp b/include/thingset++/ThingSetResult.hpp new file mode 100644 index 0000000..ae344fc --- /dev/null +++ b/include/thingset++/ThingSetResult.hpp @@ -0,0 +1,26 @@ +/* + * Copyright (c) 2026 Brill Power. + * + * SPDX-License-Identifier: Apache-2.0 + */ +#pragma once + +#include "thingset++/ThingSetStatus.hpp" + +namespace ThingSet { + +class ThingSetResult +{ +private: + const ThingSetStatusCode _code; + +public: + ThingSetResult(const ThingSetStatusCode code); + + bool success() const; + ThingSetStatusCode code() const; + + explicit operator bool() const; +}; + +} \ No newline at end of file diff --git a/include/thingset++/can/zephyr/ThingSetZephyrCanRequestResponseContext.hpp b/include/thingset++/can/zephyr/ThingSetZephyrCanRequestResponseContext.hpp index b8d6602..a906c33 100644 --- a/include/thingset++/can/zephyr/ThingSetZephyrCanRequestResponseContext.hpp +++ b/include/thingset++/can/zephyr/ThingSetZephyrCanRequestResponseContext.hpp @@ -12,6 +12,8 @@ extern "C" { #include } +#define THINGSET_PLUS_PLUS_ZEPHYR_CAN_FILTER_ID_NONE -1 + namespace ThingSet::Can::Zephyr { class ThingSetZephyrCanInterface; @@ -33,6 +35,7 @@ class ThingSetZephyrCanRequestResponseContext { std::array &txBuffer) : _canInterface(canInterface), _rxBuffer(rxBuffer.data()), _rxBufferSize(RxSize), _txBuffer(txBuffer.data()), _txBufferSize(TxSize) { + _requestResponseContext.filter_id = THINGSET_PLUS_PLUS_ZEPHYR_CAN_FILTER_ID_NONE; k_sem_init(&_lock, 1, 1); } ~ThingSetZephyrCanRequestResponseContext(); @@ -45,6 +48,7 @@ class ThingSetZephyrCanRequestResponseContext { bool send(const uint8_t otherNodeAddress, uint8_t *buffer, size_t len); private: + void unbindIfNecessary(); static void onRequestResponseReceived(net_buf *buffer, int remainingLength, isotp_fast_addr address, void *arg); void onRequestResponseReceived(net_buf *buffer, int remainingLength, isotp_fast_addr address); static const isotp_fast_opts flowControlOptions; diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 75c0f0c..b741401 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -27,7 +27,7 @@ endif() if(ENABLE_CLIENT) message("ThingSet++ client enabled") - target_sources(thingset++ PRIVATE ThingSetClient.cpp) + target_sources(thingset++ PRIVATE ThingSetClient.cpp ThingSetResult.cpp) endif() if(ENABLE_ASIO OR ENABLE_SOCKETS) diff --git a/src/ThingSetClient.cpp b/src/ThingSetClient.cpp index a512ef7..d2ecd7f 100644 --- a/src/ThingSetClient.cpp +++ b/src/ThingSetClient.cpp @@ -20,11 +20,11 @@ bool ThingSetClient::connect() return _transport.connect(); } -bool ThingSetClient::read(uint8_t **responseBuffer, size_t &responseSize) +ThingSetResult ThingSetClient::read(uint8_t **responseBuffer, size_t &responseSize) { responseSize = _transport.read(_rxBuffer, _rxBufferSize); if (responseSize == 0) { - return false; + return ThingSetResult(ThingSetStatusCode::internalServerError); } #ifdef DEBUG_LOGGING @@ -38,25 +38,21 @@ bool ThingSetClient::read(uint8_t **responseBuffer, size_t &responseSize) printf("\n"); #endif - switch (_rxBuffer[0]) { - case ThingSetStatusCode::content: - case ThingSetStatusCode::changed: - case ThingSetStatusCode::deleted: - break; - default: - return false; + ThingSetResult result = ThingSetResult((ThingSetStatusCode)_rxBuffer[0]); + if (!result) { + return result; } // first value is always a CBOR null if (_rxBuffer[1] != 0xF6) { - return false; + return ThingSetResult(ThingSetStatusCode::internalServerError); } // return size having accounted for response code and null responseSize -= 2; *responseBuffer = &_rxBuffer[2]; - return true; + return result; } } // namespace ThingSet \ No newline at end of file diff --git a/src/ThingSetResult.cpp b/src/ThingSetResult.cpp new file mode 100644 index 0000000..5f73af4 --- /dev/null +++ b/src/ThingSetResult.cpp @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2026 Brill Power. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "thingset++/ThingSetResult.hpp" + +namespace ThingSet { + +ThingSetResult::ThingSetResult(ThingSetStatusCode code) : _code(code) +{} + +ThingSetStatusCode ThingSetResult::code() const { + return _code; +} + +bool ThingSetResult::success() const { + switch (_code) { + case ThingSetStatusCode::content: + case ThingSetStatusCode::changed: + case ThingSetStatusCode::created: + case ThingSetStatusCode::deleted: + return true; + default: + return false; + } +} + +ThingSetResult::operator bool() const { + return success(); +} + +} \ No newline at end of file diff --git a/src/can/zephyr/ThingSetZephyrCanRequestResponseContext.cpp b/src/can/zephyr/ThingSetZephyrCanRequestResponseContext.cpp index a5e14c9..6c6acb5 100644 --- a/src/can/zephyr/ThingSetZephyrCanRequestResponseContext.cpp +++ b/src/can/zephyr/ThingSetZephyrCanRequestResponseContext.cpp @@ -40,8 +40,7 @@ static void onRequestResponseSent(int result, isotp_fast_addr addr, void *arg); ThingSetZephyrCanRequestResponseContext::~ThingSetZephyrCanRequestResponseContext() { - // TODO: check if bound - isotp_fast_unbind(&_requestResponseContext); + unbindIfNecessary(); } ThingSetZephyrCanInterface &ThingSetZephyrCanRequestResponseContext::getInterface() @@ -54,8 +53,17 @@ bool ThingSetZephyrCanRequestResponseContext::bind(std::function THINGSET_PLUS_PLUS_ZEPHYR_CAN_FILTER_ID_NONE) { + isotp_fast_unbind(&_requestResponseContext); + _requestResponseContext.filter_id = THINGSET_PLUS_PLUS_ZEPHYR_CAN_FILTER_ID_NONE; + } +} + bool ThingSetZephyrCanRequestResponseContext::bind(uint8_t otherNodeAddress, std::function callback) { + unbindIfNecessary(); auto canId = CanID() .setMessageType(MessageType::requestResponse) .setMessagePriority(MessagePriority::channel) diff --git a/tests/TestAsioIpClientServer.cpp b/tests/TestAsioIpClientServer.cpp index a00ef1b..7516348 100644 --- a/tests/TestAsioIpClientServer.cpp +++ b/tests/TestAsioIpClientServer.cpp @@ -68,12 +68,16 @@ ASIO_TEST(GetFloatByName, ASIO_TEST(NotFoundById, float tv; - ASSERT_FALSE(client.get(0x1300, tv)); + auto result = client.get(0x1300, tv); + ASSERT_FALSE(result); + ASSERT_EQ(ThingSetStatusCode::notFound, result.code()); ) ASIO_TEST(NotFoundByName, float tv; - ASSERT_FALSE(client.get("notTotalVoltage", tv)); + auto result = client.get("notTotalVoltage", tv); + ASSERT_FALSE(result); + ASSERT_EQ(ThingSetStatusCode::notFound, result.code()); ) ASIO_TEST(GetFunction, @@ -82,9 +86,11 @@ ASIO_TEST(GetFunction, ) ASIO_TEST(ExecFunction, - int result; - ASSERT_TRUE(client.exec(0x1000, &result, 2, 3)); - ASSERT_EQ(5, result); + int value; + auto result = client.exec(0x1000, &value, 2, 3); + ASSERT_TRUE(result); + ASSERT_EQ(ThingSetStatusCode::changed, result.code()); + ASSERT_EQ(5, value); ) ASIO_TEST(FetchFunctionParameters, @@ -101,19 +107,27 @@ ASIO_TEST(FetchFunctionParameters, ) ASIO_TEST(UpdateFloatByName, - ASSERT_TRUE(client.update("totalVoltage", 25.0f)); + auto result = client.update("totalVoltage", 25.0f); + ASSERT_TRUE(result); + ASSERT_EQ(ThingSetStatusCode::changed, result.code()); ASSERT_EQ(25.0, totalVoltage.getValue()); ) ASIO_TEST(UpdateFloatById, - ASSERT_TRUE(client.update(0x300, 26.0f)); + auto result = client.update(0x300, 26.0f); + ASSERT_TRUE(result); + ASSERT_EQ(ThingSetStatusCode::changed, result.code()); ASSERT_EQ(26.0, totalVoltage.getValue()); ) ASIO_TEST(CannotUpdateFunctionByName, - ASSERT_FALSE(client.update("xAddNumber", 26.0f)); + auto result = client.update("xAddNumber", 26.0f); + ASSERT_FALSE(result); + ASSERT_EQ(ThingSetStatusCode::badRequest, result.code()); ) ASIO_TEST(CannotUpdateFunctionById, - ASSERT_FALSE(client.update(0x1000, 26.0f)); + auto result = client.update(0x1000, 26.0f); + ASSERT_FALSE(result); + ASSERT_EQ(ThingSetStatusCode::badRequest, result.code()); ) \ No newline at end of file diff --git a/tests/zephyr/can/src/main.cpp b/tests/zephyr/can/src/main.cpp index 69ea65f..13ef6c5 100644 --- a/tests/zephyr/can/src/main.cpp +++ b/tests/zephyr/can/src/main.cpp @@ -40,14 +40,16 @@ K_THREAD_STACK_DEFINE(serverStack, CONFIG_ARCH_POSIX_RECOMMENDED_STACK_SIZE); k_thread clientThread; K_THREAD_STACK_DEFINE(clientStack, CONFIG_ARCH_POSIX_RECOMMENDED_STACK_SIZE); -k_sem clientCompleted; +k_sem serverStarted; k_sem serverCompleted; +k_sem clientCompleted; static void runServer(void *, void *, void *) { LOG_INF("Server starting up"); auto server = ThingSetServerBuilder::build(serverTransport); server.listen(); + k_sem_give(&serverStarted); k_sem_take(&clientCompleted, K_FOREVER); LOG_INF("Server shutting down"); k_sem_give(&serverCompleted); @@ -70,10 +72,13 @@ static k_tid_t createAndRunClient(k_thread_entry_t runner) #define ZCLIENT_SERVER_TEST(test_name, Body) \ ZTEST(ZephyrClientServer, test_name) \ { \ - k_sem_init(&clientCompleted, 0, 1); \ + k_sem_init(&serverStarted, 0, 1); \ k_sem_init(&serverCompleted, 0, 1); \ + k_sem_init(&clientCompleted, 0, 1); \ \ createAndRunServer(); \ +\ + k_sem_take(&serverStarted, K_FOREVER); \ \ createAndRunClient([](auto, auto, auto) \ { \ @@ -94,17 +99,26 @@ ZTEST(ZephyrClientServer, test_name) \ ZCLIENT_SERVER_TEST(test_get_float, float tv; - zassert_true(client.get(0x300, tv)); + auto result = client.get(0x300, tv); + zassert_true(result.success()); + LOG_INF("result.code: 0x%x", result.code()); + zassert_equal(ThingSetStatusCode::content, result.code()); ) ZCLIENT_SERVER_TEST(test_exec_function, - int result; - zassert_true(client.exec(0x1000, &result, 2, 3)); - zassert_equal(5, result); + int value; + auto result = client.exec(0x1000, &value, 2, 3); + zassert_true(result.success()); + LOG_INF("result.code: 0x%x", result.code()); + zassert_equal(ThingSetStatusCode::changed, result.code()); + zassert_equal(5, value); ) ZCLIENT_SERVER_TEST(test_update, - zassert_true(client.update("totalVoltage", 25.0f)); + auto result = client.update("totalVoltage", 25.0f); + zassert_true(result.success()); + LOG_INF("result.code: 0x%x", result.code()); + zassert_equal(ThingSetStatusCode::changed, result.code()); k_sleep(K_MSEC(100)); // `update` is async or something zassert_equal(25.0f, totalVoltage.getValue()); )