Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update encoding format on the wire #399

Merged
merged 8 commits into from
Apr 11, 2024
Merged

Update encoding format on the wire #399

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
93704c3
feat: update encoding wire change
jean-roland Apr 5, 2024
f21b518
refactor: rename codec test gen_uint to gen_uint32
jean-roland Apr 8, 2024
3947232
test: update codec test for encoding
jean-roland Apr 8, 2024
9d5a420
fix: add encoding_len codec function
jean-roland Apr 8, 2024
9b3a450
fix: deactivating interests temporarily
jean-roland Apr 9, 2024
1f615e1
build: suppress msvc warning c4127
jean-roland Apr 9, 2024
97eefed
fix: remove unused variable warning
jean-roland Apr 10, 2024
469c741
fix: rename encoding fields
jean-roland Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ if(CMAKE_BUILD_TYPE MATCHES "DEBUG")
elseif(MSVC)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /std:c11 /experimental:c11atomics")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /std:c++latest /experimental:c11atomics")
add_compile_options(/W4 /WX /Od)
add_compile_options(/W4 /WX /Od /wd4127)
elseif(CMAKE_SYSTEM_NAME MATCHES "Generic")
add_compile_options(-Wall -Wextra -Wno-unused-parameter -Wmissing-prototypes -pipe -g -O0)
endif()
Expand Down
2 changes: 1 addition & 1 deletion GNUmakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Z_FEATURE_SUBSCRIPTION?=1
Z_FEATURE_QUERY?=1
Z_FEATURE_QUERYABLE?=1
Z_FEATURE_ATTACHMENT?=1
Z_FEATURE_INTEREST?=1
Z_FEATURE_INTEREST?=0
Z_FEATURE_RAWETH_TRANSPORT?=0

# zenoh-pico/ directory
Expand Down
6 changes: 4 additions & 2 deletions include/zenoh-pico/protocol/codec/core.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@
}

/*------------------ Internal Zenoh-net Macros ------------------*/
int8_t _z_encoding_prefix_encode(_z_wbuf_t *wbf, z_encoding_prefix_t en);
int8_t _z_encoding_prefix_decode(z_encoding_prefix_t *en, _z_zbuf_t *zbf);
int8_t _z_consolidation_mode_encode(_z_wbuf_t *wbf, z_consolidation_mode_t en);
int8_t _z_consolidation_mode_decode(z_consolidation_mode_t *en, _z_zbuf_t *zbf);
int8_t _z_query_target_encode(_z_wbuf_t *wbf, z_query_target_t en);
Expand Down Expand Up @@ -76,6 +74,10 @@ int8_t _z_zbuf_read_exact(_z_zbuf_t *zbf, uint8_t *dest, size_t length);
int8_t _z_str_encode(_z_wbuf_t *buf, const char *s);
int8_t _z_str_decode(char **str, _z_zbuf_t *buf);

size_t _z_encoding_len(const _z_encoding_t *en);
int8_t _z_encoding_encode(_z_wbuf_t *wbf, const _z_encoding_t *en);
int8_t _z_encoding_decode(_z_encoding_t *en, _z_zbuf_t *zbf);

int8_t _z_keyexpr_encode(_z_wbuf_t *buf, _Bool has_suffix, const _z_keyexpr_t *ke);
int8_t _z_keyexpr_decode(_z_keyexpr_t *ke, _z_zbuf_t *buf, _Bool has_suffix);

Expand Down
50 changes: 38 additions & 12 deletions src/protocol/codec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,6 @@
#include "zenoh-pico/utils/result.h"

/*------------------ uint8 -------------------*/
int8_t _z_encoding_prefix_encode(_z_wbuf_t *wbf, z_encoding_prefix_t en) { return _z_zsize_encode(wbf, en); }

int8_t _z_encoding_prefix_decode(z_encoding_prefix_t *en, _z_zbuf_t *zbf) {
int8_t ret = _Z_RES_OK;

_z_zint_t tmp;
ret |= _z_zsize_decode(&tmp, zbf);
*en = tmp;

return ret;
}

