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

Autopaho CleanStart forced to true always #115

Closed
ashtonian opened this issue Jan 18, 2023 · 4 comments
Closed

Autopaho CleanStart forced to true always #115

ashtonian opened this issue Jan 18, 2023 · 4 comments

Comments

@ashtonian
Copy link

Describe the bug
autopaho is currently forcing clean start.

To reproduce
try to connect to autopaho without a clean start.

Expected behaviour
To be able to use autopaho with a clean start.

@ashtonian
Copy link
Author

Not sure if I need to configure the in memory stuff as well in order for this to work. Any thoughts?

@MattBrittan
Copy link
Contributor

Please see my comment on #113 (these two issues should probably be merged; ideally all being rolled into #25). My main issue with this is that without support for persistence setting CleanSession to false means we are not complying with the MQTT spec which details what is included in the session (see here re handling ACK packets). This would be a real issue at QOS2 (because we could keep receiving, for example, PUBREL, which we ignore, so from the broker perspective the message workflow never completes).

As such I think this would need a warning comment and my preference would be that it continues to default to CleanStart=true (I realise doing it the way you have is easier but this is a breaking change and even through we are pre 1.0 I'd prefer to avoid those if possible, especially one like this that users are likely to miss). This would not be such an issue if persistence was implemented; implementing that is likely to result in other breaking changes (and is not a small task).

@akindestam
Copy link

akindestam commented Mar 28, 2023

+1 for this, since connecting with CleanStart=false would be useful for my use case even without the full persistence support (my use case is mainly QoS 1)

I do agree that it might need to default to CleanStart=true somehow, though...

@MattBrittan
Copy link
Contributor

I'm going to close this as a duplicate of #25 (because support for a session is needed before CleanStart can be enabled). Support has been implemented in my test repo (see link in that issue) and I am currently testing it. Assistance with testing would be welcomed :-).

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

Successfully merging a pull request may close this issue.

3 participants