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

Fixed device not reconnecting after MQTT connection loss #611

Merged
merged 2 commits into from
Aug 3, 2019

Conversation

dakhnod
Copy link
Contributor

@dakhnod dakhnod commented Jul 15, 2019

MQTT was not reconnecting after a loss, since BootNormal::_onMqttDisconnected had to be called twice in order to periodically try to reconnect.

In my tests it got called only once, leading to only one singe reconnection attempt.

@stritti stritti requested review from timpur, kleini and luebbe July 15, 2019 18:55
@kleini
Copy link
Collaborator

kleini commented Jul 16, 2019

Tests need to be fixed first.

@dakhnod
Copy link
Contributor Author

dakhnod commented Jul 16, 2019

"Error: The program size (435040 bytes) is greater than maximum allowed (434160 bytes)"

i don't really think there is much i can do about that

@kleini
Copy link
Collaborator

kleini commented Jul 16, 2019

I tried to fix the tests for pull request #577 .

@dakhnod
Copy link
Contributor Author

dakhnod commented Jul 16, 2019

please help me understand:

i literally deleted three lines. how can that affect the outcome of build teste in that way?

@bertmelis
Copy link
Contributor

I would let pass. It's the esp01 which lacks memory.

I don't know how CircleCI handles things, but for esp01 one should test without config or mdns enabled to lower memory usage.

@kleini
Copy link
Collaborator

kleini commented Jul 17, 2019

@bertmelis I switched from esp01 to esp01_1m in pull request #577. This increases ROM from 512K to 1M and compiles do work again. We can then discuss somewhere else, whether we want to stay with esp01_1m or disable config mode for esp01.

Copy link
Collaborator

@kleini kleini left a comment

Choose a reason for hiding this comment

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

I did not test the code but it looks reasonable.

@kleini
Copy link
Collaborator

kleini commented Jul 17, 2019

please help me understand:

i literally deleted three lines. how can that affect the outcome of build teste in that way?

The resulting image size dynamically depends on the used libraries and their versions. Especially the Arduino framework seems to grow with every release. Please see issue #471 for the according discussion.

@dakhnod
Copy link
Contributor Author

dakhnod commented Jul 17, 2019

I did not test the code but it looks reasonable.

I am sure the code used to work once, i just don't get why onMqttDisconnected() should be called twice.

Is there any scenario where that would happen after a disconnect?

@kleini
Copy link
Collaborator

kleini commented Jul 17, 2019

I will try to test that by blocking the MQTT port of my MQTT server, so I can bring the devices into the problem, that a second connection retry becomes necessary.

@dakhnod
Copy link
Contributor Author

dakhnod commented Jul 17, 2019

Jup, exactly what I did

@stritti stritti mentioned this pull request Jul 23, 2019
@stritti
Copy link
Collaborator

stritti commented Jul 31, 2019

I think this is also issue in develop-v3:

@stritti stritti merged commit edb5d62 into homieiot:develop Aug 3, 2019
stritti pushed a commit that referenced this pull request Aug 3, 2019
* Fixed device not reconnecting after MQTT connection loss

* formatting

(cherry picked from commit edb5d62)
@stritti
Copy link
Collaborator

stritti commented Aug 3, 2019

I cherry picked it to branch develop-v3

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.

4 participants