Make client results more informative; fix Zephyr CAN transport

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 <typename Result, typename... TArg> bool exec(const uint16_t &id, Result *result, TArg... args)
+    template <typename Result, typename... TArg> 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 <typename... TArg> bool exec(const uint16_t &id, TArg... args)
+    template <typename... TArg> 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 <typename T> bool get(const uint16_t &id, T &result)
+    template <typename T> 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 <typename T> bool get(const std::string &id, T &result)
+    template <typename T> ThingSetResult get(const std::string &id, T &result)
     {
         return doRequest(id, ThingSetBinaryRequestType::get, [](auto) { return true; }, &result);
     }
 
     template <typename Id>
         requires std::is_integral_v<Id> or std::is_convertible_v<Id, std::string_view>
-    bool fetch(const Id &id, std::vector<Id> &result)
+    ThingSetResult fetch(const Id &id, std::vector<Id> &result)
     {
         return doRequest(id, ThingSetBinaryRequestType::fetch, [](auto encoder) { return encoder->encodeNull(); }, &result);
     }
 
     template <typename T>
-    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 <typename T> bool update(const std::string &fullyQualifiedName, const T &value)
+    template <typename T> 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 <typename Id, typename T>
         requires std::is_integral_v<Id> or std::is_convertible_v<Id, std::string_view>
-    bool doRequest(const Id &id, ThingSetBinaryRequestType type, std::function<bool (ThingSetBinaryEncoder *)> encode, T *result)
+    ThingSetResult doRequest(const Id &id, ThingSetBinaryRequestType type, std::function<bool (ThingSetBinaryEncoder *)> 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 <typename Id>
         requires std::is_integral_v<Id> or std::is_convertible_v<Id, std::string_view>
-    bool doRequest(const Id &id, ThingSetBinaryRequestType type, std::function<bool (ThingSetBinaryEncoder *)> encode)
+    ThingSetResult doRequest(const Id &id, ThingSetBinaryRequestType type, std::function<bool (ThingSetBinaryEncoder *)> encode)
     {
         uint8_t *responseBuffer;
         size_t responseSize;
@@ -143,28 +147,28 @@ class ThingSetClient
 
     template <typename Id>
         requires std::is_integral_v<Id> or std::is_convertible_v<Id, std::string_view>
-    bool doRequestCore(const Id &id, ThingSetBinaryRequestType type, std::function<bool (ThingSetBinaryEncoder *)> encode, uint8_t **responseBuffer, size_t &responseSize)
+    ThingSetResult doRequestCore(const Id &id, ThingSetBinaryRequestType type, std::function<bool (ThingSetBinaryEncoder *)> 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

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;
+};
+
+}

include/thingset++/can/zephyr/ThingSetZephyrCanRequestResponseContext.hpp

@@ -12,6 +12,8 @@ extern "C" {
 #include <canbus/isotp_fast.h>
 }
 
+#define THINGSET_PLUS_PLUS_ZEPHYR_CAN_FILTER_ID_NONE -1
+
 namespace ThingSet::Can::Zephyr {
 
 class ThingSetZephyrCanInterface;
@@ -33,6 +35,7 @@ class ThingSetZephyrCanRequestResponseContext {
         std::array<uint8_t, TxSize> &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;

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)

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

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();
+}
+
+}

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<int(const CanID
     return bind(CanID::broadcastAddress, callback);
 }
 
+void ThingSetZephyrCanRequestResponseContext::unbindIfNecessary()
+{
+    if (_requestResponseContext.filter_id > 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<int(const CanID &, uint8_t *, size_t, uint8_t *, size_t)> callback)
 {
+    unbindIfNecessary();
     auto canId = CanID()
                       .setMessageType(MessageType::requestResponse)
                       .setMessagePriority(MessagePriority::channel)

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());
 )