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

Fix for ignored certificate information #133

Closed
wants to merge 3 commits into from

Conversation

cvellan
Copy link
Contributor

@cvellan cvellan commented Dec 4, 2017

_tlsOptions was never assigned when building an instance of MqttClientOptions using an MQttClientOptionsBuilder, and certificate information was being lost.

This was a part of the cause of issue #115.

Correctly assigns _tlsOptions to the active client options when Build() is called
_tlsOptions was never assigned when building an instance of MqttClientOptions using an MQttClientOptionsBuilder, and certificate information was being lost.
@chkr1011 chkr1011 changed the base branch from master to develop December 5, 2017 12:18
@chkr1011
Copy link
Collaborator

chkr1011 commented Dec 5, 2017

Hi,
please fix the conflicts. Then I will merge the pull request.
Best regards
Christian

@cvellan
Copy link
Contributor Author

cvellan commented Dec 6, 2017

I did try to fix it but seem to have made more of a mess of things; I'm unused to the pull request process. Sorry for the hassle.

The effective changes are lines 122 and 126, where the TlsOptions of the active channeloptions needs to be set, instead of setting the channel options themselves (as that also happens later at line 130).

I'll try to fix the code up, if not I'll submit a clean PR. Or feel free to do it yourself if it takes too long.

@chkr1011
Copy link
Collaborator

chkr1011 commented Dec 6, 2017

Don't worry. I will fix the bug on my own. With the previous pull request you are added to the contributor list 😄

@chkr1011 chkr1011 closed this Dec 7, 2017
@cvellan
Copy link
Contributor Author

cvellan commented Dec 7, 2017

Glad I could help! 😃

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.

2 participants