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

Add unicast interest feature #349

Merged

OlivierHecart merged 41 commits into eclipse-zenoh:protocol_changes from jean-roland:ft_interests

Mar 5, 2024

Contributor

jean-roland commented Feb 20, 2024 •

edited

Loading

This PR is the first concerning the upcoming Declare interest feature, it contains mainly the modification for unicast/client but also prepare the terrain for multicast/peer.

Add a config token to toggle the non-mandatory part of the feature
Add the interest module that handles interest management at the session level and the processing of interest related messages (tx and rx)
Add the filtering module that build on the interest module to add a writer side filtering for publishers (they won't write on the network until they receive a declare subscriber).
Add the basic multicast logic to reply to interest messages.

This is a partial implementation of #331

jean-roland added 30 commits

February 16, 2024 10:04


          refactor: rename make final interest function

ccf2a25


          feat: add Interest config token

d15bc99


          fix: PA copy paste error

461a8c1


          feat: add session interest type

33a8e3f


          feat: add intrest type

80707df


          feat: add session interest module

7e95df8


          feat: add interest primitives

b29af1e


          feat: add session mutex function

bf7bdfc


          feat: add interest message processing

9682d98


          fix: modularize process declare interest function

bf99b97


          feat: implement trigger_interest function

432a341


          fix: clone entitiy lists before sending declarations

99fafd8


          fix: remove unused function

979d486


          fix: rename file

0cd5ec4


          fix: set interest_msg fields as public

d40a4be


          feat: process undeclares as well

c056220


          fix: remove unecessary enum value

864a979


          fix: remove dropper function

ab18413


          feat: add write side filtering entity

51dee17


          feat: implement filtering

60b4fcb


          fix: remove interest by default (for now)

3b73e07


          fix: rework publisher put actions order

9c0be52


          fix: add missing const

6f66d03


          fix: add missing dummy functions

46528e9


          fix: exclude dummy functions from config token

6c6abf9


          fix: remove unused defines

2add7ed


          fix: give keyexpr ownership to declare

e4385dc


          feat: add refcount copy function

14d6310


          fix: duplicate key for declare

4d04ac5


          feat: add resource copy function

0e3ea7c

jean-roland added 4 commits

February 19, 2024 11:26


          fix: use keyexpr alias instead of duplicate

02bb3a7


          fix: flip early return condition

dc64050


          feat: send final interest

ecdd57b


          refactor: rename ambiguous functions

18649c3

eclipse-zenoh-bot commented Feb 20, 2024

@jean-roland If this pull request contains a bugfix or a new feature, then please consider using Closes #ISSUE-NUMBER syntax to link it to an issue.

1 similar comment

eclipse-zenoh-bot commented Feb 20, 2024

@jean-roland If this pull request contains a bugfix or a new feature, then please consider using Closes #ISSUE-NUMBER syntax to link it to an issue.

github-advanced-security bot found potential problems

View reviewed changes

github-advanced-security bot left a comment

Cppcheck (reported by Codacy) found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

eclipse-zenoh-bot commented Feb 20, 2024

@jean-roland If this pull request contains a bugfix or a new feature, then please consider using Closes #ISSUE-NUMBER syntax to link it to an issue.


          fix: fuse identical functions

68eca1f

jean-roland marked this pull request as ready for review

February 20, 2024 11:13

jean-roland changed the base branch from protocol_changes to main

February 20, 2024 15:48

eclipse-zenoh-bot commented Feb 20, 2024

@jean-roland If this pull request contains a bugfix or a new feature, then please consider using Closes #ISSUE-NUMBER syntax to link it to an issue.

jean-roland changed the base branch from main to protocol_changes

February 20, 2024 15:49

eclipse-zenoh-bot commented Feb 20, 2024

@jean-roland If this pull request contains a bugfix or a new feature, then please consider using Closes #ISSUE-NUMBER syntax to link it to an issue.

p-avital force-pushed the protocol_changes branch from 316cf04 to 16e9eb3 Compare

February 21, 2024 16:06

OlivierHecart reviewed

View reviewed changes

src/api/api.c Outdated

                   // Trigger local subscriptions
                   _z_trigger_local_subscriptions(&pub._val->_zn.in->val, pub._val->_key, payload, len
               #if Z_FEATURE_ATTACHMENT == 1
                                                  ,
                                                  opt.attachment
               #endif
                   );
-                  return ret;
+                  // Check if write filter is active before writing

Contributor

OlivierHecart Feb 22, 2024

Push messages are now sent to the network after local subscriptions trigger. We tend to have the previous order on purpose.

Contributor

OlivierHecart commented Feb 22, 2024

I guess we should add a toggle for Z_FEATURE_INTEREST in CMakeLists.txt and GNUMakefile (enabled by default ?).
And probably enable it in .github/workflows/build-check.yaml, .github/workflows/freertos_plus_tcp.yaml and docs/conf.py.

OlivierHecart reviewed

View reviewed changes

src/session/interest.c Outdated Show resolved Hide resolved

jean-roland added 6 commits

February 27, 2024 15:22


          fix: trigger local subs after push message

ba254d5


          fix: add missing switch default label

de5c901


          build: add Z_FEATURE_INTEREST to build system

a6a9c14


          fix: remove unused function

679d1ac


          refactor: regroup static functions

0415cbf


          fix: add flag filtering on interest look-up

2dd09f0

OlivierHecart approved these changes

View reviewed changes

OlivierHecart merged commit ebdb01e into eclipse-zenoh:protocol_changes

48 checks passed

jean-roland deleted the ft_interests branch

April 9, 2024 07:56

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet