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

Expose timeout option in z_get_options_t #375

Merged
merged 2 commits into from
Mar 21, 2024
Merged

Expose timeout option in z_get_options_t #375

merged 2 commits into from
Mar 21, 2024

Conversation

sashacmc
Copy link
Member

@sashacmc sashacmc commented Mar 19, 2024 •
edited
Loading

Closes #215

@eclipse-zenoh-bot
Copy link

@sashacmc 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.

@sashacmc sashacmc force-pushed the expose_timeout_option branch from 3062c1c to c6cfc8a Compare March 19, 2024 16:19
@sashacmc sashacmc marked this pull request as ready for review March 19, 2024 16:44
jean-roland
jean-roland reviewed Mar 20, 2024
View reviewed changes
src/api/api.c Outdated
@@ -788,6 +788,7 @@ z_get_options_t z_get_options_default(void) {
#if Z_FEATURE_ATTACHMENT == 1
// TODO:ATT.attachment = z_attachment_null()
#endif
.timeout_ms = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

To make it consistent with Zenoh it should have the same default value of 10 seconds. Although you might need to double check that value.

@Mallets
Copy link
Member

Mallets commented Mar 20, 2024

This PR does not seem to implement the timeout behaviour but only to add the option in the options. Will a query ever timeout locally? Or it will always depend on a router sending the timeout? What about p2p multicast case?

@jean-roland
Copy link
Contributor

Right, it eluded me but yeah we need to implement the timeout locally, like zenoh iirc.

@sashacmc
Copy link
Member Author

sashacmc commented Mar 20, 2024 •
edited
Loading

@Mallets as far as I see, there we have no any waiting/blocking inside, just send the request (response comes asynchronously)
Should we introduce some one?

@Mallets
Copy link
Member

Mallets commented Mar 20, 2024

We could piggyback on the lease_task which basically acts as a timer for events, what do you think?

@sashacmc sashacmc requested a review from jean-roland March 20, 2024 15:41
@Mallets
Copy link
Member

Mallets commented Mar 21, 2024

Let's get this PR in and leave the internal timeout implementation to a different PR.

@Mallets Mallets merged commit 0d9f25b into eclipse-zenoh:main Mar 21, 2024
49 checks passed
@Mallets Mallets deleted the expose_timeout_option branch March 21, 2024 10:18
Mallets added a commit that referenced this pull request Apr 5, 2024
@jean-roland @DenisBiryukov91
@sashacmc @milyin @fuzzypixelz @OlivierHecart @oteffahi @gmartin82 @vortex314 @Mallets
Fix CI failures (#394)
265405d 
* add support for qos settings in sample

* - silence cpp compiler regading compound literals and type conversions,
- add missing dots in docs

* fix _z_qos_t definition and doc

* fix z_qos_t definition and doc

* - replace binary literals (non-core C) with hexadecimal ones
- replace _z_n_qos_unmake with _z_n_qos_unmake_public in subscription.c

* make _z_n_qos_unmake static inline to prevent linking errors

* fix format

* reduce qos size to 1 byte; use getters to extract individual qos settings

* fix _z_n_qos_make to no longer use compund literals to avoid warnings from zenoh-cpp

* fix z_qos_default() signature

* Add Flipper platform (#351)

* Add Flipper platform

* Fix warnings and other platform builds

* move z_n_qos_t related functions into .c (to hide designated initializers from c++)

* z_sample_t docs update to include qos

* move qos functions back to header and remove usage of designated initializers

* format

* fix: Bump sphinx doc build dep to `7.2.6` (#361)

* replac zp_ prefix for platform functions that are also available in zenoh-c

* fix format

* replace zp_ prefix with z_ for zp_random_u64

* put back previously removed zp_ functions as deprecated

* added missing new line at the end of the file

* typo fix

* move deprecated platform header to a separate folder

* Add config endpoint in raweth locator (#364)

* feat: add ethtype to reth endpoint config

* feat: type renaming

* fix: remove unused macro call

* feat: remove static raweth config

* feat: use defines for separators

* feat: add mapping function

* fix: add default raweth mapping

* feat: add debug traces on raweth config  parsing

* fix: ethtype validity test

* fix: strtok parsing calls

* chore: clang-format

* fix: include first mapping entry in lookup

* fix: appease the tyran codacy

* Fix bug reading from ws (#370)

* Expose timeout option in z_get_options_t (#375)

* Expose timeout option in z_get_options_t

* Add default get timeout

* Align examples and remove reading from stdin (#359)

* Remove reading from stdin, align example implementations

* Update examples tests

* Adjust z_sub_thr output

* Fix z_get single query examples

* Replace pthread uses with z_mutex and z_condvar

* Add multi-thread feature condition to z_get examples

* Update error message for features absence

* Update z_get expected output in modularity test

* Update sample count for freertos single-thread examples

* fix: remove unused variable warning (#379)

* Changing references to zenoh:master to zenoh:main. (#381)

* correct unsigned atomic in refcount.h (#382)

* Fix refcount cast (#384)

* Fix esp32 CI (#386)

* fix: const discard warning

* build: fix platformio dependency check

* Serial timeout (#383)

* correct unsigned atomic in refcount.h

* made espidf freertos compatible with timeout on read

---------

Co-authored-by: Luca Cominardi <[email protected]>

* Run clang-format (#388)

* Fix misra violations (#390)

* chore: run clang format

* fix: misra issues

* ci: Allow building zenoh from arbitrary branch in build-check workflow (#389)

* ci: Allow building Zenoh from arbitrary branch in build-check workflow

* Update other jobs

---------

Co-authored-by: OlivierHecart <[email protected]>

* build: point ci to zenoh protocol_changes branch

* fix: check declare <I> flag at declare level

* fix: missing function prototype warning

* build: switch zenoh branch to interests

* fix: ignore unknown final interest

* fix: missing argument in tigger_local_subs

* fix: add wait join step in z_pub for raweth testing

* fix: raweth memory leaks

* feat: add message number option to z_sub

* fix: null malloc when cloning rc list

* doc: remove obsolete types

* fix: encoding option made fragment test fail

* fix: replace all deprecated system calls

* fix: improve raweth test stability

---------

Co-authored-by: Denis Biryukov <[email protected]>
Co-authored-by: Alexander <[email protected]>
Co-authored-by: Michael Ilyin <[email protected]>
Co-authored-by: Mahmoud Mazouz <[email protected]>
Co-authored-by: OlivierHecart <[email protected]>
Co-authored-by: Alexander <[email protected]>
Co-authored-by: oteffahi <[email protected]>
Co-authored-by: Geoff Martin <[email protected]>
Co-authored-by: Lieven <[email protected]>
Co-authored-by: Luca Cominardi <[email protected]>
Co-authored-by: OlivierHecart <[email protected]>
Mallets added a commit that referenced this pull request Aug 1, 2024
@sashacmc @DenisBiryukov91
@milyin @fuzzypixelz @jean-roland @OlivierHecart @oteffahi @gmartin82 @vortex314 @Mallets @nashif @diogomatsubara
Merge main into dev/1.0.0 (#567)
96ce437 
* add support for qos settings in sample

* - silence cpp compiler regading compound literals and type conversions,
- add missing dots in docs

* fix _z_qos_t definition and doc

* fix z_qos_t definition and doc

* - replace binary literals (non-core C) with hexadecimal ones
- replace _z_n_qos_unmake with _z_n_qos_unmake_public in subscription.c

* make _z_n_qos_unmake static inline to prevent linking errors

* fix format

* reduce qos size to 1 byte; use getters to extract individual qos settings

* fix _z_n_qos_make to no longer use compund literals to avoid warnings from zenoh-cpp

* fix z_qos_default() signature

* Add Flipper platform (#351)

* Add Flipper platform

* Fix warnings and other platform builds

* move z_n_qos_t related functions into .c (to hide designated initializers from c++)

* z_sample_t docs update to include qos

* move qos functions back to header and remove usage of designated initializers

* format

* fix: Bump sphinx doc build dep to `7.2.6` (#361)

* replac zp_ prefix for platform functions that are also available in zenoh-c

* fix format

* replace zp_ prefix with z_ for zp_random_u64

* put back previously removed zp_ functions as deprecated

* added missing new line at the end of the file

* typo fix

* move deprecated platform header to a separate folder

* Add config endpoint in raweth locator (#364)

* feat: add ethtype to reth endpoint config

* feat: type renaming

* fix: remove unused macro call

* feat: remove static raweth config

* feat: use defines for separators

* feat: add mapping function

* fix: add default raweth mapping

* feat: add debug traces on raweth config  parsing

* fix: ethtype validity test

* fix: strtok parsing calls

* chore: clang-format

* fix: include first mapping entry in lookup

* fix: appease the tyran codacy

* Fix bug reading from ws (#370)

* Expose timeout option in z_get_options_t (#375)

* Expose timeout option in z_get_options_t

* Add default get timeout

* Align examples and remove reading from stdin (#359)

* Remove reading from stdin, align example implementations

* Update examples tests

* Adjust z_sub_thr output

* Fix z_get single query examples

* Replace pthread uses with z_mutex and z_condvar

* Add multi-thread feature condition to z_get examples

* Update error message for features absence

* Update z_get expected output in modularity test

* Update sample count for freertos single-thread examples

* fix: remove unused variable warning (#379)

* Changing references to zenoh:master to zenoh:main. (#381)

* correct unsigned atomic in refcount.h (#382)

* Fix refcount cast (#384)

* Fix esp32 CI (#386)

* fix: const discard warning

* build: fix platformio dependency check

* Serial timeout (#383)

* correct unsigned atomic in refcount.h

* made espidf freertos compatible with timeout on read

---------

Co-authored-by: Luca Cominardi <[email protected]>

* Run clang-format (#388)

* Fix misra violations (#390)

* chore: run clang format

* fix: misra issues

* ci: Allow building zenoh from arbitrary branch in build-check workflow (#389)

* ci: Allow building Zenoh from arbitrary branch in build-check workflow

* Update other jobs

---------

Co-authored-by: OlivierHecart <[email protected]>

* support for integration as a west module in zephyr (#395)

Integrate zenoh as a west module and enable adding zenoh as a library
using kconfig.

This will allows easier integration with zephyr by including in the west
manifest (west.yml), i.e.:

@@ -2,7 +2,13 @@ manifest:
   remotes:
     - name: upstream
       url-base: https://github.com/zephyrproject-rtos
+    - name: zenoh
+      url-base: https://github.com/eclipse-zenoh
   projects:
+    - name: zenoh-pico
+      revision: main
+      path: external/zenoh-pico
+      remote: zenoh
     - name: canopennode
       revision: dec12fa3f0d790cafa8414a4c2930ea71ab72ffd
       path: modules/lib/canopennode

and then enabling the publication example with:

CONFIG_ZENOH_PICO=y
CONFIG_ZENOH_PICO_LINK_SERIAL=y
CONFIG_ZENOH_PICO_PUBLICATION=y

application can then be built with west:

west build -b reel_board zephyr/samples/publication

Signed-off-by: Anas Nashif <[email protected]>

* fix: concurrency issue on task_running init value (#401)

* feat: Automate Release (#402)

* feat: Store project version in `version.txt`

* fix: Add `CMakeFiles` to `.gitignore`

* feat: Automate Release

* fix: Remove enforce-linking-issues workflow

* fix: Remove unecessary SSH passphrase/privatekey inputs

* fix: Force push release branch

* fix: Make build-macos need pre-build

* fix: Typo in pre-build.bash path

* fix: Typo in examples build path

* Fix windows socket timeout value (#411)

* fix: replace windows sockopt timeout value type

* fix: void functions returning value

* fix: use z_reliability enum type to set flag (#413)

* ci: change raweth workflow (#416)

* Enable releasing from any branch (#421)

* Align version with zenoh-c (#438)

As per eclipse-zenoh/zenoh-c#466, align how we
configure zenoh CMake based projects version.

* feat: add malloc value guards for freertos (#527)

* fix: add mutex free value guard (#551)

* Add missed merge changes

---------

Signed-off-by: Anas Nashif <[email protected]>
Co-authored-by: Denis Biryukov <[email protected]>
Co-authored-by: Michael Ilyin <[email protected]>
Co-authored-by: Mahmoud Mazouz <[email protected]>
Co-authored-by: Jean-Roland Gosse <[email protected]>
Co-authored-by: OlivierHecart <[email protected]>
Co-authored-by: oteffahi <[email protected]>
Co-authored-by: Geoff Martin <[email protected]>
Co-authored-by: Lieven <[email protected]>
Co-authored-by: Luca Cominardi <[email protected]>
Co-authored-by: OlivierHecart <[email protected]>
Co-authored-by: Anas Nashif <[email protected]>
Co-authored-by: Diogo Matsubara <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jean-roland jean-roland jean-roland approved these changes

@Mallets Mallets Mallets approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Expose timeout option in z_get_options_t
4 participants
@sashacmc @eclipse-zenoh-bot @Mallets @jean-roland