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

Fixes wrong QoS condition in publish message flow in mqtt-streaming c… #1582

Merged
merged 1 commit into from
Mar 15, 2019

Conversation

jcroig
Copy link
Contributor

@jcroig jcroig commented Mar 14, 2019

…onnector.

Pull Request Checklist

Fixes

Fixes wrong QoS condition in publish message flow in mqtt-streaming connector. There's no functional change. Actually old code was working properly even when the check was incorrect by chance since the boolean check evaluates true for both contants ControlPacketFlags.QoSAtMostOnceDelivery and
ControlPacketFlags.QoSExactlyOnceDelivery when message QoS is 2. However the code is misleading and doesn't reflect its real intention which is verify the QoS is "exactly once delivery" instead of "at most once delivery"

Purpose

Substitute wrong QoS check by the correct one so that code express its intention beyond following the MQTT spec.

Background Context

Simple change it is only a wrong constant value and used the right one.

References

http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html

Copy link
Contributor

@huntc huntc left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

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

LGTM.

@ennru ennru merged commit 62292b0 into akka:master Mar 15, 2019
@ennru
Copy link
Member

ennru commented Mar 15, 2019

Thank you, great to see more people trying the MQTT streaming connector!

@ennru ennru added this to the 1.0-M4/RC1 milestone Mar 15, 2019
@jcroig
Copy link
Contributor Author

jcroig commented Mar 15, 2019

My pleasure!

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

Successfully merging this pull request may close these issues.

None yet

3 participants