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 issue that message properties are not sent out when using MQTT #39

Closed
wants to merge 2 commits into from
Closed

fix issue that message properties are not sent out when using MQTT #39

wants to merge 2 commits into from

Conversation

erich-wang
Copy link
Member

the Message's properties are sent out when using AMQP/HTTPS but MQTT

@msftclas
Copy link

Hi @erich-wang, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor. If you're full-time or an intern, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

prmathur-microsoft
prmathur-microsoft reviewed Feb 22, 2017
View reviewed changes
...ce-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/mqtt/MqttMessaging.java
@@ -167,10 +168,34 @@ public void send(Message message) throws IOException
throw new IOException("Message cannot be null");
}

MessageProperty[] messageProperties = message.getProperties();
String messagePublishTopic;
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for your change. This looks good and is much needed. Can you please also add a test case around this in device\iot-device-client\src\test\java\tests\unit\com\microsoft\azure\sdk\iot\device\transport\mqtt\MqttMessagingTest.java ?

Copy link
Member Author

Choose a reason for hiding this comment

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

test case has been added, please review, thanks.

@prmathur-microsoft
Copy link
Member

Added a new requirement, squashed all the commits and checked in @f9e2af997932722357fa15b8eba13e3bbc2e7d76 commit. Thank you for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prmathur-microsoft prmathur-microsoft prmathur-microsoft approved these changes

Assignees

@prmathur-microsoft prmathur-microsoft

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@erich-wang @msftclas @prmathur-microsoft