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

Jump to bottom

Refactor link capabilities #280

Merged
merged 24 commits into from
Nov 17, 2023
Merged

Refactor link capabilities #280

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
627c7d9
feat: add modular test
jean-roland Oct 20, 2023
c0afb69
build: add cmake generator option for build
jean-roland Oct 24, 2023
a1c44f7
style: run clang-format
jean-roland Oct 24, 2023
f8e809c
fix: keep a command with default cmake generator
jean-roland Oct 24, 2023
2d0bce8
build: remove unnecessary target
jean-roland Oct 25, 2023
3b2c23a
fix: remove old modular test
jean-roland Nov 6, 2023
c29a1c2
build: update actions deps
jean-roland Nov 8, 2023
64d90bc
fix: revert codacy workflow changes
jean-roland Nov 8, 2023
23ab70d
feat: switch capabilities from bitfield to enum
jean-roland Nov 14, 2023
b27a6c2
feat: change unsafe wbuf signatures
jean-roland Nov 14, 2023
da4aaec
feat: update link capabilities init
jean-roland Nov 14, 2023
7ab167c
feat: update capabilities in unicast
jean-roland Nov 14, 2023
a53f7f2
feat: update capabilities in multicast
jean-roland Nov 14, 2023
d86a1bb
feat: update capabilities in common transport
jean-roland Nov 14, 2023
0b247b0
feat: update capabilities in link
jean-roland Nov 14, 2023
18645d9
doc: add missing capabilities enum value
jean-roland Nov 14, 2023
cf76ee3
fix: add needed blocks to switch
jean-roland Nov 14, 2023
b680332
fix: typo
jean-roland Nov 14, 2023
d73b3d1
fix: define all enum values
jean-roland Nov 16, 2023
c525a18
feat: switch to register-like structure
jean-roland Nov 17, 2023
b29b95f
feat: update link files
jean-roland Nov 17, 2023
6e7a3c9
feat: update transport files
jean-roland Nov 17, 2023
16d5e3f
fix: run clang-format
jean-roland Nov 17, 2023
52cdb36
fix: use same time in the struct to reduce size
jean-roland Nov 17, 2023
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
21 changes: 9 additions & 12 deletions include/zenoh-pico/link/link.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,18 @@
* Link capabilities values, defined as a bitmask.
*
* Enumerators:
* Z_LINK_CAPABILITY_NONE: Bitmask to define that link has no capabilities.
* Z_LINK_CAPABILITY_RELIABLE: Bitmask to define and check if link is reliable.
* Z_LINK_CAPABILITY_STREAMED: Bitmask to define and check if link is streamed.
* Z_LINK_CAPABILITY_MULTICAST: Bitmask to define and check if link is multicast.
* Z_LINK_CAP_UNICAST_STREAM: Link has unicast stream capabilities.
* Z_LINK_CAP_UNICAST_DATAGRAM: Link has unicast datagram capabilities.
* Z_LINK_CAP_MULTICAST_STREAM: Link has multicast stream capabilities.
* Z_LINK_CAP_MULTICAST_DATAGRAM: Link has multicast datagram capabilities.
*/
typedef enum {
Z_LINK_CAPABILITY_NONE = 0x00, // 0
Z_LINK_CAPABILITY_RELIABLE = 0x01, // 1 << 0
Z_LINK_CAPABILITY_STREAMED = 0x02, // 1 << 1
Z_LINK_CAPABILITY_MULTICAST = 0x04 // 1 << 2
Z_LINK_CAP_UNICAST_STREAM = 0,
Z_LINK_CAP_UNICAST_DATAGRAM,
Z_LINK_CAP_MULTICAST_STREAM,
Z_LINK_CAP_MULTICAST_DATAGRAM,
jean-roland marked this conversation as resolved.
Show resolved Hide resolved
} _z_link_capabilities_t;

#define _Z_LINK_IS_RELIABLE(X) ((X & Z_LINK_CAPABILITY_RELIABLE) == Z_LINK_CAPABILITY_RELIABLE)
#define _Z_LINK_IS_STREAMED(X) ((X & Z_LINK_CAPABILITY_STREAMED) == Z_LINK_CAPABILITY_STREAMED)
#define _Z_LINK_IS_MULTICAST(X) ((X & Z_LINK_CAPABILITY_MULTICAST) == Z_LINK_CAPABILITY_MULTICAST)

