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.

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

Fix CI failures #394

Merged

Conversation

jean-roland
Copy link
Contributor

@jean-roland jean-roland commented Apr 3, 2024

Following #389, CI is functional again on this branch and it revealed many small issues that are fixed in this PR, along with a merge of the main branch to bring the aforementioned changes.

There is still an issue regarding fragmentation that seems to involve zenoh. Edit: solved

DenisBiryukov91 and others added 30 commits February 15, 2024 17:52
- replace _z_n_qos_unmake with _z_n_qos_unmake_public in subscription.c
* Add Flipper platform

* Fix warnings and other platform builds
…ority-in-sample

Add support for qos settings in sample
…qos-designated-initializers

Fix/remove qos designated initializers
…tform-api-sync-with-zenoh-c

platform api synchronization with zenoh-c
* 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
* Expose timeout option in z_get_options_t

* Add default get timeout
* 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
@jean-roland jean-roland mentioned this pull request Apr 5, 2024
@@ -37,7 +37,7 @@ int main(int argc, char **argv) {
const char *mode = "client";
char *clocator = NULL;
char *llocator = NULL;
int n = -1;
int n = 2147483647; // max int value by default
Copy link
Member

Choose a reason for hiding this comment

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

It's safer to use int n = ~0; in such a way we always have the guarantee n is max(int) regardless the actual size of int.

@@ -30,6 +30,7 @@

#define KEYEXPR "demo/example/zenoh-pico-pub"
#define VALUE "[FreeRTOS-Plus-TCP] Pub from Zenoh-Pico!"
#define N 2147483647 // max int value by default
Copy link
Member

Choose a reason for hiding this comment

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

It's safer to use int n = ~0; in such a way we always have the guarantee n is max(int) regardless the actual size of int.

Copy link
Contributor Author

@jean-roland jean-roland Apr 5, 2024

Choose a reason for hiding this comment

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

With a signed integer ~0 will be interpreted as -1. As this is also an example thing, I suggest we fix this with the z_sleep later.

Copy link
Member

Choose a reason for hiding this comment

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

You are right. Technically it should be an unsigned int and not an int.

@@ -27,13 +27,17 @@
#endif

#define KEYEXPR "demo/example/**"
#define N 2147483647 // max int value by default
Copy link
Member

Choose a reason for hiding this comment

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

It's safer to use int n = ~0; in such a way we always have the guarantee n is max(int) regardless the actual size of int.

@@ -30,7 +30,7 @@ int main(int argc, char **argv) {
const char *mode = "client";
char *clocator = NULL;
char *llocator = NULL;
int n = 10;
int n = 2147483647; // max int value by default
Copy link
Member

Choose a reason for hiding this comment

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

It's safer to use int n = ~0; in such a way we always have the guarantee n is max(int) regardless the actual size of int.

@@ -27,7 +27,7 @@ int main(int argc, char **argv) {
const char *mode = "client";
char *clocator = NULL;
char *llocator = NULL;
int n = 10;
int n = 2147483647; // max int value by default
Copy link
Member

Choose a reason for hiding this comment

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

It's safer to use int n = ~0; in such a way we always have the guarantee n is max(int) regardless the actual size of int.

@@ -26,7 +26,7 @@ int main(int argc, char **argv) {
const char *mode = "client";
char *clocator = NULL;
char *llocator = NULL;
int n = 10;
int n = 2147483647; // max int value by default
Copy link
Member

Choose a reason for hiding this comment

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

It's safer to use int n = ~0; in such a way we always have the guarantee n is max(int) regardless the actual size of int.

@@ -26,9 +26,10 @@ int main(int argc, char **argv) {
const char *mode = "client";
char *clocator = NULL;
char *llocator = NULL;
int n = 2147483647; // max int value by default
Copy link
Member

Choose a reason for hiding this comment

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

It's safer to use int n = ~0; in such a way we always have the guarantee n is max(int) regardless the actual size of int.

@@ -18,6 +18,8 @@
#include <stdlib.h>
#include <zenoh-pico.h>

#define N 2147483647 // max int value by default
Copy link
Member

Choose a reason for hiding this comment

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

It's safer to use int n = ~0; in such a way we always have the guarantee n is max(int) regardless the actual size of int.

@@ -18,13 +18,18 @@
#include <stdlib.h>
#include <zenoh-pico.h>

#define N 2147483647 // max int value by default
Copy link
Member

Choose a reason for hiding this comment

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

It's safer to use int n = ~0; in such a way we always have the guarantee n is max(int) regardless the actual size of int.

uart_wait_tx_done(sock->_serial, 1000);
uart_flush(sock->_serial);
uart_driver_delete(sock->_serial);
zp_free(sock->after_cobs);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be z_free?

uart_flush(sock->_serial);
uart_driver_delete(sock->_serial);
zp_free(sock->after_cobs);
zp_free(sock->before_cobs);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be z_free?

Copy link
Member

@Mallets Mallets left a comment

Choose a reason for hiding this comment

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

Some minor changes, everything else LGTM.

@jean-roland
Copy link
Contributor Author

We noticed some issues with examples:

  • Some still use sleep instead of z_sleep
  • We should use an unsigned int set to ~0 for the default loop count
    But this is not the scope of this PR so this shall be adressed by another PR

@Mallets
Copy link
Member

Mallets commented Apr 5, 2024

We noticed some issues with examples:

  • Some still use sleep instead of z_sleep
  • We should use an unsigned int set to ~0 for the default loop count
    But this is not the scope of this PR so this shall be adressed by another PR

Agree

@Mallets Mallets merged commit 265405d into eclipse-zenoh:protocol_changes Apr 5, 2024
50 checks passed
@jean-roland jean-roland deleted the ft_protocol_change branch May 27, 2024 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.