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

#noissue MQTT metadata topic name doc update #433

Conversation

rohitjoshi-c8y
Copy link
Contributor

Based on @sagscmi feedback to improve the Java doc around the topic feild.

@@ -103,6 +103,8 @@ public String getResponseTopic() {
}

/**
* Retrieves the MQTT topic. If necessary, this property must be explicitly configured when employing an MQTT WebSocket Producer.
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about rephrasing it to something like:

Represents MQTT topic name when receiving messages via {@link MqttMessageListener}. When publishing the message via {@link MqttPublisher.publish} this field is currently ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if that makes it clear.
The concern was that it does not say this field is not automatically set in case of using WS producer and consuming messages from WS consumer.

Choose a reason for hiding this comment

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

You're kind of both right :-) My concern was that you can only rely on this field when it was set by the MQTT engine, on a message published by an MQTT device. In that case it does reflect the topic that the message was published to. However, it is essentially meaningless for messages published by a microservice, through the SDK.

Can we put Javadoc on the generated constructor? As a user of the SDK I would want to know that setting this field does not affect the MQTT topic that a message will actually be published on (it would be reasonable for a user to assume that it does control the topic). I would also want to know that the value will be passed through unchanged to any SDK consumers that receive the message (I think that is true?) which could be confusing if it's set to a different value than the actual topic.

On the getter side, shouldn't we just say:

  1. For messages published by an MQTT client, this field contains the name of the topic that the message was published to.
  2. For messages published by an SDK client, this field contains the value passed to the MqttMetadata constructor, which is not necessarily the same as the MQTT topic the message was published to.

Sorry for the long comment!

@rohitjoshi-c8y
Copy link
Contributor Author

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 this pull request may close these issues.

None yet

3 participants