struct _z_link_t; // Forward declaration to be used in _z_f_link_*

typedef int8_t (*_z_f_link_open)(struct _z_link_t *self);
Expand Down Expand Up @@ -105,6 +101,7 @@ typedef struct _z_link_t {

uint16_t _mtu;
uint8_t _capabilities;
bool _is_reliable;
Copy link
Member

Choose a reason for hiding this comment

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

_is_reliable should be a bit in the _capabilities.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_capabilities is no longer a bitfield though, eventually capabilities could be a structure including an enum and the reliable flag

Copy link
Member

Choose a reason for hiding this comment

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

The idea of the bitfield was to reduce the overhead. These values will be booleans, so they can be store in a single bit.
If this was discussed within the team, you can ignore. Otherwise, I suggest to be rethought because this was done intentionally.

Copy link
Member

Choose a reason for hiding this comment

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

tagging @p-avital

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also invoking @Mallets

Copy link
Member

Choose a reason for hiding this comment

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

In this specific case I don't think it will make any difference.
capabilities is uint8_t and bool takes the same space of a uint8_t.
Unless you are running on a 8-bit platform, and because of alignment, the two solutions are equivalent w.r.t. space usage.

The bitfield usually is used to express the combination of different independent capabilities, which may be invalid. Looking at the code re-organisation I believe that having specific enum variants instead of bitfields will make the logic more explicit.

Copy link
Member

Choose a reason for hiding this comment

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

As this is zenoh-pico, I see 8-bit as a target.
Also, you are just considering one capability, so you are not being fair in the analysis.

Today, we are considering 3 capabilities: _Z_LINK_IS_STREAMED, _Z_LINK_IS_RELIABLE, _Z_LINK_IS_MULTICAST.
Tomorrow we may have others capabilities like _Z_LINK_IS_SECURE or _Z_LINK_IS_UNIDIRECTIONAL.

I am still in favor of the bitfield, also because it scales better than to enumerate all the possible combinations.

Copy link
Contributor Author

@jean-roland jean-roland Nov 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Just to weigh in on the discussion, the current bitfield structure isn't super well suited because only RELIABLE is a true flag/bool, for multicast and streamed it really represents enums:

enum LINK_CAPABILITY_TRANSPORT {
    UNICAST,
    MULTICAST,
    <New values>,
}

and

enum LINK_CAPABILITY_FLOW (just made this up) {
    DATAGRAM,
    STREAM,
}

Ideally we would want to have both enums + any flags + new future values as their own separate fields. Of course that would cost more memory

So if we really want to represent things correctly and use as little memory as possible then we have the possibility to do a register like structure:

struct capability {
    uint8_t transport: 2;
    uint8_t flow: 1;
    bool reliable: 1;
    uin8_t reserved: 4;
}

It does open a lot of problems and future headache if we have to change the fields but that's a possibility.

Another one is a compromise with a bitfield and a combined enum which is almost this PR (albeit the bitfield is only the reliable flag). That won't scale well if we the enums grow too much or if we have to add more enums.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do like the register-like structure proposal a bit better than the enum one. Not extremely familiar with it, but then couldn't we do

struct capability {
    enum LINK_CAPABILITY_TRANSPORT transport: 2;
    enum LINK_CAPABILITY_FLOW flow: 1;
    _Bool reliable: 1;
    uin8_t reserved: 4;
}

to make each field unambiguous as to its purpose? (since C enums are dumb, this might backfire and make capability int-sized though)

Copy link
Member

Choose a reason for hiding this comment

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

My main goal is to avoid ambiguity as well as avoiding the possibility to represent invalid states. For that reason, I believe the current PR is a good tradeoff, as mentioned by @jean-roland .

I don't expect the enum to significantly grow and have a large number of variants. We are targeting fundamental characteristics of transports and links, which have an impact on the Zenoh protocol itself...

} _z_link_t;

void _z_link_clear(_z_link_t *zl);
Expand Down
4 changes: 2 additions & 2 deletions include/zenoh-pico/transport/common/tx.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
#include "zenoh-pico/net/session.h"
#include "zenoh-pico/transport/transport.h"

