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

ConnectionSettings should use "clean_start" instead of "clean_session" #83

Open
cartertinney opened this issue Nov 22, 2023 · 4 comments

Comments

@cartertinney
Copy link
Contributor

"clean_session" is a term used from MQTT 3.1.1, "clean_start" would be the correct term for usage with MQTT 5 to prevent confusion

@rido-min
Copy link
Contributor

yes this is a good point, before making any change I'd like to understand how the different MQTT libraries used in this repo behave when using MQTT5 and CleanSession:

https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901039

  • Should they fail since it's a MQTT5 concept?
  • In the same way, should CleanStart fail when used with 3?

I believe there is also some dependency with SessionExpiryInterval, without setting it I'm not able to get a persistent session with MQTTNet, see https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901048

We might want to add SessionExpiryInterval as a new ConnectionSetting, with a default value that will be applied when CleanStart = False

@cartertinney
Copy link
Contributor Author

cartertinney commented Nov 27, 2023

In Python, the Paho library will raise an exception if you attempt to set a "clean_session" with MQTTv5, yes. And similarly, any attempt to set a "clean_start" (besides the default value) while using MQTTv3.1.1 will also result in an exception. This makes sense to me, given what you point out about the Session Expiry Interval property, as well as the change to when the option is provided (i.e. lifespan of the option) - it appears the semantics change between 3 and 5, it's not just a naming change, and thus it is even more important that we handle this change correctly and idiomatically.

I would reiterate, that I really think that we ought to be concerned with the MQTT5 spec, and the high level concepts rather than minutia of specific library implementations. Whether or not any particular library allows us to use a deprecated value/option with MQTT5 configuration should not be what informs our decisions.

@rido-min
Copy link
Contributor

and I agree !!!

We could focus on MQTT5 only, and rename CleanSession to CleanStart.

@cartertinney
Copy link
Contributor Author

Yes, I agree, this is what we should do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants