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.

Sign up for GitHub

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

Jump to bottom

fix: add processing of missing Mqtt subscribe options #536

Merged
merged 2 commits into from Dec 17, 2022
Merged

fix: add processing of missing Mqtt subscribe options #536

merged 2 commits into from Dec 17, 2022

Conversation

brianmay
Copy link
Contributor

@brianmay brianmay commented Dec 11, 2022 •
edited

Fix for #487.

@brianmay
Copy link
Contributor Author

Seems to work. Note that initially I wasn't getting retained messages. This is because the default for retain_forward_rule is Never:

rumqtt/rumqttc/src/v5/mqttbytes/mod.rs

Line 336 in 29f5b1e

Self::Never

This default was somewhat unexpected, and because the value was previously ignored it had no affect.

@brianmay brianmay marked this pull request as ready for review December 11, 2022 21:47
brianmay added a commit to brianmay/robotica-rust that referenced this pull request Dec 11, 2022
@brianmay
Don't process outgoing mqtt packets twice
754d889 
Fix the nolocal mqtt subscribe option. See
bytebeamio/rumqtt#536
@de-sh de-sh changed the title Add processing of missing Mqtt subscribe options fix: add processing of missing Mqtt subscribe options Dec 15, 2022
@h3nill
Copy link
Member

h3nill commented Dec 16, 2022

ported the changes to rumqttd's protocol. and updated the default for RetainForwardRule to OnEverySubscribe because Never by default is not very intuitive.

@h3nill h3nill requested a review from de-sh December 16, 2022 15:48
de-sh
de-sh approved these changes Dec 17, 2022
View reviewed changes
Copy link
Member

@de-sh de-sh left a comment •
edited

Choose a reason for hiding this comment

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

LGTM 👍🏼

@h3nill h3nill merged commit 51b53b4 into bytebeamio:main Dec 17, 2022
@brianmay brianmay deleted the fix_487 branch December 17, 2022 22:18
@brianmay brianmay mentioned this pull request Dec 21, 2022
Closed
carlocorradini pushed a commit to carlocorradini/rumqtt that referenced this pull request Aug 3, 2023
@brianmay @henil
fix: add processing of missing Mqtt subscribe options (bytebeamio#536)
4955149 
* Add processing of missing Mqtt subscribe options

* port subscribe options fix to rumqttd as well & change default of RetainForwardRule to OnEverySubscribe

Co-authored-by: henil <dedaniahenil@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@h3nill h3nill h3nill left review comments

@de-sh de-sh de-sh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@brianmay @h3nill @de-sh @henil