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

Support message buffering before initial connect #286

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

nofarius
Copy link

The codeformatting changes to develop caused code conflicts in PR #198. This is the same PR but with the code conflicts removed. All code changes should be attributed to @matsukaz as he was the original contributor. Submitting a new PR as I don't have access to update the original PR

This should resolve issue #197

Original PR description

This change will enable offline buffering even if initial connection has never been established yet.

I've changed MqttAndoidClient, MqttService, MqttConnection in order to instantiate these instances before connection get established.

Please make sure that the following boxes are checked before submitting your Pull Request, thank you!

  • You have signed the Eclipse ECA
  • All of your commits have been signed-off with the correct email address (The same one that you used to sign the CLA)
  • If This PR fixes an issue, that you reference the issue below. OR if this is a new issue that you are fixing straight away that you add some Description about the bug and how this will fix it.
  • If this is new functionality, You have added the appropriate Unit tests.

Signed-off-by: Sam Craig <sam@gigobin.com>
@matsukaz
Copy link

matsukaz commented Feb 1, 2018

@nofarius Thank you for creating this PR!
I had no time to check my code to see why the code conflicts.
Hope this PR get merged.

@nofarius
Copy link
Author

nofarius commented Feb 1, 2018

@matsukaz no problem, looking forward to this functionality being part of this project!

Unfortunately, there are some unit tests that are not working. I have managed to resolve one of them, which I haven't committed yet as it then caused others to break....

Basically, it has to do with the removal of the clientHandle cleanup in MqttAndroidClient and MqttService files, which means that the connectOptions are persisting between connect / disconnect cycles. See the testCleanSession test, where cleanSession is being changed in the connectOptions, but the original value is still being used.

If you're able, I would appreciate your contribution to reviewing these unit tests. I have added you as a collaborator to the https://github.com/nofarius/paho.mqtt.android project, pushing to the msg_buffer_before_connect brach should update this PR

Signed-off-by: Sam Craig <sam@gigobin.com>
Signed-off-by: Sam Craig <sam@gigobin.com>
@nofarius
Copy link
Author

nofarius commented Feb 2, 2018

Since it is not possible to enable / disable network connection inside a unit test (as far as I'm aware), I have added an Enable Message Buffer option to the sample app so that testing message buffer while app is offline (eg airplane mode) can be done manually

@nofarius
Copy link
Author

nofarius commented Feb 5, 2018

Is there a known issue with the unit tests randomly failing when they should pass?

I can't replicate the test failure in the auto build locally. When I run the whole test suite locally usually they all pass, and sometimes random tests fail for no real reason... I then run the test suite again and the same test that failed then succeeds with no code changes...

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

2 participants