void __unsafe_z_prepare_wbuf(_z_wbuf_t *buf, _Bool is_streamed);
void __unsafe_z_finalize_wbuf(_z_wbuf_t *buf, _Bool is_streamed);
void __unsafe_z_prepare_wbuf(_z_wbuf_t *buf, uint8_t link_capabilities);
void __unsafe_z_finalize_wbuf(_z_wbuf_t *buf, uint8_t link_capabilities);
/*This function is unsafe because it operates in potentially concurrent
data.*Make sure that the following mutexes are locked before calling this function : *-ztu->mutex_tx */
int8_t __unsafe_z_serialize_zenoh_fragment(_z_wbuf_t *dst, _z_wbuf_t *src, z_reliability_t reliability, size_t sn);
Expand Down
14 changes: 13 additions & 1 deletion src/link/link.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,19 @@ size_t _z_link_recv_exact_zbuf(const _z_link_t *link, _z_zbuf_t *zbf, size_t len

int8_t _z_link_send_wbuf(const _z_link_t *link, const _z_wbuf_t *wbf) {
int8_t ret = _Z_RES_OK;
_Bool link_is_streamed = _Z_LINK_IS_STREAMED(link->_capabilities);
_Bool link_is_streamed = false;

switch (link->_capabilities) {
Copy link
Member

Choose a reason for hiding this comment

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

This should be a bitwise operation as defined in the existing _Z_LINK_IS_STREAMED

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal is to remove the bitwise operations and use a switch as future transport will not necessarily map neatly to the stream/cast separation and it's easier to expand.

Copy link
Member

Choose a reason for hiding this comment

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

Future transports will support or not a given set of functionalities. And the way, the core code should be handled is accordingly to the support or not these functionalities.
As far as I remember, this is how the Rust implementation is also tackling this issue.

Can you give an example of a future transport that does not map? Note that if new properties must be added, they can be done is a clean way with the current approach.

Copy link
Member

Choose a reason for hiding this comment

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

tagging @Mallets

Copy link
Member

@Mallets Mallets Nov 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

In accordance with my previous comment, bitwise operations have the tendency to be interpreted as a combination of indipendent parameters. This has led to some confusion and misconception on how things should be done and structured. I believe in this case having a clear variant separation helps in understanding what are the real cardinality of available options.

Copy link
Contributor Author

@jean-roland jean-roland Nov 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I added MULTICAST_STREAM only because bluetooth was declared multicast and streamed. Tagged you both at the location

Copy link
Member

Choose a reason for hiding this comment

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

Then I withdraw my comment: As far as I know there are no multicast protocols that are byte-streamed.
Now I know :)

Copy link
Contributor

@p-avital p-avital Nov 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

My take on this is that regardless of the existence of a multicast-streamed transport, I think we always treat multicast-ness and streamed-ness independently (since one affects routing and the other serialization), so while the enum might highlight that some don't exist, is that really a relevant fact we need to encode in here?

In general, I take an enum going through the combinations of X and Y as X_Y to be a sign that these properties are rather independent, angling toward the bitfield. But bitfields are a bit ugly: I like the register-struct proposal @jean-roland made in the other thread better if we end up returning to the bitfield approach.

Copy link
Member

Choose a reason for hiding this comment

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

Then I propose to move forward with @jean-roland proposal then.

Copy link
Member

Choose a reason for hiding this comment

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

Disclaimer: Bluetooth is not MULTICAST STREAM per se. This was a workaround to make it work with Zenoh-Pico only and for the selected profile, since Zenoh Rust did not support Multicast or Bluetooth.

As @p-avital mentioned, each property addresses two distinct mechanisms within Zenoh, and it was the fact that they were independent that allowed us to be flexible on supporting new protocols even if part of the entire system was not fully supporting them.

I am fine with the suggestion of @jean-roland here ( #280 (comment) ). But note that there are only two transports in Zenoh and, as far as I am aware of the roadmap, there is not any plan to change it further.

case Z_LINK_CAP_UNICAST_STREAM:
case Z_LINK_CAP_MULTICAST_STREAM:
link_is_streamed = true;
break;
case Z_LINK_CAP_UNICAST_DATAGRAM:
case Z_LINK_CAP_MULTICAST_DATAGRAM:
default:
link_is_streamed = false;
break;
}
for (size_t i = 0; (i < _z_wbuf_len_iosli(wbf)) && (ret == _Z_RES_OK); i++) {
_z_bytes_t bs = _z_iosli_to_bytes(_z_wbuf_get_iosli(wbf, i));
size_t n = bs.len;
Expand Down
3 changes: 2 additions & 1 deletion src/link/multicast/bt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ uint16_t _z_get_link_mtu_bt(void) { return SPP_MAXIMUM_PAYLOAD; }
int8_t _z_new_link_bt(_z_link_t *zl, _z_endpoint_t endpoint) {
int8_t ret = _Z_RES_OK;

zl->_capabilities = Z_LINK_CAPABILITY_STREAMED | Z_LINK_CAPABILITY_MULTICAST;
jean-roland marked this conversation as resolved.
Show resolved Hide resolved
zl->_capabilities = Z_LINK_CAP_MULTICAST_STREAM;
zl->_is_reliable = false;
zl->_mtu = _z_get_link_mtu_bt();

zl->_endpoint = endpoint;
Expand Down
3 changes: 2 additions & 1 deletion src/link/multicast/udp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ uint16_t _z_get_link_mtu_udp_multicast(void) {
int8_t _z_new_link_udp_multicast(_z_link_t *zl, _z_endpoint_t endpoint) {
int8_t ret = _Z_RES_OK;

zl->_capabilities = Z_LINK_CAPABILITY_MULTICAST;
zl->_capabilities = Z_LINK_CAP_MULTICAST_DATAGRAM;
zl->_is_reliable = false;
zl->_mtu = _z_get_link_mtu_udp_multicast();

zl->_endpoint = endpoint;
Expand Down
3 changes: 2 additions & 1 deletion src/link/unicast/serial.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ uint16_t _z_get_link_mtu_serial(void) { return _Z_SERIAL_MTU_SIZE; }
int8_t _z_new_link_serial(_z_link_t *zl, _z_endpoint_t endpoint) {
int8_t ret = _Z_RES_OK;

zl->_capabilities = Z_LINK_CAPABILITY_NONE;
zl->_capabilities = Z_LINK_CAP_UNICAST_DATAGRAM;
zl->_is_reliable = false;
zl->_mtu = _z_get_link_mtu_serial();

zl->_endpoint = endpoint;
Expand Down
3 changes: 2 additions & 1 deletion src/link/unicast/tcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ uint16_t _z_get_link_mtu_tcp(void) {
int8_t _z_new_link_tcp(_z_link_t *zl, _z_endpoint_t *endpoint) {
int8_t ret = _Z_RES_OK;

zl->_capabilities = Z_LINK_CAPABILITY_RELIABLE | Z_LINK_CAPABILITY_STREAMED;
zl->_capabilities = Z_LINK_CAP_UNICAST_STREAM;
zl->_is_reliable = true;
zl->_mtu = _z_get_link_mtu_tcp();

zl->_endpoint = *endpoint;
Expand Down
3 changes: 2 additions & 1 deletion src/link/unicast/udp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ uint16_t _z_get_link_mtu_udp_unicast(void) {
int8_t _z_new_link_udp_unicast(_z_link_t *zl, _z_endpoint_t endpoint) {
int8_t ret = _Z_RES_OK;

zl->_capabilities = Z_LINK_CAPABILITY_NONE;
zl->_capabilities = Z_LINK_CAP_UNICAST_DATAGRAM;
zl->_is_reliable = false;
zl->_mtu = _z_get_link_mtu_udp_unicast();

zl->_endpoint = endpoint;
Expand Down
3 changes: 2 additions & 1 deletion src/link/unicast/ws.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ uint16_t _z_get_link_mtu_ws(void) {
int8_t _z_new_link_ws(_z_link_t *zl, _z_endpoint_t *endpoint) {
int8_t ret = _Z_RES_OK;

zl->_capabilities = Z_LINK_CAPABILITY_RELIABLE;
zl->_capabilities = Z_LINK_CAP_UNICAST_DATAGRAM;
zl->_is_reliable = true;
zl->_mtu = _z_get_link_mtu_ws();

zl->_endpoint = *endpoint;
Expand Down
51 changes: 30 additions & 21 deletions src/transport/common/rx.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,32 +28,41 @@ int8_t _z_link_recv_t_msg(_z_transport_message_t *t_msg, const _z_link_t *zl) {
_z_zbuf_t zbf = _z_zbuf_make(Z_BATCH_UNICAST_SIZE);
_z_zbuf_reset(&zbf);

if (_Z_LINK_IS_STREAMED(zl->_capabilities) == true) {
// Read the message length
if (_z_link_recv_exact_zbuf(zl, &zbf, _Z_MSG_LEN_ENC_SIZE, NULL) == _Z_MSG_LEN_ENC_SIZE) {
size_t len = 0;
for (uint8_t i = 0; i < _Z_MSG_LEN_ENC_SIZE; i++) {
len |= (size_t)(_z_zbuf_read(&zbf) << (i * (uint8_t)8));
}
switch (zl->_capabilities) {
jean-roland marked this conversation as resolved.
Show resolved Hide resolved
// Stream capable links
case Z_LINK_CAP_UNICAST_STREAM:
case Z_LINK_CAP_MULTICAST_STREAM:
// Read the message length
if (_z_link_recv_exact_zbuf(zl, &zbf, _Z_MSG_LEN_ENC_SIZE, NULL) == _Z_MSG_LEN_ENC_SIZE) {
size_t len = 0;
for (uint8_t i = 0; i < _Z_MSG_LEN_ENC_SIZE; i++) {
len |= (size_t)(_z_zbuf_read(&zbf) << (i * (uint8_t)8));
}

size_t writable = _z_zbuf_capacity(&zbf) - _z_zbuf_len(&zbf);
if (writable >= len) {
// Read enough bytes to decode the message
if (_z_link_recv_exact_zbuf(zl, &zbf, len, NULL) != len) {
ret = _Z_ERR_TRANSPORT_RX_FAILED;
size_t writable = _z_zbuf_capacity(&zbf) - _z_zbuf_len(&zbf);
if (writable >= len) {
// Read enough bytes to decode the message
if (_z_link_recv_exact_zbuf(zl, &zbf, len, NULL) != len) {
ret = _Z_ERR_TRANSPORT_RX_FAILED;
}
} else {
ret = _Z_ERR_TRANSPORT_NO_SPACE;
}
} else {
ret = _Z_ERR_TRANSPORT_NO_SPACE;
ret = _Z_ERR_TRANSPORT_RX_FAILED;
}
} else {
ret = _Z_ERR_TRANSPORT_RX_FAILED;
}
} else {
if (_z_link_recv_zbuf(zl, &zbf, NULL) == SIZE_MAX) {
ret = _Z_ERR_TRANSPORT_RX_FAILED;
}
break;
// Datagram capable links
case Z_LINK_CAP_UNICAST_DATAGRAM:
case Z_LINK_CAP_MULTICAST_DATAGRAM:
if (_z_link_recv_zbuf(zl, &zbf, NULL) == SIZE_MAX) {
ret = _Z_ERR_TRANSPORT_RX_FAILED;
}
break;
default:
ret = _Z_ERR_GENERIC;
break;
}

if (ret == _Z_RES_OK) {
_z_transport_message_t l_t_msg;
ret = _z_transport_message_decode(&l_t_msg, &zbf);
Expand Down
87 changes: 64 additions & 23 deletions src/transport/common/tx.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,23 @@
* Make sure that the following mutexes are locked before calling this function:
* - ztu->mutex_tx
*/
void __unsafe_z_prepare_wbuf(_z_wbuf_t *buf, _Bool is_streamed) {
void __unsafe_z_prepare_wbuf(_z_wbuf_t *buf, uint8_t link_capabilities) {
_z_wbuf_reset(buf);

if (is_streamed == true) {
for (uint8_t i = 0; i < _Z_MSG_LEN_ENC_SIZE; i++) {
_z_wbuf_put(buf, 0, i);
}
_z_wbuf_set_wpos(buf, _Z_MSG_LEN_ENC_SIZE);
switch (link_capabilities) {
jean-roland marked this conversation as resolved.
Show resolved Hide resolved
// Stream capable links
case Z_LINK_CAP_UNICAST_STREAM:
case Z_LINK_CAP_MULTICAST_STREAM:
for (uint8_t i = 0; i < _Z_MSG_LEN_ENC_SIZE; i++) {
_z_wbuf_put(buf, 0, i);
}
_z_wbuf_set_wpos(buf, _Z_MSG_LEN_ENC_SIZE);
break;
// Datagram capable links
case Z_LINK_CAP_UNICAST_DATAGRAM:
case Z_LINK_CAP_MULTICAST_DATAGRAM:
default:
break;
}
}

Expand All @@ -43,12 +52,22 @@ void __unsafe_z_prepare_wbuf(_z_wbuf_t *buf, _Bool is_streamed) {
* Make sure that the following mutexes are locked before calling this function:
* - ztu->mutex_tx
*/
void __unsafe_z_finalize_wbuf(_z_wbuf_t *buf, _Bool is_streamed) {
if (is_streamed == true) {
size_t len = _z_wbuf_len(buf) - _Z_MSG_LEN_ENC_SIZE;
for (uint8_t i = 0; i < _Z_MSG_LEN_ENC_SIZE; i++) {
_z_wbuf_put(buf, (uint8_t)((len >> (uint8_t)8 * i) & (uint8_t)0xFF), i);
void __unsafe_z_finalize_wbuf(_z_wbuf_t *buf, uint8_t link_capabilities) {
switch (link_capabilities) {
jean-roland marked this conversation as resolved.
Show resolved Hide resolved
// Stream capable links
case Z_LINK_CAP_UNICAST_STREAM:
case Z_LINK_CAP_MULTICAST_STREAM: {
size_t len = _z_wbuf_len(buf) - _Z_MSG_LEN_ENC_SIZE;
for (uint8_t i = 0; i < _Z_MSG_LEN_ENC_SIZE; i++) {
_z_wbuf_put(buf, (uint8_t)((len >> (uint8_t)8 * i) & (uint8_t)0xFF), i);
}
break;
}
// Datagram capable links
case Z_LINK_CAP_UNICAST_DATAGRAM:
case Z_LINK_CAP_MULTICAST_DATAGRAM:
default:
break;
}
}

Expand All @@ -74,24 +93,46 @@ int8_t _z_link_send_t_msg(const _z_link_t *zl, const _z_transport_message_t *t_m
// Create and prepare the buffer to serialize the message on
uint16_t mtu = (zl->_mtu < Z_BATCH_UNICAST_SIZE) ? zl->_mtu : Z_BATCH_UNICAST_SIZE;
_z_wbuf_t wbf = _z_wbuf_make(mtu, false);
if (_Z_LINK_IS_STREAMED(zl->_capabilities) == true) {
for (uint8_t i = 0; i < _Z_MSG_LEN_ENC_SIZE; i++) {
_z_wbuf_put(&wbf, 0, i);
}
_z_wbuf_set_wpos(&wbf, _Z_MSG_LEN_ENC_SIZE);
}

switch (zl->_capabilities) {
// Stream capable links
case Z_LINK_CAP_UNICAST_STREAM:
case Z_LINK_CAP_MULTICAST_STREAM:
for (uint8_t i = 0; i < _Z_MSG_LEN_ENC_SIZE; i++) {
_z_wbuf_put(&wbf, 0, i);
}
_z_wbuf_set_wpos(&wbf, _Z_MSG_LEN_ENC_SIZE);
break;
// Datagram capable links
case Z_LINK_CAP_UNICAST_DATAGRAM:
case Z_LINK_CAP_MULTICAST_DATAGRAM:
break;
default:
ret = _Z_ERR_GENERIC;
break;
}
// Encode the session message
ret = _z_transport_message_encode(&wbf, t_msg);
if (ret == _Z_RES_OK) {
// Write the message length in the reserved space if needed
if (_Z_LINK_IS_STREAMED(zl->_capabilities) == true) {
size_t len = _z_wbuf_len(&wbf) - _Z_MSG_LEN_ENC_SIZE;
for (uint8_t i = 0; i < _Z_MSG_LEN_ENC_SIZE; i++) {
_z_wbuf_put(&wbf, (uint8_t)((len >> (uint8_t)8 * i) & (uint8_t)0xFF), i);
switch (zl->_capabilities) {
// Stream capable links
case Z_LINK_CAP_UNICAST_STREAM:
case Z_LINK_CAP_MULTICAST_STREAM: {
// Write the message length in the reserved space if needed
size_t len = _z_wbuf_len(&wbf) - _Z_MSG_LEN_ENC_SIZE;
for (uint8_t i = 0; i < _Z_MSG_LEN_ENC_SIZE; i++) {
_z_wbuf_put(&wbf, (uint8_t)((len >> (uint8_t)8 * i) & (uint8_t)0xFF), i);
}
break;
}
// Datagram capable links
case Z_LINK_CAP_UNICAST_DATAGRAM:
case Z_LINK_CAP_MULTICAST_DATAGRAM:
break;
default:
ret = _Z_ERR_GENERIC;
break;
}

// Send the wbuf on the socket
ret = _z_link_send_wbuf(zl, &wbf);
}
Expand Down
Loading
Loading