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

z_open should accept z_open_options_t to allow for future extensions #352

Open
DenisBiryukov91 opened this issue May 6, 2024 · 3 comments
Labels
api fix Problem in the API api sync Synchronize API with other bindings release Part of the next release

Comments

@DenisBiryukov91
Copy link
Contributor

Describe the release item

z_open signature should be changed to
z_error_t z_open(struct z_owned_session_t *this_, struct z_owned_config_t *config, const z_open_options_t *options);

This will allow providing more options in the future without breaking the api.

@DenisBiryukov91 DenisBiryukov91 added the release Part of the next release label May 6, 2024
@milyin
Copy link
Contributor

milyin commented May 29, 2024

There was also a request from @kydos to provide open function without required config parameter, as usually our config is default: eclipse-zenoh/zenoh#805, This is not decided yet and may be it makes sense to think about these issues together.

On the zenoh-c side we can put pointer to config inside options structure, allow it to be nullptr and ,provide z_open_options_default with null pointer to config. Or better name it z_session_options_t ?

The code will look like:

z_owned_session_t session;
int err = z_open(&session, z_open_options_default());

Here I assume that we passing options by value, which is not the case at this moment. Issue #401 was created for this

@milyin milyin added api sync Synchronize API with other bindings api fix Problem in the API labels May 29, 2024
@milyin
Copy link
Contributor

milyin commented May 30, 2024

z_owned_config_t config;
z_config_new(&config);
z_owned_session_t session;
z_open_options_t opts;
z_open_options_default(&opts);
opts.config = z_move(config);
int err = z_open(&session, &opts);

vs

z_owned_config_t config;
z_config_new(&config);
z_owned_session_t session;
z_open_options_t opts = z_open_options_default();
opts.config = z_move(config);
int err = z_open(&session, opts);

@DenisBiryukov91
Copy link
Contributor Author

To allow providing options inline and specifying only non-default values we can probably try to use macros as proposed here https://stackoverflow.com/questions/13716913/default-value-for-struct-member-in-c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api fix Problem in the API api sync Synchronize API with other bindings release Part of the next release
Projects
Status: No status
Development

No branches or pull requests

2 participants