int8_t _z_consolidation_mode_encode(_z_wbuf_t *wbf, z_consolidation_mode_t en) { return _z_zsize_encode(wbf, en); }

int8_t _z_consolidation_mode_decode(z_consolidation_mode_t *en, _z_zbuf_t *zbf) {
Expand Down Expand Up @@ -298,3 +286,41 @@ int8_t _z_str_decode(char **str, _z_zbuf_t *zbf) {

return ret;
}

/*------------------ encoding ------------------*/
#define _Z_ENCODING_FLAG_S 0x01

size_t _z_encoding_len(const _z_encoding_t *en) {
size_t en_len = _z_zint_len((uint32_t)(en->prefix) << 1);
if (_z_bytes_check(en->suffix)) {
en_len += _z_bytes_encode_len(&en->suffix);
}
return en_len;
}

int8_t _z_encoding_encode(_z_wbuf_t *wbf, const _z_encoding_t *en) {
_Bool has_suffix = _z_bytes_check(en->suffix);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For naming alignment, it should be called has_schema and en->schema.

uint32_t id = (uint32_t)(en->prefix) << 1;
if (has_suffix) {
id |= _Z_ENCODING_FLAG_S;
}
_Z_RETURN_IF_ERR(_z_zint32_encode(wbf, id));
if (has_suffix) {
_Z_RETURN_IF_ERR(_z_bytes_encode(wbf, &en->suffix));
}
return _Z_RES_OK;
}

int8_t _z_encoding_decode(_z_encoding_t *en, _z_zbuf_t *zbf) {
uint32_t id = 0;
_Bool has_suffix = false;
_Z_RETURN_IF_ERR(_z_zint32_decode(&id, zbf));
if ((id & _Z_ENCODING_FLAG_S) != 0) {
has_suffix = true;
}
en->prefix = (z_encoding_prefix_t)(id >> 1);
if (has_suffix) {
_Z_RETURN_IF_ERR(_z_bytes_decode(&en->suffix, zbf));
}
return _Z_RES_OK;
}
24 changes: 8 additions & 16 deletions src/protocol/codec/message.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,7 @@ int8_t _z_push_body_encode(_z_wbuf_t *wbf, const _z_push_body_t *pshb) {
}

if (has_encoding) {
_Z_RETURN_IF_ERR(_z_encoding_prefix_encode(wbf, pshb->_body._put._encoding.prefix));
_Z_RETURN_IF_ERR(_z_bytes_encode(wbf, &pshb->_body._put._encoding.suffix));
_Z_RETURN_IF_ERR(_z_encoding_encode(wbf, &pshb->_body._put._encoding));
}

if (has_source_info) {
Expand Down Expand Up @@ -352,8 +351,7 @@ int8_t _z_push_body_decode(_z_push_body_t *pshb, _z_zbuf_t *zbf, uint8_t header)
_Z_RETURN_IF_ERR(_z_timestamp_decode(&pshb->_body._put._commons._timestamp, zbf));
}
if ((ret == _Z_RES_OK) && _Z_HAS_FLAG(header, _Z_FLAG_Z_P_E)) {
_Z_RETURN_IF_ERR(_z_encoding_prefix_decode(&pshb->_body._put._encoding.prefix, zbf));
_Z_RETURN_IF_ERR(_z_bytes_decode(&pshb->_body._put._encoding.suffix, zbf));
_Z_RETURN_IF_ERR(_z_encoding_decode(&pshb->_body._put._encoding, zbf));
}
if ((ret == _Z_RES_OK) && _Z_HAS_FLAG(header, _Z_FLAG_Z_Z)) {
_Z_RETURN_IF_ERR(_z_msg_ext_decode_iter(zbf, _z_push_body_decode_extensions, pshb));
Expand Down Expand Up @@ -437,11 +435,9 @@ int8_t _z_query_encode(_z_wbuf_t *wbf, const _z_msg_query_t *msg) {
extheader |= _Z_FLAG_Z_Z;
}
_Z_RETURN_IF_ERR(_z_uint8_encode(wbf, extheader));
_Z_RETURN_IF_ERR(_z_zsize_encode(wbf, _z_zint_len(msg->_ext_value.encoding.prefix) +
_z_bytes_encode_len(&msg->_ext_value.encoding.suffix) +
msg->_ext_value.payload.len));
_Z_RETURN_IF_ERR(_z_encoding_prefix_encode(wbf, msg->_ext_value.encoding.prefix));
_Z_RETURN_IF_ERR(_z_bytes_encode(wbf, &msg->_ext_value.encoding.suffix));
_Z_RETURN_IF_ERR(
_z_zsize_encode(wbf, _z_encoding_len(&msg->_ext_value.encoding) + msg->_ext_value.payload.len));
_Z_RETURN_IF_ERR(_z_encoding_encode(wbf, &msg->_ext_value.encoding));
_Z_RETURN_IF_ERR(_z_bytes_val_encode(wbf, &msg->_ext_value.payload));
}
if (required_exts.info) {
Expand Down Expand Up @@ -474,8 +470,7 @@ int8_t _z_query_decode_extensions(_z_msg_ext_t *extension, void *ctx) {
}
case _Z_MSG_EXT_ENC_ZBUF | 0x03: { // Payload
_z_zbuf_t zbf = _z_zbytes_as_zbuf(extension->_body._zbuf._val);
ret = _z_encoding_prefix_decode(&msg->_ext_value.encoding.prefix, &zbf);
ret |= _z_bytes_decode(&msg->_ext_value.encoding.suffix, &zbf);
ret = _z_encoding_decode(&msg->_ext_value.encoding, &zbf);
_z_bytes_t bytes = _z_bytes_wrap((uint8_t *)_z_zbuf_start(&zbf), _z_zbuf_len(&zbf));
_z_bytes_copy(&msg->_ext_value.payload, &bytes);
break;
Expand Down Expand Up @@ -533,7 +528,6 @@ int8_t _z_reply_encode(_z_wbuf_t *wbf, const _z_msg_reply_t *reply) {
}
int8_t _z_reply_decode_extension(_z_msg_ext_t *extension, void *ctx) {
int8_t ret = _Z_RES_OK;
_z_msg_reply_t *reply = (_z_msg_reply_t *)ctx;
switch (_Z_EXT_FULL_ID(extension->_header)) {
default:
ret = _z_msg_ext_unknown_error(extension, 0x0a);
Expand Down Expand Up @@ -574,8 +568,7 @@ int8_t _z_err_encode(_z_wbuf_t *wbf, const _z_msg_err_t *err) {
_Z_RETURN_IF_ERR(_z_uint8_encode(wbf, header));
// Encode encoding
if (has_encoding) {
_Z_RETURN_IF_ERR(_z_encoding_prefix_encode(wbf, err->encoding.prefix));
_Z_RETURN_IF_ERR(_z_bytes_encode(wbf, &err->encoding.suffix));
_Z_RETURN_IF_ERR(_z_encoding_encode(wbf, &err->encoding));
}
// Encode extensions
if (has_sinfo_ext) {
Expand Down Expand Up @@ -607,8 +600,7 @@ int8_t _z_err_decode(_z_msg_err_t *err, _z_zbuf_t *zbf, uint8_t header) {
*err = (_z_msg_err_t){0};

if (_Z_HAS_FLAG(header, _Z_FLAG_Z_E_E)) {
_Z_RETURN_IF_ERR(_z_encoding_prefix_decode(&err->encoding.prefix, zbf));
_Z_RETURN_IF_ERR(_z_bytes_decode(&err->encoding.suffix, zbf));
_Z_RETURN_IF_ERR(_z_encoding_decode(&err->encoding, zbf));
}
if (_Z_HAS_FLAG(header, _Z_FLAG_Z_Z)) {
_Z_RETURN_IF_ERR(_z_msg_ext_decode_iter(zbf, _z_err_decode_extension, err));
Expand Down
28 changes: 16 additions & 12 deletions tests/z_msgcodec_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ uint64_t gen_uint64(void) {
return ret;
}

unsigned int gen_uint(void) {
uint32_t gen_uint32(void) {
unsigned int ret = 0;
z_random_fill(&ret, sizeof(ret));
return ret;
Expand Down Expand Up @@ -236,15 +236,20 @@ _z_locator_array_t gen_locator_array(size_t size) {
return la;
}

_z_value_t gen_value(void) {
_z_value_t val;
val.encoding.prefix = gen_zint();
_z_encoding_t gen_encoding(void) {
_z_encoding_t en;
en.prefix = gen_uint32() & 0x7fffffff;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefix should be represented as an uint16_t.

if (gen_bool()) {
val.encoding.suffix = gen_bytes(8);
en.suffix = gen_bytes(16);
} else {
val.encoding.suffix = _z_bytes_empty();
en.suffix = _z_bytes_empty();
}
return en;
}

_z_value_t gen_value(void) {
_z_value_t val;
val.encoding = gen_encoding();
if (gen_bool()) {
val.payload = _z_bytes_empty();
} else {
Expand Down Expand Up @@ -532,7 +537,6 @@ void assert_eq_source_info(const _z_source_info_t *left, const _z_source_info_t
assert(left->_entity_id == right->_entity_id);
assert(memcmp(left->_id.id, right->_id.id, 16) == 0);
}
_z_encoding_t gen_encoding(void) { return (_z_encoding_t){.prefix = gen_uint64(), .suffix = gen_bytes(16)}; }
void assert_eq_encoding(const _z_encoding_t *left, const _z_encoding_t *right) {
assert(left->prefix == right->prefix);
assert_eq_bytes(&left->suffix, &right->suffix);
Expand Down Expand Up @@ -1310,7 +1314,7 @@ _z_n_msg_response_t gen_response(void) {
._ext_timestamp = gen_bool() ? gen_timestamp() : _z_timestamp_null(),
._ext_responder = {._eid = gen_uint16(), ._zid = gen_zid()},
};
switch (gen_uint() % 2) {
switch (gen_uint32() % 2) {
case 0: {
ret._tag = _Z_RESPONSE_BODY_ERR;
ret._body._err = gen_err();
Expand Down Expand Up @@ -1463,9 +1467,9 @@ void init_message(void) {

_z_transport_message_t gen_open(void) {
if (gen_bool()) {
return _z_t_msg_make_open_syn(gen_uint(), gen_uint(), gen_bytes(16));
return _z_t_msg_make_open_syn(gen_uint32(), gen_uint32(), gen_bytes(16));
} else {
return _z_t_msg_make_open_ack(gen_uint(), gen_uint());
return _z_t_msg_make_open_ack(gen_uint32(), gen_uint32());
}
}
void assert_eq_open(const _z_t_msg_open_t *left, const _z_t_msg_open_t *right) {
Expand Down Expand Up @@ -1583,7 +1587,7 @@ _z_network_message_vec_t gen_net_msgs(size_t n) {
}

_z_transport_message_t gen_frame(void) {
return _z_t_msg_make_frame(gen_uint(), gen_net_msgs(gen_uint8() % 16), gen_bool());
return _z_t_msg_make_frame(gen_uint32(), gen_net_msgs(gen_uint8() % 16), gen_bool());
}
void assert_eq_frame(const _z_t_msg_frame_t *left, const _z_t_msg_frame_t *right) {
assert(left->_sn == right->_sn);
Expand All @@ -1609,7 +1613,7 @@ void frame_message(void) {
}

_z_transport_message_t gen_fragment(void) {
return _z_t_msg_make_fragment(gen_uint(), gen_bytes(gen_uint8()), gen_bool(), gen_bool());
return _z_t_msg_make_fragment(gen_uint32(), gen_bytes(gen_uint8()), gen_bool(), gen_bool());
}
void assert_eq_fragment(const _z_t_msg_fragment_t *left, const _z_t_msg_fragment_t *right) {
assert(left->_sn == right->_sn);
Expand Down
Loading