Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims to improve usability of Mqttoptions and fixes #434 and #270 . some of the changes are:
port isn't required argument to
new()
:localhost:1337
, if not specified, defaults ports will be used as per MQTT specifications.set_port()
can be used to set/override the portPassing partial url as address works:
localhost
,localhost:1883
,ws://localhost
,mqtts://localhost:1884
,wss://example.org/mqtt
etc. are all valid addressesswanx://localhost
,localhost:999999
,localhost:x2
return errors regarding scheme / port.Same behavior of MqttOptions for both tcp and webscokets :)
Issues / queries:
instead of returning Error from
MqttOptions::new()
, shall we panic if provided options are wrong?why?: because we do same for other fns like
set_keep_alive
/set_inflight
.why not?: rumqttc is used as library, returning error so users can handle em as they want sound better than panics.
we aren't performing any host validations, i.e.
MqttOptions::new("client_id", "[@-1")
won't return any error. Rather it would try to connect to the host and fail with lookup error:Io(Custom { kind: Uncategorized, error: "failed to lookup address information: Name or service not known" })
. Shall we do strict hostname checking?why?: we would catch some of the invalid address early ( like empty hostname, invalid IPv6 etc.)
why not?: It is hard to validate! using other dependency like
url::Host
seems good but it doesn't supportno_std
which we would like to have :(Please comment your thoughts, I will mark the PR as ready once above/any other queries are resolved :)
Type of change
Checklist:
cargo fmt
CHANGELOG.md
if it's relevant to the users of the library. If it's not relevant mention why.TODO: once we have finalized it!