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

mqtt_version of CreateOptionsBuilder seems to be ignored #140

Closed
koepalex opened this issue Oct 22, 2021 · 2 comments
Closed

mqtt_version of CreateOptionsBuilder seems to be ignored #140

koepalex opened this issue Oct 22, 2021 · 2 comments
Assignees
Labels
fix added A fix was added to an unreleased branch
Milestone

Comments

@koepalex
Copy link

In my first approach of using the paho mqtt crate I added my settings to CreateOptions like

let create_opts = mqtt::CreateOptionsBuilder::new()
        .server_uri(host)
        .mqtt_version(mqtt::MQTT_VERSION_5)
        .client_id("my-publisher-id")
        .finalize();

let cli = mqtt::AsyncClient::new(create_opts).unwrap_or_else(|err| {
        error!("Error creating the client: {}", err);
        process::exit(1);
    });

match cli.connect(None).await {
        Ok(res) => info!("Connected with Response: {}", res.reason_code()),
        Err(err) => panic!("Error connecting to MQTT broker:: {}", err),
    }

let msg = mqtt::Message::new(topic, data.dump().into_bytes(), qos);
let delivery_token = cli.publish(msg).wait();

by using cli.connect(None) I expect to use the settings of CreateOptions but at least the mqtt_version is ignored and mqtt_version 0 is used.

@fpagliughi
Copy link
Contributor

Sorry for the slow response...

Yeah, I agree this is confusing. I simply exposed the behavior of the underlying C library, and I'm not particularly happy about having to specify the version twice, either.

I think the C lib wants to know if you might use a v5 connection when you create the client so it can create some persistence information differently. But it obviously still lets you create a v3 connection... which you found out by accident :-)

We could take the hint from the create option to be the default when connecting. That would certainly be better than the way it works now.

But even better than that maybe either:

  • Default to a v5 create option and then let the connect option decide the requested version.
  • or, remove the create option entirely from the API, always create a v5 client in the C lib, and let the rust app chose the version for each connection.

So, either way, chose the version in the connect options, but decide whether or not to even keep the option when creating the client.

@fpagliughi fpagliughi self-assigned this Jan 16, 2022
@fpagliughi fpagliughi added this to the v0.10 milestone Jan 16, 2022
@fpagliughi
Copy link
Contributor

OK. The connection will default to the version of MQTT specified in the client's create options. Basically, now you can chose to connect with an older version of the protocol than the one specified when creating the client, but you can not connect with a newer one...

Meaning:

If you create the client as v5:

  • The connection will default to v5 if none is specified in the connect options
  • You can still connect with v3 by specifying it in the connect options

If you create the client as v3.x:

  • You can only connect with v3
  • You do not need to specify it in the connect options

@fpagliughi fpagliughi added the fix added A fix was added to an unreleased branch label Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix added A fix was added to an unreleased branch
Projects
None yet
Development

No branches or pull requests

2 participants