Fix fragmentation #297
Fix fragmentation #297
Conversation
| @@ -45,6 +45,8 @@ | |||
| #define _Z_FLAG_Z_T 0x20 // 1 << 5 | QueryTarget if T==1 then the query target is present | |||
| #define _Z_FLAG_Z_X 0x00 // Unused flags are set to zero | |||
|
|
|||
| #define _Z_FRAG_BUFF_BASE_SIZE 128 // Base size of the buffer to encode a fragment message header | |||
|
|
|||
| // Flags: | |||
There was a problem hiding this comment.
Why 128bytes? If there is a reason for it, please document it.
There was a problem hiding this comment.
It's an arbitrary value we decided with @Mallets, that seemed not to big and not to small. The buffer being expandable if needed.
There was a problem hiding this comment.
In fragmentation, the zbuf is used to copy the header bytes and to append the pointer to the payload so as to have a linear reading of the bytes.
128 is a rules of thumb heuristic about the size of memory you need to serialize the header. If more is needed, another block can be allocated straight after and so on an so forth.
| @@ -296,8 +296,7 @@ int8_t _z_bytes_val_encode(_z_wbuf_t *wbf, const _z_bytes_t *bs) { | |||
| int8_t ret = _Z_RES_OK; | |||
|
|
|||
| if ((wbf->_expansion_step != 0) && (bs->len > Z_TSID_LENGTH)) { | |||
| // ret |= _z_wbuf_wrap_bytes(wbf, bs->start, 0, bs->len); | |||
| ret |= _z_wbuf_write_bytes(wbf, bs->start, 0, bs->len); | |||
| ret |= _z_wbuf_wrap_bytes(wbf, bs->start, 0, bs->len); | |||
| @@ -0,0 +1,90 @@ | |||
| // | |||
There was a problem hiding this comment.
Why not to integrate the test in the client and/or multicast tests instead of having a new one? Also, because this shall be tested while interoperating with the Rust implementation.
There was a problem hiding this comment.
I think a standalone test has value as fragmentation will eventually be a removable feature.
There was a problem hiding this comment.
I agree with both, but the fact we don't have a test that actually test fragmentation with the Rust implementation is the at root of #291. So, if I have to prioritise, having a test that tests against Rust is more important than on UDP multicast. Keeping in mind that we don't have reliability on UDP multicast.
jean-roland
force-pushed
the
fix_fragment
branch
from
9bffb65
to
5ac84e7
Compare
jean-roland
force-pushed
the
fix_fragment
branch
from
ffe4f04
to
f5972b0
Compare
|
Approving this PR. @jean-roland I'd like to see in a separate PR the fragmentation tests working as well with the Zenoh router. |
There are currently a few issues regarding fragmentation messages:
This PR is attempting to resolve that and also adds a CI non-regression test for this feature.