Rework attachments (#423)

* feat: switch attachment to z_bytes * fix: remove attachment from build and example * feat: remove attachment feature token * fix: init missing query reply attachment option * feat: add api functions for attachments * fix: conversion issue * feat: add attachment examples * fix: check for null value before in drop function * fix: copy attachment * ci: add attachment test * fix: query/reply memory leaks * feat: add get/queryable attachment example * tests: add query/queryable attachment test * fix: rename kv_pairs_length
eclipse-zenoh · Jun 17, 2024 · aa7e7bc · aa7e7bc
1 parent 9aa9ed1
commit aa7e7bc
Show file tree

Hide file tree

Showing 43 changed files with 1,369 additions and 767 deletions.
diff --git a/.github/workflows/build-check.yaml b/.github/workflows/build-check.yaml
@@ -115,7 +115,7 @@ jobs:
         run: |
           sudo apt install -y ninja-build
           CMAKE_GENERATOR=Ninja make
-          python3 ./build/tests/modularity.py --pub $Z_FEATURE_PUBLICATION --sub $Z_FEATURE_SUBSCRIPTION --queryable $Z_FEATURE_QUERYABLE --query $Z_FEATURE_QUERY --attachment 0
+          python3 ./build/tests/modularity.py --pub $Z_FEATURE_PUBLICATION --sub $Z_FEATURE_SUBSCRIPTION --queryable $Z_FEATURE_QUERYABLE --query $Z_FEATURE_QUERY
         timeout-minutes: 5
         env:
           Z_FEATURE_PUBLICATION: ${{ matrix.feature_publication }}
@@ -196,9 +196,9 @@ jobs:
         if: always()
         run: kill ${{ steps.run-zenoh.outputs.zenohd-pid }}
 
-  no_attachment_test:
+  attachment_test:
     needs: zenoh_build
-    name: Build and test without attachment on ubuntu-latest
+    name: Test attachments
     runs-on: ubuntu-latest
     steps:
       - name: Checkout code
@@ -222,10 +222,8 @@ jobs:
         run: |
           sudo apt install -y ninja-build
           CMAKE_GENERATOR=Ninja make
-          python3 ./build/tests/modularity.py --pub 1 --sub 1 --queryable 1 --query 1 --attachment 0
+          python3 ./build/tests/attachment.py
         timeout-minutes: 5
-        env:
-          Z_FEATURE_ATTACHMENT: 0
 
       - name: Kill Zenoh router
         if: always()

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -132,15 +132,13 @@ set(Z_FEATURE_SUBSCRIPTION 1 CACHE STRING "Toggle subscription feature")
 set(Z_FEATURE_QUERY 1 CACHE STRING "Toggle query feature")
 set(Z_FEATURE_QUERYABLE 1 CACHE STRING "Toggle queryable feature")
 set(Z_FEATURE_RAWETH_TRANSPORT 0 CACHE STRING "Toggle raw ethernet transport feature")
-set(Z_FEATURE_ATTACHMENT 0 CACHE STRING "Toggle attachment feature")
 set(Z_FEATURE_INTEREST 0 CACHE STRING "Toggle interest feature") # Toggle to 1 when protocol changes are merged
 add_definition(Z_FEATURE_MULTI_THREAD=${Z_FEATURE_MULTI_THREAD})
 add_definition(Z_FEATURE_PUBLICATION=${Z_FEATURE_PUBLICATION})
 add_definition(Z_FEATURE_SUBSCRIPTION=${Z_FEATURE_SUBSCRIPTION})
 add_definition(Z_FEATURE_QUERY=${Z_FEATURE_QUERY})
 add_definition(Z_FEATURE_QUERYABLE=${Z_FEATURE_QUERYABLE})
 add_definition(Z_FEATURE_RAWETH_TRANSPORT=${Z_FEATURE_RAWETH_TRANSPORT})
-add_definition(Z_FEATURE_ATTACHMENT=${Z_FEATURE_ATTACHMENT})
 add_definition(Z_FEATURE_INTEREST=${Z_FEATURE_INTEREST})
 add_compile_definitions("Z_BUILD_DEBUG=$<CONFIG:Debug>")
 message(STATUS "Building with feature confing:\n\
@@ -149,7 +147,6 @@ message(STATUS "Building with feature confing:\n\
 * SUBSCRIPTION: ${Z_FEATURE_SUBSCRIPTION}\n\
 * QUERY: ${Z_FEATURE_QUERY}\n\
 * QUERYABLE: ${Z_FEATURE_QUERYABLE}\n\
-* ATTACHMENT: ${Z_FEATURE_ATTACHMENT}\n\
 * INTEREST: ${Z_FEATURE_INTEREST}\n\
 * RAWETH: ${Z_FEATURE_RAWETH_TRANSPORT}")
 
@@ -180,6 +177,7 @@ endif()
 
 if(CMAKE_BUILD_TYPE MATCHES "DEBUG")
   if(UNIX)
+    # FIXME: seems to not apply to library files, Issue #422
     add_compile_options(-c -Wall -Wextra -Werror -Wshadow -Wpedantic -Wunused -Wstrict-prototypes -pipe -g -O0)
     # add_compile_options(-Wconversion)
     # add_link_options(-fsanitize=address)
@@ -363,6 +361,7 @@ if(UNIX OR MSVC)
     configure_file(${PROJECT_SOURCE_DIR}/tests/raweth.py ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/raweth.py COPYONLY)
     configure_file(${PROJECT_SOURCE_DIR}/tests/fragment.py ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/fragment.py COPYONLY)
     configure_file(${PROJECT_SOURCE_DIR}/tests/single_thread.py ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/single_thread.py COPYONLY)
+    configure_file(${PROJECT_SOURCE_DIR}/tests/attachment.py ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/attachment.py COPYONLY)
 
     enable_testing()
     add_test(z_data_struct_test ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/z_data_struct_test)

diff --git a/GNUmakefile b/GNUmakefile
@@ -56,7 +56,6 @@ Z_FEATURE_PUBLICATION?=1
 Z_FEATURE_SUBSCRIPTION?=1
 Z_FEATURE_QUERY?=1
 Z_FEATURE_QUERYABLE?=1
-Z_FEATURE_ATTACHMENT?=0
 Z_FEATURE_INTEREST?=1
 Z_FEATURE_RAWETH_TRANSPORT?=0
 
@@ -76,7 +75,7 @@ CROSSIMG_PREFIX=zenoh-pico_
 CMAKE_OPT=-DZENOH_DEBUG=$(ZENOH_DEBUG) -DBUILD_EXAMPLES=$(BUILD_EXAMPLES) -DCMAKE_BUILD_TYPE=$(BUILD_TYPE) -DBUILD_TESTING=$(BUILD_TESTING) -DBUILD_MULTICAST=$(BUILD_MULTICAST)\
  -DZ_FEATURE_MULTI_THREAD=$(Z_FEATURE_MULTI_THREAD) -DZ_FEATURE_INTEREST=$(Z_FEATURE_INTEREST) \
  -DZ_FEATURE_PUBLICATION=$(Z_FEATURE_PUBLICATION) -DZ_FEATURE_SUBSCRIPTION=$(Z_FEATURE_SUBSCRIPTION) -DZ_FEATURE_QUERY=$(Z_FEATURE_QUERY) -DZ_FEATURE_QUERYABLE=$(Z_FEATURE_QUERYABLE)\
- -DZ_FEATURE_RAWETH_TRANSPORT=$(Z_FEATURE_RAWETH_TRANSPORT) -DZ_FEATURE_ATTACHMENT=$(Z_FEATURE_ATTACHMENT) -DBUILD_INTEGRATION=$(BUILD_INTEGRATION) -DBUILD_TOOLS=$(BUILD_TOOLS) -DBUILD_SHARED_LIBS=$(BUILD_SHARED_LIBS) -H.
+ -DZ_FEATURE_RAWETH_TRANSPORT=$(Z_FEATURE_RAWETH_TRANSPORT) -DBUILD_INTEGRATION=$(BUILD_INTEGRATION) -DBUILD_TOOLS=$(BUILD_TOOLS) -DBUILD_SHARED_LIBS=$(BUILD_SHARED_LIBS) -H.
 
 ifeq ($(FORCE_C99), ON)
 	CMAKE_OPT += -DCMAKE_C_STANDARD=99

diff --git a/docs/conf.py b/docs/conf.py
@@ -33,7 +33,6 @@
     "-DZ_FEATURE_SUBSCRIPTION=1",
     "-DZ_FEATURE_QUERY=1",
     "-DZ_FEATURE_QUERYABLE=1",
-    "-DZ_FEATURE_ATTACHMENT=1",
 ]
 
 # -- Options for HTML output -------------------------------------------------

diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt
@@ -36,14 +36,18 @@ if(UNIX)
         add_example(z_put unix/c11/z_put.c)
         add_example(z_pub unix/c11/z_pub.c)
         add_example(z_pub_st unix/c11/z_pub_st.c)
+        add_example(z_pub_attachment unix/c11/z_pub_attachment.c)
         add_example(z_sub unix/c11/z_sub.c)
         add_example(z_sub_channel unix/c11/z_sub_channel.c)
         add_example(z_sub_st unix/c11/z_sub_st.c)
+        add_example(z_sub_attachment unix/c11/z_sub_attachment.c)
         add_example(z_pull unix/c11/z_pull.c)
         add_example(z_get unix/c11/z_get.c)
         add_example(z_get_channel unix/c11/z_get_channel.c)
+        add_example(z_get_attachment unix/c11/z_get_attachment.c)
         add_example(z_queryable unix/c11/z_queryable.c)
         add_example(z_queryable_channel unix/c11/z_queryable_channel.c)
+        add_example(z_queryable_attachment unix/c11/z_queryable_attachment.c)
         add_example(z_info unix/c11/z_info.c)
         add_example(z_scout unix/c11/z_scout.c)
         add_example(z_ping unix/c11/z_ping.c)

diff --git a/examples/unix/c11/z_get.c b/examples/unix/c11/z_get.c
@@ -29,14 +29,6 @@ void reply_dropper(void *ctx) {
     z_condvar_free(&cond);
 }
 
-#if Z_FEATURE_ATTACHMENT == 1
-int8_t attachment_handler(z_bytes_t key, z_bytes_t att_value, void *ctx) {
-    (void)ctx;
-    printf(">>> %.*s: %.*s\n", (int)key.len, key.start, (int)att_value.len, att_value.start);
-    return 0;
-}
-#endif
-
 void reply_handler(const z_loaned_reply_t *reply, void *ctx) {
     (void)(ctx);
     if (z_reply_is_ok(reply)) {
@@ -47,12 +39,6 @@ void reply_handler(const z_loaned_reply_t *reply, void *ctx) {
         z_bytes_decode_into_string(z_sample_payload(sample), &replystr);
 
         printf(">> Received ('%s': '%s')\n", z_string_data(z_loan(keystr)), z_string_data(z_loan(replystr)));
-#if Z_FEATURE_ATTACHMENT == 1
-        if (z_attachment_check(&sample.attachment)) {
-            printf("Attachement found\n");
-            z_attachment_iterate(sample.attachment, attachment_handler, NULL);
-        }
-#endif
         z_drop(z_move(keystr));
         z_drop(z_move(replystr));
     } else {
@@ -140,11 +126,6 @@ int main(int argc, char **argv) {
         z_bytes_encode_from_string(&payload, value);
         opts.payload = &payload;
     }
-#if Z_FEATURE_ATTACHMENT == 1
-    z_owned_bytes_map_t map = z_bytes_map_new();
-    z_bytes_map_insert_by_alias(&map, z_bytes_from_str("hi"), z_bytes_from_str("there"));
-    opts.attachment = z_bytes_map_as_attachment(&map);
-#endif
 
     z_owned_closure_reply_t callback;
     z_closure(&callback, reply_handler, reply_dropper);
@@ -160,11 +141,6 @@ int main(int argc, char **argv) {
     zp_stop_lease_task(z_loan_mut(s));
 
     z_close(z_move(s));
-
-#if Z_FEATURE_ATTACHMENT == 1
-    z_bytes_map_drop(&map);
-#endif
-
     return 0;
 }
 #else